After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 581464 - xing data is being ignored
xing data is being ignored
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-ugly
git master
Other Linux
: Normal normal
: 0.10.12
Assigned To: Sebastian Dröge (slomo)
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2009-05-05 12:59 UTC by Anthony
Modified: 2009-05-06 14:50 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
make mp3parse more lenient when resynching. (1.65 KB, patch)
2009-05-06 14:30 UTC, Jan Schmidt
committed Details | Review

Description Anthony 2009-05-05 12:59:15 UTC
When reading mp3s with Xing data that contains the duration of the song, the information is ignored. In the test I was using, the maximum duration is given as about 55 minutes, when the real duration is about 5 minutes. gstreamer's duration becomes more accurate as the song progresses, but this is unnecessary, as the information is already available in the header.
Comment 1 Anthony 2009-05-05 13:06:30 UTC
http://bnovc.com/foo.mp3
Comment 2 Anthony 2009-05-05 14:19:30 UTC
I believe this is failing in gst_mp3parse_handle_first_frame.

In the mp3 I posted it says it is mpeg version 2, channel 2 (I'm not sure if this is right), and gets the read_id far too low in the headers.
Comment 3 Anthony 2009-05-06 13:19:33 UTC
"data" appears be off by 0x9C.

  data -= 0x9C;
  read_id = GST_READ_UINT32_BE (data);

will read the correct position (so that read_id==xing_id), but it seemed to still have invalid data, and that isn't a permanent solution.
Comment 4 Anthony 2009-05-06 13:42:51 UTC
0000500: 0000 0000 0000 00ff f360 0000 0000 0000  .........`......
0000510: 0000 0000 0000 0000 0000 0000 5869 6e67  ............Xing
0000520: 0000 0007 0000 30a1 0042 3a79 0103 0608  ......0..B:y....
0000530: 0b0e 1013 1618 1a1d 1f22 2427 292c 2f31  ........."$'),/1
0000540: 3436 393c 3e41 4346 494b 4e50 5355 585b  469<>ACFIKNPSUX[
0000550: 5d60 6365 686b 6d70 7376 787b 7e80 8385  ]`cehkmpsvx{~...
0000560: 888b 8d90 9395 989b 9ea0 a3a5 a8ab aeb0  ................
0000570: b3b6 b8bb bec0 c3c6 c9cc cfd2 d4d7 dadd  ................
0000580: dfe2 e5e8 eaed f0f3 f5f8 fafc feff ff00  ................
0000590: 0000 0000 0000 0000 0000 0000 0000 0000  ................
00005a0: 0000 00ff f310 540e 0000 012e 0000 0000  ......T.........
00005b0: 00a8 025c ac00 0000 8003 c08e 08ff f320  ...\........... 
00005c0: 5406 0000 012e 0000 0000 00a8 025c ac00  T............\..
00005d0: 0000 21f0 a000 0000 00ff 8007 c08e 0821  ..!............!
00005e0: f0a0 0000 0000 ff80 00c0 8e08 21f0 a000  ............!...

This is the header of the file that is having problems.

even if I set my offset to 0x01 and don't add offset afterward, then read_id is 0xfff31054, which I believe indicates that gst thinks the headers are starting a lot later than they are.
Comment 5 Jan Schmidt 2009-05-06 14:23:44 UTC
I have a patch for this here.
Comment 6 Anthony 2009-05-06 14:28:52 UTC
(In reply to comment #5)
> I have a patch for this here.
> 

Sorry if I'm just missing it, but where is the patch you're referring to?
Comment 7 Jan Schmidt 2009-05-06 14:30:18 UTC
Created attachment 134109 [details] [review]
make mp3parse more lenient when resynching.
Comment 8 Jan Schmidt 2009-05-06 14:31:07 UTC
Patch is attached now. Hopefully Sebastian agrees with it :)
Comment 9 Sebastian Dröge (slomo) 2009-05-06 14:37:43 UTC
Yeah, looks good :) You already had a patch before I started to look at this bug at all ;)
Comment 10 Jan Schmidt 2009-05-06 14:50:59 UTC
Thanks, pushed:

commit 85a88a0a6499ebc9049c47aed55d24a8358bed50
Author: Jan Schmidt <thaytan@noraisin.net>
Date:   Wed May 6 15:27:01 2009 +0100

    mp3parse: Allow more bits to change in headers during resynch
    
    Be more lenient about what we accept as changing bits in a header - basicall
    only require that the mp3 sync marker is present, for the mpeg version,
    layer and samplerate.
    
    Fixes: #581464