GNOME Bugzilla – Bug 660598
playbin2: Make sure that elements that are plugged are compatible with the fixed sink
Last modified: 2011-10-03 13:21:57 UTC
Created attachment 197918 [details] [review] Aims at fixing the issue avoiding to plug hardware accelerated video decoders in the case the sink doesn't support them. In the case where we have hardware accelerated decoders on the system (especially vaapi elements that are actually plugged), and the user is providing a sink that doesn't support the surface, we end up having incompatibility between the decoder and the sink. A simple example that shows how it crashes on a system where gstreamer-vaapi is installed: gst-launch playbin2 video-sink=xvimagesink uri=/codec/supported/by/vaapi What should be done in this case, is either, avoid using the accelerated decoder and plug a "normal" decoder instead (if available), or plug a converter after the decoder. Also after fixing this issue, we might want to finally raise the rank of the vdpau elements so they are actually plugged in the playbin2 when available on the system.
This is a duplicate of bug #656923, or more correct the fix for it, right? I'll review it later, thanks!
Review of attachment 197918 [details] [review]: Looks good in general, just two comments: ::: gst/playback/gstplaybin2.c @@ +3231,3 @@ + /* If it is not a video element and we don't have a fixed video sink + * we try the factory */ + if (!strstr (klass, "Video") || !group->video_sink) { Don't do this only for video but also for audio and the audio-sink @@ +3240,3 @@ + + if ((sinkpad = gst_element_get_static_pad (group->video_sink, "sink"))) { + caps = gst_pad_get_caps (sinkpad); gst_pad_get_caps_reffed() here, and don't forget to unref the caps after use
*** Bug 656923 has been marked as a duplicate of this bug. ***
Review of attachment 197918 [details] [review]: ::: gst/playback/gstplaybin2.c @@ +3231,3 @@ + /* If it is not a video element and we don't have a fixed video sink + * we try the factory */ + if (!strstr (klass, "Video") || !group->video_sink) { Also use the gst_element_factory_list_is_type() API here with MEDIA_VIDEO and MEDIA_IMAGE (and for audio with MEDIA_AUDIO) instead of the manual comparison. @@ +3242,3 @@ + caps = gst_pad_get_caps (sinkpad); + + compatible = gst_element_factory_can_src_any_caps (factory, caps); Do this compatibility check only for GST_ELEMENT_FACTORY_TYPE_DECODER, otherwise you'll never plug parsers or demuxers anymore
Created attachment 198081 [details] [review] New version of the patch taking into account the comments
commit 12a54ae4dd8fb54a0e17949580ac854c62d1086c Author: Sebastian Dröge <sebastian.droege@collabora.co.uk> Date: Mon Oct 3 15:20:06 2011 +0200 playbin2: Minor cleanup of decoder-sink compatibility checking code commit a123195dd0ef850b65a87724d60a25061bb1e14d Author: Thibault Saunier <thibault.saunier@collabora.com> Date: Fri Sep 30 12:29:34 2011 -0300 playbin2: Make sure that the decoders we plug are compatible with the fixed sink The fact that a decoder is not compatible with the fixed sink is currently happenning in the case where we have hardware accelerated video decoders on the system (especially vaapi elements that are actually plugged), and the user is providing a sink that doesn't support the surface. A simple example that shows how it used to crash on a system where gstreamer-vaapi is installed: gst-launch playbin2 video-sink=xvimagesink uri=/codec/supported/by/vaapi What we are now doing in this case, is avoid using the accelerated decoder and plug a "normal" decoder instead (if avalaible). This commit doesn't handle the case where we have hardware accelerated demuxing.