GNOME Bugzilla – Bug 578926
[h264parse] Make sure h264parse are autoplugged when required
Last modified: 2011-09-07 10:06:56 UTC
When I view an MPEG4 stream from a Sony camera with playbin or playbin2 (or decodebin or decodebin2), the decoded stream is weird. It looks like only the top row or so of macroblocks is really decoded, and the rest only has differential info (probably the I-frames aren't detected). I get the same effect with a manually constructed pipeline: souphttpsrc location=... ! ffdec_mpeg4 ! autovideosink Inserting an mpeg4videoparse solves this: souphttpsrc location=... ! mpeg4videoparse ! ffdec_mpeg4 ! autovideosink This is using -base 0.10.21, -bad 0.10.9 and -ffmpeg 0.10.4.
Created attachment 132639 [details] about 8 seconds of MPEG4 captured from Sony camera This is about 8 seconds (truncated at 950K) of raw MPEG4 video captured from the Sony camera using gst-launch souphttpsrc location=... ! filesink location=sony.mp4 Just in case you can't reproduce the problem, the video is actually a screenshot of what you get on my installation.
To solve this, I propose adding the 'parsed=(boolean)false' field to the caps found by typefind, and 'parsed=(boolean)true' to the caps of ffmpeg. The same issue also applies to h264. Currently, h264parse doesn't have the 'parsed' field, so I'm going to add that as well. Patches follow. I'll clone this bug for -ffmpeg.
Created attachment 132822 [details] [review] Assume MPEG-4 stream is not parsed. Set the 'parsed' field of the MPEG-4 caps to 'false'. This forces an mpeg4parse element to be inserted before the stream can be decoded. Unfortunately, it is not possible to detect if the stream is already parsed or not. However, that typically happens only if a demuxer or depayloader comes in front of it, which also sets the caps anyway. Only a multifilesrc (with a sufficiently high buffer size) may produce a parsed stream.
For mp3, we fixed this by raising the rank of the parser high enough that it's always preferred over all the decoders, and having 'parsed=false' on the sinkpad and 'parsed=true' of the srcpad of the parser, to avoid having an endless chain of them plugged. It's slightly better than changing the typefind caps, imho, because it's a bit more explicit, and a demuxer that does output parsed data, can add 'parsed=true' to the caps it produces, to avoid the parser if it wants.
Sounds good! Except... that means that the mpeg4videoparse will always be inserted, even if the stream is already parsed (i.e. coming out of a demuxer or depayloader). That incurs some overhead because mpeg4videoparse is scanning over the stream byte-by-byte. However, I just did some measurements and it looks like mpeg4videoparse takes less than 10% of the time of ffdec_mpeg4 (which is already pretty fast). Conclusion: raise the rank of mpeg4videoparse.
Like I said, we can mark 'parsed=true' on such demuxers/depayloaders' caps and it'll skip mpeg4videoparse
Created attachment 133032 [details] [review] mpeg4videoparse: increased rank above that of ffdec_mpeg4 As proposed.
Created attachment 133034 [details] [review] [gst-plugins-good] Add 'parsed=true' to caps of rtpmp4vdepay, rtpmp4gdepay, and matroska MPEG-4 These are the main depayloaders/demuxers for MPEG-4. Missing: * ffdemux_mpeg4: rank is NONE anyway. * avidemux/ogmdemux: riff-media doesn't even set systemstream=false, so I don't dare touching it.
The same is actually true for h264 (and probably others). Patches follow.
Created attachment 133035 [details] [review] Make sure h264parse is autoplugged before H.264 decoder Added 'parsed' property to h264parse caps and increased its rank above that of ffdec_h264.
Created attachment 133037 [details] [review] [gst-plugins-good] Add 'parsed=true' to caps of rtph264depay and matroska MPEG-4. Same comments as for MPEG-4.
*** Bug 556133 has been marked as a duplicate of this bug. ***
Except the rank changes this could immediately be committed. Could you separate the rank change of h264parse from the parsed=true setting? The other patches are good and I'll commit them all in one go later. Also, MPEG 4 video will also often be inside qt/mp4 files so qtdemux should be adjusted too.
Created attachment 141625 [details] [review] Make sure h264parse can be autoplugged I removed the one line from the h264parse patch that does the rank change. I didn't actually do this on the git tree but just modified the patch, so it may not apply cleanly to HEAD... qtdemux is still todo.
It looks like these patches never got committed. Is there still a problem with them?
*** Bug 601262 has been marked as a duplicate of this bug. ***
mpeg4videoparse has a high enough rank now, only h264parse is left. I don't know why its rank was not increased yet.
Created attachment 189603 [details] [review] fix video parsers rank following this idea of David: https://bugzilla.gnome.org/show_bug.cgi?id=649542#c10 I propose to set rank of all parsers in -bad/gst/videoparsers/ directory to PRIMARY -10.
Comment on attachment 189603 [details] [review] fix video parsers rank This should be done on a case-by-case basis. Also h264parse has PRIMARY+1 now
This is fixed in -bad git now. h264parse is autoplugged, but only h264parse and none of the other parsers in the videoparsers plugin.