⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 287059 in webkit


Ignore:
Timestamp:
Dec 14, 2021, 5:25:11 PM (5 years ago)
Author:
Jean-Yves Avenard
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:
Location:
trunk/Source
Files:
14 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r287053 r287059  
     12021-12-14  Jean-Yves Avenard  <jya@apple.com>
     2
     3        SourceBufferParser should be using contiguous SharedBuffer
     4        https://bugs.webkit.org/show_bug.cgi?id=233865
     5        rdar://problem/86085253
     6
     7        Reviewed by Eric Carlson.
     8
     9        The SharedBuffer sent to the SourceBufferParser can only ever contain one
     10        DataSegment and we had assertions to that effect. The SharedBuffer class
     11        type now guarantees how the data is structured and allow for more explicit
     12        code which improves readability.
     13        We also had some workarounds for the fact that SharedBuffer didn't use
     14        thread-safe refcounting and instead we referenced the inner DataSegment.
     15        This can be removed.
     16
     17        Covered by existing tests, no observable differences.
     18
     19        * Modules/mediasource/SourceBuffer.cpp:
     20        (WebCore::SourceBuffer::appendBufferTimerFired):
     21        * Modules/mediasource/SourceBuffer.h:
     22        * platform/audio/cocoa/AudioFileReaderCocoa.cpp:
     23        * platform/graphics/SourceBufferPrivate.cpp:
     24        (WebCore::SourceBufferPrivate::append):
     25        * platform/graphics/SourceBufferPrivate.h:
     26        * platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.h:
     27        * platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:
     28        (WebCore::SourceBufferPrivateAVFObjC::append):
     29        * platform/graphics/cocoa/SourceBufferParser.cpp:
     30        (WebCore::SourceBufferParser::Segment::Segment):
     31        (WebCore::SourceBufferParser::Segment::size const):
     32        (WebCore::SourceBufferParser::Segment::read const):
     33        (WebCore::SourceBufferParser::Segment::takeSharedBuffer):
     34        (WebCore::SourceBufferParser::Segment::getSharedBuffer const):
     35        * platform/graphics/cocoa/SourceBufferParser.h:
     36        * platform/graphics/cocoa/SourceBufferParserWebM.cpp:
     37
    1382021-12-14  Jean-Yves Avenard  <jya@apple.com>
    239

  • trunk/Source/WebCore/Modules/mediasource/SourceBuffer.cpp

    r287021 r287059  
    531531    // When the segment parser loop algorithm is invoked, run the following steps:
    532532
    533     RefPtr<FragmentedSharedBuffer> appendData = WTFMove(m_pendingAppendData);
     533    RefPtr<SharedBuffer> appendData = WTFMove(m_pendingAppendData);
    534534    // 1. Loop Top: If the input buffer is empty, then jump to the need more data step below.
    535535    if (!appendData || !appendData->size()) {

  • trunk/Source/WebCore/Modules/mediasource/SourceBuffer.h

    r287021 r287059  
    223223    WTF::Observer<void*()> m_opaqueRootProvider;
    224224
    225     RefPtr<FragmentedSharedBuffer> m_pendingAppendData;
     225    RefPtr<SharedBuffer> m_pendingAppendData;
    226226    Timer m_appendBufferTimer;
    227227

  • trunk/Source/WebCore/platform/audio/cocoa/AudioFileReaderCocoa.cpp

    r287021 r287059  
    131131
    132132public:
    133     Ref<FragmentedSharedBuffer> m_buffer;
     133    Ref<SharedBuffer> m_buffer;
    134134#if ENABLE(MEDIA_SOURCE)
    135135    Ref<AudioTrackPrivateWebM> m_track;

  • trunk/Source/WebCore/platform/graphics/SourceBufferPrivate.cpp

    r287021 r287059  
    13191319}
    13201320
    1321 void SourceBufferPrivate::append(Ref<FragmentedSharedBuffer>&& buffer)
     1321void SourceBufferPrivate::append(Ref<SharedBuffer>&& buffer)
    13221322{
    13231323    append(buffer->extractData());

  • trunk/Source/WebCore/platform/graphics/SourceBufferPrivate.h

    r287021 r287059  
    5050namespace WebCore {
    5151
    52 class FragmentedSharedBuffer;
     52class SharedBuffer;
    5353class TimeRanges;
    5454
    … …  
    6969
    7070    virtual void setActive(bool) = 0;
    71     WEBCORE_EXPORT virtual void append(Ref<FragmentedSharedBuffer>&&);
     71    WEBCORE_EXPORT virtual void append(Ref<SharedBuffer>&&);
    7272    virtual void abort() = 0;
    7373    virtual void resetParserState() = 0;

  • trunk/Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.h

    r287021 r287059  
    154154
    155155    // SourceBufferPrivate overrides
    156     void append(Ref<FragmentedSharedBuffer>&&) final;
     156    void append(Ref<SharedBuffer>&&) final;
    157157    void abort() final;
    158158    void resetParserState() final;

  • trunk/Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm

    r287021 r287059  
    564564}
    565565
    566 void SourceBufferPrivateAVFObjC::append(Ref<FragmentedSharedBuffer>&& data)
     566void SourceBufferPrivateAVFObjC::append(Ref<SharedBuffer>&& data)
    567567{
    568568    ALWAYS_LOG(LOGIDENTIFIER, "data length = ", data->size());

  • trunk/Source/WebCore/platform/graphics/cocoa/SourceBufferParser.cpp

    r287021 r287059  
    8181}
    8282
    83 SourceBufferParser::Segment::Segment(Ref<FragmentedSharedBuffer>&& buffer)
     83SourceBufferParser::Segment::Segment(Ref<SharedBuffer>&& buffer)
    8484    : m_segment(WTFMove(buffer))
    8585{
    … …  
    102102        },
    103103#endif
    104         [](const Ref<FragmentedSharedBuffer>& buffer)
     104        [](const Ref<SharedBuffer>& buffer)
    105105        {
    106106            return buffer->size();
    … …  
    126126        },
    127127#endif
    128         [&](const Ref<FragmentedSharedBuffer>& buffer) -> ReadResult
     128        [&](const Ref<SharedBuffer>& buffer) -> ReadResult
    129129        {
    130130            buffer->copyTo(destination, position, sizeToRead);
    … …  
    134134}
    135135
    136 Ref<FragmentedSharedBuffer> SourceBufferParser::Segment::takeSharedBuffer()
     136Ref<SharedBuffer> SourceBufferParser::Segment::takeSharedBuffer()
    137137{
    138138    return WTF::switchOn(m_segment,
    … …  
    143143            auto readResult = read(0, vector.size(), vector.data());
    144144            if (!readResult.has_value())
    145                 return FragmentedSharedBuffer::create();
     145                return SharedBuffer::create();
    146146            vector.shrink(readResult.value());
    147             return FragmentedSharedBuffer::create(WTFMove(vector));
     147            return SharedBuffer::create(WTFMove(vector));
    148148        },
    149149#endif
    150         [&](Ref<FragmentedSharedBuffer>& buffer)
     150        [&](Ref<SharedBuffer>& buffer)
    151151        {
    152             return std::exchange(buffer, FragmentedSharedBuffer::create());
     152            return std::exchange(buffer, SharedBuffer::create());
    153153        }
    154154    );
    155155}
    156156
    157 RefPtr<FragmentedSharedBuffer> SourceBufferParser::Segment::getSharedBuffer() const
     157RefPtr<SharedBuffer> SourceBufferParser::Segment::getSharedBuffer() const
    158158{
    159159    return WTF::switchOn(m_segment,
    160160#if HAVE(MT_PLUGIN_FORMAT_READER)
    161         [&](const RetainPtr<MTPluginByteSourceRef>&) -> RefPtr<FragmentedSharedBuffer>
     161        [&](const RetainPtr<MTPluginByteSourceRef>&) -> RefPtr<SharedBuffer>
    162162        {
    163163            return nullptr;
    164164        },
    165165#endif
    166         [&](const Ref<FragmentedSharedBuffer>& buffer) -> RefPtr<FragmentedSharedBuffer>
     166        [&](const Ref<SharedBuffer>& buffer) -> RefPtr<SharedBuffer>
    167167        {
    168168            return buffer.ptr();

  • trunk/Source/WebCore/platform/graphics/cocoa/SourceBufferParser.h

    r287021 r287059  
    4646class ContentType;
    4747class MediaSample;
    48 class FragmentedSharedBuffer;
     48class SharedBuffer;
    4949
    5050class WEBCORE_EXPORT SourceBufferParser : public ThreadSafeRefCounted<SourceBufferParser> {
    … …  
    7070        Segment(RetainPtr<MTPluginByteSourceRef>&&);
    7171#endif
    72         Segment(Ref<FragmentedSharedBuffer>&&);
     72        Segment(Ref<SharedBuffer>&&);
    7373        Segment(Segment&&) = default;
    74         Ref<FragmentedSharedBuffer> takeSharedBuffer();
    75         // Will return nullptr if Segment's backend isn't a FragmentedSharedBuffer.
    76         RefPtr<FragmentedSharedBuffer> getSharedBuffer() const;
     74        Ref<SharedBuffer> takeSharedBuffer();
     75        // Will return nullptr if Segment's backend isn't a SharedBuffer.
     76        RefPtr<SharedBuffer> getSharedBuffer() const;
    7777
    7878        size_t size() const;
    … …  
    8888            RetainPtr<MTPluginByteSourceRef>,
    8989#endif
    90             Ref<FragmentedSharedBuffer>
     90            Ref<SharedBuffer>
    9191        > m_segment;
    9292    };

  • trunk/Source/WebCore/platform/graphics/cocoa/SourceBufferParserWebM.cpp

    r287021 r287059  
    366366    }
    367367
    368     static void FreeDataSegment(void* refcon, void*, size_t)
     368    static void FreeSharedBuffer(void* refcon, void*, size_t)
    369369    {
    370         auto* buffer = reinterpret_cast<DataSegment*>(refcon);
     370        auto* buffer = reinterpret_cast<SharedBuffer*>(refcon);
    371371        buffer->deref();
    372372    }
    … …  
    389389                continue;
    390390            }
    391             RefPtr<FragmentedSharedBuffer> sharedBuffer = currentSegment.getSharedBuffer();
     391            RefPtr<SharedBuffer> sharedBuffer = currentSegment.getSharedBuffer();
    392392            CMBlockBufferRef rawBlockBuffer = nullptr;
    393393            uint64_t lastRead = 0;
    … …  
    409409                destinationOffset = 0;
    410410            } else {
    411                 ASSERT(sharedBuffer->hasOneSegment(), "Can only deal with sharedBuffer containing a single DataSegment");
    412                 // A FragmentedSharedBuffer doesn't have thread-safe refcounting, as such we must keep a reference to the DataSegment instead.
    413411                // TODO: could we only create a new CMBlockBuffer if the backend memory changed since the previous one?
    414                 auto firstSegment = sharedBuffer->begin()->segment;
    415                 size_t canRead = std::min<size_t>(numToRead, firstSegment->size() - m_positionWithinSegment);
     412                size_t canRead = std::min<size_t>(numToRead, sharedBuffer->size() - m_positionWithinSegment);
    416413                // From CMBlockBufferCustomBlockSource documentation:
    417414                // Note that for 64-bit architectures, this struct contains misaligned function pointers.
    … …  
    421418                allocator.version = 0;
    422419                allocator.AllocateBlock = nullptr;
    423                 allocator.FreeBlock = FreeDataSegment;
    424                 allocator.refCon = firstSegment.ptr();
    425                 firstSegment->ref();
    426                 auto err = PAL::CMBlockBufferCreateWithMemoryBlock(nullptr, static_cast<void*>(const_cast<uint8_t*>(firstSegment->data())), firstSegment->size(), nullptr, &allocator, m_positionWithinSegment, canRead, 0, &rawBlockBuffer);
     420                allocator.FreeBlock = FreeSharedBuffer;
     421                allocator.refCon = sharedBuffer.get();
     422                sharedBuffer->ref();
     423                auto err = PAL::CMBlockBufferCreateWithMemoryBlock(nullptr, static_cast<void*>(const_cast<uint8_t*>(sharedBuffer->data())), sharedBuffer->size(), nullptr, &allocator, m_positionWithinSegment, canRead, 0, &rawBlockBuffer);
    427424                if (err != kCMBlockBufferNoErr)
    428425                    return Status(Status::kNotEnoughMemory);

  • trunk/Source/WebKit/ChangeLog

    r287056 r287059  
     12021-12-14  Jean-Yves Avenard  <jya@apple.com>
     2
     3        SourceBufferParser should be using contiguous shared buffer
     4        https://bugs.webkit.org/show_bug.cgi?id=233865
     5        rdar://problem/86085253
     6
     7        Reviewed by Eric Carlson.
     8
     9        * WebProcess/GPU/media/SourceBufferPrivateRemote.cpp:
     10        (WebKit::SourceBufferPrivateRemote::append):
     11        * WebProcess/GPU/media/SourceBufferPrivateRemote.h:
     12
    1132021-12-14  Alex Christensen  <achristensen@webkit.org>
    214

  • trunk/Source/WebKit/WebProcess/GPU/media/SourceBufferPrivateRemote.cpp

    r287021 r287059  
    7979}
    8080
    81 void SourceBufferPrivateRemote::append(Ref<FragmentedSharedBuffer>&& data)
     81void SourceBufferPrivateRemote::append(Ref<SharedBuffer>&& data)
    8282{
    8383    if (!m_gpuProcessConnection)

  • trunk/Source/WebKit/WebProcess/GPU/media/SourceBufferPrivateRemote.h

    r287021 r287059  
    7373    // SourceBufferPrivate overrides
    7474    void setActive(bool) final;
    75     void append(Ref<WebCore::FragmentedSharedBuffer>&&) final;
     75    void append(Ref<WebCore::SharedBuffer>&&) final;
    7676    void abort() final;
    7777    void resetParserState() final;
Note: See TracChangeset for help on using the changeset viewer.