Title: [287059] trunk/Source
Revision
287059
Author
[email protected]
Date
2021-12-14 17:25:11 -0800 (Tue, 14 Dec 2021)

Log Message

Source/WebCore:
SourceBufferParser should be using contiguous SharedBuffer
https://bugs.webkit.org/show_bug.cgi?id=233865
rdar://problem/86085253

Reviewed by Eric Carlson.

The SharedBuffer sent to the SourceBufferParser can only ever contain one
DataSegment and we had assertions to that effect. The SharedBuffer class
type now guarantees how the data is structured and allow for more explicit
code which improves readability.
We also had some workarounds for the fact that SharedBuffer didn't use
thread-safe refcounting and instead we referenced the inner DataSegment.
This can be removed.

Covered by existing tests, no observable differences.

* Modules/mediasource/SourceBuffer.cpp:
(WebCore::SourceBuffer::appendBufferTimerFired):
* Modules/mediasource/SourceBuffer.h:
* platform/audio/cocoa/AudioFileReaderCocoa.cpp:
* platform/graphics/SourceBufferPrivate.cpp:
(WebCore::SourceBufferPrivate::append):
* platform/graphics/SourceBufferPrivate.h:
* platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.h:
* platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:
(WebCore::SourceBufferPrivateAVFObjC::append):
* platform/graphics/cocoa/SourceBufferParser.cpp:
(WebCore::SourceBufferParser::Segment::Segment):
(WebCore::SourceBufferParser::Segment::size const):
(WebCore::SourceBufferParser::Segment::read const):
(WebCore::SourceBufferParser::Segment::takeSharedBuffer):
(WebCore::SourceBufferParser::Segment::getSharedBuffer const):
* platform/graphics/cocoa/SourceBufferParser.h:
* platform/graphics/cocoa/SourceBufferParserWebM.cpp:

Source/WebKit:
SourceBufferParser should be using contiguous shared buffer
https://bugs.webkit.org/show_bug.cgi?id=233865
rdar://problem/86085253

Reviewed by Eric Carlson.

* WebProcess/GPU/media/SourceBufferPrivateRemote.cpp:
(WebKit::SourceBufferPrivateRemote::append):
* WebProcess/GPU/media/SourceBufferPrivateRemote.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (287058 => 287059)


--- trunk/Source/WebCore/ChangeLog	2021-12-15 01:23:39 UTC (rev 287058)
+++ trunk/Source/WebCore/ChangeLog	2021-12-15 01:25:11 UTC (rev 287059)
@@ -1,5 +1,42 @@
 2021-12-14  Jean-Yves Avenard  <[email protected]>
 
+        SourceBufferParser should be using contiguous SharedBuffer
+        https://bugs.webkit.org/show_bug.cgi?id=233865
+        rdar://problem/86085253
+
+        Reviewed by Eric Carlson.
+
+        The SharedBuffer sent to the SourceBufferParser can only ever contain one
+        DataSegment and we had assertions to that effect. The SharedBuffer class
+        type now guarantees how the data is structured and allow for more explicit
+        code which improves readability.
+        We also had some workarounds for the fact that SharedBuffer didn't use
+        thread-safe refcounting and instead we referenced the inner DataSegment.
+        This can be removed.
+
+        Covered by existing tests, no observable differences.
+
+        * Modules/mediasource/SourceBuffer.cpp:
+        (WebCore::SourceBuffer::appendBufferTimerFired):
+        * Modules/mediasource/SourceBuffer.h:
+        * platform/audio/cocoa/AudioFileReaderCocoa.cpp:
+        * platform/graphics/SourceBufferPrivate.cpp:
+        (WebCore::SourceBufferPrivate::append):
+        * platform/graphics/SourceBufferPrivate.h:
+        * platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.h:
+        * platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:
+        (WebCore::SourceBufferPrivateAVFObjC::append):
+        * platform/graphics/cocoa/SourceBufferParser.cpp:
+        (WebCore::SourceBufferParser::Segment::Segment):
+        (WebCore::SourceBufferParser::Segment::size const):
+        (WebCore::SourceBufferParser::Segment::read const):
+        (WebCore::SourceBufferParser::Segment::takeSharedBuffer):
+        (WebCore::SourceBufferParser::Segment::getSharedBuffer const):
+        * platform/graphics/cocoa/SourceBufferParser.h:
+        * platform/graphics/cocoa/SourceBufferParserWebM.cpp:
+
+2021-12-14  Jean-Yves Avenard  <[email protected]>
+
         Make PreviewConverterProvider not modify the SharedBuffer once returned
         https://bugs.webkit.org/show_bug.cgi?id=233923
         rdar://problem/86149850

Modified: trunk/Source/WebCore/Modules/mediasource/SourceBuffer.cpp (287058 => 287059)


--- trunk/Source/WebCore/Modules/mediasource/SourceBuffer.cpp	2021-12-15 01:23:39 UTC (rev 287058)
+++ trunk/Source/WebCore/Modules/mediasource/SourceBuffer.cpp	2021-12-15 01:25:11 UTC (rev 287059)
@@ -530,7 +530,7 @@
     // https://dvcs.w3.org/hg/html-media/raw-file/tip/media-source/media-source.html#sourcebuffer-segment-parser-loop
     // When the segment parser loop algorithm is invoked, run the following steps:
 
-    RefPtr<FragmentedSharedBuffer> appendData = WTFMove(m_pendingAppendData);
+    RefPtr<SharedBuffer> appendData = WTFMove(m_pendingAppendData);
     // 1. Loop Top: If the input buffer is empty, then jump to the need more data step below.
     if (!appendData || !appendData->size()) {
         sourceBufferPrivateAppendComplete(AppendResult::AppendSucceeded);

Modified: trunk/Source/WebCore/Modules/mediasource/SourceBuffer.h (287058 => 287059)


--- trunk/Source/WebCore/Modules/mediasource/SourceBuffer.h	2021-12-15 01:23:39 UTC (rev 287058)
+++ trunk/Source/WebCore/Modules/mediasource/SourceBuffer.h	2021-12-15 01:25:11 UTC (rev 287059)
@@ -222,7 +222,7 @@
 
     WTF::Observer<void*()> m_opaqueRootProvider;
 
-    RefPtr<FragmentedSharedBuffer> m_pendingAppendData;
+    RefPtr<SharedBuffer> m_pendingAppendData;
     Timer m_appendBufferTimer;
 
     RefPtr<VideoTrackList> m_videoTracks;

Modified: trunk/Source/WebCore/platform/audio/cocoa/AudioFileReaderCocoa.cpp (287058 => 287059)


--- trunk/Source/WebCore/platform/audio/cocoa/AudioFileReaderCocoa.cpp	2021-12-15 01:23:39 UTC (rev 287058)
+++ trunk/Source/WebCore/platform/audio/cocoa/AudioFileReaderCocoa.cpp	2021-12-15 01:25:11 UTC (rev 287059)
@@ -130,7 +130,7 @@
     WTF_MAKE_FAST_ALLOCATED;
 
 public:
-    Ref<FragmentedSharedBuffer> m_buffer;
+    Ref<SharedBuffer> m_buffer;
 #if ENABLE(MEDIA_SOURCE)
     Ref<AudioTrackPrivateWebM> m_track;
 #endif

Modified: trunk/Source/WebCore/platform/graphics/SourceBufferPrivate.cpp (287058 => 287059)


--- trunk/Source/WebCore/platform/graphics/SourceBufferPrivate.cpp	2021-12-15 01:23:39 UTC (rev 287058)
+++ trunk/Source/WebCore/platform/graphics/SourceBufferPrivate.cpp	2021-12-15 01:25:11 UTC (rev 287059)
@@ -1318,7 +1318,7 @@
     updateHighestPresentationTimestamp();
 }
 
-void SourceBufferPrivate::append(Ref<FragmentedSharedBuffer>&& buffer)
+void SourceBufferPrivate::append(Ref<SharedBuffer>&& buffer)
 {
     append(buffer->extractData());
 }

Modified: trunk/Source/WebCore/platform/graphics/SourceBufferPrivate.h (287058 => 287059)


--- trunk/Source/WebCore/platform/graphics/SourceBufferPrivate.h	2021-12-15 01:23:39 UTC (rev 287058)
+++ trunk/Source/WebCore/platform/graphics/SourceBufferPrivate.h	2021-12-15 01:25:11 UTC (rev 287059)
@@ -49,7 +49,7 @@
 
 namespace WebCore {
 
-class FragmentedSharedBuffer;
+class SharedBuffer;
 class TimeRanges;
 
 enum class SourceBufferAppendMode : uint8_t {
@@ -68,7 +68,7 @@
     WEBCORE_EXPORT virtual ~SourceBufferPrivate();
 
     virtual void setActive(bool) = 0;
-    WEBCORE_EXPORT virtual void append(Ref<FragmentedSharedBuffer>&&);
+    WEBCORE_EXPORT virtual void append(Ref<SharedBuffer>&&);
     virtual void abort() = 0;
     virtual void resetParserState() = 0;
     virtual void removedFromMediaSource() = 0;

Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.h (287058 => 287059)


--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.h	2021-12-15 01:23:39 UTC (rev 287058)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.h	2021-12-15 01:25:11 UTC (rev 287059)
@@ -153,7 +153,7 @@
     void didProvideMediaDataForTrackId(Ref<MediaSample>&&, uint64_t trackId, const String& mediaType);
 
     // SourceBufferPrivate overrides
-    void append(Ref<FragmentedSharedBuffer>&&) final;
+    void append(Ref<SharedBuffer>&&) final;
     void abort() final;
     void resetParserState() final;
     void removedFromMediaSource() final;

Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm (287058 => 287059)


--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm	2021-12-15 01:23:39 UTC (rev 287058)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm	2021-12-15 01:25:11 UTC (rev 287059)
@@ -563,7 +563,7 @@
     UNUSED_PARAM(hasSessionSemaphore);
 }
 
-void SourceBufferPrivateAVFObjC::append(Ref<FragmentedSharedBuffer>&& data)
+void SourceBufferPrivateAVFObjC::append(Ref<SharedBuffer>&& data)
 {
     ALWAYS_LOG(LOGIDENTIFIER, "data length = ", data->size());
 

Modified: trunk/Source/WebCore/platform/graphics/cocoa/SourceBufferParser.cpp (287058 => 287059)


--- trunk/Source/WebCore/platform/graphics/cocoa/SourceBufferParser.cpp	2021-12-15 01:23:39 UTC (rev 287058)
+++ trunk/Source/WebCore/platform/graphics/cocoa/SourceBufferParser.cpp	2021-12-15 01:25:11 UTC (rev 287059)
@@ -80,7 +80,7 @@
 {
 }
 
-SourceBufferParser::Segment::Segment(Ref<FragmentedSharedBuffer>&& buffer)
+SourceBufferParser::Segment::Segment(Ref<SharedBuffer>&& buffer)
     : m_segment(WTFMove(buffer))
 {
 }
@@ -101,7 +101,7 @@
             return clampTo<size_t>(MTPluginByteSourceGetLength(byteSource.get()));
         },
 #endif
-        [](const Ref<FragmentedSharedBuffer>& buffer)
+        [](const Ref<SharedBuffer>& buffer)
         {
             return buffer->size();
         }
@@ -125,7 +125,7 @@
             return Unexpected<ReadError> { ReadError::FatalError };
         },
 #endif
-        [&](const Ref<FragmentedSharedBuffer>& buffer) -> ReadResult
+        [&](const Ref<SharedBuffer>& buffer) -> ReadResult
         {
             buffer->copyTo(destination, position, sizeToRead);
             return sizeToRead;
@@ -133,7 +133,7 @@
     );
 }
 
-Ref<FragmentedSharedBuffer> SourceBufferParser::Segment::takeSharedBuffer()
+Ref<SharedBuffer> SourceBufferParser::Segment::takeSharedBuffer()
 {
     return WTF::switchOn(m_segment,
 #if HAVE(MT_PLUGIN_FORMAT_READER)
@@ -142,28 +142,28 @@
             Vector<uint8_t> vector(size());
             auto readResult = read(0, vector.size(), vector.data());
             if (!readResult.has_value())
-                return FragmentedSharedBuffer::create();
+                return SharedBuffer::create();
             vector.shrink(readResult.value());
-            return FragmentedSharedBuffer::create(WTFMove(vector));
+            return SharedBuffer::create(WTFMove(vector));
         },
 #endif
-        [&](Ref<FragmentedSharedBuffer>& buffer)
+        [&](Ref<SharedBuffer>& buffer)
         {
-            return std::exchange(buffer, FragmentedSharedBuffer::create());
+            return std::exchange(buffer, SharedBuffer::create());
         }
     );
 }
 
-RefPtr<FragmentedSharedBuffer> SourceBufferParser::Segment::getSharedBuffer() const
+RefPtr<SharedBuffer> SourceBufferParser::Segment::getSharedBuffer() const
 {
     return WTF::switchOn(m_segment,
 #if HAVE(MT_PLUGIN_FORMAT_READER)
-        [&](const RetainPtr<MTPluginByteSourceRef>&) -> RefPtr<FragmentedSharedBuffer>
+        [&](const RetainPtr<MTPluginByteSourceRef>&) -> RefPtr<SharedBuffer>
         {
             return nullptr;
         },
 #endif
-        [&](const Ref<FragmentedSharedBuffer>& buffer) -> RefPtr<FragmentedSharedBuffer>
+        [&](const Ref<SharedBuffer>& buffer) -> RefPtr<SharedBuffer>
         {
             return buffer.ptr();
         }

Modified: trunk/Source/WebCore/platform/graphics/cocoa/SourceBufferParser.h (287058 => 287059)


--- trunk/Source/WebCore/platform/graphics/cocoa/SourceBufferParser.h	2021-12-15 01:23:39 UTC (rev 287058)
+++ trunk/Source/WebCore/platform/graphics/cocoa/SourceBufferParser.h	2021-12-15 01:25:11 UTC (rev 287059)
@@ -45,7 +45,7 @@
 
 class ContentType;
 class MediaSample;
-class FragmentedSharedBuffer;
+class SharedBuffer;
 
 class WEBCORE_EXPORT SourceBufferParser : public ThreadSafeRefCounted<SourceBufferParser> {
 public:
@@ -69,11 +69,11 @@
 #if HAVE(MT_PLUGIN_FORMAT_READER)
         Segment(RetainPtr<MTPluginByteSourceRef>&&);
 #endif
-        Segment(Ref<FragmentedSharedBuffer>&&);
+        Segment(Ref<SharedBuffer>&&);
         Segment(Segment&&) = default;
-        Ref<FragmentedSharedBuffer> takeSharedBuffer();
-        // Will return nullptr if Segment's backend isn't a FragmentedSharedBuffer.
-        RefPtr<FragmentedSharedBuffer> getSharedBuffer() const;
+        Ref<SharedBuffer> takeSharedBuffer();
+        // Will return nullptr if Segment's backend isn't a SharedBuffer.
+        RefPtr<SharedBuffer> getSharedBuffer() const;
 
         size_t size() const;
 
@@ -87,7 +87,7 @@
 #if HAVE(MT_PLUGIN_FORMAT_READER)
             RetainPtr<MTPluginByteSourceRef>,
 #endif
-            Ref<FragmentedSharedBuffer>
+            Ref<SharedBuffer>
         > m_segment;
     };
 

Modified: trunk/Source/WebCore/platform/graphics/cocoa/SourceBufferParserWebM.cpp (287058 => 287059)


--- trunk/Source/WebCore/platform/graphics/cocoa/SourceBufferParserWebM.cpp	2021-12-15 01:23:39 UTC (rev 287058)
+++ trunk/Source/WebCore/platform/graphics/cocoa/SourceBufferParserWebM.cpp	2021-12-15 01:25:11 UTC (rev 287059)
@@ -365,9 +365,9 @@
         return Status(Status::kWouldBlock);
     }
 
-    static void FreeDataSegment(void* refcon, void*, size_t)
+    static void FreeSharedBuffer(void* refcon, void*, size_t)
     {
-        auto* buffer = reinterpret_cast<DataSegment*>(refcon);
+        auto* buffer = reinterpret_cast<SharedBuffer*>(refcon);
         buffer->deref();
     }
 
@@ -388,7 +388,7 @@
                 advanceToNextSegment();
                 continue;
             }
-            RefPtr<FragmentedSharedBuffer> sharedBuffer = currentSegment.getSharedBuffer();
+            RefPtr<SharedBuffer> sharedBuffer = currentSegment.getSharedBuffer();
             CMBlockBufferRef rawBlockBuffer = nullptr;
             uint64_t lastRead = 0;
             size_t destinationOffset = m_positionWithinSegment;
@@ -408,11 +408,8 @@
                 lastRead = readResult.value();
                 destinationOffset = 0;
             } else {
-                ASSERT(sharedBuffer->hasOneSegment(), "Can only deal with sharedBuffer containing a single DataSegment");
-                // A FragmentedSharedBuffer doesn't have thread-safe refcounting, as such we must keep a reference to the DataSegment instead.
                 // TODO: could we only create a new CMBlockBuffer if the backend memory changed since the previous one?
-                auto firstSegment = sharedBuffer->begin()->segment;
-                size_t canRead = std::min<size_t>(numToRead, firstSegment->size() - m_positionWithinSegment);
+                size_t canRead = std::min<size_t>(numToRead, sharedBuffer->size() - m_positionWithinSegment);
                 // From CMBlockBufferCustomBlockSource documentation:
                 // Note that for 64-bit architectures, this struct contains misaligned function pointers.
                 // To avoid link-time issues, it is recommended that clients fill CMBlockBufferCustomBlockSource's function pointer fields
@@ -420,10 +417,10 @@
                 CMBlockBufferCustomBlockSource allocator;
                 allocator.version = 0;
                 allocator.AllocateBlock = nullptr;
-                allocator.FreeBlock = FreeDataSegment;
-                allocator.refCon = firstSegment.ptr();
-                firstSegment->ref();
-                auto err = PAL::CMBlockBufferCreateWithMemoryBlock(nullptr, static_cast<void*>(const_cast<uint8_t*>(firstSegment->data())), firstSegment->size(), nullptr, &allocator, m_positionWithinSegment, canRead, 0, &rawBlockBuffer);
+                allocator.FreeBlock = FreeSharedBuffer;
+                allocator.refCon = sharedBuffer.get();
+                sharedBuffer->ref();
+                auto err = PAL::CMBlockBufferCreateWithMemoryBlock(nullptr, static_cast<void*>(const_cast<uint8_t*>(sharedBuffer->data())), sharedBuffer->size(), nullptr, &allocator, m_positionWithinSegment, canRead, 0, &rawBlockBuffer);
                 if (err != kCMBlockBufferNoErr)
                     return Status(Status::kNotEnoughMemory);
                 lastRead = canRead;

Modified: trunk/Source/WebKit/ChangeLog (287058 => 287059)


--- trunk/Source/WebKit/ChangeLog	2021-12-15 01:23:39 UTC (rev 287058)
+++ trunk/Source/WebKit/ChangeLog	2021-12-15 01:25:11 UTC (rev 287059)
@@ -1,3 +1,15 @@
+2021-12-14  Jean-Yves Avenard  <[email protected]>
+
+        SourceBufferParser should be using contiguous shared buffer
+        https://bugs.webkit.org/show_bug.cgi?id=233865
+        rdar://problem/86085253
+
+        Reviewed by Eric Carlson.
+
+        * WebProcess/GPU/media/SourceBufferPrivateRemote.cpp:
+        (WebKit::SourceBufferPrivateRemote::append):
+        * WebProcess/GPU/media/SourceBufferPrivateRemote.h:
+
 2021-12-14  Alex Christensen  <[email protected]>
 
         Remove properties set by NSURLProtocol on NSURLRequest before serializing

Modified: trunk/Source/WebKit/WebProcess/GPU/media/SourceBufferPrivateRemote.cpp (287058 => 287059)


--- trunk/Source/WebKit/WebProcess/GPU/media/SourceBufferPrivateRemote.cpp	2021-12-15 01:23:39 UTC (rev 287058)
+++ trunk/Source/WebKit/WebProcess/GPU/media/SourceBufferPrivateRemote.cpp	2021-12-15 01:25:11 UTC (rev 287059)
@@ -78,7 +78,7 @@
     m_gpuProcessConnection->messageReceiverMap().removeMessageReceiver(Messages::SourceBufferPrivateRemote::messageReceiverName(), m_remoteSourceBufferIdentifier.toUInt64());
 }
 
-void SourceBufferPrivateRemote::append(Ref<FragmentedSharedBuffer>&& data)
+void SourceBufferPrivateRemote::append(Ref<SharedBuffer>&& data)
 {
     if (!m_gpuProcessConnection)
         return;

Modified: trunk/Source/WebKit/WebProcess/GPU/media/SourceBufferPrivateRemote.h (287058 => 287059)


--- trunk/Source/WebKit/WebProcess/GPU/media/SourceBufferPrivateRemote.h	2021-12-15 01:23:39 UTC (rev 287058)
+++ trunk/Source/WebKit/WebProcess/GPU/media/SourceBufferPrivateRemote.h	2021-12-15 01:25:11 UTC (rev 287059)
@@ -72,7 +72,7 @@
 
     // SourceBufferPrivate overrides
     void setActive(bool) final;
-    void append(Ref<WebCore::FragmentedSharedBuffer>&&) final;
+    void append(Ref<WebCore::SharedBuffer>&&) final;
     void abort() final;
     void resetParserState() final;
     void removedFromMediaSource() final;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to