[webkit-reviews] review granted: [Bug 233826] [GStreamer] Fill in client-name property on audio sinks : [Attachment 446036] Patch

[bugzilla-daemon at webkit.org]

Xabier Rodríguez Calvar <calvaris at igalia.com> has granted Philippe Normand
<pnormand at igalia.com>'s request for review:
Bug 233826: [GStreamer] Fill in client-name property on audio sinks
https://bugs.webkit.org/show_bug.cgi?id=233826

Attachment 446036: Patch

https://bugs.webkit.org/attachment.cgi?id=446036&action=review




--- Comment #3 from Xabier Rodríguez Calvar <calvaris at igalia.com> ---
Comment on attachment 446036
  --> https://bugs.webkit.org/attachment.cgi?id=446036
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=446036&action=review

> Source/WebCore/platform/audio/gstreamer/AudioDestinationGStreamer.cpp:145
> -	   g_signal_connect(audioSink.get(), "child-added",
G_CALLBACK(autoAudioSinkChildAddedCallback), nullptr);
> +	   g_signal_connect(audioSink.get(), "child-added",
G_CALLBACK(+[](GstChildProxy*, GObject* object, gchar*, gpointer) {
> +	       if (GST_IS_AUDIO_BASE_SINK(object))
> +		   g_object_set(GST_AUDIO_BASE_SINK(object), "buffer-time",
static_cast<gint64>(100000), nullptr);
> +	   }), nullptr);

Please, mention this drive-by reorganization on the changelog.

>> Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp:458
>> +	}), role.releaseImpl().leakRef());
> 
> Hm, this is not looking good... i'll revisit this part.

Definitely looking not good. I guess you'll have to g_strdup and use
g_signal_connect_data to provide the GClosureNotify to destroy it.

> Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h:310
> +GstElement* createAutoAudioSink(String role);
> +GstElement* createPlatformAudioSink(const char* role);

Why don't you use const String& for both?