Bug 291277 – [GStreamer] triggerRepaint() should allow a null previousBuffer

• Component: New Bugs
• Status: RESOLVED FIXED

[Enrique Ocaña]

We're getting some crashes downstream in https://ytlr-cert.appspot.com/2021/main.html?&test_type=progressive-test&command=run on Raspberry Pi, and they're caused by RELEASE_ASSERT(previousBuffer) in MediaPlayerPrivateGStreamer::triggerRepaint() [1]. When debugged, I realized that the null previousBuffer was set by the gst_buffer_copy_deep() in MediaPlayerPrivateGStreamer::flushCurrentBuffer() [2]. This means that the deep copy failed to produce a valid buffer. This is sort of expected in Raspberry Pi when using the OMX decoder, since those buffers come from a special buffer pool managed by OMX and reference scarce GPU memory (caps: video/x-raw(memory:GLMemory)).

This special situation needs to be had into account and the assert removed.

[1] https://github.com/WebKit/WebKit/blob/main/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp#L3785
[2] https://github.com/WebKit/WebKit/blob/main/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp#L3886

[Enrique Ocaña]

I can take care of the change, but I'd like to CC Phil, since he was the one who added the assert in https://github.com/WebKit/WebKit/commit/1ce41f04b7758febc0912d7cbd117e82e0d4f34d.

[Enrique Ocaña]

Oh, also I forgot to paste the extra log lines that I added (copying only the most relevant chunk)...

@@ -4074,9 +4079,11 @@ void MediaPlayerPrivateGStreamer::flushCurrentBuffer()
         // necessary because the sample might have been allocated by a hardware decoder and memory
         // might have to be reclaimed by a non-sysmem buffer pool.
         const GstStructure* info = gst_sample_get_info(m_sample.get());
+        GRefPtr<GstBuffer> oldBuffer = gst_sample_get_buffer(m_sample.get());
         auto buffer = adoptGRef(gst_buffer_copy_deep(gst_sample_get_buffer(m_sample.get())));
         m_sample = adoptGRef(gst_sample_new(buffer.get(), gst_sample_get_caps(m_sample.get()),
             gst_sample_get_segment(m_sample.get()), info ? gst_structure_copy(info) : nullptr));
+        GST_DEBUG("(A) m_sample %p -> buffer = %p, copied buffer = %p", m_sample.get(), oldBuffer.get(), gst_sample_get_buffer(m_sample.get()));
     }

...which prove that the buffer can't be copied:

0:00:33.648841549   966  0x11dd320 DEBUG      webkitmediaplayer Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:230:~MediaPlayerPrivateGStreamer: ~MediaPlayerPrivateGStreamer(): 0x61783b40
0:00:33.966290195   966  0x11dd320 DEBUG      webkitmediaplayer Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:188:MediaPlayerPrivateGStreamer: MediaPlayerPrivateGStreamer(): 0x5f467000
0:00:34.687093632   966 0x537166f0 DEBUG      webkitmediaplayer Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:3956:triggerRepaint: (B) m_sample 0x6b81ecb0 -> buffer = 0x53737950
0:00:36.801996027   966 0x537166f0 DEBUG      webkitmediaplayer Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:3956:triggerRepaint: (B) m_sample 0x6b81ec10 -> buffer = 0x53737950
0:00:36.856909256   966 0x537166f0 DEBUG      webkitmediaplayer Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:3956:triggerRepaint: (B) m_sample 0x64d6f960 -> buffer = 0x535cfbf0
0:00:36.870268267   966  0x11dd320 DEBUG      webkitmediaplayer Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:4086:flushCurrentBuffer: (A) m_sample 0x19b5820 -> buffer = 0x535cfbf0, copied buffer = (nil)
0:00:40.443328526   966 0x537166f0 ERROR      webkitmediaplayer Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:3951:triggerRepaint: No previousBuffer. MediaPlayerPrivateGStreamer 0x5f467000

[Enrique Ocaña]

Pull request: https://github.com/WebKit/WebKit/pull/43808

[EWS]

Committed 293459@main (29aaf6c0d8ef): <https://commits.webkit.org/293459@main>

Reviewed commits have been landed. Closing PR #43808 and removing active labels.

[Radar WebKit Bug Importer]

<rdar://problem/148904602>