GNOME Bugzilla – Bug 719636
deinterlace: alters caps in passthrough mode preventing hardware decode
Last modified: 2018-01-29 10:21:29 UTC
I've been trying to get playbin to work with the vaapi plugins out of the box and with the help of several people on the #gstreamer channel we finally figured out one of the problem was in the deinterlace plugin. It seems deinterlace alterates caps even though it's in passthrough mode, which strips out the meta:GstVideoGLTextureUploadMeta capabilities of our video sink. What follows is that the vaapidecode doesn't get to know about this caps and it just happens to not link or not do software decode. Attached is a patch to prevent the filtering in passthrough mode.
Created attachment 263232 [details] [review] deinterlace: don't alterate peer's caps in passthrough mode
Comment on attachment 263232 [details] [review] deinterlace: don't alterate peer's caps in passthrough mode You can't check self->passthrough in getcaps, as this is supposed to give you everything that you can do, not what is currently configured. In this case, I think deinterlace should potentially just prepend the peer caps.
That said, we can't just append them. playbin probably needs to add some kind of passthrough mode. Something like a autoconvert containing identity and deinterlace.
Also that patch is incomplete, deinterlace can also handle all caps that are progressive in automatic mode, and can handle all caps always in disabled mode. In interlaced mode it can only handle the caps that it can actually handle. I think basing the getcaps behaviour on the mode would be a good idea. However that will make negotiation fail in auto mode for all caps that deinterlace does not understand and which are interlaced, e.g. "video/x-raw(meta:GstVideoGLTextureUploadMeta),interlaced=true". Alternatively we could also add a third mode that always allows everything, and only does deinterlacing if it can do that and the input is interlaced. Some kind of best-effort-auto mode.
(In reply to comment #3) > That said, we can't just append them. playbin probably needs to add some kind > of passthrough mode. Something like a autoconvert containing identity and > deinterlace. Then you're moving the problem from deinterlace to playbin without making it easier ;)
Created attachment 263687 [details] [review] deinterlace: support ANY caps features if deinterlace mode allows it Here is an attempt to solve the issue, taking into account Sebastian's remarks.
Created attachment 263688 [details] [review] deinterlace: support ANY caps features if deinterlace mode allows it
See also this related patch which enable the deinterlaced element to disable itself when it does not support the input caps features: https://bugzilla.gnome.org/show_bug.cgi?id=720388 And this patch for letting playbin plugs elements with caps features: https://bugzilla.gnome.org/show_bug.cgi?id=720205
Review of attachment 263688 [details] [review]: ::: gst/deinterlace/gstdeinterlace.c @@ +279,3 @@ +#define DEINTERLACE_ALL_CAPS DEINTERLACE_CAPS ";" \ + GST_VIDEO_CAPS_MAKE_WITH_FEATURES ("ANY", DEINTERLACE_VIDEO_FORMATS) This is not correct IMHO, it should support any format in passthrough mode. Not just DEINTERLACE_VIDEO_FORMATS.
Created attachment 264286 [details] [review] deinterlace: support any video formats and any caps features if deinterlace mode allows it
Looks good, will merge it when #720388 is ready and we have a unit test
Comment on attachment 264286 [details] [review] deinterlace: support any video formats and any caps features if deinterlace mode allows it Actually not completely. In the !peercaps case you should also do the same instead of just using the template caps.
Created attachment 264306 [details] [review] deinterlace: support any video formats and any caps features if deinterlace mode allows it
commit 0bbdb9bb1d9408dbbe753b47c3b919ddc285511c Author: Matthieu Bouron <matthieu.bouron@collabora.com> Date: Fri Dec 6 17:08:54 2013 +0000 deinterlace: support any video formats and any caps features if deinterlace mode allows it https://bugzilla.gnome.org/show_bug.cgi?id=719636
Since that patch playing: http://files.sounddevices.com/prores422_proxy_29.97.mov fails with: ERROR:gstdeinterlace.c:426:gst_deinterlace_set_method: assertion failed: (method_type != G_TYPE_INVALID) Reverting it makes it working.
Created attachment 266444 [details] [review] deinterlace: do no try to set deinterlace method if passthrough is enabled
It would appear that deinterlace doesn't really support all the formats which the recent patches make it claim to support. And while the most recent patch fixes issues where content is in unsupported formats but already progressive it doesn't handle cases where the content is in an unsupported format and still interlaced. For example interlaced I422_10LE content will fail, this can be triggered with the following pipeline: gst-launch-1.0 videotestsrc ! video/x-raw,format=I422_10LE,interlace-mode=mixed ! deinterlace ! fakesink If deinterlace was reporting the correct sink caps this could be avoided via a videoconvert element, but due to the incorrect caps videoconvert doesn't do anything (since its already in a format that all elements claim to accept).
Yeah, there's something not really right here. This needs further changes, especially something more along the lines of the videobalance/textoverlay changes that Matthieu currently works on.
Comment on attachment 266444 [details] [review] deinterlace: do no try to set deinterlace method if passthrough is enabled commit 200eb7498d01937685a0e2d48df4d7b7b62e83d6 Author: Matthieu Bouron <matthieu.bouron@collabora.com> Date: Thu Jan 16 11:26:41 2014 +0000 deinterlace: do not try set deinterlace method if passthrough is enabled Fixes an issue with progressive content and unsupported video formats for the deinterlace method. https://bugzilla.gnome.org/show_bug.cgi?id=719636
Not sure I understand what needs to be done here. The pipeline from comment #17 seems to work just fine here these days?
I don't really remember :)
Yeah, it's been a while. Let's close this then. If anyone remembers, please re-open, otherwise I'm sure people will file new bugs if there are still issues.