[GStreamer] GstVideoFrame leaks

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

Unreviewed, revert due to incomplete fix.

Canonical link: https://commits.webkit.org/279063@main

Source/WebCore/platform/graphics/gstreamer/ImageDecoderGStreamer.cpp

@@ -26,6 +26,7 @@
 #include "GStreamerRegistryScanner.h"
 #include "ImageGStreamer.h"
 #include "MediaSampleGStreamer.h"
+#include "NotImplemented.h"
 #include "RuntimeApplicationChecks.h"
 #include "VideoFrameGStreamer.h"
 #include <gst/base/gsttypefindhelper.h>
@@ -38,25 +39,13 @@ GST_DEBUG_CATEGORY(webkit_image_decoder_debug);
 #define GST_CAT_DEFAULT webkit_image_decoder_debug
 
 static Lock s_decoderLock;
-static Vector<WeakPtr<ImageDecoderGStreamer>> s_imageDecoders;
-
-static void ensureDebugCategoryIsInitialized()
-{
-    static std::once_flag onceFlag;
-    std::call_once(onceFlag, [] {
-        GST_DEBUG_CATEGORY_INIT(webkit_image_decoder_debug, "webkitimagedecoder", 0, "WebKit image decoder");
-    });
-}
+static Vector<RefPtr<ImageDecoderGStreamer>> s_imageDecoders;
 
 void teardownGStreamerImageDecoders()
 {
-    ensureDebugCategoryIsInitialized();
     Locker lock { s_decoderLock };
-    GST_DEBUG("Tearing down %zu image decoders", s_imageDecoders.size());
-    for (auto& weakDecoder : s_imageDecoders) {
-        if (RefPtr decoder = weakDecoder.get())
-            decoder->tearDown();
-    }
+    for (auto& decoder : s_imageDecoders)
+        decoder->tearDown();
     s_imageDecoders.clear();
 }
 
@@ -71,11 +60,12 @@ class ImageDecoderGStreamerSample final : public MediaSampleGStreamer {
     {
         if (!m_image)
             return nullptr;
-        return m_image->image();
+        return m_image->image().nativeImage()->platformImage();
     }
     void dropImage()
     {
         m_image = nullptr;
+        m_frame = nullptr;
     }
 
     SampleFlags flags() const override
@@ -86,8 +76,8 @@ class ImageDecoderGStreamerSample final : public MediaSampleGStreamer {
 private:
     ImageDecoderGStreamerSample(GRefPtr<GstSample>&& sample, const FloatSize& presentationSize)
         : MediaSampleGStreamer(WTFMove(sample), presentationSize, { })
-        , m_frame(VideoFrameGStreamer::createWrappedSample(this->sample()))
     {
+        m_frame = VideoFrameGStreamer::create(GRefPtr(platformSample().sample.gstSample), presentationSize);
         m_image = m_frame->convertToImage();
     }
 
@@ -111,15 +101,19 @@ RefPtr<ImageDecoderGStreamer> ImageDecoderGStreamer::create(FragmentedSharedBuff
     RefPtr decoder = adoptRef(*new ImageDecoderGStreamer(data, mimeType, alphaOption, gammaAndColorProfileOption));
     {
         Locker lock { s_decoderLock };
-        s_imageDecoders.append(WeakPtr { *decoder });
+        s_imageDecoders.append(decoder);
     }
     return decoder;
 }
 
 ImageDecoderGStreamer::ImageDecoderGStreamer(FragmentedSharedBuffer& data, const String& mimeType, AlphaOption, GammaAndColorProfileOption)
     : m_mimeType(mimeType)
 {
-    ensureDebugCategoryIsInitialized();
+    static std::once_flag onceFlag;
+    std::call_once(onceFlag, [] {
+        GST_DEBUG_CATEGORY_INIT(webkit_image_decoder_debug, "webkitimagedecoder", 0, "WebKit image decoder");
+    });
+
     static Atomic<uint32_t> decoderId;
     GRefPtr<GstElement> parsebin = gst_element_factory_make("parsebin", makeString("image-decoder-parser-"_s, decoderId.exchangeAdd(1)).utf8().data());
     m_parserHarness = GStreamerElementHarness::create(WTFMove(parsebin), [](auto&, auto&&) { }, [this](auto& pad) -> RefPtr<GStreamerElementHarness> {

Source/WebCore/platform/graphics/gstreamer/ImageDecoderGStreamer.h

@@ -29,7 +29,6 @@
 #include "SharedBuffer.h"
 #include <wtf/Forward.h>
 #include <wtf/Lock.h>
-#include <wtf/WeakPtr.h>
 
 namespace WebCore {
 
@@ -38,7 +37,7 @@ class ImageDecoderGStreamerSample;
 
 void teardownGStreamerImageDecoders();
 
-class ImageDecoderGStreamer final : public ImageDecoder, public CanMakeWeakPtr<ImageDecoderGStreamer> {
+class ImageDecoderGStreamer final : public ImageDecoder {
     WTF_MAKE_FAST_ALLOCATED;
     WTF_MAKE_NONCOPYABLE(ImageDecoderGStreamer);
 public:

Source/WebCore/platform/graphics/gstreamer/ImageGStreamer.h

@@ -21,50 +21,56 @@
 
 #if ENABLE(VIDEO) && USE(GSTREAMER)
 
+#include "BitmapImage.h"
 #include "FloatRect.h"
 #include "GStreamerCommon.h"
-#include "PlatformImage.h"
 
+#include <gst/gst.h>
 #include <gst/video/video-frame.h>
 
-#include <wtf/Forward.h>
+#include <wtf/Ref.h>
+#include <wtf/RefCounted.h>
+#include <wtf/RefPtr.h>
 
 namespace WebCore {
 class IntSize;
 
 class ImageGStreamer : public RefCounted<ImageGStreamer> {
 public:
-    static Ref<ImageGStreamer> create(GRefPtr<GstSample>&& sample)
+    static Ref<ImageGStreamer> createImage(GRefPtr<GstSample>&& sample)
     {
         return adoptRef(*new ImageGStreamer(WTFMove(sample)));
     }
     ~ImageGStreamer();
 
     operator bool() const { return !!m_image; }
 
-    PlatformImagePtr image() const { return m_image; }
+    BitmapImage& image()
+    {
+        ASSERT(m_image);
+        return *m_image.get();
+    }
 
     void setCropRect(FloatRect rect) { m_cropRect = rect; }
     FloatRect rect()
     {
         ASSERT(m_image);
         if (!m_cropRect.isEmpty())
             return FloatRect(m_cropRect);
-        return FloatRect(0, 0, m_size.width(), m_size.height());
+        return FloatRect(0, 0, m_image->size().width(), m_image->size().height());
     }
 
     bool hasAlpha() const { return m_hasAlpha; }
 
 private:
     ImageGStreamer(GRefPtr<GstSample>&&);
     GRefPtr<GstSample> m_sample;
-    PlatformImagePtr m_image;
+    RefPtr<BitmapImage> m_image;
     FloatRect m_cropRect;
 #if USE(CAIRO)
     GstVideoFrame m_videoFrame;
     bool m_frameMapped { false };
 #endif
-    FloatSize m_size;
     bool m_hasAlpha { false };
 };
 

Source/WebCore/platform/graphics/gstreamer/ImageGStreamerCairo.cpp

@@ -74,8 +74,6 @@ ImageGStreamer::ImageGStreamer(GRefPtr<GstSample>&& sample)
     int height = GST_VIDEO_FRAME_HEIGHT(&m_videoFrame);
     RefPtr<cairo_surface_t> surface;
 
-    m_size = { static_cast<float>(width), static_cast<float>(height) };
-
     if (m_hasAlpha || componentSwapRequired) {
         uint8_t* surfaceData = static_cast<uint8_t*>(fastMalloc(height * stride));
         uint8_t* surfacePixel = surfaceData;
@@ -158,7 +156,7 @@ ImageGStreamer::ImageGStreamer(GRefPtr<GstSample>&& sample)
         surface = adoptRef(cairo_image_surface_create_for_data(bufferData, CAIRO_FORMAT_ARGB32, width, height, stride));
 
     ASSERT(cairo_surface_status(surface.get()) == CAIRO_STATUS_SUCCESS);
-    m_image = WTFMove(surface);
+    m_image = BitmapImage::create(WTFMove(surface));
 
     if (GstVideoCropMeta* cropMeta = gst_buffer_get_video_crop_meta(buffer))
         setCropRect(FloatRect(cropMeta->x, cropMeta->y, cropMeta->width, cropMeta->height));

Source/WebCore/platform/graphics/gstreamer/ImageGStreamerSkia.cpp

@@ -28,39 +28,35 @@
 
 #if ENABLE(VIDEO) && USE(GSTREAMER) && USE(SKIA)
 
+#include "GStreamerCommon.h"
 #include "NotImplemented.h"
-#include <skia/ColorSpaceSkia.h>
+#include <gst/gst.h>
+#include <gst/video/gstvideometa.h>
 #include <skia/core/SkImage.h>
 
 IGNORE_CLANG_WARNINGS_BEGIN("cast-align")
 #include <skia/core/SkPixmap.h>
 IGNORE_CLANG_WARNINGS_END
 
-#include <wtf/StdLibExtras.h>
-#include <wtf/glib/WTFGType.h>
-
 namespace WebCore {
 
-struct BufferData {
-    std::span<const uint8_t> planeData;
-};
-WEBKIT_DEFINE_ASYNC_DATA_STRUCT(BufferData);
-
 ImageGStreamer::ImageGStreamer(GRefPtr<GstSample>&& sample)
     : m_sample(WTFMove(sample))
 {
     GstBuffer* buffer = gst_sample_get_buffer(m_sample.get());
     if (UNLIKELY(!GST_IS_BUFFER(buffer)))
         return;
 
-    auto videoFrame = makeUnique<GstMappedFrame>(m_sample, GST_MAP_READ);
-    if (!*videoFrame)
+    GstVideoInfo videoInfo;
+    if (!gst_video_info_from_caps(&videoInfo, gst_sample_get_caps(m_sample.get())))
         return;
 
-    auto videoInfo = videoFrame->info();
+    auto videoFrame = makeUnique<GstMappedFrame>(buffer, &videoInfo, GST_MAP_READ);
+    if (!*videoFrame)
+        return;
 
     // The frame has to be RGB so we can paint it.
-    ASSERT(GST_VIDEO_INFO_IS_RGB(videoInfo));
+    ASSERT(GST_VIDEO_INFO_IS_RGB(&videoInfo));
 
     // The video buffer may have these formats in these cases:
     // { BGRx, BGRA } on little endian
@@ -95,24 +91,17 @@ ImageGStreamer::ImageGStreamer(GRefPtr<GstSample>&& sample)
         break;
     }
 
-    m_size = { static_cast<float>(videoFrame->width()), static_cast<float>(videoFrame->height()) };
-
     auto toSkiaColorSpace = [](const PlatformVideoColorSpace&) {
         notImplemented();
         return SkColorSpace::MakeSRGB();
     };
-    auto imageInfo = SkImageInfo::Make(videoFrame->width(), videoFrame->height(), colorType, alphaType, toSkiaColorSpace(videoColorSpaceFromInfo(*videoInfo)));
-
-    // Copy the buffer data. Keeping the whole mapped GstVideoFrame alive would increase memory
-    // pressure and the file descriptor(s) associated with the buffer pool open. We only need the
-    // data here.
-    auto bufferData = createBufferData();
-    bufferData->planeData = { reinterpret_cast<const uint8_t*>(videoFrame->planeData(0)), static_cast<unsigned>(videoFrame->planeStride(0)) };
-
-    SkPixmap pixmap(imageInfo, bufferData->planeData.data(), bufferData->planeData.size_bytes());
-    m_image = SkImages::RasterFromPixmap(pixmap, [](const void*, void* context) {
-        destroyBufferData(reinterpret_cast<BufferData*>(context));
-    }, bufferData);
+    auto imageInfo = SkImageInfo::Make(videoFrame->width(), videoFrame->height(), colorType, alphaType, toSkiaColorSpace(videoColorSpaceFromInfo(videoInfo)));
+    SkPixmap pixmap(imageInfo, videoFrame->planeData(0), videoFrame->planeStride(0));
+    auto image = SkImages::RasterFromPixmap(pixmap, [](const void*, void* context) {
+        std::unique_ptr<GstMappedFrame> videoFrame(static_cast<GstMappedFrame*>(context));
+    }, videoFrame.release());
+
+    m_image = BitmapImage::create(WTFMove(image));
 
     if (auto* cropMeta = gst_buffer_get_video_crop_meta(buffer))
         m_cropRect = FloatRect(cropMeta->x, cropMeta->y, cropMeta->width, cropMeta->height);

Source/WebCore/platform/graphics/gstreamer/MediaSampleGStreamer.h

@@ -54,8 +54,6 @@ class MediaSampleGStreamer : public MediaSample {
     PlatformSample::Type platformSampleType() const override { return PlatformSample::GStreamerSampleType; }
     void dump(PrintStream&) const override;
 
-    const GRefPtr<GstSample>& sample() const { return m_sample; }
-
 protected:
     MediaSampleGStreamer(GRefPtr<GstSample>&&, const FloatSize& presentationSize, TrackID);
     virtual ~MediaSampleGStreamer() = default;

Source/WebCore/platform/graphics/gstreamer/VideoFrameGStreamer.cpp

@@ -24,7 +24,6 @@
 
 #if ENABLE(VIDEO) && USE(GSTREAMER)
 
-#include "BitmapImage.h"
 #include "GLContext.h"
 #include "GStreamerCommon.h"
 #include "GraphicsContext.h"
@@ -83,7 +82,7 @@ static RefPtr<ImageGStreamer> convertSampleToImage(const GRefPtr<GstSample>& sam
     if (!convertedSample)
         return nullptr;
 
-    return ImageGStreamer::create(WTFMove(convertedSample));
+    return ImageGStreamer::createImage(WTFMove(convertedSample));
 }
 
 RefPtr<VideoFrame> VideoFrame::fromNativeImage(NativeImage& image)
@@ -541,7 +540,7 @@ void VideoFrame::copyTo(std::span<uint8_t> destination, VideoPixelFormat pixelFo
     callback({ });
 }
 
-void VideoFrame::paintInContext(GraphicsContext& context, const FloatRect& destination, const ImageOrientation& destinationImageOrientation, bool shouldDiscardAlpha)
+void VideoFrame::paintInContext(GraphicsContext & context, const FloatRect& destination, const ImageOrientation& destinationImageOrientation, bool shouldDiscardAlpha)
 {
     auto image = convertSampleToImage(downcast<VideoFrameGStreamer>(*this).sample());
     if (!image)
@@ -550,11 +549,7 @@ void VideoFrame::paintInContext(GraphicsContext& context, const FloatRect& desti
     auto imageRect = image->rect();
     auto source = destinationImageOrientation.usesWidthAsHeight() ? FloatRect(imageRect.location(), imageRect.size().transposedSize()) : imageRect;
     auto compositeOperator = !shouldDiscardAlpha && image->hasAlpha() ? CompositeOperator::SourceOver : CompositeOperator::Copy;
-    auto platformImage = image->image();
-    auto bitmapImage = BitmapImage::create(WTFMove(platformImage));
-    if (!bitmapImage)
-        return;
-    context.drawImage(*bitmapImage.get(), destination, source, { compositeOperator, destinationImageOrientation });
+    context.drawImage(image->image(), destination, source, { compositeOperator, destinationImageOrientation });
 }
 
 uint32_t VideoFrameGStreamer::pixelFormat() const