Skip to content

Commit

Permalink
[GTK][GStreamer] VA+DMABuf videos flicker
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=253807

Reviewed by Xabier Rodriguez-Calvar.

This is the dumb approach of storing references and dropping them upon new samples. It still has
flaws since the is-released check is not necessarily consistent and can lead to retained GstMemory
references, draining the decoder of available dmabuf targets for decoding.

Co-authored-by: Philippe Normand <philn@igalia.com>

* Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.cpp:
(WTF::adoptGRef):
(WTF::refGPtr<GstMemory>):
(WTF::derefGPtr<GstMemory>):
* Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.h:
* Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
(WebCore::MediaPlayerPrivateGStreamer::pushDMABufToCompositor):
* Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h:

Canonical link: https://commits.webkit.org/270971@main
  • Loading branch information
zdobersek authored and philn committed Nov 20, 2023
1 parent a655840 commit 2f37937
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 8 deletions.
18 changes: 18 additions & 0 deletions Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,24 @@ template<> void derefGPtr<GstBufferPool>(GstBufferPool* ptr)
gst_object_unref(ptr);
}

template<> GRefPtr<GstMemory> adoptGRef(GstMemory* ptr)
{
return GRefPtr<GstMemory>(ptr, GRefPtrAdopt);
}

template<> GstMemory* refGPtr<GstMemory>(GstMemory* ptr)
{
if (ptr)
gst_memory_ref(ptr);
return ptr;
}

template<> void derefGPtr<GstMemory>(GstMemory* ptr)
{
if (ptr)
gst_memory_unref(ptr);
}

template<> GRefPtr<GstSample> adoptGRef(GstSample* ptr)
{
return GRefPtr<GstSample>(ptr, GRefPtrAdopt);
Expand Down
4 changes: 4 additions & 0 deletions Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.h
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,10 @@ template<> GRefPtr<GstBufferPool> adoptGRef(GstBufferPool*);
template<> GstBufferPool* refGPtr<GstBufferPool>(GstBufferPool*);
template<> void derefGPtr<GstBufferPool>(GstBufferPool*);

template<> GRefPtr<GstMemory> adoptGRef(GstMemory*);
template<> GstMemory* refGPtr<GstMemory>(GstMemory*);
template<> void derefGPtr<GstMemory>(GstMemory*);

template<> GRefPtr<GstSample> adoptGRef(GstSample* ptr);
template<> GstSample* refGPtr<GstSample>(GstSample* ptr);
template<> void derefGPtr<GstSample>(GstSample* ptr);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3356,8 +3356,21 @@ static DMABufColorSpace colorSpaceForColorimetry(const GstVideoColorimetry* cinf
return DMABufColorSpace::Invalid;
}

struct DMABufMemoryQuarkData {
DMABufReleaseFlag releaseFlag;
};

G_DEFINE_QUARK(DMABufMemoryQuarkData, dmabuf_memory);

void MediaPlayerPrivateGStreamer::pushDMABufToCompositor()
{
m_dmabufMemory.removeIf([](auto& memory) -> bool {
auto* quarkData = static_cast<DMABufMemoryQuarkData*>(gst_mini_object_get_qdata(GST_MINI_OBJECT_CAST(memory.get()), dmabuf_memory_quark()));
if (!quarkData)
return true;
return quarkData->releaseFlag.released();
});

Locker sampleLocker { m_sampleMutex };
if (!GST_IS_SAMPLE(m_sample.get()))
return;
Expand Down Expand Up @@ -3404,11 +3417,12 @@ void MediaPlayerPrivateGStreamer::pushDMABufToCompositor()
if (isDMABufMemory) {
// In case of a hardware decoder that's yielding dmabuf memory, we can take the relevant data and
// push it into the composition process.
auto memory = adoptGRef(gst_buffer_get_memory(buffer, 0));

// Provide the DMABufObject with a relevant handle (memory address). When provided for the first time,
// the lambda will be invoked and all dmabuf data is filled in.
downcast<TextureMapperPlatformLayerProxyDMABuf>(proxy).pushDMABuf(
DMABufObject(reinterpret_cast<uintptr_t>(gst_buffer_peek_memory(buffer, 0))),
DMABufObject(reinterpret_cast<uintptr_t>(memory.get())),
[&](auto&& object) {
bool infoHasDrmFormat = false;
uint32_t fourcc = 0;
Expand All @@ -3426,13 +3440,18 @@ void MediaPlayerPrivateGStreamer::pushDMABufToCompositor()
object.width = GST_VIDEO_INFO_WIDTH(&videoInfo);
object.height = GST_VIDEO_INFO_HEIGHT(&videoInfo);

// TODO: release mechanism for a decoder-provided dmabuf is a bit tricky. The dmabuf object
// itself doesn't provide anything useful, but the decoder won't reuse the dmabuf until the
// relevant GstSample reference is dropped by the downstream pipeline. So for this to work,
// there's a need to somehow associate the GstSample reference with this release flag so that
// the reference is dropped once the release flag is signalled. There's ways to achieve that,
// but left for later.
object.releaseFlag = DMABufReleaseFlag { };
// The dmabuf object itself doesn't provide anything useful, but the decoder won't
// reuse the dmabuf until the relevant GstSample reference is dropped by the
// downstream pipeline. So for this to work, we associate the GstMemory reference
// count with this release flag so that the reference is dropped once the release
// flag is signalled.
object.releaseFlag = DMABufReleaseFlag(DMABufReleaseFlag::Initialize);

gst_mini_object_set_qdata(GST_MINI_OBJECT_CAST(memory.get()), dmabuf_memory_quark(),
new DMABufMemoryQuarkData { object.releaseFlag.dup() },
[](gpointer data) {
delete static_cast<DMABufMemoryQuarkData*>(data);
});

// For each plane, the relevant data (stride, offset, skip, dmabuf fd) is retrieved and assigned
// as appropriate.
Expand Down Expand Up @@ -3460,6 +3479,11 @@ void MediaPlayerPrivateGStreamer::pushDMABufToCompositor()
}
return WTFMove(object);
}, m_textureMapperFlags);

auto* quarkData = static_cast<DMABufMemoryQuarkData*>(gst_mini_object_get_qdata(GST_MINI_OBJECT_CAST(memory.get()), dmabuf_memory_quark()));
if (quarkData)
m_dmabufMemory.add(WTFMove(memory));

return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,7 @@ class MediaPlayerPrivateGStreamer : public MediaPlayerPrivateInterface
String m_errorMessage;

#if USE(TEXTURE_MAPPER_DMABUF)
HashSet<GRefPtr<GstMemory>> m_dmabufMemory;
RefPtr<GBMBufferSwapchain> m_swapchain;
#endif

Expand Down

0 comments on commit 2f37937

Please sign in to comment.