1650281 - browser.privatebrowsing.forceMediaMemoryCache breaks mp4 video site

Status: VERIFIED FIXED

(Core :: Audio/Video: Playback, defect, P1)

Description

[Sylvain]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Firefox/78.0

Steps to reproduce:

Add this to user.js:
user_pref("browser.privatebrowsing.forceMediaMemoryCache", true);
user_pref("media.memory_cache_max_size", 393216); // Optional

Then go to https://www.kindgirls.com/ (yeah I know)
Click on Videos at the top right
Click on the first video (it's a link to an mp4 and a webm)
Let it play for 5 seconds
Click BACK and pick another video
Repeat this process a few times

Actual results:

Usually, after opening the third video, we get an error instead of the video, saying there are no mime types for this media. I usually get it after the 3rd one, but sometimes I get it on the second try, sometimes on the 4th; I cannot start playing more than a few videos until none will play. And mind you, I do not have to play them much, just a few seconds before trying out another one...

Expected results:

As you can see I tried increasing the size of the RAM cache, but it makes no difference. Yeah I know, pr0n site, but it's good for testing because it does not use a custom player, it's just pure links to small mp4 videos. I am guessing this might be an uplift from Tor? Tor will soon be using the new ESR and might then be affected too (for now it is not affected, it's still based on v68).

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 2

[C.M.Chang[:chunmin]]

I can reproduce this in private window with the provided link. Are you able to reproduce this on other sites, or non-private window?

Comment 3

[Sylvain]

Well the pref only affects Private Browsing, it disables the disk media cache in private mode (used to buffer video).
I found another site for testing this but really, any site that has a link to a mp4 will do. So go here:
http://www.html5videoplayer.net/html5video/mp4-h-264-video-test/
Start playing the video, let it play for 10 seconds then refresh the page; repeat a few times.
Regards,

Comment 4

[C.M.Chang[:chunmin]]

I can take a look since I can reproduce it.

Comment 5

[C.M.Chang[:chunmin]]

Finally got time to check on this.

At first glance, I would guess there is something wrong around gCombinedSizes since gCombinedSizes never grows but gCombinedSizes can be shrank.

Maybe this is a regression of bug 1356046, since gCombinedSizes += .. is removed from changeset 485993.

I'll check if adding gCombinedSizes += .. to MemoryBlockCache::EnsureBufferCanContain works

Comment 6

[C.M.Chang[:chunmin]]

Attachment: Bug 1650281 - P1: Widen `gCombinedSizes` once the buffers grow

Comment 7

[C.M.Chang[:chunmin]]

Attachment: Bug 1650281 - P2: Make sure `gCombinedSizes` won't be underflowed

Depends on D84273

Comment 8

[C.M.Chang[:chunmin]]

In this bug, when the video cannot be played, the MemoryBlockCache::EnsureBufferCanContain will return false and make MemoryBlockCache::Init() fails, so MediaCache::GetMediaCache get nothing and then MediaCacheStream::Init returns an error. As a result, ChannelMediaDecoder cannot be loaded in HTMLMediaElement since ChannelMediaResource::Open returns an error from MediaCacheStream::Init.

The cause is that the gCombinedSizes is underflowed to a super big value v such that v + k is larger than the size of memory_caches_limit, where k is the first buffer size allocated for MemoryBlockCache's inner buffer.

The gCombinedSizes is initialized to 0 at first, but it never grows. Once MemoryBlockCache is released (it's no longer needed), the gCombinedSizes will be updated to gCombinedSizes -= mBuffer.Length(). So gCombinedSizes is underflowed to 0 - mBuffer.Length(), and becomes a super big positive value v.

When the next MemoryBlockCache is created, asking for a buffer size k, the code will think now the total buffer size v + k is larger than the system-limit, and so it will reject the request.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3c239f976c21
P1: Widen `gCombinedSizes` once the buffers grow r=gerald
https://hg.mozilla.org/integration/autoland/rev/6c83e21dd295
P2: Make sure `gCombinedSizes` won't be underflowed r=gerald

Comment 10

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/3c239f976c21
https://hg.mozilla.org/mozilla-central/rev/6c83e21dd295

Comment 11

[Ardelean Oana, Desktop Test Engineering [:oardelean]]

I managed to reproduce this issue on a 2020-07-02 Nightly build using the STR and the URL from Comment 3, on macOS 12. Verified as fixed on Firefox 108.0b1(build ID: 20221114145411) Nightly 109.0a1(build ID: 20221114214403) on macOS 12, Windows 10, Ubuntu 22.