Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MSE SourceBuffer] Reset extra memory cost inside removedFromMediaSou… #1264

Conversation

asurdej-comcast
Copy link

…rce()

This allows SourceBuffer that was removed from media source to report memory that is actually in use. Otherwise it reports last value even if memory was relased already.

I think m_reportedExtraMemoryCost doesn't need to be reset as memory can not grow after removing SourceBuffer from media source. There is no way to reattach and data are not accepted also.

The same we did in 2.28 34e8375

…rce()

This allows SourceBuffer that was removed from media source to report
memory that is actually in use. Otherwise it reports last value even
if memory was relased already.
@modeveci modeveci requested a review from eocanha January 9, 2024 12:00
@eocanha eocanha added the upstream Related to an upstream bug (or should be at some point) label Jan 10, 2024
@eocanha
Copy link
Member

eocanha commented Jan 10, 2024

Submitted the patch upstream for review as https://bugs.webkit.org/show_bug.cgi?id=267350 / WebKit/WebKit#22602.

webkit-commit-queue pushed a commit to eocanha/WebKit that referenced this pull request Jan 11, 2024
https://bugs.webkit.org/show_bug.cgi?id=267350

Reviewed by Xabier Rodriguez-Calvar.

When the SourceBuffer is removed from a MediaSource, its TrackBuffers are cleared (and so is
its data, which can take a good amount of memory), but the SourceBuffer still accounts for
the cost of its old data, as m_extraMemoryCost isn't reset to zero.

See: WebPlatformForEmbedded/WPEWebKit#1264

This patch resets m_extraMemoryCost when the SourceBuffer is removed.

Original author: Andrzej Surdej <Andrzej_Surdej@comcast.com>

* Source/WebCore/Modules/mediasource/SourceBuffer.cpp:
(WebCore::SourceBuffer::removedFromMediaSource): Reset m_extraMemoryCost.

Canonical link: https://commits.webkit.org/272908@main
eocanha added a commit that referenced this pull request Jan 11, 2024
https://bugs.webkit.org/show_bug.cgi?id=267350

Reviewed by Xabier Rodriguez-Calvar.

When the SourceBuffer is removed from a MediaSource, its TrackBuffers are cleared (and so is
its data, which can take a good amount of memory), but the SourceBuffer still accounts for
the cost of its old data, as m_extraMemoryCost isn't reset to zero.

See: #1264

This patch resets m_extraMemoryCost when the SourceBuffer is removed.

Original author: Andrzej Surdej <Andrzej_Surdej@comcast.com>

* Source/WebCore/Modules/mediasource/SourceBuffer.cpp:
(WebCore::SourceBuffer::removedFromMediaSource): Reset m_extraMemoryCost.

Canonical link: https://commits.webkit.org/272908@main
eocanha added a commit that referenced this pull request Jan 11, 2024
https://bugs.webkit.org/show_bug.cgi?id=267350

Reviewed by Xabier Rodriguez-Calvar.

When the SourceBuffer is removed from a MediaSource, its TrackBuffers are cleared (and so is
its data, which can take a good amount of memory), but the SourceBuffer still accounts for
the cost of its old data, as m_extraMemoryCost isn't reset to zero.

See: #1264

This patch resets m_extraMemoryCost when the SourceBuffer is removed.

Original author: Andrzej Surdej <Andrzej_Surdej@comcast.com>

* Source/WebCore/Modules/mediasource/SourceBuffer.cpp:
(WebCore::SourceBuffer::removedFromMediaSource): Reset m_extraMemoryCost.

Canonical link: https://commits.webkit.org/272908@main
@eocanha
Copy link
Member

eocanha commented Jan 11, 2024

Patch landed upstream as WebKit/WebKit@4fa545a and packported to wpe-2.38 as da14ef5 and to wpe-2.42 as 8472a26.
Closing PR.

@eocanha eocanha closed this Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
upstream Related to an upstream bug (or should be at some point) wpe-2.38
Development

Successfully merging this pull request may close these issues.

None yet

3 participants