Attachment 350115 Details for Bug 189699 – Patch

[patch] Patch

bug-189699-20180919154909.patch (text/plain), 14.49 KB, created by Philippe Normand on 2018-09-19 07:49:10 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Philippe Normand

Created: 2018-09-19 07:49:10 PDT

Size: 14.49 KB

patch

obsolete

>Subversion Revision: 236205
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 694b58f86bfefbf74faa02e7b5aabfa04f31654e..d8d616b971f0d8f96dee5079746fa28a020d60b2 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,33 @@
>+2018-09-19  Philippe Normand  <pnormand@igalia.com>
>+
>+        [GStreamer] Utilities cleanups
>+        https://bugs.webkit.org/show_bug.cgi?id=189699
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        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-19  Jer Noble  <jer.noble@apple.com>
> 
>         REGRESSION (r236006): New waitingForKey() requirement breaks Modern EME tests.
>diff --git a/Source/WebCore/platform/audio/gstreamer/WebKitWebAudioSourceGStreamer.cpp b/Source/WebCore/platform/audio/gstreamer/WebKitWebAudioSourceGStreamer.cpp
>index 55cdb42cc6edd0158fc91645d40998b6682f95fc..51264ac69b99cba9af620c56a72c6bbb5248c1a3 100644
>--- a/Source/WebCore/platform/audio/gstreamer/WebKitWebAudioSourceGStreamer.cpp
>+++ b/Source/WebCore/platform/audio/gstreamer/WebKitWebAudioSourceGStreamer.cpp
>@@ -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);
>@@ -325,39 +325,49 @@ static void webKitWebAudioSrcLoop(WebKitWebAudioSrc* src)
>     priv->numberOfSamples += priv->framesToPull;
>     GstClockTime duration = gst_util_uint64_scale(priv->numberOfSamples, GST_SECOND, priv->sampleRate) - timestamp;
> 
>-    Vector<GRefPtr<GstBuffer>> channelBufferList;
>-    channelBufferList.reserveInitialCapacity(priv->sources.size());
>+    Vector<GRefPtr<GstBuffer>> channelBufferList(priv->sources.size());
>+    Vector<GstMappedBuffer> mappedBuffers(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);
>diff --git a/Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp b/Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp
>index 62353e4f3e6847f9f0ced35de246b051a82dd963..605d6727214492904b32e37a7c13c945219998a7 100644
>--- a/Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp
>+++ b/Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp
>@@ -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;
>diff --git a/Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h b/Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h
>index 793b4dd7564897fd1338eec203ce9e9711499b11..5d707d4b38fba4edc2eb83a6ca7391962d852f24 100644
>--- a/Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h
>+++ b/Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h
>@@ -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,22 @@ inline GstClockTime toGstClockTime(const MediaTime &mediaTime)
> class GstMappedBuffer {
>     WTF_MAKE_NONCOPYABLE(GstMappedBuffer);
> public:
>+
>+    GstMappedBuffer() = default;
>+    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 +104,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 +112,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 };
> };
> 
>diff --git a/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp b/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp
>index 4f1a02fd2eb3a6d2004d4a8bf55068038f89f013..231cfd2d1f309c79b985ccd56d30825a12b103d5 100644
>--- a/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp
>+++ b/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp
>@@ -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)

Actions: View | Formatted Diff | Diff

Attachments on bug 189699: 350030 | 350100 | 350115 | 350417