[GStreamer] Crash in AudioTrackPrivate with playbin3 enabled

• Component: Media
• Status: RESOLVED FIXED

[Philippe Normand]

https://www.reddit.com/r/WTF/comments/bcqcar/engine_cold_start_turkish_style/

(gdb) bt
#0  0x00007f40b2c38510 in _ZN7WebCore17AudioTrackPrivate10setEnabledEb (enabled=true, this=0x0) at ../Source/WebCore/platform/graphics/AudioTrackPrivate.h:53
#1  0x00007f40b2c38510 in _ZN7WebCore26AudioTrackPrivateGStreamer12markAsActiveEv (this=0x0) at ../Source/WebCore/platform/graphics/gstreamer/AudioTrackPrivateGStreamer.cpp:83
#2  0x00007f40b2ab9c86 in _ZN7WebCore27MediaPlayerPrivateGStreamer13handleMessageEP11_GstMessage (this=0x7f27492b9c00, message=0x5650ca7a86a0)
    at ../Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1451
#3  0x00007f40a7c918ee in ffi_call_unix64 () at /usr/lib/x86_64-linux-gnu/libffi.so.6
#4  0x00007f40a7c912bf in ffi_call () at /usr/lib/x86_64-linux-gnu/libffi.so.6
#9  0x00007f40aa2c191f in <emit signal message:streams-selected on instance 0x5650c99cab90 [GstBus]> (instance=instance@entry=0x5650c99cab90, signal_id=<optimized out>, detail=<optimized out>)
    at ../../../gobject/gsignal.c:3447
    #5  0x00007f40aa2a5472 in g_cclosure_marshal_generic
    (closure=0x5650ca76c190, return_gvalue=0x0, n_param_values=<optimized out>, param_values=<optimized out>, invocation_hint=<optimized out>, marshal_data=<optimized out>) at ../../../gobject/gclosure.c:1496
    #6  0x00007f40aa2a4c7d in g_closure_invoke (closure=0x5650ca76c190, return_value=0x0, n_param_values=2, param_values=0x7ffe1bf88fd0, invocation_hint=0x7ffe1bf88f50) at ../../../gobject/gclosure.c:810
    #7  0x00007f40aa2b8345 in signal_emit_unlocked_R
    (node=node@entry=0x5650ca3ad310, detail=detail@entry=1632, instance=instance@entry=0x5650c99cab90, emission_return=emission_return@entry=0x0, instance_and_params=instance_and_params@entry=0x7ffe1bf88fd0)
    at ../../../gobject/gsignal.c:3635
    #8  0x00007f40aa2c125e in g_signal_emit_valist (instance=<optimized out>, signal_id=<optimized out>, detail=<optimized out>, var_args=var_args@entry=0x7ffe1bf891a0) at ../../../gobject/gsignal.c:3391
#10 0x00007f40aaec9c34 in gst_bus_async_signal_func (bus=0x5650c99cab90 [GstBus], message=0x5650ca7a86a0, data=<optimized out>) at ../subprojects/gstreamer/gst/gstbus.c:1261
#11 0x00007f40aaecaa4d in gst_bus_source_dispatch (source=0x5650ca73b390, callback=0x7f40aaec9be0 <gst_bus_async_signal_func>, user_data=0x0) at ../subprojects/gstreamer/gst/gstbus.c:839
#12 0x00007f40aa1c2dd8 in g_main_dispatch (context=0x5650c993d020) at ../../../glib/gmain.c:3182
#13 0x00007f40aa1c2dd8 in g_main_context_dispatch (context=context@entry=0x5650c993d020) at ../../../glib/gmain.c:3847
#14 0x00007f40aa1c31c8 in g_main_context_iterate (context=0x5650c993d020, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at ../../../glib/gmain.c:3920
#15 0x00007f40aa1c34c2 in g_main_loop_run (loop=0x5650c9b0f990) at ../../../glib/gmain.c:4116
#16 0x00007f40aedd2a70 in _ZN3WTF7RunLoop3runEv () at ../Source/WTF/wtf/glib/RunLoopGLib.cpp:96
#17 0x00007f40b160c8c8 in _ZN6WebKit20AuxiliaryProcessMainINS_10WebProcessENS_14WebProcessMainEEEiiPPc (argc=<optimized out>, argv=0x7ffe1bf89588) at ../Source/WebKit/Shared/unix/AuxiliaryProcessMain.h:66
#18 0x00007f40a916309b in __libc_start_main (main=0x5650c7b94f20 <main(int, char**)>, argc=3, argv=0x7ffe1bf89588, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7ffe1bf89578)
    at ../csu/libc-start.c:308
#19 0x00005650c7b94faa in _start ()

[Philippe Normand]

m_audioTracks looks empty? This is with a RelWithDebInfo build:

(gdb) f 2
#2  0x00007f40b2ab9c86 in _ZN7WebCore27MediaPlayerPrivateGStreamer13handleMessageEP11_GstMessage (this=0x7f27492b9c00, message=0x5650ca7a86a0)
    at ../Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1451
1451	                track->markAsActive();
(gdb) p track
$1 = <optimized out>
(gdb) p m_audioTracks
warning: can't find linker symbol for virtual table for `WebCore::MediaPlayerPrivateGStreamer' value
warning:   found `_ZTVN7WebCore30MediaPlayerPrivateGStreamerMSEE' instead
$2 = {m_impl = {static m_maxLoad = 2, static m_minLoad = 6, m_table = 0x0, m_tableSize = 0, m_tableSizeMask = 0, m_keyCount = 0, m_deletedCount = 0}}
(gdb) p streamId
$3 = "511baabfffee5fa8069fe40b80215508"

[Philippe Normand]

Created attachment 367421 [details]
Patch

[Thibault Saunier]

Comment on attachment 367421 [details]
Patch

lgtm, but if someone set USE_PLAYBIN3, it will still break... should we unset it on `gst_init`?

[Philippe Normand]

(In reply to Thibault Saunier from comment #3)
> Comment on attachment 367421 [details]
> Patch
> 
> lgtm, but if someone set USE_PLAYBIN3, it will still break... should we
> unset it on `gst_init`?

What would break? I've added a test to ensure we process stream collections only when the new var is set.

[Philippe Normand]

Comment on attachment 367421 [details]
Patch

I found more issues, unfortunately :( Will update the patch, sorry.

[Philippe Normand]

(In reply to Thibault Saunier from comment #3)
> Comment on attachment 367421 [details]
> Patch
> 
> lgtm, but if someone set USE_PLAYBIN3, it will still break... should we
> unset it on `gst_init`?

Good idea :P

[Philippe Normand]

Created attachment 367516 [details]
Patch

[Xabier Rodríguez Calvar]

Comment on attachment 367516 [details]
Patch

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

From the GStreamer PoV, this is ok. I'll let Michael have the final word about the env variables.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:2372
> +    if ((!isMediaSource() && g_getenv("WEBKIT_GST_USE_PLAYBIN3")) || (url.protocolIs("mediastream")))

I think there are issues here with too many () .

[Michael Catanzaro]

Comment on attachment 367516 [details]
Patch

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

> Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp:230
> +        // USE_PLAYBIN3 is dangerous for us because its potential sneaky effect
> +        // is to register the playbin3 element under the playbin namespace. We
> +        // can't allow this, when we create playbin, we want playbin2, not
> +        // playbin3.
> +        unsetenv("USE_PLAYBIN3");

This will need a rethink. See https://bugs.webkit.org/show_bug.cgi?id=194370#c6 for why you can't do this.

[Philippe Normand]

Created attachment 367536 [details]
Patch

[Michael Catanzaro]

This got lost.

[Xabier Rodríguez Calvar]

Comment on attachment 367536 [details]
Patch

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

> Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp:230
> +        if (getenv("USE_PLAYBIN3"))

I'd use g_getenv and we're good to go.

[Philippe Normand]

Committed r244584: <https://trac.webkit.org/changeset/244584>

[Radar WebKit Bug Importer]

<rdar://problem/50164498>