Bug 219812 – [Media in GPU Process][MSE] Implement SourceBuffer::reportExtraMemoryAllocated()

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Peng Liu]

[Media in GPU Process][MSE] Implement SourceBuffer::reportExtraMemoryAllocated()

[Peng Liu]

Created attachment 416068 [details]
Patch

[Eric Carlson]

Comment on attachment 416068 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=416068&action=review

> Source/WebCore/Modules/mediasource/SourceBuffer.cpp:1194
> +void SourceBuffer::sourceBufferPrivateReportExtraMemoryCost(uint64_t extraMemory)

I don't think "sourceBufferPrivate" should be part of the method name.

> Source/WebCore/Modules/mediasource/SourceBuffer.h:161
> +    void sourceBufferPrivateReportExtraMemoryCost(uint64_t extraMemory) final;

Nit: parameter name isn't necessary.

> Source/WebCore/platform/graphics/SourceBufferPrivate.cpp:624
> +    DEBUG_LOG(LOGIDENTIFIER, "currentTime = ", currentTime, ", require ", totalTrackBufferSizeInBytes() + newDataSize, " bytes, maximum buffer size is ", maximumBufferSize);
> +    uint64_t initialBufferedSize = totalTrackBufferSizeInBytes();

if you switch the order of these lines you would only need to call the method once.

> Source/WebCore/platform/graphics/SourceBufferPrivate.cpp:644
> +        DEBUG_LOG(LOGIDENTIFIER, "evicted ", initialBufferedSize - totalTrackBufferSizeInBytes());

Did you mean to use the `initialBufferedSize` variable here?

[Peng Liu]

Comment on attachment 416068 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=416068&action=review

>> Source/WebCore/Modules/mediasource/SourceBuffer.cpp:1194
>> +void SourceBuffer::sourceBufferPrivateReportExtraMemoryCost(uint64_t extraMemory)
> 
> I don't think "sourceBufferPrivate" should be part of the method name.

Hmm, I agree that "sourceBufferPrivate" in the method name looks strange. I choose to keep it because this will make its name consistent with other methods of SourceBufferPrivateClient.

>> Source/WebCore/Modules/mediasource/SourceBuffer.h:161
>> +    void sourceBufferPrivateReportExtraMemoryCost(uint64_t extraMemory) final;
> 
> Nit: parameter name isn't necessary.

Right! Will fix it.

>> Source/WebCore/platform/graphics/SourceBufferPrivate.cpp:624
>> +    uint64_t initialBufferedSize = totalTrackBufferSizeInBytes();
> 
> if you switch the order of these lines you would only need to call the method once.

Good point! Will fix it.

>> Source/WebCore/platform/graphics/SourceBufferPrivate.cpp:644
>> +        DEBUG_LOG(LOGIDENTIFIER, "evicted ", initialBufferedSize - totalTrackBufferSizeInBytes());
> 
> Did you mean to use the `initialBufferedSize` variable here?

Yes. It will log how much memory is freed from the track buffer.

[Peng Liu]

Created attachment 416071 [details]
Patch for landing

[EWS]

Committed r270731: <https://trac.webkit.org/changeset/270731>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 416071 [details].

[Radar WebKit Bug Importer]

<rdar://problem/72250848>