Unreviewed, rolling out r236255.
authormcatanzaro@igalia.com <mcatanzaro@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Sep 2018 20:07:02 +0000 (20:07 +0000)
committermcatanzaro@igalia.com <mcatanzaro@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Sep 2018 20:07:02 +0000 (20:07 +0000)
Many WebAudio crashes

Reverted changeset:

"[GStreamer] Utilities cleanups"
https://bugs.webkit.org/show_bug.cgi?id=189699
https://trac.webkit.org/changeset/236255

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@236352 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 035f04f..69078ed 100644 (file)
@@ -1,3 +1,15 @@
+2018-09-21  Michael Catanzaro  <mcatanzaro@igalia.com>
+
+        Unreviewed, rolling out r236255.
+
+        Many WebAudio crashes
+
+        Reverted changeset:
+
+        "[GStreamer] Utilities cleanups"
+        https://bugs.webkit.org/show_bug.cgi?id=189699
+        https://trac.webkit.org/changeset/236255
+
 2018-09-21  Jer Noble  <jer.noble@apple.com>
 
         Move AVVideoPerformanceMetrics into AVFoundationSPI.h
index 51264ac..55cdb42 100644 (file)
@@ -308,7 +308,7 @@ static void webKitWebAudioSrcGetProperty(GObject* object, guint propertyId, GVal
     }
 }
 
-static std::optional<Vector<GRefPtr<GstBuffer>>> webKitWebAudioSrcAllocateBuffersAndRenderAudio(WebKitWebAudioSrc* src)
+static void webKitWebAudioSrcLoop(WebKitWebAudioSrc* src)
 {
     WebKitWebAudioSourcePrivate* priv = src->priv;
 
@@ -317,7 +317,7 @@ static std::optional<Vector<GRefPtr<GstBuffer>>> webKitWebAudioSrcAllocateBuffer
     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 std::nullopt;
+        return;
     }
 
     ASSERT(priv->pool);
@@ -325,49 +325,39 @@ static std::optional<Vector<GRefPtr<GstBuffer>>> webKitWebAudioSrcAllocateBuffer
     priv->numberOfSamples += priv->framesToPull;
     GstClockTime duration = gst_util_uint64_scale(priv->numberOfSamples, GST_SECOND, priv->sampleRate) - timestamp;
 
-    Vector<GRefPtr<GstBuffer>> channelBufferList(priv->sources.size());
-    Vector<GstMappedBuffer> mappedBuffers(priv->sources.size());
+    Vector<GRefPtr<GstBuffer>> channelBufferList;
+    channelBufferList.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)));
-            return std::nullopt;
+            gst_task_stop(src->priv->task.get());
+            return;
         }
 
         ASSERT(buffer);
         GST_BUFFER_TIMESTAMP(buffer.get()) = timestamp;
         GST_BUFFER_DURATION(buffer.get()) = duration;
-        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);
+        mapGstBuffer(buffer.get(), GST_MAP_READWRITE);
+        priv->bus->setChannelMemory(i, reinterpret_cast<float*>(getGstBufferDataPointer(buffer.get())), priv->framesToPull);
         channelBufferList.uncheckedAppend(WTFMove(buffer));
     }
 
     // FIXME: Add support for local/live audio input.
     priv->provider->render(nullptr, priv->bus, priv->framesToPull);
 
-    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());
-
+    ASSERT(channelBufferList.size() == priv->sources.size());
     bool failed = false;
     for (unsigned i = 0; i < priv->sources.size(); ++i) {
-        auto& buffer = channelBufferList.value()[i];
+        // Unmap before passing on the buffer.
+        auto& buffer = channelBufferList[i];
+        unmapGstBuffer(buffer.get());
 
         if (priv->enableGapBufferSupport && priv->bus->channel(i)->isSilent())
             GST_BUFFER_FLAG_SET(buffer.get(), GST_BUFFER_FLAG_GAP);
index 605d672..62353e4 100644 (file)
@@ -40,6 +40,8 @@
 
 namespace WebCore {
 
+const char* webkitGstMapInfoQuarkString = "webkit-gst-map-info";
+
 GstPad* webkitGstGhostPadFromStaticTemplate(GstStaticPadTemplate* staticPadTemplate, const gchar* name, GstPad* target)
 {
     GstPad* pad;
@@ -141,6 +143,26 @@ 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)
 {
@@ -183,6 +205,38 @@ 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 5d707d4..793b4dd 100644 (file)
@@ -63,9 +63,14 @@ 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);
@@ -79,22 +84,11 @@ 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)
@@ -104,7 +98,6 @@ 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); }
@@ -112,8 +105,8 @@ public:
 
     explicit operator bool() const { return m_isValid; }
 private:
-    GstBuffer* m_buffer { nullptr };
-    GstMapInfo m_info GST_MAP_INFO_INIT;
+    GstBuffer* m_buffer;
+    GstMapInfo m_info;
     bool m_isValid { false };
 };
 
index 231cfd2..4f1a02f 100644 (file)
@@ -103,6 +103,7 @@ struct _WebKitWebSrcPrivate {
     uint64_t minimumBlocksize;
 
     RefPtr<MainThreadNotifier<MainThreadSourceNotification>> notifier;
+    GRefPtr<GstBuffer> buffer;
 };
 
 enum {
@@ -365,6 +366,11 @@ 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;
@@ -885,8 +891,16 @@ 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;
     }
 
@@ -895,6 +909,7 @@ 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;
         }
 
@@ -902,12 +917,21 @@ 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;
@@ -932,7 +956,7 @@ void CachedResourceStreamingClient::dataReceived(PlatformMediaResource&, const c
         uint64_t subBufferOffset = startingOffset + currentOffset;
         uint64_t currentOffsetSize = std::min(blockSize, bufferSize - currentOffset);
 
-        GstBuffer* subBuffer = gst_buffer_new_wrapped(g_memdup(data + currentOffset, currentOffsetSize), currentOffsetSize);
+        GstBuffer* subBuffer = gst_buffer_copy_region(priv->buffer.get(), GST_BUFFER_COPY_ALL, currentOffset, currentOffsetSize);
         if (UNLIKELY(!subBuffer)) {
             GST_ELEMENT_ERROR(src, CORE, FAILED, ("Failed to allocate sub-buffer"), (nullptr));
             break;
@@ -961,6 +985,8 @@ void CachedResourceStreamingClient::dataReceived(PlatformMediaResource&, const c
             break;
         }
     }
+
+    priv->buffer.clear();
 }
 
 void CachedResourceStreamingClient::accessControlCheckFailed(PlatformMediaResource&, const ResourceError& error)