• Status Closed
  • Percent Complete
  • Task Type Bugs
  • Category ID3 / meta data
  • Assigned To No-one
  • Operating System All players
  • Severity Low
  • Priority Very Low
  • Reported Version Daily build (which?)
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by salty-horse - 2011-03-12
Last edited by Buschel - 2011-03-16

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

Using latest svn on uisimulator and on Sansa e200.

The following file has playback problems in Rockbox:

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

Closed by  Buschel
2011-03-16 18:47
Reason for closing:  Fixed
Additional comments about closing:   Warning: Undefined array key "typography" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 371 Warning: Undefined array key "camelcase" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 407

Fixed with r29603.

Some detailed information about the tag/file:

1. The real mp3 data begins at byte position 0×10674. 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 0×040776. From this the first mp3 frame position is calculated as 0×10400.
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.

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.

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…

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.

Patch version v05 implemented the idea from my last post.

Remove parameter from parser-functions. In consequence also remove a static variable and a function from mpeg.c (only built for !SW_CODEC).

MikeS commented on 2011-03-14 22:26

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.

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.

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).


Available keyboard shortcuts


Task Details

Task Editing