⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 271290 in webkit


Ignore:
Timestamp:
Jan 8, 2021, 5:16:25 AM (6 years ago)
Author:
Philippe Normand
Message:

[GStreamer] WebAudio provider should clean-up its bin when the client disappears
​https://bugs.webkit.org/show_bug.cgi?id=219245

Reviewed by Xabier Rodriguez-Calvar.

Source/WebCore:

Clean-up elements downstream of the deinterleave element when the provider client changes or
is removed.

  • platform/audio/gstreamer/AudioSourceProviderGStreamer.cpp:

(WebCore::copyGStreamerBuffersToAudioChannel):
(WebCore::AudioSourceProviderGStreamer::provideInput):
(WebCore::AudioSourceProviderGStreamer::handleSample):
(WebCore::AudioSourceProviderGStreamer::setClient):
(WebCore::AudioSourceProviderGStreamer::handleRemovedDeinterleavePad):
(WebCore::AudioSourceProviderGStreamer::deinterleavePadsConfigured):

  • platform/graphics/gstreamer/GStreamerCommon.cpp:

(WebCore::disconnectSimpleBusMessageCallback): Drive-by, remove bus signal handler.
(WebCore::connectSimpleBusMessageCallback):

  • platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp: Drive-by, define

GST_CAT_DEFAULT earlier so that all GST_DEBUG call sites actually log something.

LayoutTests:

Unflag tests no longer crashing.

  • platform/glib/TestExpectations:
  • platform/gtk-wayland/TestExpectations:
  • platform/gtk/TestExpectations:
Location:
trunk
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r271284 r271290  
     12021-01-08  Philippe Normand  <pnormand@igalia.com>
     2
     3        [GStreamer] WebAudio provider should clean-up its bin when the client disappears
     4        https://bugs.webkit.org/show_bug.cgi?id=219245
     5
     6        Reviewed by Xabier Rodriguez-Calvar.
     7
     8        Unflag tests no longer crashing.
     9
     10        * platform/glib/TestExpectations:
     11        * platform/gtk-wayland/TestExpectations:
     12        * platform/gtk/TestExpectations:
     13
    1142021-01-07  Zalan Bujtas  <zalan@apple.com>
    215

  • trunk/LayoutTests/platform/glib/TestExpectations

    r271196 r271290  
    452452
    453453webkit.org/b/218580 fast/mediastream/captureStream/canvas2d.html [ Failure Timeout ]
    454 
    455 webkit.org/b/219245 webrtc/audio-replace-track.html [ Crash ]
    456 webkit.org/b/219245 webrtc/peer-connection-audio-mute.html [ Crash ]
    457 webkit.org/b/219245 webrtc/peer-connection-audio-mute2.html [ Crash ]
    458 webkit.org/b/219245 webrtc/peer-connection-audio-unmute.html [ Crash ]
    459 webkit.org/b/219245 webrtc/peer-connection-remote-audio-mute.html [ Crash ]
    460 webkit.org/b/219245 webrtc/peer-connection-remote-audio-mute2.html [ Crash ]
    461454
    462455#////////////////////////////////////////////////////////////////////////////////////////

  • trunk/LayoutTests/platform/gtk-wayland/TestExpectations

    r270242 r271290  
    3333
    3434# WebRTC
    35 webkit.org/b/219245 webkit.org/b/212892 webrtc/peer-connection-audio-mute2.html [ Failure Timeout Pass Crash ]
     35webkit.org/b/212892 webrtc/peer-connection-audio-mute2.html [ Failure Timeout Pass Crash ]
    3636
    3737# Workers

  • trunk/LayoutTests/platform/gtk/TestExpectations

    r271142 r271290  
    14051405webkit.org/b/193311 fast/images/decode-render-animated-image.html [ ImageOnlyFailure Pass ]
    14061406
    1407 webkit.org/b/193318 webkit.org/b/219245 webrtc/audio-replace-track.html [ Timeout Pass Crash ]
     1407webkit.org/b/193318 webrtc/audio-replace-track.html [ Timeout Pass ]
    14081408
    14091409webkit.org/b/193490 [ Debug ] animations/play-state-suspend.html [ Pass Failure ]

  • trunk/Source/WebCore/ChangeLog

    r271289 r271290  
     12021-01-08  Philippe Normand  <pnormand@igalia.com>
     2
     3        [GStreamer] WebAudio provider should clean-up its bin when the client disappears
     4        https://bugs.webkit.org/show_bug.cgi?id=219245
     5
     6        Reviewed by Xabier Rodriguez-Calvar.
     7
     8        Clean-up elements downstream of the deinterleave element when the provider client changes or
     9        is removed.
     10
     11        * platform/audio/gstreamer/AudioSourceProviderGStreamer.cpp:
     12        (WebCore::copyGStreamerBuffersToAudioChannel):
     13        (WebCore::AudioSourceProviderGStreamer::provideInput):
     14        (WebCore::AudioSourceProviderGStreamer::handleSample):
     15        (WebCore::AudioSourceProviderGStreamer::setClient):
     16        (WebCore::AudioSourceProviderGStreamer::handleRemovedDeinterleavePad):
     17        (WebCore::AudioSourceProviderGStreamer::deinterleavePadsConfigured):
     18        * platform/graphics/gstreamer/GStreamerCommon.cpp:
     19        (WebCore::disconnectSimpleBusMessageCallback): Drive-by, remove bus signal handler.
     20        (WebCore::connectSimpleBusMessageCallback):
     21        * platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp: Drive-by, define
     22        GST_CAT_DEFAULT earlier so that all GST_DEBUG call sites actually log something.
     23
    1242021-01-08  Xabier Rodriguez Calvar  <calvaris@igalia.com>
    225

  • trunk/Source/WebCore/platform/audio/gstreamer/AudioSourceProviderGStreamer.cpp

    r270184 r271290  
    6868    auto available = gst_adapter_available(adapter);
    6969    if (!available) {
     70        GST_TRACE("Adapter empty, silencing bus");
    7071        bus->zero();
    7172        return;
    7273    }
    7374
     75    GST_TRACE("%zu samples available for channel %d (%zu frames requested)", available, channelNumber, framesToProcess);
    7476    size_t bytes = framesToProcess * sizeof(float);
    7577    if (available >= bytes) {
    … …  
    151153void AudioSourceProviderGStreamer::provideInput(AudioBus* bus, size_t framesToProcess)
    152154{
     155    GST_TRACE("Fetching buffers from adapters");
    153156    auto locker = holdLock(m_adapterMutex);
    154157    for (auto& it : m_adapters)
    … …  
    158161GstFlowReturn AudioSourceProviderGStreamer::handleSample(GstAppSink* sink, bool isPreroll)
    159162{
     163    GST_TRACE("Pulling audio sample from the sink");
    160164    auto sample = adoptGRef(isPreroll ? gst_app_sink_try_pull_preroll(sink, 0) : gst_app_sink_try_pull_sample(sink, 0));
    161165    if (!sample)
    … …  
    169173        return GST_FLOW_ERROR;
    170174
     175    GST_TRACE("Storing audio sample %" GST_PTR_FORMAT, sample.get());
    171176    {
    172177        auto locker = holdLock(m_adapterMutex);
    173178        GQuark quark = g_quark_from_static_string("channel-id");
    174179        int channelId = GPOINTER_TO_INT(g_object_get_qdata(G_OBJECT(sink), quark));
     180        GST_DEBUG("Channel ID: %d", channelId);
     181
    175182        auto result = m_adapters.ensure(channelId, [&] {
    176183            return gst_adapter_new();
    … …  
    190197        return;
    191198
     199    GST_DEBUG("Setting up client %p (previous: %p)", client, m_client);
     200    bool previousClientWasValid = m_client;
    192201    m_client = client;
    193 
    194 #if ENABLE(MEDIA_STREAM)
    195     if (m_pipeline)
    196         gst_element_set_state(m_pipeline.get(), m_client ? GST_STATE_PLAYING : GST_STATE_NULL);
    197 #endif
    198 
    199     // FIXME: This early return should ideally be replaced by a removal of the m_audioSinkBin from
    200     // its parent pipeline. https://bugs.webkit.org/show_bug.cgi?id=219245
    201     if (!m_client)
    202         return;
    203202
    204203    // The volume element is used to mute audio playback towards the
    … …  
    208207    auto volumeElement = adoptGRef(gst_bin_get_by_name(GST_BIN_CAST(m_audioSinkBin.get()), "volume"));
    209208
    210     if (volumeElement)
    211         g_object_set(volumeElement.get(), "mute", TRUE, nullptr);
    212 
    213     // The audioconvert and audioresample elements are needed to
    214     // ensure deinterleave and the sinks downstream receive buffers in
    215     // the format specified by the capsfilter.
    216     GstElement* audioQueue = gst_element_factory_make("queue", nullptr);
    217     GstElement* audioConvert  = gst_element_factory_make("audioconvert", nullptr);
    218     GstElement* audioResample = gst_element_factory_make("audioresample", nullptr);
    219     GstElement* capsFilter = gst_element_factory_make("capsfilter", nullptr);
    220     GstElement* deInterleave = gst_element_factory_make("deinterleave", "deinterleave");
    221 
    222     g_object_set(deInterleave, "keep-positions", TRUE, nullptr);
    223     m_deinterleavePadAddedHandlerId = g_signal_connect(deInterleave, "pad-added", G_CALLBACK(onGStreamerDeinterleavePadAddedCallback), this);
    224     m_deinterleaveNoMorePadsHandlerId = g_signal_connect(deInterleave, "no-more-pads", G_CALLBACK(onGStreamerDeinterleaveReadyCallback), this);
    225     m_deinterleavePadRemovedHandlerId = g_signal_connect(deInterleave, "pad-removed", G_CALLBACK(onGStreamerDeinterleavePadRemovedCallback), this);
    226 
    227     auto caps = adoptGRef(gst_caps_new_simple("audio/x-raw", "rate", G_TYPE_INT, static_cast<int>(gSampleBitRate),
    228         "format", G_TYPE_STRING, GST_AUDIO_NE(F32), "layout", G_TYPE_STRING, "interleaved", nullptr));
    229     g_object_set(capsFilter, "caps", caps.get(), nullptr);
    230 
    231     gst_bin_add_many(GST_BIN_CAST(m_audioSinkBin.get()), audioQueue, audioConvert, audioResample, capsFilter, deInterleave, nullptr);
    232 
    233     auto audioTee = adoptGRef(gst_bin_get_by_name(GST_BIN(m_audioSinkBin.get()), "audioTee"));
    234 
    235     // Link a new src pad from tee to queue ! audioconvert !
    236     // audioresample ! capsfilter ! deinterleave. Later
    237     // on each deinterleaved planar audio channel will be routed to an
    238     // appsink for data extraction and processing.
    239     gst_element_link_pads_full(audioTee.get(), "src_%u", audioQueue, "sink", GST_PAD_LINK_CHECK_NOTHING);
    240     gst_element_link_pads_full(audioQueue, "src", audioConvert, "sink", GST_PAD_LINK_CHECK_NOTHING);
    241     gst_element_link_pads_full(audioConvert, "src", audioResample, "sink", GST_PAD_LINK_CHECK_NOTHING);
    242     gst_element_link_pads_full(audioResample, "src", capsFilter, "sink", GST_PAD_LINK_CHECK_NOTHING);
    243     gst_element_link_pads_full(capsFilter, "src", deInterleave, "sink", GST_PAD_LINK_CHECK_NOTHING);
    244 
    245     gst_element_sync_state_with_parent(audioQueue);
    246     gst_element_sync_state_with_parent(audioConvert);
    247     gst_element_sync_state_with_parent(audioResample);
    248     gst_element_sync_state_with_parent(capsFilter);
    249     gst_element_sync_state_with_parent(deInterleave);
     209    if (volumeElement) {
     210        bool shouldMute = m_client;
     211        g_object_set(volumeElement.get(), "mute", shouldMute, nullptr);
     212    }
     213
     214    auto audioTee = adoptGRef(gst_bin_get_by_name(GST_BIN_CAST(m_audioSinkBin.get()), "audioTee"));
     215    if (!m_client || previousClientWasValid) {
     216        auto audioQueue = adoptGRef(gst_bin_get_by_name(GST_BIN_CAST(m_audioSinkBin.get()), "queue"));
     217        auto audioConvert = adoptGRef(gst_bin_get_by_name(GST_BIN_CAST(m_audioSinkBin.get()), "audioconvert"));
     218        auto audioResample = adoptGRef(gst_bin_get_by_name(GST_BIN_CAST(m_audioSinkBin.get()), "audioresample"));
     219        auto capsFilter = adoptGRef(gst_bin_get_by_name(GST_BIN_CAST(m_audioSinkBin.get()), "capsfilter"));
     220        auto deInterleave = adoptGRef(gst_bin_get_by_name(GST_BIN_CAST(m_audioSinkBin.get()), "deinterleave"));
     221        auto queueSinkPad = adoptGRef(gst_element_get_static_pad(audioQueue.get(), "sink"));
     222        auto teeSrcPad = adoptGRef(gst_pad_get_peer(queueSinkPad.get()));
     223
     224        GST_DEBUG("Cleaning up audio deinterleave chain");
     225        gst_element_set_locked_state(m_audioSinkBin.get(), true);
     226
     227        gst_element_set_state(audioQueue.get(), GST_STATE_NULL);
     228        gst_element_set_state(audioConvert.get(), GST_STATE_NULL);
     229        gst_element_set_state(audioResample.get(), GST_STATE_NULL);
     230        gst_element_set_state(capsFilter.get(), GST_STATE_NULL);
     231        gst_element_set_state(deInterleave.get(), GST_STATE_NULL);
     232        gst_element_unlink_many(audioTee.get(), audioQueue.get(), audioConvert.get(), audioResample.get(), capsFilter.get(), deInterleave.get(), nullptr);
     233        gst_element_set_locked_state(m_audioSinkBin.get(), false);
     234        gst_bin_remove_many(GST_BIN_CAST(m_audioSinkBin.get()), audioQueue.get(), audioConvert.get(), audioResample.get(), capsFilter.get(), deInterleave.get(), nullptr);
     235        gst_element_release_request_pad(audioTee.get(), teeSrcPad.get());
     236    }
     237
     238    if (m_client) {
     239        // The audioconvert and audioresample elements are needed to
     240        // ensure deinterleave and the sinks downstream receive buffers in
     241        // the format specified by the capsfilter.
     242        auto* audioQueue = gst_element_factory_make("queue", "queue");
     243        auto* audioConvert = gst_element_factory_make("audioconvert", "audioconvert");
     244        auto* audioResample = gst_element_factory_make("audioresample", "audioresample");
     245        auto* capsFilter = gst_element_factory_make("capsfilter", "capsfilter");
     246        auto* deInterleave = gst_element_factory_make("deinterleave", "deinterleave");
     247
     248        GST_DEBUG("Setting up audio deinterleave chain");
     249        g_object_set(deInterleave, "keep-positions", TRUE, nullptr);
     250        m_deinterleavePadAddedHandlerId = g_signal_connect(deInterleave, "pad-added", G_CALLBACK(onGStreamerDeinterleavePadAddedCallback), this);
     251        m_deinterleaveNoMorePadsHandlerId = g_signal_connect(deInterleave, "no-more-pads", G_CALLBACK(onGStreamerDeinterleaveReadyCallback), this);
     252        m_deinterleavePadRemovedHandlerId = g_signal_connect(deInterleave, "pad-removed", G_CALLBACK(onGStreamerDeinterleavePadRemovedCallback), this);
     253
     254        auto caps = adoptGRef(gst_caps_new_simple("audio/x-raw", "rate", G_TYPE_INT, static_cast<int>(gSampleBitRate),
     255            "format", G_TYPE_STRING, GST_AUDIO_NE(F32), "layout", G_TYPE_STRING, "interleaved", nullptr));
     256        g_object_set(capsFilter, "caps", caps.get(), nullptr);
     257
     258        gst_bin_add_many(GST_BIN_CAST(m_audioSinkBin.get()), audioQueue, audioConvert, audioResample, capsFilter, deInterleave, nullptr);
     259
     260        // Link a new src pad from tee to queue ! audioconvert !
     261        // audioresample ! capsfilter ! deinterleave. Later
     262        // on each deinterleaved planar audio channel will be routed to an
     263        // appsink for data extraction and processing.
     264        gst_element_link_pads_full(audioTee.get(), "src_%u", audioQueue, "sink", GST_PAD_LINK_CHECK_NOTHING);
     265        gst_element_link_pads_full(audioQueue, "src", audioConvert, "sink", GST_PAD_LINK_CHECK_NOTHING);
     266        gst_element_link_pads_full(audioConvert, "src", audioResample, "sink", GST_PAD_LINK_CHECK_NOTHING);
     267        gst_element_link_pads_full(audioResample, "src", capsFilter, "sink", GST_PAD_LINK_CHECK_NOTHING);
     268        gst_element_link_pads_full(capsFilter, "src", deInterleave, "sink", GST_PAD_LINK_CHECK_NOTHING);
     269
     270        gst_element_sync_state_with_parent(audioQueue);
     271        gst_element_sync_state_with_parent(audioConvert);
     272        gst_element_sync_state_with_parent(audioResample);
     273        gst_element_sync_state_with_parent(capsFilter);
     274        gst_element_sync_state_with_parent(deInterleave);
     275    }
     276
     277    m_deinterleaveSourcePads = 0;
     278    clearAdapters();
     279#if ENABLE(MEDIA_STREAM)
     280    if (m_pipeline)
     281        gst_element_set_state(m_pipeline.get(), m_client ? GST_STATE_PLAYING : GST_STATE_NULL);
     282#endif
    250283}
    251284
    … …  
    306339void AudioSourceProviderGStreamer::handleRemovedDeinterleavePad(GstPad* pad)
    307340{
     341    if (GST_PAD_DIRECTION(pad) != GST_PAD_SRC)
     342        return;
     343
    308344    GST_DEBUG("Pad %" GST_PTR_FORMAT " gone", pad);
    309345    m_deinterleaveSourcePads--;
    … …  
    321357void AudioSourceProviderGStreamer::deinterleavePadsConfigured()
    322358{
    323     GST_DEBUG("Deinterleave configured, notifying client");
     359    GST_DEBUG("Deinterleave configured with %d channels, notifying client", m_deinterleaveSourcePads);
    324360    m_notifier->notify(MainThreadNotification::DeinterleavePadsConfigured, [numberOfChannels = m_deinterleaveSourcePads, sampleRate = gSampleBitRate, client = m_client] {
    325361        if (client)

  • trunk/Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp

    r271197 r271290  
    405405void disconnectSimpleBusMessageCallback(GstElement* pipeline)
    406406{
    407     GRefPtr<GstBus> bus = adoptGRef(gst_pipeline_get_bus(GST_PIPELINE(pipeline)));
     407    auto bus = adoptGRef(gst_pipeline_get_bus(GST_PIPELINE(pipeline)));
    408408    g_signal_handlers_disconnect_by_func(bus.get(), reinterpret_cast<gpointer>(simpleBusMessageCallback), pipeline);
     409    gst_bus_remove_signal_watch(bus.get());
    409410}
    410411
    411412void connectSimpleBusMessageCallback(GstElement* pipeline)
    412413{
    413     GRefPtr<GstBus> bus = adoptGRef(gst_pipeline_get_bus(GST_PIPELINE(pipeline)));
     414    auto bus = adoptGRef(gst_pipeline_get_bus(GST_PIPELINE(pipeline)));
    414415    gst_bus_add_signal_watch_full(bus.get(), RunLoopSourcePriority::RunLoopDispatcher);
    415416    g_signal_connect(bus.get(), "message", G_CALLBACK(simpleBusMessageCallback), pipeline);

  • trunk/Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp

    r270906 r271290  
    5252    GST_STATIC_CAPS("audio/x-raw(ANY);"));
    5353
     54GST_DEBUG_CATEGORY_STATIC(webkitMediaStreamSrcDebug);
     55#define GST_CAT_DEFAULT webkitMediaStreamSrcDebug
     56
    5457GRefPtr<GstTagList> mediaStreamTrackPrivateGetTags(MediaStreamTrackPrivate* track)
    5558{
    … …  
    305308}
    306309
    307 GST_DEBUG_CATEGORY_STATIC(webkitMediaStreamSrcDebug);
    308 #define GST_CAT_DEFAULT webkitMediaStreamSrcDebug
    309 
    310310#define doInit \
    311311    G_IMPLEMENT_INTERFACE(GST_TYPE_URI_HANDLER, webkitMediaStreamSrcUriHandlerInit); \
Note: See TracChangeset for help on using the changeset viewer.