[GStreamer] Clean-up the TextSink

[Philippe Normand]

Currently the text-sink implementation is spread between a custom AppSink sub-class and some AppSink code in the player. In order to reduce clutter in the player, this sink could be a bin and the implementation could host the appsink implementation details.

[Philippe Normand]

Created attachment 417691 [details]
Patch

[Xabier Rodríguez Calvar]

Comment on attachment 417691 [details]
Patch

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

> Source/WebCore/platform/graphics/gstreamer/TextSinkGStreamer.cpp:84
> +    g_object_set(priv->appSink.get(), "emit-signals", true, "enable-last-sample", false, "caps", textCaps.get(), nullptr);

I think these should be TRUE and FALSE

> Source/WebCore/platform/graphics/gstreamer/TextSinkGStreamer.cpp:88
> +        auto sample = adoptGRef(gst_app_sink_pull_sample(GST_APP_SINK(appSink)));
> +        webkitTextSinkHandleSample(sink, WTFMove(sample));

What about webkitTextSinkHandleSample(sink, adoptGRef(gst_app_sink_pull_sample(GST_APP_SINK(appSink)))); ?

> Source/WebCore/platform/graphics/gstreamer/TextSinkGStreamer.cpp:94
> +        auto sample = adoptGRef(gst_app_sink_pull_preroll(GST_APP_SINK(appSink)));
> +        webkitTextSinkHandleSample(sink, WTFMove(sample));

Ditto.

[Philippe Normand]

WeakPtr asserts in debug, this needs another iteration :)

[Philippe Normand]

Created attachment 417692 [details]
Patch

[Xabier Rodríguez Calvar]

Comment on attachment 417692 [details]
Patch

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

> Source/WebCore/platform/graphics/gstreamer/TextSinkGStreamer.cpp:130
> +GstElement* webkitTextSinkNew(WeakPtr<MediaPlayerPrivateGStreamer> player)

I think you can && here and move when assigning to the priv

[Philippe Normand]

Committed r271517: <https://trac.webkit.org/changeset/271517>

[Radar WebKit Bug Importer]

<rdar://problem/73246106>