Rockbox

This is the bug/patch tracker for Rockbox. Click here for more information.

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#12007 - File with Corrupt metadata reported with wrong playing time, seeking broken

Attached to Project: Rockbox
Opened by Ori Avtalion (salty-horse) - Sunday, 13 March 2011, 00:07 GMT+2
Last edited by Andree Buschmann (Buschel) - Wednesday, 16 March 2011, 19:47 GMT+2
Task Type Bugs
Category ID3 / meta data
Status Closed
Assigned To No-one
Player Type All players
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

Using latest svn on uisimulator and on Sansa e200.

The following file has playback problems in Rockbox:
http://www.archive.org/download/MIT9.00F04/L9.mp3

Players such as vlc and totem report is to have 76 minutes. Rockbox reports it to have 5.
Trying to seek by holding the back/forward buttons causes it to jump to unexpected places. (This is probably due to the cause of the first problem)

Rebuilding the stream with Foobar2000 causes rockbox to play it (and display metadata) correctly
This task depends upon

Closed by  Andree Buschmann (Buschel)
Wednesday, 16 March 2011, 19:47 GMT+2
Reason for closing:  Fixed
Additional comments about closing:  Fixed with r29603.
Comment by Andree Buschmann (Buschel) - Sunday, 13 March 2011, 11:37 GMT+2
Some detailed information about the tag/file:

1. The real mp3 data begins at byte position 0x10674. If everything before this position is deleted the file plays fine and has the right duration.
2. The tag length stated in its header is wrong. The header states an 7-to-8-bit encoded length of 0x040776. From this the first mp3 frame position is calculated as 0x10400.
3. If you correct the id3 tag length to 0x040C6A in the file, it plays fine, has the right duration and reads the metadata. Residual issue: The album art is only displaying the upper half.

Edit: Needed to correct the real offset of the first mp3 header.
Comment by Andree Buschmann (Buschel) - Sunday, 13 March 2011, 14:28 GMT+2
Further information:

4. The tag is stating it is a ID3v2.4 tag. But it is a ID3v2.3 tag. When correcting this in the file, the length of all frames is correct. As a result the embedded image is fully loaded and displayed.
5. The wrong id3v2-length that is stated in the header itself points to a position within the embedded album art where -- by random -- a byte sequence is placed that looks is allowed to be interpreted as a valid mpeg header: mp1 format is detected. If you simply enlarge the wrong tag length by 1 byte -- what is still wrong -- the metadata parser will parse to the next valid mp3 header and play back the file with correct duration.
Comment by Andree Buschmann (Buschel) - Sunday, 13 March 2011, 16:47 GMT+2
With this patch applied the file plays back correctly. The patch changes the mp3 header parser to search for two consecutive valid MPEG frame headers with identical main types (= same MPEG version, same layer, same sampling rate). The first frame header of this pair is assumed to be the first frame header of the whole file.

Important: When using the patch with the file from above, the parser locks to the _second_ valid header. This is because there is another byte-array in the first frame that is assumed to be a valid header as well. This may happen with any other file as well -> There might be frames skipped.

Another, more sophisticated, way would be to calculate the frame size of any detected valid MPEG frame header and check whether there is another MPEG frame header with identical main types is placed at the calculated byte position.

Edit: I am not sure this patch works when using Xing-VBR-headers. Maybe we need to use different functions to o either parse towards the first valid header (e.g. for locking to Xing headers) or to check for the first of a pair of identical frames...
   mp3_header_parsing_v01.patch (2.8 KiB) 
 apps/mp3data.c |   59 +++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 39 insertions(+), 20 deletions(-)

Comment by Andree Buschmann (Buschel) - Monday, 14 March 2011, 08:04 GMT+2
Proof-of-concept for a parser that seeks for a valid for a valid MPEG frame header and checks whether the next frame header is located at the expected byte position:
1. Parse until valid MPEG frame header is found
2. Extract information from frame header, including frame size in bytes
3. Skip the frame according to frame size
4. Check if there is another valid frame header with the same type (= same MPEG version, layer, sampling rate).

If so, the parser assumes the frame header found in 1) is the first MPEG frame header of the file.

This is still a proof-of-concept. The implementation does not take into account that there might be a 'Xing'-header that uses different types than the stream itself. Therefor this patch needs to be enhanced to support
a) Seek to first valid frame header without checking for a valid pair. -> For 'Xing' and similar headers.
b) Seek to first frame of a pair (see 1-4). -> For the audio stream.
   mp3_header_parsing_v02.patch (4.7 KiB) 
 apps/mp3data.c |   78 +++++++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 57 insertions(+), 21 deletions(-)

Comment by Andree Buschmann (Buschel) - Monday, 14 March 2011, 21:52 GMT+2
Patch version v05 implemented the idea from my last post.
   mp3_header_parsing_v05.patch (6.9 KiB) 
 apps/mp3data.c |  108 ++++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 77 insertions(+), 31 deletions(-)

Comment by Andree Buschmann (Buschel) - Monday, 14 March 2011, 22:26 GMT+2
Remove parameter from parser-functions. In consequence also remove a static variable and a function from mpeg.c (only built for !SW_CODEC).
   mp3_header_parsing_v07.patch (12.1 KiB) 
 apps/mp3data.h |    6 --
 apps/mpeg.c    |   31 +-------------
 apps/plugin.h  |    4 -
 apps/mp3data.c |  119 +++++++++++++++++++++++++++++++++++++--------------------
 4 files changed, 86 insertions(+), 74 deletions(-)

Comment by Michael Sevakis (MikeS) - Monday, 14 March 2011, 23:26 GMT+2
I get 1:16:30 for the duration of L9.mp3. Most files seem fine except for the few outliers I mentioned in IRC. I'll check to make sure they have the XING headers and didn't mysteriously lose them since most others from the same album seem to be alright, then get back about that.

ETA: :) Seems I somehow managed to encode a whole album without Xing tags. So, the problem's on my end with those.
Comment by Andree Buschmann (Buschel) - Tuesday, 15 March 2011, 22:14 GMT+2
Just to make sure why the attached file plays back with correct duration on !SWCODEC players *without* this patch: The wrong ID3v2 tag length lets the metadata parser jump to a wrong position within the tag. At this position there is a byte sequence which is a valid MPEG *Layer1* frame header. SWCODEC players accept this byte sequence as valid MPEG frame header and will assume this byte sequence is the first frame header of the MPEG audio stream -- which is wrong. !SWCODEC players do not accept *Layer1* frame headers as valid headers. Therefor the parser will move forward to the next valid MPEG frame header -- which results in finding the correct header.
Comment by Andree Buschmann (Buschel) - Tuesday, 15 March 2011, 23:03 GMT+2
This version does not change mpeg.c and keeps parsing until a header is found that is of the same type as the reference_header (if the reference_header is != 0).
   mp3_header_parsing_v09.patch (11 KiB) 
 apps/mp3data.h |   32 +++++++++-----
 apps/plugin.h  |    2 
 apps/mp3data.c |  129 +++++++++++++++++++++++++++++++++++++++------------------
 3 files changed, 113 insertions(+), 50 deletions(-)

Loading...