From ae3ba533914d7961248190bc56734d7b6fe2b716 Mon Sep 17 00:00:00 2001 From: Vincent Penquerc'h Date: Mon, 17 Oct 2011 21:05:30 +0000 Subject: [PATCH] playsink: handle after-the-fact changes in converters/volume booleans The playsink was nastily poking a boolean in the structure. Make those booleans properties, so we are told when they change, and rebuild the conversion bin when they do. Some cleanup to go with it too. https://bugzilla.gnome.org/show_bug.cgi?id=661262 --- gst/playback/gstplaysink.c | 23 ++++----- gst/playback/gstplaysinkaudioconvert.c | 88 ++++++++++++++++++++++++++++++++-- gst/playback/gstplaysinkaudioconvert.h | 3 +- gst/playback/gstplaysinkconvertbin.c | 68 +++++++++++++------------- gst/playback/gstplaysinkconvertbin.h | 5 +- gst/playback/gstplaysinkvideoconvert.c | 10 ++-- gst/playback/gstplaysinkvideoconvert.h | 1 - 7 files changed, 142 insertions(+), 56 deletions(-) diff --git a/gst/playback/gstplaysink.c b/gst/playback/gstplaysink.c index d642fa0..3639f6c 100644 --- a/gst/playback/gstplaysink.c +++ b/gst/playback/gstplaysink.c @@ -1799,9 +1799,15 @@ gen_audio_chain (GstPlaySink * playsink, gboolean raw) if (!(playsink->flags & GST_PLAY_FLAG_NATIVE_AUDIO) || (!have_volume && playsink->flags & GST_PLAY_FLAG_SOFT_VOLUME)) { - GST_DEBUG_OBJECT (playsink, "creating audioconvert"); + gboolean use_converters = !(playsink->flags & GST_PLAY_FLAG_NATIVE_AUDIO); + gboolean use_volume = + !have_volume && playsink->flags & GST_PLAY_FLAG_SOFT_VOLUME; + GST_DEBUG_OBJECT (playsink, + "creating audioconvert with use-converters %d, use-volume %d", + use_converters, use_volume); chain->conv = - g_object_new (GST_TYPE_PLAY_SINK_AUDIO_CONVERT, "name", "aconv", NULL); + g_object_new (GST_TYPE_PLAY_SINK_AUDIO_CONVERT, "name", "aconv", + "use-converters", use_converters, "use-volume", use_volume, NULL); gst_bin_add (bin, chain->conv); if (prev) { if (!gst_element_link_pads_full (prev, "src", chain->conv, "sink", @@ -1812,11 +1818,6 @@ gen_audio_chain (GstPlaySink * playsink, gboolean raw) } prev = chain->conv; - GST_PLAY_SINK_AUDIO_CONVERT_CAST (chain->conv)->use_converters = - !(playsink->flags & GST_PLAY_FLAG_NATIVE_AUDIO); - GST_PLAY_SINK_AUDIO_CONVERT_CAST (chain->conv)->use_volume = (!have_volume - && playsink->flags & GST_PLAY_FLAG_SOFT_VOLUME); - if (!have_volume && playsink->flags & GST_PLAY_FLAG_SOFT_VOLUME) { GstPlaySinkAudioConvert *conv = GST_PLAY_SINK_AUDIO_CONVERT_CAST (chain->conv); @@ -1964,13 +1965,13 @@ setup_audio_chain (GstPlaySink * playsink, gboolean raw) G_CALLBACK (notify_mute_cb), playsink); } - GST_PLAY_SINK_AUDIO_CONVERT_CAST (chain->conv)->use_volume = FALSE; + g_object_set (chain->conv, "use-volume", FALSE, NULL); } else { GstPlaySinkAudioConvert *conv = GST_PLAY_SINK_AUDIO_CONVERT_CAST (chain->conv); /* no volume, we need to add a volume element when we can */ - conv->use_volume = TRUE; + g_object_set (chain->conv, "use-volume", TRUE, NULL); GST_DEBUG_OBJECT (playsink, "the sink has no volume property"); /* Disconnect signals */ @@ -3028,14 +3029,14 @@ caps_notify_cb (GstPad * pad, GParamSpec * unused, GstPlaySink * playsink) if (pad == playsink->audio_pad) { raw = is_raw_pad (pad); - reconfigure = (! !playsink->audio_pad_raw != ! !raw) + reconfigure = (!!playsink->audio_pad_raw != !!raw) && playsink->audiochain; GST_DEBUG_OBJECT (pad, "Audio caps changed: raw %d reconfigure %d caps %" GST_PTR_FORMAT, raw, reconfigure, caps); } else if (pad == playsink->video_pad) { raw = is_raw_pad (pad); - reconfigure = (! !playsink->video_pad_raw != ! !raw) + reconfigure = (!!playsink->video_pad_raw != !!raw) && playsink->videochain; GST_DEBUG_OBJECT (pad, "Video caps changed: raw %d reconfigure %d caps %" GST_PTR_FORMAT, raw, diff --git a/gst/playback/gstplaysinkaudioconvert.c b/gst/playback/gstplaysinkaudioconvert.c index b470939..acfd6ef 100644 --- a/gst/playback/gstplaysinkaudioconvert.c +++ b/gst/playback/gstplaysinkaudioconvert.c @@ -34,15 +34,26 @@ GST_DEBUG_CATEGORY_STATIC (gst_play_sink_audio_convert_debug); G_DEFINE_TYPE (GstPlaySinkAudioConvert, gst_play_sink_audio_convert, GST_TYPE_PLAY_SINK_CONVERT_BIN); +enum +{ + PROP_0, + PROP_USE_CONVERTERS, + PROP_USE_VOLUME, +}; + static gboolean -gst_play_sink_audio_convert_add_conversion_elements (GstPlaySinkConvertBin * - cbin) +gst_play_sink_audio_convert_add_conversion_elements (GstPlaySinkAudioConvert * + self) { - GstPlaySinkAudioConvert *self = GST_PLAY_SINK_AUDIO_CONVERT (cbin); + GstPlaySinkConvertBin *cbin = GST_PLAY_SINK_CONVERT_BIN (self); GstElement *el, *prev = NULL; g_assert (cbin->conversion_elements == NULL); + GST_DEBUG_OBJECT (self, + "Building audio conversion with use-converters %d, use-volume %d", + self->use_converters, self->use_volume); + if (self->use_converters) { el = gst_play_sink_convert_bin_add_conversion_element_factory (cbin, "audioconvert", "conv"); @@ -73,6 +84,7 @@ gst_play_sink_audio_convert_add_conversion_elements (GstPlaySinkConvertBin * } prev = el; } + return TRUE; link_failed: @@ -91,6 +103,59 @@ gst_play_sink_audio_convert_finalize (GObject * object) } static void +gst_play_sink_audio_convert_set_property (GObject * object, guint prop_id, + const GValue * value, GParamSpec * pspec) +{ + GstPlaySinkAudioConvert *self = GST_PLAY_SINK_AUDIO_CONVERT_CAST (object); + gboolean v, changed = FALSE; + + switch (prop_id) { + case PROP_USE_CONVERTERS: + v = g_value_get_boolean (value); + if (v != self->use_converters) { + self->use_converters = v; + changed = TRUE; + } + break; + case PROP_USE_VOLUME: + v = g_value_get_boolean (value); + if (v != self->use_volume) { + self->use_volume = v; + changed = TRUE; + } + break; + default: + break; + } + + if (changed) { + GstPlaySinkConvertBin *cbin = GST_PLAY_SINK_CONVERT_BIN (self); + GST_DEBUG_OBJECT (self, "Rebuilding converter bin"); + gst_play_sink_convert_bin_remove_elements (cbin); + gst_play_sink_audio_convert_add_conversion_elements (self); + gst_play_sink_convert_bin_cache_converter_caps (cbin); + } +} + +static void +gst_play_sink_audio_convert_get_property (GObject * object, guint prop_id, + GValue * value, GParamSpec * pspec) +{ + GstPlaySinkAudioConvert *self = GST_PLAY_SINK_AUDIO_CONVERT_CAST (object); + + switch (prop_id) { + case PROP_USE_CONVERTERS: + g_value_set_boolean (value, self->use_converters); + break; + case PROP_USE_VOLUME: + g_value_set_boolean (value, self->use_volume); + break; + default: + break; + } +} + +static void gst_play_sink_audio_convert_class_init (GstPlaySinkAudioConvertClass * klass) { GObjectClass *gobject_class; @@ -103,6 +168,18 @@ gst_play_sink_audio_convert_class_init (GstPlaySinkAudioConvertClass * klass) gstelement_class = (GstElementClass *) klass; gobject_class->finalize = gst_play_sink_audio_convert_finalize; + gobject_class->set_property = gst_play_sink_audio_convert_set_property; + gobject_class->get_property = gst_play_sink_audio_convert_get_property; + + g_object_class_install_property (gobject_class, PROP_USE_CONVERTERS, + g_param_spec_boolean ("use-converters", "Use converters", + "Whether to use conversion elements", FALSE, + G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY | G_PARAM_STATIC_STRINGS)); + + g_object_class_install_property (gobject_class, PROP_USE_VOLUME, + g_param_spec_boolean ("use-volume", "Use volume", + "Whether to use a volume element", FALSE, + G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY | G_PARAM_STATIC_STRINGS)); gst_element_class_set_details_simple (gstelement_class, "Player Sink Audio Converter", "Audio/Bin/Converter", @@ -116,8 +193,6 @@ gst_play_sink_audio_convert_init (GstPlaySinkAudioConvert * self) GstPlaySinkConvertBin *cbin = GST_PLAY_SINK_CONVERT_BIN (self); cbin->audio = TRUE; - cbin->add_conversion_elements = - gst_play_sink_audio_convert_add_conversion_elements; /* FIXME: Only create this on demand but for now we need * it to always exist because of playsink's volume proxying @@ -126,4 +201,7 @@ gst_play_sink_audio_convert_init (GstPlaySinkAudioConvert * self) self->volume = gst_element_factory_make ("volume", "volume"); if (self->volume) gst_object_ref_sink (self->volume); + + gst_play_sink_audio_convert_add_conversion_elements (self); + gst_play_sink_convert_bin_cache_converter_caps (cbin); } diff --git a/gst/playback/gstplaysinkaudioconvert.h b/gst/playback/gstplaysinkaudioconvert.h index 5f80af6..ad532e2 100644 --- a/gst/playback/gstplaysinkaudioconvert.h +++ b/gst/playback/gstplaysinkaudioconvert.h @@ -36,7 +36,6 @@ G_BEGIN_DECLS (G_TYPE_CHECK_INSTANCE_TYPE ((obj), GST_TYPE_PLAY_SINK_AUDIO_CONVERT)) #define GST_IS_PLAY_SINK_AUDIO_CONVERT_CLASS(klass) \ (G_TYPE_CHECK_CLASS_TYPE ((klass), GST_TYPE_PLAY_SINK_AUDIO_CONVERT)) - typedef struct _GstPlaySinkAudioConvert GstPlaySinkAudioConvert; typedef struct _GstPlaySinkAudioConvertClass GstPlaySinkAudioConvertClass; @@ -46,8 +45,8 @@ struct _GstPlaySinkAudioConvert /* < pseudo public > */ GstElement *volume; - gboolean use_volume; gboolean use_converters; + gboolean use_volume; }; struct _GstPlaySinkAudioConvertClass diff --git a/gst/playback/gstplaysinkconvertbin.c b/gst/playback/gstplaysinkconvertbin.c index 35faf4a..afd4b75 100644 --- a/gst/playback/gstplaysinkconvertbin.c +++ b/gst/playback/gstplaysinkconvertbin.c @@ -147,6 +147,8 @@ gst_play_sink_convert_bin_set_targets (GstPlaySinkConvertBin * self, GstPad *pad; GstElement *head, *tail; + GST_DEBUG_OBJECT (self, "Setting pad targets with passthrough %d", + passthrough); if (self->conversion_elements == NULL || passthrough) { if (!passthrough) { GST_WARNING_OBJECT (self, @@ -159,10 +161,12 @@ gst_play_sink_convert_bin_set_targets (GstPlaySinkConvertBin * self, } pad = gst_element_get_static_pad (head, "sink"); + GST_DEBUG_OBJECT (self, "Ghosting bin sink pad to %" GST_PTR_FORMAT, pad); gst_ghost_pad_set_target (GST_GHOST_PAD_CAST (self->sinkpad), pad); gst_object_unref (pad); pad = gst_element_get_static_pad (tail, "src"); + GST_DEBUG_OBJECT (self, "Ghosting bin src pad to %" GST_PTR_FORMAT, pad); gst_ghost_pad_set_target (GST_GHOST_PAD_CAST (self->srcpad), pad); gst_object_unref (pad); } @@ -289,6 +293,7 @@ gst_play_sink_convert_bin_sink_setcaps (GstPad * pad, GstCaps * caps) gboolean reconfigure = FALSE; gboolean raw; + GST_DEBUG_OBJECT (pad, "setcaps: %" GST_PTR_FORMAT, caps); GST_PLAY_SINK_CONVERT_BIN_LOCK (self); s = gst_caps_get_structure (caps, 0); name = gst_structure_get_name (s); @@ -299,6 +304,8 @@ gst_play_sink_convert_bin_sink_setcaps (GstPad * pad, GstCaps * caps) raw = g_str_has_prefix (name, "video/x-raw-"); } + GST_DEBUG_OBJECT (self, "raw %d, self->raw %d, blocked %d", + raw, self->raw, gst_pad_is_blocked (self->sink_proxypad)); if (raw) { if (!self->raw && !gst_pad_is_blocked (self->sink_proxypad)) { GST_DEBUG_OBJECT (self, "Changing caps from non-raw to raw"); @@ -376,24 +383,49 @@ gst_play_sink_convert_bin_getcaps (GstPad * pad) return ret; } +void +gst_play_sink_convert_bin_remove_elements (GstPlaySinkConvertBin * self) +{ + if (self->conversion_elements) { + g_list_foreach (self->conversion_elements, + (GFunc) gst_play_sink_convert_bin_remove_element, self); + g_list_free (self->conversion_elements); + self->conversion_elements = NULL; + } + if (self->identity) { + gst_element_set_state (self->identity, GST_STATE_NULL); + gst_bin_remove (GST_BIN_CAST (self), self->identity); + self->identity = NULL; + } + if (self->converter_caps) { + gst_caps_unref (self->converter_caps); + self->converter_caps = NULL; + } +} + static void gst_play_sink_convert_bin_finalize (GObject * object) { GstPlaySinkConvertBin *self = GST_PLAY_SINK_CONVERT_BIN_CAST (object); + gst_play_sink_convert_bin_remove_elements (self); + gst_object_unref (self->sink_proxypad); g_mutex_free (self->lock); G_OBJECT_CLASS (parent_class)->finalize (object); } -static void +void gst_play_sink_convert_bin_cache_converter_caps (GstPlaySinkConvertBin * self) { GstElement *head; GstPad *pad; - self->converter_caps = NULL; + if (self->converter_caps) { + gst_caps_unref (self->converter_caps); + self->converter_caps = NULL; + } if (!self->conversion_elements) { GST_WARNING_OBJECT (self, "No conversion elements"); @@ -422,17 +454,6 @@ gst_play_sink_convert_bin_change_state (GstElement * element, GstPlaySinkConvertBin *self = GST_PLAY_SINK_CONVERT_BIN_CAST (element); switch (transition) { - case GST_STATE_CHANGE_NULL_TO_READY: - GST_PLAY_SINK_CONVERT_BIN_LOCK (self); - g_assert (self->add_conversion_elements); - if (!(*self->add_conversion_elements) (self)) { - GST_ELEMENT_ERROR (self, CORE, PAD, - (NULL), ("Failed to configure the converter bin.")); - } - gst_play_sink_convert_bin_cache_converter_caps (self); - gst_play_sink_convert_bin_add_identity (self); - GST_PLAY_SINK_CONVERT_BIN_UNLOCK (self); - break; case GST_STATE_CHANGE_PAUSED_TO_READY: GST_PLAY_SINK_CONVERT_BIN_LOCK (self); if (gst_pad_is_blocked (self->sink_proxypad)) @@ -465,25 +486,6 @@ gst_play_sink_convert_bin_change_state (GstElement * element, (GDestroyNotify) gst_object_unref); GST_PLAY_SINK_CONVERT_BIN_UNLOCK (self); break; - case GST_STATE_CHANGE_READY_TO_NULL: - GST_PLAY_SINK_CONVERT_BIN_LOCK (self); - if (self->conversion_elements) { - g_list_foreach (self->conversion_elements, - (GFunc) gst_play_sink_convert_bin_remove_element, self); - g_list_free (self->conversion_elements); - self->conversion_elements = NULL; - } - if (self->identity) { - gst_element_set_state (self->identity, GST_STATE_NULL); - gst_bin_remove (GST_BIN_CAST (self), self->identity); - self->identity = NULL; - } - if (self->converter_caps) { - gst_caps_unref (self->converter_caps); - self->converter_caps = NULL; - } - GST_PLAY_SINK_CONVERT_BIN_UNLOCK (self); - break; default: break; } @@ -547,4 +549,6 @@ gst_play_sink_convert_bin_init (GstPlaySinkConvertBin * self) GST_DEBUG_FUNCPTR (gst_play_sink_convert_bin_getcaps)); gst_element_add_pad (GST_ELEMENT_CAST (self), self->srcpad); gst_object_unref (templ); + + gst_play_sink_convert_bin_add_identity (self); } diff --git a/gst/playback/gstplaysinkconvertbin.h b/gst/playback/gstplaysinkconvertbin.h index 7fac18d..d8379ef 100644 --- a/gst/playback/gstplaysinkconvertbin.h +++ b/gst/playback/gstplaysinkconvertbin.h @@ -78,7 +78,6 @@ struct _GstPlaySinkConvertBin /* configuration for derived classes */ gboolean audio; - gboolean (*add_conversion_elements)(GstPlaySinkConvertBin *); }; struct _GstPlaySinkConvertBinClass @@ -93,6 +92,10 @@ gst_play_sink_convert_bin_add_conversion_element_factory (GstPlaySinkConvertBin void gst_play_sink_convert_bin_add_conversion_element (GstPlaySinkConvertBin *self, GstElement *el); +void +gst_play_sink_convert_bin_cache_converter_caps (GstPlaySinkConvertBin * self); +void +gst_play_sink_convert_bin_remove_elements (GstPlaySinkConvertBin * self); G_END_DECLS #endif /* __GST_PLAY_SINK_CONVERT_BIN_H__ */ diff --git a/gst/playback/gstplaysinkvideoconvert.c b/gst/playback/gstplaysinkvideoconvert.c index 0159f7c..a8f3d81 100644 --- a/gst/playback/gstplaysinkvideoconvert.c +++ b/gst/playback/gstplaysinkvideoconvert.c @@ -35,9 +35,10 @@ G_DEFINE_TYPE (GstPlaySinkVideoConvert, gst_play_sink_video_convert, GST_TYPE_PLAY_SINK_CONVERT_BIN); static gboolean -gst_play_sink_video_convert_add_conversion_elements (GstPlaySinkConvertBin * - cbin) +gst_play_sink_video_convert_add_conversion_elements (GstPlaySinkVideoConvert * + self) { + GstPlaySinkConvertBin *cbin = GST_PLAY_SINK_CONVERT_BIN (self); GstElement *el, *prev = NULL; el = gst_play_sink_convert_bin_add_conversion_element_factory (cbin, @@ -87,6 +88,7 @@ gst_play_sink_video_convert_init (GstPlaySinkVideoConvert * self) { GstPlaySinkConvertBin *cbin = GST_PLAY_SINK_CONVERT_BIN (self); cbin->audio = FALSE; - cbin->add_conversion_elements = - gst_play_sink_video_convert_add_conversion_elements; + + gst_play_sink_video_convert_add_conversion_elements (self); + gst_play_sink_convert_bin_cache_converter_caps (cbin); } diff --git a/gst/playback/gstplaysinkvideoconvert.h b/gst/playback/gstplaysinkvideoconvert.h index 33030ad..df56d97 100644 --- a/gst/playback/gstplaysinkvideoconvert.h +++ b/gst/playback/gstplaysinkvideoconvert.h @@ -36,7 +36,6 @@ G_BEGIN_DECLS (G_TYPE_CHECK_INSTANCE_TYPE ((obj), GST_TYPE_PLAY_SINK_VIDEO_CONVERT)) #define GST_IS_PLAY_SINK_VIDEO_CONVERT_CLASS(klass) \ (G_TYPE_CHECK_CLASS_TYPE ((klass), GST_TYPE_PLAY_SINK_VIDEO_CONVERT)) - typedef struct _GstPlaySinkVideoConvert GstPlaySinkVideoConvert; typedef struct _GstPlaySinkVideoConvertClass GstPlaySinkVideoConvertClass; -- 2.7.4