[webkit-reviews] review granted: [Bug 219812] [Media in GPU Process][MSE] Implement SourceBuffer::reportExtraMemoryAllocated() : [Attachment 416068] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted Peng Liu
<peng.liu6 at apple.com>'s request for review:
Bug 219812: [Media in GPU Process][MSE] Implement
SourceBuffer::reportExtraMemoryAllocated()
https://bugs.webkit.org/show_bug.cgi?id=219812

Attachment 416068: Patch

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




--- Comment #2 from Eric Carlson <eric.carlson at apple.com> ---
Comment on attachment 416068
  --> https://bugs.webkit.org/attachment.cgi?id=416068
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?