Subject: Re: [ecasound] ecasound 2.3.0 mp3 header problem
From: Julian Dobson (juliand_AT_braverock.com)
Date: Thu Sep 25 2003 - 18:44:50 EEST
As promised, here is the patch for libecasound/audioio-mp3.cpp... On
review it's not perfect as it skips a block into the file for the second
header check and if it doesn't find a second header there, it starts
looking for headers from that point, when really it should go back to
where it found the first header to continue looking. However, I've yet
to find an mp3 it fails on (unlike the orginal code).
---CUT HERE---
--- audioio-mp3.cpp Wed Feb 26 18:46:08 2003
+++ audioio-mp3.cpp Wed Sep 3 12:33:17 2003
@@ -261,8 +261,10 @@
if((file = std::fopen(filename, "rb")) == NULL) {
return false;
}
- if (std::fread(tmp, 1, 4, file) != 4)
+ if (std::fread(tmp, 1, 4, file) != 4) {
+ ECA_LOG_MSG(ECA_LOGGER::info, "(audioio-mp3) Failed to
read first 4 bytes of file!");
goto done;
+ }
buf = new unsigned char [1024];
head = convert_to_header(tmp);
while(!mpg123_head_check(head)) {
@@ -285,30 +287,59 @@
if(mpg123_head_check(head))
{
std::fseek(file, i+1-in_buf, SEEK_CUR);
- break;
+ if (mpg123_decode_header(frp, head))
+ {
+ /*
+ * We found something which looks like a MPEG-header.
+ * We check the next frame too, to be sure
+ */
+ if (std::fseek(file, frp->framesize, SEEK_CUR) != 0) {
+ ECA_LOG_MSG(ECA_LOGGER::info, "(audioio-mp3)
Seeking to next frame failed!");
+ goto done;
+ }
+ if (std::fread(tmp, 1, 4, file) != 4) {
+ ECA_LOG_MSG(ECA_LOGGER::info, "(audioio-mp3)
Failed to read the next 4 bytes!");
+ goto done;
+ }
+ head = convert_to_header(tmp);
+ if (mpg123_head_check(head) && mpg123_decode_header(frp, head))
+ {
+ std::fclose(file);
+ return true;
+ }
+ ECA_LOG_MSG(ECA_LOGGER::info, "(audioio-mp3) head check and
decode failed on second frame!");
+ }
+ else
+ ECA_LOG_MSG(ECA_LOGGER::info, "(audioio-mp3) Failed to find
second valid header!");
+ // break;
}
}
}
delete[] buf;
- if (mpg123_decode_header(frp, head))
- {
- /*
- * We found something which looks like a MPEG-header.
- * We check the next frame too, to be sure
- */
- if (std::fseek(file, frp->framesize, SEEK_CUR) != 0) {
- goto done;
- }
- if (std::fread(tmp, 1, 4, file) != 4) {
- goto done;
- }
- head = convert_to_header(tmp);
- if (mpg123_head_check(head) && mpg123_decode_header(frp, head))
- {
- std::fclose(file);
- return true;
- }
- }
+// if (mpg123_decode_header(frp, head))
+// {
+// /*
+// * We found something which looks like a MPEG-header.
+// * We check the next frame too, to be sure
+// */
+// if (std::fseek(file, frp->framesize, SEEK_CUR) != 0) {
+// ECA_LOG_MSG(ECA_LOGGER::info, "(audioio-mp3)
Seeking to next frame failed!");
+// goto done;
+// }
+// if (std::fread(tmp, 1, 4, file) != 4) {
+// ECA_LOG_MSG(ECA_LOGGER::info, "(audioio-mp3)
Failed to read the next 4 bytes!");
+// goto done;
+// }
+// head = convert_to_header(tmp);
+// if (mpg123_head_check(head) && mpg123_decode_header(frp, head))
+// {
+// std::fclose(file);
+// return true;
+// }
+// ECA_LOG_MSG(ECA_LOGGER::info, "(audioio-mp3) head check and
decode failed on second frame!");
+// }
+// else
+// ECA_LOG_MSG(ECA_LOGGER::info, "(audioio-mp3) Failed to find
second valid header!");
done:
std::fclose(file);
---CUT HERE---
Julian
Julian Dobson wrote:
> Kai,
>
> I did not know that code came from XMMS, so you might be right about the
> problem being fixed in XMMS already... Either way, I'll scrape the code
> together and post it up.
>
> Julian
>
> Kai Vehmanen wrote:
>
>> On Wed, 3 Sep 2003, Julian Dobson wrote:
>>
>>
>>> On investigation, it turned out that while the code had been updated
>>> to search the whole file for a header, the code checked for a second
>>> header incorrectly. Specifically, the first header is found inside a
>>> loop while the second header is found outside the loop. In my
>>> particular situation, the id3v2 tags appear to fake a header, so the
>>> second check fails.
>>
>>
>> [...]
>>
>>> The solution was to move the second check inside the loop, so the
>>> loop only exits after two consecutive good frame headers.
>>
>>
>> [...]
>>
>>> If anyone would like to look over the code, please let me know and I
>>> can post it to the list.
>>
>>
>>
>> Great, I'd be definitely interested in the fixes! The mp3 header
>> parsing code is originally from XMMS-1.2.5. I haven't been following
>> the XMMS codebase, so there might indeed be some bugs... (possibly
>> already fixed in XMMS).
>>
>
> --
> To unsubscribe send message 'unsubscribe' in the body of the
> message to <ecasound-list-request_AT_wakkanet.fi>.
This archive was generated by hypermail 2b28 : Thu Sep 25 2003 - 18:44:07 EEST