GNOME Bugzilla – Bug 725341
playbin: detect if video-sink supports deinterlacing
Last modified: 2018-11-03 11:29:09 UTC
The idea here is to make playbin able to detect if the video-sink supports deinterlacing and insert whether or not the deinterlace element (and its relative bin). This should improve the situation of hardware decoders vs interlaced content when the decoder does not do the deinterlacing himself. Two solutions have been proposed: * introduce a deinterlace interface, and implement it in the sink. The interface can add the possibility to choose a particular deinterlacing method from what the element supports. * use the element Klass, but there is no convention yet to do something like Video/Sink + Filter/Effect/Video/Deinterlace. Maybe Video/Sink/[Filter list ...] ? The main issue I see here is, if overlaying is required (thinking about subtitles), it should only be done after deinterlacing, so the sink might also need the overlay feature if we don't insert the deinterlace element.
Created attachment 270505 [details] [review] WIP playsink: check if video sink supports deinterlace and insert the deinterlace element accordingly Proof of concept attached. Missing: if deinterlacing is disabled, the deinterlacing feature should be disabled on the sink.
Review of attachment 270505 [details] [review]: Good PoC. Here's my list of missing: - As you mention if there is subtitle and sink don't support it we are a bit screwed, not sure how to detect that and fallback correctly - Totem uses the flags to enable/disable interlacing, though with that solution there is not mean to tell the sink not to de-interlace, at the same time I feel like this option was there to let user see something even though they didn't have a fast enough CPU. So mostly a relic. Even the worst de-interlacing looks better then no-deinterlacing imho. I'm open to discussion, though I personally like the idea of keeping it simple like this PoC is doing. ::: gst/playback/gstplaysink.c @@ +1499,3 @@ + *ret |= TRUE; + } else { + *ret |= FALSE; A bit too complex, just "*ret = TRUE" would do ;-P.
Review of attachment 270505 [details] [review]: No idea about the subtitle overlay part here. That would currently be a bit annoying to handle as we can't easily get here from the sink if it does support overlay composition or not, and also don't know if the subtitle renderer supports it. Needs some more thoughts. Same goes for rescaling btw, but I would say it's save to assume that any sink that can do deinterlacing can also do scaling ;) But otherwise this is how it is supposed to be, yes. ::: gst/playback/gstplaysink.c @@ +1516,3 @@ + } + + it = gst_bin_iterate_elements (GST_BIN (element)); iterate_recurse() (probably same change needed for the colorbalance stuff). bins might be nested @@ +1518,3 @@ + it = gst_bin_iterate_elements (GST_BIN (element)); + while (gst_iterator_foreach (it, iterate_deinterlace_elements, + &ret) == GST_ITERATOR_RESYNC) Why not gst_iterator_find_custom()? @@ +3127,3 @@ + need_deinterlace = + !video_sink_supports_deinterlacing (GST_ELEMENT + (playsink->video_sink)); Please make sure that this is reconfigure-safe. Especially if previously we needed a deinterlace element but now don't, we need to make sure that it is properly removed.
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/issues/113.