Title: [228271] trunk
- Revision
- 228271
- Author
- [email protected]
- Date
- 2018-02-08 05:10:48 -0800 (Thu, 08 Feb 2018)
Log Message
[GStreamer] LayoutTest webaudio/silent-audio-interrupted-in-background.html makes its subsequent test flaky crash
https://bugs.webkit.org/show_bug.cgi?id=173916
Reviewed by Xabier Rodriguez Calvar.
Source/WebCore:
This patch fixes two crashes and a runtime warning:
- The provider client configuration should be done from the main
thread but the no-more-pads signal of deinterleave was fired from
a non-main thread.
- The deinterleave pad-removed signal can be fired for a not fully
configured pipeline if the audio context is interrupted. So the
peer quark of the removed pad needs to be checked, it might be a
null pointer.
- The provider connects to the deinterleave signals only when a
client is provided, so the signal disconnection needs to check
that to avoid runtime warnings.
* platform/audio/gstreamer/AudioSourceProviderGStreamer.cpp:
(WebCore::AudioSourceProviderGStreamer::AudioSourceProviderGStreamer):
Create a main thread notifier.
(WebCore::AudioSourceProviderGStreamer::~AudioSourceProviderGStreamer):
Invalidate notifier and check a client was set before
disconnecting from deinterleave signals.
(WebCore::AudioSourceProviderGStreamer::handleRemovedDeinterleavePad):
Check validity of the pad peer.
(WebCore::AudioSourceProviderGStreamer::deinterleavePadsConfigured):
Set client from main thread.
* platform/audio/gstreamer/AudioSourceProviderGStreamer.h:
LayoutTests:
* platform/gtk/TestExpectations: Unskip fixed test.
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (228270 => 228271)
--- trunk/LayoutTests/ChangeLog 2018-02-08 12:47:12 UTC (rev 228270)
+++ trunk/LayoutTests/ChangeLog 2018-02-08 13:10:48 UTC (rev 228271)
@@ -1,3 +1,12 @@
+2018-02-08 Philippe Normand <[email protected]>
+
+ [GStreamer] LayoutTest webaudio/silent-audio-interrupted-in-background.html makes its subsequent test flaky crash
+ https://bugs.webkit.org/show_bug.cgi?id=173916
+
+ Reviewed by Xabier Rodriguez Calvar.
+
+ * platform/gtk/TestExpectations: Unskip fixed test.
+
2018-02-06 Yusuke Suzuki <[email protected]>
[JSC] Implement Array.prototype.flatMap and Array.prototype.flatten
Modified: trunk/LayoutTests/platform/gtk/TestExpectations (228270 => 228271)
--- trunk/LayoutTests/platform/gtk/TestExpectations 2018-02-08 12:47:12 UTC (rev 228270)
+++ trunk/LayoutTests/platform/gtk/TestExpectations 2018-02-08 13:10:48 UTC (rev 228271)
@@ -1293,8 +1293,6 @@
webkit.org/b/172955 media/video-preload.html [ Crash Pass ]
-webkit.org/b/173916 webaudio/silent-audio-interrupted-in-background.html [ Skip ]
-
webkit.org/b/175575 imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/ready-states/autoplay-with-slow-text-tracks.html [ Crash Pass ]
webkit.org/b/176797 accessibility/aria-listbox-no-selection.html [ Crash ]
Modified: trunk/Source/WebCore/ChangeLog (228270 => 228271)
--- trunk/Source/WebCore/ChangeLog 2018-02-08 12:47:12 UTC (rev 228270)
+++ trunk/Source/WebCore/ChangeLog 2018-02-08 13:10:48 UTC (rev 228271)
@@ -1,5 +1,39 @@
2018-02-08 Philippe Normand <[email protected]>
+ [GStreamer] LayoutTest webaudio/silent-audio-interrupted-in-background.html makes its subsequent test flaky crash
+ https://bugs.webkit.org/show_bug.cgi?id=173916
+
+ Reviewed by Xabier Rodriguez Calvar.
+
+ This patch fixes two crashes and a runtime warning:
+
+ - The provider client configuration should be done from the main
+ thread but the no-more-pads signal of deinterleave was fired from
+ a non-main thread.
+
+ - The deinterleave pad-removed signal can be fired for a not fully
+ configured pipeline if the audio context is interrupted. So the
+ peer quark of the removed pad needs to be checked, it might be a
+ null pointer.
+
+ - The provider connects to the deinterleave signals only when a
+ client is provided, so the signal disconnection needs to check
+ that to avoid runtime warnings.
+
+ * platform/audio/gstreamer/AudioSourceProviderGStreamer.cpp:
+ (WebCore::AudioSourceProviderGStreamer::AudioSourceProviderGStreamer):
+ Create a main thread notifier.
+ (WebCore::AudioSourceProviderGStreamer::~AudioSourceProviderGStreamer):
+ Invalidate notifier and check a client was set before
+ disconnecting from deinterleave signals.
+ (WebCore::AudioSourceProviderGStreamer::handleRemovedDeinterleavePad):
+ Check validity of the pad peer.
+ (WebCore::AudioSourceProviderGStreamer::deinterleavePadsConfigured):
+ Set client from main thread.
+ * platform/audio/gstreamer/AudioSourceProviderGStreamer.h:
+
+2018-02-08 Philippe Normand <[email protected]>
+
[GStreamer][WebAudio] No need for version check in each loop iteration
https://bugs.webkit.org/show_bug.cgi?id=182577
Modified: trunk/Source/WebCore/platform/audio/gstreamer/AudioSourceProviderGStreamer.cpp (228270 => 228271)
--- trunk/Source/WebCore/platform/audio/gstreamer/AudioSourceProviderGStreamer.cpp 2018-02-08 12:47:12 UTC (rev 228270)
+++ trunk/Source/WebCore/platform/audio/gstreamer/AudioSourceProviderGStreamer.cpp 2018-02-08 13:10:48 UTC (rev 228271)
@@ -83,7 +83,8 @@
}
AudioSourceProviderGStreamer::AudioSourceProviderGStreamer()
- : m_client(nullptr)
+ : m_notifier(MainThreadNotifier<MainThreadNotification>::create())
+ , m_client(nullptr)
, m_deinterleaveSourcePads(0)
, m_deinterleavePadAddedHandlerId(0)
, m_deinterleaveNoMorePadsHandlerId(0)
@@ -96,8 +97,10 @@
AudioSourceProviderGStreamer::~AudioSourceProviderGStreamer()
{
+ m_notifier->invalidate();
+
GRefPtr<GstElement> deinterleave = adoptGRef(gst_bin_get_by_name(GST_BIN(m_audioSinkBin.get()), "deinterleave"));
- if (deinterleave) {
+ if (deinterleave && m_client) {
g_signal_handler_disconnect(deinterleave.get(), m_deinterleavePadAddedHandlerId);
g_signal_handler_disconnect(deinterleave.get(), m_deinterleaveNoMorePadsHandlerId);
g_signal_handler_disconnect(deinterleave.get(), m_deinterleavePadRemovedHandlerId);
@@ -318,7 +321,10 @@
// Remove the queue ! appsink chain downstream of deinterleave.
GQuark quark = g_quark_from_static_string("peer");
- GstPad* sinkPad = reinterpret_cast<GstPad*>(g_object_get_qdata(G_OBJECT(pad), quark));
+ GstPad* sinkPad = GST_PAD_CAST(g_object_get_qdata(G_OBJECT(pad), quark));
+ if (!sinkPad)
+ return;
+
GRefPtr<GstElement> queue = adoptGRef(gst_pad_get_parent_element(sinkPad));
GRefPtr<GstPad> queueSrcPad = adoptGRef(gst_element_get_static_pad(queue.get(), "src"));
GRefPtr<GstPad> appsinkSinkPad = adoptGRef(gst_pad_get_peer(queueSrcPad.get()));
@@ -331,10 +337,12 @@
void AudioSourceProviderGStreamer::deinterleavePadsConfigured()
{
- ASSERT(m_client);
- ASSERT(m_deinterleaveSourcePads == gNumberOfChannels);
+ m_notifier->notify(MainThreadNotification::DeinterleavePadsConfigured, [this] {
+ ASSERT(m_client);
+ ASSERT(m_deinterleaveSourcePads == gNumberOfChannels);
- m_client->setFormat(m_deinterleaveSourcePads, gSampleBitRate);
+ m_client->setFormat(m_deinterleaveSourcePads, gSampleBitRate);
+ });
}
void AudioSourceProviderGStreamer::clearAdapters()
Modified: trunk/Source/WebCore/platform/audio/gstreamer/AudioSourceProviderGStreamer.h (228270 => 228271)
--- trunk/Source/WebCore/platform/audio/gstreamer/AudioSourceProviderGStreamer.h 2018-02-08 12:47:12 UTC (rev 228270)
+++ trunk/Source/WebCore/platform/audio/gstreamer/AudioSourceProviderGStreamer.h 2018-02-08 13:10:48 UTC (rev 228271)
@@ -23,6 +23,7 @@
#include "AudioSourceProvider.h"
#include "GRefPtrGStreamer.h"
+#include "MainThreadNotifier.h"
#include <gst/gst.h>
#include <wtf/Forward.h>
#include <wtf/Noncopyable.h>
@@ -53,6 +54,10 @@
void clearAdapters();
private:
+ enum MainThreadNotification {
+ DeinterleavePadsConfigured = 1 << 0,
+ };
+ Ref<MainThreadNotifier<MainThreadNotification>> m_notifier;
GRefPtr<GstElement> m_audioSinkBin;
AudioSourceProviderClient* m_client;
int m_deinterleaveSourcePads;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
