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 678011 - typefinding: some mpeg files are not identified as mpeg files
typefinding: some mpeg files are not identified as mpeg files
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other All
: Normal major
: 1.2.2
Assigned To: Reynaldo H. Verdejo Pinochet
GStreamer Maintainers
Depends on:
Reported: 2012-06-13 12:14 UTC by gsbbj
Modified: 2013-11-21 16:48 UTC
See Also:
GNOME target: ---
GNOME version: ---

testcase (999.00 KB, video/mpg)
2012-06-13 12:33 UTC, gsbbj
Sample_1 (999.00 KB, video/mpg)
2013-02-05 02:30 UTC, greg
Sample_2 (999.00 KB, video/mpg)
2013-02-05 02:31 UTC, greg
Sample_3 (999.00 KB, video/mpg)
2013-02-05 02:31 UTC, greg
Sample_4 (999.00 KB, video/mpg)
2013-02-05 02:32 UTC, greg
Sample_5 (999.00 KB, video/mpg)
2013-02-05 02:32 UTC, greg
Sample_6 (999.00 KB, video/mpg)
2013-02-05 02:32 UTC, greg
Sample_7 (999.00 KB, video/mpg)
2013-02-05 02:33 UTC, greg
proposed fix (1.01 KB, patch)
2013-11-14 04:51 UTC, Reynaldo H. Verdejo Pinochet
none Details | Review
proposed fix (1.01 KB, patch)
2013-11-14 04:59 UTC, Reynaldo H. Verdejo Pinochet
accepted-commit_now Details | Review
proposed fix (1008 bytes, patch)
2013-11-14 15:25 UTC, Reynaldo H. Verdejo Pinochet
committed Details | Review

Description gsbbj 2012-06-13 12:14:01 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.)
Comment 1 Tim-Philipp Müller 2012-06-13 12:22:56 UTC
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)
Comment 2 gsbbj 2012-06-13 12:33:35 UTC
Created attachment 216269 [details]

Let me know if you need more "content."
Comment 3 greg 2013-01-30 19:34:58 UTC
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.

Comment 4 Tim-Philipp Müller 2013-02-04 05:25:17 UTC
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.
Comment 5 greg 2013-02-05 02:25:24 UTC
Thanks Tim-Philipp Muller. Much appreciated.
Comment 6 greg 2013-02-05 02:30:27 UTC
Created attachment 235185 [details]
Comment 7 greg 2013-02-05 02:31:08 UTC
Created attachment 235186 [details]
Comment 8 greg 2013-02-05 02:31:36 UTC
Created attachment 235187 [details]
Comment 9 greg 2013-02-05 02:32:03 UTC
Created attachment 235188 [details]
Comment 10 greg 2013-02-05 02:32:29 UTC
Created attachment 235189 [details]
Comment 11 greg 2013-02-05 02:32:57 UTC
Created attachment 235190 [details]
Comment 12 greg 2013-02-05 02:33:25 UTC
Created attachment 235191 [details]
Comment 13 Pavel Bludov 2013-10-28 07:17:19 UTC
The detection of mpegps was broken in this commit:

Instead of
- do {
- len = len / 2;
- data = gst_type_find_peek (tf, 0, 5 + len);
- } while (data == NULL && len >= 32);
we have
+ do {
+ len = len / 2;
+ data = gst_type_find_peek (tf, 0, 5 + len);
+ } while (data == NULL && len >= 32);

To fix, simply revert


to original


or reduce MPEG2_MIN_SYS_HEADERS twice.
Comment 14 Jan Schmidt 2013-11-14 00:39:27 UTC
yes, doubling the initial len value, or moving the len = len / 2; line to after the type_find_peek() would work.
Comment 15 Reynaldo H. Verdejo Pinochet 2013-11-14 04:51:29 UTC
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.
Comment 16 Reynaldo H. Verdejo Pinochet 2013-11-14 04:59:49 UTC
Created attachment 259785 [details] [review]
proposed fix

wrong email in the previous one
Comment 17 Jan Schmidt 2013-11-14 06:35:52 UTC
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.
Comment 18 Reynaldo H. Verdejo Pinochet 2013-11-14 15:25:28 UTC
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
Comment 19 Jan Schmidt 2013-11-15 02:41:26 UTC
Review of attachment 259814 [details] [review]:

Comment 20 Tim-Philipp Müller 2013-11-21 16:48:04 UTC
Cherry-picked into 1.2 branch.