Skip to content

Commit

Permalink
[GStreamer][WebCodecs][Debug] Assertions due to incorrect usage of We…
Browse files Browse the repository at this point in the history 
…akPtr

https://bugs.webkit.org/show_bug.cgi?id=264704

Reviewed by Xabier Rodriguez-Calvar.

Adapt audio/video encoders to rely on ThreadSafeWeakPtr, that can be used from non-main threads.

* Source/WebCore/platform/audio/gstreamer/AudioEncoderGStreamer.cpp:
(WebCore::GStreamerAudioEncoder::encode):
(WebCore::GStreamerInternalAudioEncoder::GStreamerInternalAudioEncoder):
* Source/WebCore/platform/graphics/gstreamer/VideoEncoderGStreamer.cpp:
(WebCore::GStreamerVideoEncoder::encode):
(WebCore::GStreamerInternalVideoEncoder::GStreamerInternalVideoEncoder):

Canonical link: https://commits.webkit.org/270632@main
  • Loading branch information
@philn
philn committed Nov 13, 2023
1 parent 711b192 commit 54140a7
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 30 deletions.
35 changes: 17 additions & 18 deletions Source/WebCore/platform/audio/gstreamer/AudioEncoderGStreamer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ static WorkQueue& gstEncoderWorkQueue()
return queue.get();
}

class GStreamerInternalAudioEncoder : public ThreadSafeRefCounted<GStreamerInternalAudioEncoder>
, public CanMakeWeakPtr<GStreamerInternalAudioEncoder, WeakPtrFactoryInitialization::Eager> {
class GStreamerInternalAudioEncoder : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<GStreamerInternalAudioEncoder, WTF::DestructionThread::Main> {
WTF_MAKE_NONCOPYABLE(GStreamerInternalAudioEncoder);
WTF_MAKE_FAST_ALLOCATED;

public:
Expand Down Expand Up @@ -152,10 +152,9 @@ void GStreamerAudioEncoder::encode(RawFrame&& frame, EncodeCallback&& callback)
else
resultString = "Encoding failed"_s;

encoder->postTask([weakEncoder = WeakPtr { encoder.get() }, encoder, result = WTFMove(resultString), callback = WTFMove(callback)]() mutable {
if (!weakEncoder)
return;
if (encoder->isClosed())
encoder->postTask([weakEncoder = ThreadSafeWeakPtr { encoder.get() }, result = WTFMove(resultString), callback = WTFMove(callback)]() mutable {
auto encoder = weakEncoder.get();
if (!encoder || encoder->isClosed())
return;

callback(WTFMove(result));
Expand Down Expand Up @@ -205,7 +204,8 @@ GStreamerInternalAudioEncoder::GStreamerInternalAudioEncoder(const String& codec

auto pad = adoptGRef(gst_element_get_static_pad(m_encoder.get(), "src"));
g_signal_connect_data(pad.get(), "notify::caps", G_CALLBACK(+[](GObject* pad, GParamSpec*, gpointer userData) {
WeakPtr<GStreamerInternalAudioEncoder> encoder(*static_cast<WeakPtr<GStreamerInternalAudioEncoder>*>(userData));
auto weakEncoder = static_cast<ThreadSafeWeakPtr<GStreamerInternalAudioEncoder>*>(userData);
auto encoder = weakEncoder->get();
if (!encoder)
return;

Expand All @@ -214,8 +214,9 @@ GStreamerInternalAudioEncoder::GStreamerInternalAudioEncoder(const String& codec
if (!caps)
return;

encoder.get()->postTask([weakEncoder = WeakPtr { *encoder.get() }, caps = WTFMove(caps)] {
if (!weakEncoder)
encoder->postTask([weakEncoder = WTFMove(weakEncoder), caps = WTFMove(caps)] {
auto encoder = weakEncoder->get();
if (!encoder)
return;

auto structure = gst_caps_get_structure(caps.get(), 0);
Expand Down Expand Up @@ -243,14 +244,14 @@ GStreamerInternalAudioEncoder::GStreamerInternalAudioEncoder(const String& codec
if (gst_structure_get_int(structure, "rate", &sampleRate))
configuration.sampleRate = sampleRate;

weakEncoder.get()->m_descriptionCallback(WTFMove(configuration));
encoder->m_descriptionCallback(WTFMove(configuration));
});
}), new WeakPtr { *this }, [](void* data, GClosure*) {
delete static_cast<WeakPtr<GStreamerInternalAudioEncoder>*>(data);
}), new ThreadSafeWeakPtr { *this }, [](void* data, GClosure*) {
delete static_cast<ThreadSafeWeakPtr<GStreamerInternalAudioEncoder>*>(data);
}, static_cast<GConnectFlags>(0));

m_harness = GStreamerElementHarness::create(WTFMove(harnessedElement), [weakThis = WeakPtr { *this }, this](auto&, const GRefPtr<GstBuffer>& outputBuffer) {
if (!weakThis)
m_harness = GStreamerElementHarness::create(WTFMove(harnessedElement), [weakThis = ThreadSafeWeakPtr { *this }, this](auto&, const GRefPtr<GstBuffer>& outputBuffer) {
if (!weakThis.get())
return;
if (m_isClosed)
return;
Expand All @@ -269,10 +270,8 @@ GStreamerInternalAudioEncoder::GStreamerInternalAudioEncoder(const String& codec
GST_TRACE_OBJECT(m_harness->element(), "Notifying encoded%s frame", isKeyFrame ? " key" : "");
GstMappedBuffer mappedBuffer(outputBuffer.get(), GST_MAP_READ);
AudioEncoder::EncodedFrame encodedFrame { mappedBuffer.createVector(), isKeyFrame, m_timestamp, m_duration };
m_postTaskCallback([weakThis = WeakPtr { *this }, this, encodedFrame = WTFMove(encodedFrame)]() mutable {
if (!weakThis)
return;
if (m_isClosed)
m_postTaskCallback([protectedThis = Ref { *this }, this, encodedFrame = WTFMove(encodedFrame)]() mutable {
if (protectedThis->m_isClosed)
return;
m_outputCallback({ WTFMove(encodedFrame) });
});
Expand Down
27 changes: 15 additions & 12 deletions Source/WebCore/platform/graphics/gstreamer/VideoEncoderGStreamer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ static WorkQueue& gstEncoderWorkQueue()
return queue.get();
}

class GStreamerInternalVideoEncoder : public ThreadSafeRefCounted<GStreamerInternalVideoEncoder>
, public CanMakeWeakPtr<GStreamerInternalVideoEncoder, WeakPtrFactoryInitialization::Eager> {
class GStreamerInternalVideoEncoder : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<GStreamerInternalVideoEncoder, WTF::DestructionThread::Main> {
WTF_MAKE_NONCOPYABLE(GStreamerInternalVideoEncoder);
WTF_MAKE_FAST_ALLOCATED;

public:
Expand Down Expand Up @@ -143,8 +143,9 @@ void GStreamerVideoEncoder::encode(RawFrame&& frame, bool shouldGenerateKeyFrame
else
resultString = "Encoding failed"_s;

encoder->postTask([weakEncoder = WeakPtr { encoder.get() }, result = WTFMove(resultString), callback = WTFMove(callback)]() mutable {
if (!weakEncoder || weakEncoder->isClosed())
encoder->postTask([weakEncoder = ThreadSafeWeakPtr { encoder.get() }, result = WTFMove(resultString), callback = WTFMove(callback)]() mutable {
auto encoder = weakEncoder.get();
if (!encoder || encoder->isClosed())
return;

callback(WTFMove(result));
Expand Down Expand Up @@ -181,7 +182,8 @@ GStreamerInternalVideoEncoder::GStreamerInternalVideoEncoder(const String& codec

auto pad = adoptGRef(gst_element_get_static_pad(element.get(), "src"));
g_signal_connect_data(pad.get(), "notify::caps", G_CALLBACK(+[](GObject* pad, GParamSpec*, gpointer userData) {
WeakPtr<GStreamerInternalVideoEncoder> encoder(*static_cast<WeakPtr<GStreamerInternalVideoEncoder>*>(userData));
auto weakEncoder = static_cast<ThreadSafeWeakPtr<GStreamerInternalVideoEncoder>*>(userData);
auto encoder = weakEncoder->get();
if (!encoder)
return;

Expand All @@ -190,8 +192,9 @@ GStreamerInternalVideoEncoder::GStreamerInternalVideoEncoder(const String& codec
if (!caps)
return;

encoder.get()->postTask([weakEncoder = WeakPtr { *encoder.get() }, caps = WTFMove(caps)] {
if (!weakEncoder)
encoder->postTask([weakEncoder = WTFMove(weakEncoder), caps = WTFMove(caps)] {
auto encoder = weakEncoder->get();
if (!encoder)
return;

VideoEncoder::ActiveConfiguration configuration;
Expand All @@ -213,14 +216,14 @@ GStreamerInternalVideoEncoder::GStreamerInternalVideoEncoder(const String& codec
GstMappedBuffer buffer(header, GST_MAP_READ);
configuration.description = { { buffer.data(), buffer.size() } };
}
weakEncoder.get()->m_descriptionCallback(WTFMove(configuration));
encoder->m_descriptionCallback(WTFMove(configuration));
});
}), new WeakPtr { *this }, [](void* data, GClosure*) {
delete static_cast<WeakPtr<GStreamerInternalVideoEncoder>*>(data);
}), new ThreadSafeWeakPtr { *this }, [](void* data, GClosure*) {
delete static_cast<ThreadSafeWeakPtr<GStreamerInternalVideoEncoder>*>(data);
}, static_cast<GConnectFlags>(0));

m_harness = GStreamerElementHarness::create(WTFMove(element), [weakThis = WeakPtr { *this }, this](auto&, const GRefPtr<GstBuffer>& outputBuffer) {
if (!weakThis)
m_harness = GStreamerElementHarness::create(WTFMove(element), [weakThis = ThreadSafeWeakPtr { *this }, this](auto&, const GRefPtr<GstBuffer>& outputBuffer) {
if (!weakThis.get())
return;
if (m_isClosed)
return;
Expand Down

0 comments on commit 54140a7

Please sign in to comment.