WebKit Bugzilla
Attachment 350115 Details for
Bug 189699
: [GStreamer] Utilities cleanups
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[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:
Patch
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)
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 189699
:
350030
|
350100
|
350115
|
350417