Modified: trunk/LayoutTests/ChangeLog (264594 => 264595)
--- trunk/LayoutTests/ChangeLog 2020-07-20 09:40:06 UTC (rev 264594)
+++ trunk/LayoutTests/ChangeLog 2020-07-20 10:26:11 UTC (rev 264595)
@@ -1,5 +1,14 @@
2020-07-20 Philippe Normand <[email protected]>
+ [GStreamer] ImageDecoder hits more Debug ASSERTs
+ https://bugs.webkit.org/show_bug.cgi?id=214191
+
+ Reviewed by Xabier Rodriguez-Calvar.
+
+ * platform/glib/TestExpectations: Unflag no-longer-crashing tests.
+
+2020-07-20 Philippe Normand <[email protected]>
+
Unreviewed, GTK gardening
* platform/gtk/TestExpectations: Unflag now-stable webrtc tests, no longer flaky since r263836.
Modified: trunk/LayoutTests/platform/glib/TestExpectations (264594 => 264595)
--- trunk/LayoutTests/platform/glib/TestExpectations 2020-07-20 09:40:06 UTC (rev 264594)
+++ trunk/LayoutTests/platform/glib/TestExpectations 2020-07-20 10:26:11 UTC (rev 264595)
@@ -216,10 +216,6 @@
webkit.org/b/207547 media/media-can-play-av1.html [ Failure ]
-webkit.org/b/214191 [ Debug ] fast/images/animated-png-loop-count.html [ Crash ]
-webkit.org/b/214191 [ Debug ] http/tests/images/jpeg-partial-load.html [ Crash ]
-webkit.org/b/214191 [ Debug ] http/tests/images/png-partial-load.html [ Crash ]
-
#////////////////////////////////////////////////////////////////////////////////////////
# End of GStreamer-related bugs
#////////////////////////////////////////////////////////////////////////////////////////
Modified: trunk/Source/WebCore/ChangeLog (264594 => 264595)
--- trunk/Source/WebCore/ChangeLog 2020-07-20 09:40:06 UTC (rev 264594)
+++ trunk/Source/WebCore/ChangeLog 2020-07-20 10:26:11 UTC (rev 264595)
@@ -1,3 +1,22 @@
+2020-07-20 Philippe Normand <[email protected]>
+
+ [GStreamer] ImageDecoder hits more Debug ASSERTs
+ https://bugs.webkit.org/show_bug.cgi?id=214191
+
+ Reviewed by Xabier Rodriguez-Calvar.
+
+ Process decoded video samples from the decoder parent thread, in order to guarantee the
+ underlying RefPtrs stored in the ImageDecoderGStreamerSample keep a consistent owning
+ runloop.
+
+ * platform/graphics/gstreamer/ImageDecoderGStreamer.cpp:
+ (WebCore::ImageDecoderGStreamer::InnerDecoder::connectDecoderPad):
+ (WebCore::ImageDecoderGStreamer::setHasEOS):
+ (WebCore::ImageDecoderGStreamer::notifySample):
+ (WebCore::ImageDecoderGStreamer::InnerDecoder::handleMessage):
+ (WebCore::ImageDecoderGStreamer::pushEncodedData):
+ * platform/graphics/gstreamer/ImageDecoderGStreamer.h:
+
2020-07-20 Fujii Hironori <[email protected]>
Use forward template declarations for ColorComponents instead of including ColorComponents.h
Modified: trunk/Source/WebCore/platform/graphics/gstreamer/ImageDecoderGStreamer.cpp (264594 => 264595)
--- trunk/Source/WebCore/platform/graphics/gstreamer/ImageDecoderGStreamer.cpp 2020-07-20 09:40:06 UTC (rev 264594)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/ImageDecoderGStreamer.cpp 2020-07-20 10:26:11 UTC (rev 264595)
@@ -29,6 +29,7 @@
#include "MediaSampleGStreamer.h"
#include "NotImplemented.h"
#include <gst/app/gstappsink.h>
+#include <wtf/Lock.h>
#include <wtf/MainThread.h>
#include <wtf/Optional.h>
#include <wtf/Threading.h>
@@ -228,12 +229,12 @@
nullptr,
[](GstAppSink* sink, gpointer userData) -> GstFlowReturn {
auto sample = adoptGRef(gst_app_sink_try_pull_preroll(sink, 0));
- static_cast<ImageDecoderGStreamer*>(userData)->handleSample(WTFMove(sample));
+ static_cast<ImageDecoderGStreamer*>(userData)->notifySample(WTFMove(sample));
return GST_FLOW_OK;
},
[](GstAppSink* sink, gpointer userData) -> GstFlowReturn {
auto sample = adoptGRef(gst_app_sink_try_pull_sample(sink, 0));
- static_cast<ImageDecoderGStreamer*>(userData)->handleSample(WTFMove(sample));
+ static_cast<ImageDecoderGStreamer*>(userData)->notifySample(WTFMove(sample));
return GST_FLOW_OK;
},
{ nullptr }
@@ -253,16 +254,33 @@
gst_element_sync_state_with_parent(sink);
}
-void ImageDecoderGStreamer::handleSample(GRefPtr<GstSample>&& sample)
+void ImageDecoderGStreamer::setHasEOS()
{
- auto* caps = gst_sample_get_caps(sample.get());
- GST_DEBUG("Handling sample with caps %" GST_PTR_FORMAT, caps);
- auto presentationSize = getVideoResolutionFromCaps(caps);
- if (presentationSize && !presentationSize->isEmpty() && (!m_size || m_size != roundedIntSize(*presentationSize)))
- m_size = roundedIntSize(*presentationSize);
- m_sampleData.addSample(ImageDecoderGStreamerSample::create(WTFMove(sample), *m_size));
+ GST_DEBUG("EOS on decoder %p", this);
+ {
+ LockHolder lock(m_sampleMutex);
+ m_eos = true;
+ m_sampleCondition.notifyOne();
+ }
+ {
+ LockHolder lock(m_handlerMutex);
+ m_handlerCondition.wait(m_handlerMutex);
+ }
}
+void ImageDecoderGStreamer::notifySample(GRefPtr<GstSample>&& sample)
+{
+ {
+ LockHolder lock(m_sampleMutex);
+ m_sample = WTFMove(sample);
+ m_sampleCondition.notifyOne();
+ }
+ {
+ LockHolder lock(m_handlerMutex);
+ m_handlerCondition.wait(m_handlerMutex);
+ }
+}
+
void ImageDecoderGStreamer::InnerDecoder::handleMessage(GstMessage* message)
{
ASSERT(&m_runLoop == &RunLoop::current());
@@ -272,8 +290,7 @@
switch (GST_MESSAGE_TYPE(message)) {
case GST_MESSAGE_EOS:
- gst_element_set_state(m_pipeline.get(), GST_STATE_NULL);
- m_runLoop.stop();
+ m_decoder.setHasEOS();
break;
case GST_MESSAGE_WARNING:
gst_message_parse_warning(message, &error.outPtr(), &debug.outPtr());
@@ -282,8 +299,7 @@
case GST_MESSAGE_ERROR:
gst_message_parse_error(message, &error.outPtr(), &debug.outPtr());
g_warning("Error: %d, %s. Debug output: %s", error->code, error->message, debug.get());
- gst_element_set_state(m_pipeline.get(), GST_STATE_NULL);
- m_runLoop.stop();
+ m_decoder.setHasEOS();
break;
default:
break;
@@ -351,9 +367,32 @@
auto thread = Thread::create("ImageDecoderGStreamer", [this, data = "" size = buffer.size()] {
m_innerDecoder = ImageDecoderGStreamer::InnerDecoder::create(*this, data, size);
m_innerDecoder->run();
- });
- thread->waitForCompletion();
- m_eos = true;
+ }, ThreadType::Graphics);
+ thread->detach();
+ bool isEOS = false;
+ {
+ LockHolder lock(m_sampleMutex);
+ isEOS = m_eos;
+ }
+ while (!isEOS) {
+ {
+ LockHolder lock(m_sampleMutex);
+ m_sampleCondition.wait(m_sampleMutex);
+ isEOS = m_eos;
+ if (m_sample) {
+ auto* caps = gst_sample_get_caps(m_sample.get());
+ GST_DEBUG("Handling sample with caps %" GST_PTR_FORMAT " on decoder %p", caps, this);
+ auto presentationSize = getVideoResolutionFromCaps(caps);
+ if (presentationSize && !presentationSize->isEmpty() && (!m_size || m_size != roundedIntSize(*presentationSize)))
+ m_size = roundedIntSize(*presentationSize);
+ m_sampleData.addSample(ImageDecoderGStreamerSample::create(WTFMove(m_sample), *m_size));
+ }
+ }
+ {
+ LockHolder lock(m_handlerMutex);
+ m_handlerCondition.notifyAll();
+ }
+ }
m_innerDecoder = nullptr;
callOnMainThread([this] {
if (m_encodedDataStatusChangedCallback)
Modified: trunk/Source/WebCore/platform/graphics/gstreamer/ImageDecoderGStreamer.h (264594 => 264595)
--- trunk/Source/WebCore/platform/graphics/gstreamer/ImageDecoderGStreamer.h 2020-07-20 09:40:06 UTC (rev 264594)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/ImageDecoderGStreamer.h 2020-07-20 10:26:11 UTC (rev 264595)
@@ -74,6 +74,9 @@
bool isAllDataReceived() const final { return m_eos; }
void clearFrameBufferCache(size_t) final;
+ void setHasEOS();
+ void notifySample(GRefPtr<GstSample>&&);
+
private:
class InnerDecoder : public ThreadSafeRefCounted<InnerDecoder>, public CanMakeWeakPtr<InnerDecoder> {
WTF_MAKE_FAST_ALLOCATED;
@@ -91,6 +94,11 @@
m_memoryStream = adoptGRef(g_memory_input_stream_new_from_data(data, size, nullptr));
}
+ ~InnerDecoder()
+ {
+ gst_element_set_state(m_pipeline.get(), GST_STATE_NULL);
+ }
+
void run();
EncodedDataStatus encodedDataStatus() const;
@@ -119,7 +127,11 @@
Optional<IntSize> m_size;
String m_mimeType;
RefPtr<ImageDecoderGStreamer::InnerDecoder> m_innerDecoder;
+ Condition m_sampleCondition;
+ Lock m_sampleMutex;
+ GRefPtr<GstSample> m_sample;
+ Condition m_handlerCondition;
+ Lock m_handlerMutex;
};
-
}
#endif
