GNOME Bugzilla – Bug 699398
mpegvideoparse: couple of issues
Last modified: 2013-05-04 19:49:35 UTC
When encountering new sequence header, the parser compares its size to previous one and when it differs it updates codec data and sends caps again. This is not necessary. Unless the actual header changes (first 11 bytes) the quantization matrices in codec data should not be updated, otherwise it causes unnecessary decoder reopen. Second issue is that when starting the parsing in the middle of GOP, the parser outputs frames before it has chance to parse the sequence header. This results in completely wrong caps (i.e. mpegversion=1 for mpeg 2).
Created attachment 242999 [details] [review] don't compare sequence header size
Created attachment 243000 [details] [review] don't send buffers until sequence header is parsed
Comment on attachment 242999 [details] [review] don't compare sequence header size commit 946ffd0da5a7bd9b004f02e306e061d1b7f52b4b Author: Matej Knopp <matej.knopp@gmail.com> Date: Wed May 1 17:19:07 2013 +0200 mpegvideoparse: don't compare buffer size when checking whether to update caps https://bugzilla.gnome.org/show_bug.cgi?id=699398
Review of attachment 243000 [details] [review]: Is this still needed after commit f1a6d84a6c4595a94c5654131a74b6a979f53a04 Author: Tim-Philipp Müller <tim.muller@collabora.co.uk> Date: Wed May 1 17:36:47 2013 +0100 mpegvideoparse: don't announce incomplete source caps Don't send any source caps yet if we're still in drop-buffers-until-we-get-a-sequence-header mode. Fixes transmuxing of many MPEG-TS/PS streams into formats which require things like width, height or codec_data on the input caps. Also fixes issues when using playbin with decoder sinks that want width/height etc. https://bugzilla.gnome.org/show_bug.cgi?id=695879 ::: gst/videoparsers/gstmpegvideoparse.c @@ +480,3 @@ ret = TRUE; + if (!mpvparse->config) + ret = FALSE; Wouldn't this cause the frames before we have a config to be either dropped or be sent together with the next frame after we got a config? I.e. either lose data or accumulate multiple frames plus config in the middle in a single buffer?
It shouldn't be needed anymore. I think it should cause the parser to skip it, but I might be wrong.
I also think it shouldn't be needed any more, so let's close this.