Summary: | [Media in GPU Process][MSE] Implement SourceBuffer::reportExtraMemoryAllocated() | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Peng Liu <peng.liu6> | ||||||
Component: | New Bugs | Assignee: | Peng Liu <peng.liu6> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | calvaris, eric.carlson, ews-watchlist, glenn, jer.noble, philipj, sergio, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Peng Liu
2020-12-11 16:17:57 PST
Created attachment 416068 [details]
Patch
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? 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. Created attachment 416071 [details]
Patch for landing
Committed r270731: <https://trac.webkit.org/changeset/270731> All reviewed patches have been landed. Closing bug and clearing flags on attachment 416071 [details]. |