[GStreamer] Utilities cleanups
authorphiln@webkit.org <philn@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 24 Sep 2018 10:20:38 +0000 (10:20 +0000)
committerphiln@webkit.org <philn@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 24 Sep 2018 10:20:38 +0000 (10:20 +0000)
https://bugs.webkit.org/show_bug.cgi?id=189699
<rdar://problem/44634143>

Reviewed by Xabier Rodriguez-Calvar.

The GstMappedBuffer now has a move constructor so that it can be easily
reused in the webaudiosrc element. The now-unused corresponding
buffer-mapping utilities are removed from the code-base.

The HTTP source element used to handle a GstBuffer in its private
structure but this is no longer required since data is now pushed
in chunks, see bug #182829.

* platform/audio/gstreamer/WebKitWebAudioSourceGStreamer.cpp:
(webKitWebAudioSrcLoop):
* platform/graphics/gstreamer/GStreamerCommon.cpp:
(WebCore::createGstBuffer): Deleted.
(WebCore::createGstBufferForData): Deleted.
(WebCore::getGstBufferDataPointer): Deleted.
(WebCore::mapGstBuffer): Deleted.
(WebCore::unmapGstBuffer): Deleted.
* platform/graphics/gstreamer/GStreamerCommon.h:
(WebCore::GstMappedBuffer::create): New method returning a
reference to a newly created GstMappedBuffer instance.
* platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:
(webKitWebSrcStop): Remove reference to unused GstBuffer.
(CachedResourceStreamingClient::dataReceived): Ditto.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@236396 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/audio/gstreamer/WebKitWebAudioSourceGStreamer.cpp
Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp
Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h
Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp

index 762b3e0..9050da2 100644 (file)
@@ -1,3 +1,34 @@
+2018-09-24  Philippe Normand  <pnormand@igalia.com>
+
+        [GStreamer] Utilities cleanups
+        https://bugs.webkit.org/show_bug.cgi?id=189699
+        <rdar://problem/44634143>
+
+        Reviewed by Xabier Rodriguez-Calvar.
+
+        The GstMappedBuffer now has a move constructor so that it can be easily
+        reused in the webaudiosrc element. The now-unused corresponding
+        buffer-mapping utilities are removed from the code-base.
+
+        The HTTP source element used to handle a GstBuffer in its private
+        structure but this is no longer required since data is now pushed
+        in chunks, see bug #182829.
+
+        * platform/audio/gstreamer/WebKitWebAudioSourceGStreamer.cpp:
+        (webKitWebAudioSrcLoop):
+        * platform/graphics/gstreamer/GStreamerCommon.cpp:
+        (WebCore::createGstBuffer): Deleted.
+        (WebCore::createGstBufferForData): Deleted.
+        (WebCore::getGstBufferDataPointer): Deleted.
+        (WebCore::mapGstBuffer): Deleted.
+        (WebCore::unmapGstBuffer): Deleted.
+        * platform/graphics/gstreamer/GStreamerCommon.h:
+        (WebCore::GstMappedBuffer::create): New method returning a
+        reference to a newly created GstMappedBuffer instance.
+        * platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:
+        (webKitWebSrcStop): Remove reference to unused GstBuffer.
+        (CachedResourceStreamingClient::dataReceived): Ditto.
+
 2018-09-24  Enrique Ocaña González  <eocanha@igalia.com>
 
         [MSE][GStreamer] Don't update duration when it was not previously NaN
index 55cdb42..9a1d197 100644 (file)
@@ -308,7 +308,7 @@ static void webKitWebAudioSrcGetProperty(GObject* object, guint propertyId, GVal
     }
 }
 
-static void webKitWebAudioSrcLoop(WebKitWebAudioSrc* src)
+static std::optional<Vector<GRefPtr<GstBuffer>>> webKitWebAudioSrcAllocateBuffersAndRenderAudio(WebKitWebAudioSrc* src)
 {
     WebKitWebAudioSourcePrivate* priv = src->priv;
 
@@ -317,7 +317,7 @@ static void webKitWebAudioSrcLoop(WebKitWebAudioSrc* src)
     if (!priv->provider || !priv->bus) {
         GST_ELEMENT_ERROR(src, CORE, FAILED, ("Internal WebAudioSrc error"), ("Can't start without provider or bus"));
         gst_task_stop(src->priv->task.get());
-        return;
+        return std::nullopt;
     }
 
     ASSERT(priv->pool);
@@ -327,37 +327,49 @@ static void webKitWebAudioSrcLoop(WebKitWebAudioSrc* src)
 
     Vector<GRefPtr<GstBuffer>> channelBufferList;
     channelBufferList.reserveInitialCapacity(priv->sources.size());
+    Vector<GstMappedBuffer> mappedBuffers;
+    mappedBuffers.reserveInitialCapacity(priv->sources.size());
     for (unsigned i = 0; i < priv->sources.size(); ++i) {
         GRefPtr<GstBuffer> buffer;
         GstFlowReturn ret = gst_buffer_pool_acquire_buffer(priv->pool.get(), &buffer.outPtr(), nullptr);
         if (ret != GST_FLOW_OK) {
-            for (auto& buffer : channelBufferList)
-                unmapGstBuffer(buffer.get());
-
             // FLUSHING and EOS are not errors.
             if (ret < GST_FLOW_EOS || ret == GST_FLOW_NOT_LINKED)
                 GST_ELEMENT_ERROR(src, CORE, PAD, ("Internal WebAudioSrc error"), ("Failed to allocate buffer for flow: %s", gst_flow_get_name(ret)));
-            gst_task_stop(src->priv->task.get());
-            return;
+            return std::nullopt;
         }
 
         ASSERT(buffer);
         GST_BUFFER_TIMESTAMP(buffer.get()) = timestamp;
         GST_BUFFER_DURATION(buffer.get()) = duration;
-        mapGstBuffer(buffer.get(), GST_MAP_READWRITE);
-        priv->bus->setChannelMemory(i, reinterpret_cast<float*>(getGstBufferDataPointer(buffer.get())), priv->framesToPull);
+        GstMappedBuffer mappedBuffer(buffer.get(), GST_MAP_READWRITE);
+        ASSERT(mappedBuffer);
+        mappedBuffers.uncheckedAppend(WTFMove(mappedBuffer));
+        priv->bus->setChannelMemory(i, reinterpret_cast<float*>(mappedBuffers[i].data()), priv->framesToPull);
         channelBufferList.uncheckedAppend(WTFMove(buffer));
     }
 
     // FIXME: Add support for local/live audio input.
     priv->provider->render(nullptr, priv->bus, priv->framesToPull);
 
-    ASSERT(channelBufferList.size() == priv->sources.size());
+    return std::make_optional(channelBufferList);
+}
+
+static void webKitWebAudioSrcLoop(WebKitWebAudioSrc* src)
+{
+    WebKitWebAudioSourcePrivate* priv = src->priv;
+
+    std::optional<Vector<GRefPtr<GstBuffer>>> channelBufferList = webKitWebAudioSrcAllocateBuffersAndRenderAudio(src);
+    if (!channelBufferList) {
+        gst_task_stop(src->priv->task.get());
+        return;
+    }
+
+    ASSERT(channelBufferList->size() == priv->sources.size());
+
     bool failed = false;
     for (unsigned i = 0; i < priv->sources.size(); ++i) {
-        // Unmap before passing on the buffer.
-        auto& buffer = channelBufferList[i];
-        unmapGstBuffer(buffer.get());
+        auto& buffer = channelBufferList.value()[i];
 
         if (priv->enableGapBufferSupport && priv->bus->channel(i)->isSilent())
             GST_BUFFER_FLAG_SET(buffer.get(), GST_BUFFER_FLAG_GAP);
index 62353e4..605d672 100644 (file)
@@ -40,8 +40,6 @@
 
 namespace WebCore {
 
-const char* webkitGstMapInfoQuarkString = "webkit-gst-map-info";
-
 GstPad* webkitGstGhostPadFromStaticTemplate(GstStaticPadTemplate* staticPadTemplate, const gchar* name, GstPad* target)
 {
     GstPad* pad;
@@ -143,26 +141,6 @@ bool getSampleVideoInfo(GstSample* sample, GstVideoInfo& videoInfo)
 }
 #endif
 
-GstBuffer* createGstBuffer(GstBuffer* buffer)
-{
-    gsize bufferSize = gst_buffer_get_size(buffer);
-    GstBuffer* newBuffer = gst_buffer_new_and_alloc(bufferSize);
-
-    if (!newBuffer)
-        return 0;
-
-    gst_buffer_copy_into(newBuffer, buffer, static_cast<GstBufferCopyFlags>(GST_BUFFER_COPY_METADATA), 0, bufferSize);
-    return newBuffer;
-}
-
-GstBuffer* createGstBufferForData(const char* data, int length)
-{
-    GstBuffer* buffer = gst_buffer_new_and_alloc(length);
-
-    gst_buffer_fill(buffer, 0, data, length);
-
-    return buffer;
-}
 
 const char* capsMediaType(const GstCaps* caps)
 {
@@ -205,38 +183,6 @@ bool areEncryptedCaps(const GstCaps* caps)
 #endif
 }
 
-char* getGstBufferDataPointer(GstBuffer* buffer)
-{
-    GstMiniObject* miniObject = reinterpret_cast<GstMiniObject*>(buffer);
-    GstMapInfo* mapInfo = static_cast<GstMapInfo*>(gst_mini_object_get_qdata(miniObject, g_quark_from_static_string(webkitGstMapInfoQuarkString)));
-    return reinterpret_cast<char*>(mapInfo->data);
-}
-
-void mapGstBuffer(GstBuffer* buffer, uint32_t flags)
-{
-    GstMapInfo* mapInfo = static_cast<GstMapInfo*>(fastMalloc(sizeof(GstMapInfo)));
-    if (!gst_buffer_map(buffer, mapInfo, static_cast<GstMapFlags>(flags))) {
-        fastFree(mapInfo);
-        gst_buffer_unref(buffer);
-        return;
-    }
-
-    GstMiniObject* miniObject = reinterpret_cast<GstMiniObject*>(buffer);
-    gst_mini_object_set_qdata(miniObject, g_quark_from_static_string(webkitGstMapInfoQuarkString), mapInfo, nullptr);
-}
-
-void unmapGstBuffer(GstBuffer* buffer)
-{
-    GstMiniObject* miniObject = reinterpret_cast<GstMiniObject*>(buffer);
-    GstMapInfo* mapInfo = static_cast<GstMapInfo*>(gst_mini_object_steal_qdata(miniObject, g_quark_from_static_string(webkitGstMapInfoQuarkString)));
-
-    if (!mapInfo)
-        return;
-
-    gst_buffer_unmap(buffer, mapInfo);
-    fastFree(mapInfo);
-}
-
 Vector<String> extractGStreamerOptionsFromCommandLine()
 {
     GUniqueOutPtr<char> contents;
index 793b4dd..96b4c93 100644 (file)
@@ -63,14 +63,9 @@ bool getVideoSizeAndFormatFromCaps(GstCaps*, WebCore::IntSize&, GstVideoFormat&,
 std::optional<FloatSize> getVideoResolutionFromCaps(const GstCaps*);
 bool getSampleVideoInfo(GstSample*, GstVideoInfo&);
 #endif
-GstBuffer* createGstBuffer(GstBuffer*);
-GstBuffer* createGstBufferForData(const char* data, int length);
-char* getGstBufferDataPointer(GstBuffer*);
 const char* capsMediaType(const GstCaps*);
 bool doCapsHaveType(const GstCaps*, const char*);
 bool areEncryptedCaps(const GstCaps*);
-void mapGstBuffer(GstBuffer*, uint32_t);
-void unmapGstBuffer(GstBuffer*);
 Vector<String> extractGStreamerOptionsFromCommandLine();
 bool initializeGStreamer(std::optional<Vector<String>>&& = std::nullopt);
 unsigned getGstPlayFlag(const char* nick);
@@ -84,11 +79,21 @@ inline GstClockTime toGstClockTime(const MediaTime &mediaTime)
 class GstMappedBuffer {
     WTF_MAKE_NONCOPYABLE(GstMappedBuffer);
 public:
+
+    GstMappedBuffer(GstMappedBuffer&& other)
+        : m_buffer(other.m_buffer)
+        , m_info(other.m_info)
+        , m_isValid(other.m_isValid)
+    {
+        other.m_isValid = false;
+    }
+
     GstMappedBuffer(GstBuffer* buffer, GstMapFlags flags)
         : m_buffer(buffer)
     {
         m_isValid = gst_buffer_map(m_buffer, &m_info, flags);
     }
+
     // Unfortunately, GST_MAP_READWRITE is defined out of line from the MapFlags
     // enum as an int, and C++ is careful to not implicity convert it to an enum.
     GstMappedBuffer(GstBuffer* buffer, int flags)
@@ -98,6 +103,7 @@ public:
     {
         if (m_isValid)
             gst_buffer_unmap(m_buffer, &m_info);
+        m_isValid = false;
     }
 
     uint8_t* data() { ASSERT(m_isValid); return static_cast<uint8_t*>(m_info.data); }
@@ -105,8 +111,8 @@ public:
 
     explicit operator bool() const { return m_isValid; }
 private:
-    GstBuffer* m_buffer;
-    GstMapInfo m_info;
+    GstBuffer* m_buffer { nullptr };
+    GstMapInfo m_info GST_MAP_INFO_INIT;
     bool m_isValid { false };
 };
 
index 4f1a02f..231cfd2 100644 (file)
@@ -103,7 +103,6 @@ struct _WebKitWebSrcPrivate {
     uint64_t minimumBlocksize;
 
     RefPtr<MainThreadNotifier<MainThreadSourceNotification>> notifier;
-    GRefPtr<GstBuffer> buffer;
 };
 
 enum {
@@ -366,11 +365,6 @@ static void webKitWebSrcStop(WebKitWebSrc* src)
 
     bool wasSeeking = std::exchange(priv->isSeeking, false);
 
-    if (priv->buffer) {
-        unmapGstBuffer(priv->buffer.get());
-        priv->buffer.clear();
-    }
-
     priv->paused = false;
 
     priv->offset = 0;
@@ -891,16 +885,8 @@ void CachedResourceStreamingClient::dataReceived(PlatformMediaResource&, const c
     WebKitWebSrc* src = WEBKIT_WEB_SRC(m_src.get());
     WebKitWebSrcPrivate* priv = src->priv;
 
-    GST_LOG_OBJECT(src, "Have %lld bytes of data", priv->buffer ? static_cast<long long>(gst_buffer_get_size(priv->buffer.get())) : length);
-
-    ASSERT(!priv->buffer || data == getGstBufferDataPointer(priv->buffer.get()));
-
-    if (priv->buffer)
-        unmapGstBuffer(priv->buffer.get());
-
     if (priv->isSeeking) {
         GST_DEBUG_OBJECT(src, "Seek in progress, ignoring data");
-        priv->buffer.clear();
         return;
     }
 
@@ -909,7 +895,6 @@ void CachedResourceStreamingClient::dataReceived(PlatformMediaResource&, const c
         if (priv->offset + length <= priv->requestedOffset) {
             // Discard all the buffers coming before the requested seek position.
             priv->offset += length;
-            priv->buffer.clear();
             return;
         }
 
@@ -917,21 +902,12 @@ void CachedResourceStreamingClient::dataReceived(PlatformMediaResource&, const c
             guint64 offset = priv->requestedOffset - priv->offset;
             data += offset;
             length -= offset;
-            if (priv->buffer)
-                gst_buffer_resize(priv->buffer.get(), offset, -1);
             priv->offset = priv->requestedOffset;
         }
 
         priv->requestedOffset = 0;
     }
 
-    // Ports using the GStreamer backend but not the soup implementation of ResourceHandle
-    // won't be using buffers provided by this client, the buffer is created here in that case.
-    if (!priv->buffer)
-        priv->buffer = adoptGRef(createGstBufferForData(data, length));
-    else
-        gst_buffer_set_size(priv->buffer.get(), static_cast<gssize>(length));
-
     checkUpdateBlocksize(length);
 
     uint64_t startingOffset = priv->offset;
@@ -956,7 +932,7 @@ void CachedResourceStreamingClient::dataReceived(PlatformMediaResource&, const c
         uint64_t subBufferOffset = startingOffset + currentOffset;
         uint64_t currentOffsetSize = std::min(blockSize, bufferSize - currentOffset);
 
-        GstBuffer* subBuffer = gst_buffer_copy_region(priv->buffer.get(), GST_BUFFER_COPY_ALL, currentOffset, currentOffsetSize);
+        GstBuffer* subBuffer = gst_buffer_new_wrapped(g_memdup(data + currentOffset, currentOffsetSize), currentOffsetSize);
         if (UNLIKELY(!subBuffer)) {
             GST_ELEMENT_ERROR(src, CORE, FAILED, ("Failed to allocate sub-buffer"), (nullptr));
             break;
@@ -985,8 +961,6 @@ void CachedResourceStreamingClient::dataReceived(PlatformMediaResource&, const c
             break;
         }
     }
-
-    priv->buffer.clear();
 }
 
 void CachedResourceStreamingClient::accessControlCheckFailed(PlatformMediaResource&, const ResourceError& error)