GNOME Bugzilla – Bug 678011
typefinding: some mpeg files are not identified as mpeg files
Last modified: 2013-11-21 16:48:04 UTC
When dealing with mpeg files in Ubuntu 12.04, Totem will not play nor show a Thumbnail in Nautilus for some mpeg files. Some mpeg files are playable and have a Thumbnail, but others are not. There is no difference between playable and non-playable files with respect to how they were created, the size of the file, the type of content, etc. It can be verified running the following command on the sample attached: $ gst-typefind <clip> The reason is that typefind function scans the first 128k of the clip and tries to find several PES packets. In playable clips there's audio muxed at the beginning of clips which uses small PES packets and the identity conditions can be satisfied. In both playable and non-playable clips, there's something quite unusual as there's a single Program Stream packet per video frame, that means that some of them are as big as 64K and the conditions to identify the clip are not satisfied. I can't tell if this clip wrongly muxed because I haven't checked on the spec but usually a video frame in mpeg PS is split in fragments of about 2K. The issue is in the typefind function. (all mpeg files that are non-playable in Totem are correctly identified as mpeg files, and playable, by video player VLC.)
Could you make one of those files available to use by any chance? The first 1MB might be enough already (head --bytes=999k foo.mpg > head.mpg)
Created attachment 216269 [details] testcase Let me know if you need more "content."
Could the priority of this bug be raised? I've got lots of video files that will not play with Totem because of this bug. And although I can play the files with VLC, no thumbnails are generated for the files because of Totem's inability to play the files. This issue has be a long-standing bug way before this bug report was even made. This issue was present even in ubuntu 10.04. Any way to add some priority to this bug so that it can get fixed sooner rather than later? I'm sure a lot of GNOME users would be most appreciative for the fix. Thanx
I can have a look at this. More sample files would be good, to make sure the fix catches all/most of these files that currently fail to typefind.
Thanks Tim-Philipp Muller. Much appreciated.
Created attachment 235185 [details] Sample_1
Created attachment 235186 [details] Sample_2
Created attachment 235187 [details] Sample_3
Created attachment 235188 [details] Sample_4
Created attachment 235189 [details] Sample_5
Created attachment 235190 [details] Sample_6
Created attachment 235191 [details] Sample_7
The detection of mpegps was broken in this commit: http://cgit.freedesktop.org/gstreamer/gst-plugins-base/commit/gst/typefind?h=BRANCH-RELEASE-0_10_19&id=1e2c32779299d99d8cff724614fc841acdb5b86b Instead of - len = MPEG2_MAX_PROBE_LENGTH * 2; - do { - len = len / 2; - data = gst_type_find_peek (tf, 0, 5 + len); - } while (data == NULL && len >= 32); we have + len = MPEG2_MAX_PROBE_LENGTH; + do { + len = len / 2; + data = gst_type_find_peek (tf, 0, 5 + len); + } while (data == NULL && len >= 32); To fix, simply revert len = MPEG2_MAX_PROBE_LENGTH; to original len = MPEG2_MAX_PROBE_LENGTH * 2; or reduce MPEG2_MIN_SYS_HEADERS twice.
yes, doubling the initial len value, or moving the len = len / 2; line to after the type_find_peek() would work.
Created attachment 259784 [details] [review] proposed fix I'm doing the latter and adjusting the rest of the code to match ('end' will be off if not doing this and wont work as expected). Pavel, thanks for spotting this but while your assumption is correct your no-op MPEG2_MAX_PROBE_LENGTH *2 && /2 at the beginning of the cycle disturbs me some. Attached patch makes all 7 samples being correctly detected.
Created attachment 259785 [details] [review] proposed fix wrong email in the previous one
Review of attachment 259785 [details] [review]: Arguably, by removing the 'no-op divide by 2', you've made the code *less* readable - since now the len when comparing and then exiting the loop is no longer the len that was actually peeked, but OK.
Created attachment 259814 [details] [review] proposed fix You're right, found it a bit weird myself but was too tired to try and refactor it. I'm committing this one instead. It should address your readability concerns.
Review of attachment 259814 [details] [review]: OK
Cherry-picked into 1.2 branch.