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

Media Cache Section #941

Closed
wants to merge 1 commit into from
Closed

Media Cache Section #941

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented May 10, 2020

Hello. I've been using your excellent user.js to customize my Firefox, and I'm pleased to finally offer back a contribution. I tested lots of variations on these settings in different situations, plus looked into the source code, to arrive at these findings. I used large video files because it's easy to observe what FF does with a large chunk of data.

The media cache is separate from the main cache. It it used instead of the main cache for HTML5-style media objects such as <video>, and only keeps them in cache while the objects are open (i.e. navigating away to another page or closing the tab removes the objects from cache.) The media cache is primarily a disk-based cache, having been originally coded that way, however a memory-based media cache was added later and will be used instead if certain conditions are met. Only one cache type is used per media object.

A media object will be stored in memory instead of disk if the size of the media object is lower than all three of:
media.memory_cache_max_size (in KiB; default 8,192)
media.memory_caches_combined_limit_kb (in KiB, default 524,288)
media.memory_caches_combined_limit_pc_sysmem (in percent of system memory; default 5)

The two combined_limit prefs appear to be intended to limit the combined size of all the media objects in memory cache, but they don't do that.

If memory cache is not used, disk cache is used instead. The size of the disk cache is controlled by:
media.cache_size (in KiB; default 512,000)
The disk cache can't be disabled by setting this to 0. In that case, FF still uses a disk cache, but it is small (~12 MB) and playback may be impacted as a result.

While in private browsing mode, and while browser.privatebrowsing.forceMediaMemoryCache is set to true, disk cache is never used. In this case, the memory cache is used even if the media object is larger than media.memory_cache_max_size. The available memory cache is used, with parts of the object evicted as needed to stay within the limit (of a single object; combined memory cache size is still not limited). The default memory cache limit of 8 MiB is not adequate for high bitrate videos; playback will be impacted because the buffer is too small. If the media object is larger than either of the combined_limit prefs, it will not load at all.

I decided to set my limits to 3 GiB, 3 GiB and 50% because I want FF to use memory instead of disk whenever possible, and I have plenty of RAM. That's why I included the two combined_limit prefs -- they also need to be increased if a user wants to be able to use memory for large media objects.

Some large videos I used for testing (note: navigating directly to the videos, or accessing them as

@Thorin-Oakenpants
Copy link
Contributor

Hi @WolfWhisperer .. thanks for the info. I haven't forgotten about this, but some very quick points/questions

  • media.memory_cache_max_size - you set as 81920, the default is 8192 (is that a typos or did you deliberately 10x it? We set as 16384 which is based on Tor Browser's experience - and if they move it up, we'd also follow.
  • kibibtyes .. can you point me to the source that shows this, I'd just like to confirm :) Namely I'm looking at the 1003 pref browser.cache.memory.capacity - but if all these prefs are consistent, they'd be the same.
  • the new prefs I'm loathe to add for a couple of reasons
    • we don't disable memory cache by default, so AFAIC if people turn disable it, that's on them
    • less prefs to deal with (maintaining, people bitching about them etc)
    • but I would really need to judge each one on it's own: e.g. media.cache_size - if setting it to zero doesn't even disable it, then why bother listing it. People fiddling with it will just cause questions and performance issues

tl;dr: I'll get back to this at some stage :)

@ghost
Copy link
Author

ghost commented May 12, 2020

is that a typos or did you deliberately 10x it?

That was deliberate. In PB mode and when using browser.privatebrowsing.forceMediaMemoryCache this pref limits the size of the memory cache. 16 MiB isn't enough to smoothly play high-bitrate video. 64-128 MiB seems to be sufficient for most reasonable cases. This isn't an issue for TOR because the slow network speed prevents smooth playback of high-bitrate video on TOR anyway.

Kibibytes:
For browser.cache.memory.capacity see about:cache which shows the correct units, matching exactly a custom number that is set for this pref.

For media.memory_cache_max_size see how it is used in dom/media/MediaCache.cpp:

  const int64_t mediaMemoryCacheMaxSize =
      static_cast<int64_t>(StaticPrefs::media_memory_cache_max_size()) * 1024;

For media.cache_size note that the largest size the disk cache file will grow to is 524,288,000 bytes.

Why I think all the prefs are needed:
We disable the main browser disk cache by default, and also enable browser.privatebrowsing.forceMediaMemoryCache by default. This is good, as using memory-only caching makes sense for a lot of reasons. Unfortunately, we can't completely disable the media disk cache. We can only reduce its use to the smallest number of situations possible, and the two combined_limit prefs are part of the key to doing that. Those prefs also control the size of the largest media file that can be loaded at all in PB mode (on a system with 8GB RAM that would be only 400MB by default). That's a new limitation we create by enabling browser.privatebrowsing.forceMediaMemoryCache and people should know about it and what they can do about it.

As for media.cache_size that is not a pref I can see much value in anyone changing. The value I see is in documenting the unusual behavior of the pref. There is a serious lack of information available about how the media cache works. People will naturally endeavor to disable disk cache wherever possible, and would discover that pref on their own. By explaining why it can't/shouldn't be disabled, hopefully we can save people a lot of time figuring it out the hard way.

@Thorin-Oakenpants
Copy link
Contributor

me

we don't disable memory cache by default

I clearly need moar coffee, lol. We actual force (more) memory cache (by disabling disk) - and this discussion is all about memory cache and how to use/improve that

@Thorin-Oakenpants
Copy link
Contributor

going to @earthlng here for his opinions

  • upping the value of media.memory_cache_max_size seems legit: although if network speed on TB for high quality/res vids is that bad, then I doubt they work much better with a media disk cache - I guess it really depends on the nodes. But, we're not TB: so I agree we should raise this: not sure if 10x is right.
  • kibibytes: excellent: those are definite fixes then

@earthlng
Copy link
Contributor

@WolfWhisperer thanks for all those very detailed infos!

In your paragraph about "private browsing mode"+forceMediaMemoryCache you write that

If the media object is larger than either of the combined_limit prefs, it will not load at all.

but that's not what I'm seeing. To test it I already had forceMediaMemoryCache enabled, then set the memory_cache_max_size to 10x default and reduced media.memory_caches_combined_limit_kb to 163840 (to make the cache smaller than your 1st video link) and the 1st video you linked works fine for me in a private window. What am I missing?
I guess it could be because that video is sent in chunks (?)

I tested another video (which seems to send the whole video unchunked, with a Content-Length of 460875880) with the same config and that works fine too.

If we're indeed breaking certain videos in private windows due to the recently added forceMediaMemoryCache pref then that's definitely something we should warn users about but I'm not able to confirm that we actually are.
Also, there's currently nothing in this PR that mentions potential breakage for 1016 which IMO is the most important part and presumably the main reason for this whole PR.

@ghost
Copy link
Author

ghost commented May 27, 2020

I checked again, and I'm still seeing the videos not load at all, including the link you provided. I'm using FF 76.0, are you too? I tried seeing if certain other prefs might be influencing what's going on to no avail. Tomorrow I'll set up a vanilla FF installation on another system and see what I can come up with.

@crssi
Copy link

crssi commented May 27, 2020

All the videos here are working flawlessly in a plain vanilla firefox profile + ghacks user.js
Even skipping, random clicking on a progress bar works very well.

What is your Internet connection bandwidth? Speedtest?

Firefox 76.0.1 (64-bit)
Windows 10 Version 1909 - up-to-date OS and APPs level
user.js version 77-alpha

Cheers

@earthlng
Copy link
Contributor

earthlng commented Jun 1, 2020

I'm using FF 76.0, are you too?

yes

While in private browsing mode, and while browser.privatebrowsing.forceMediaMemoryCache is set to true, disk cache is never used. In this case, the memory cache is used even if the media object is larger than media.memory_cache_max_size. The available memory cache is used, with parts of the object evicted as needed to stay within the limit (of a single object; combined memory cache size is still not limited).

Since we know that parts of a media object are evicted as needed, and since by default it only uses a very small media cache anyways (ie. much smaller than either of the combined_limit prefs on almost all systems, presumably), I don't see why a media object shouldn't load when it's larger than either of the combined_limit prefs.

@crssi, did you set media.memory_caches_combined_limit_kb to something like 163840 (ie much smaller than the size of the videos) and test in a private window?
If not, can you please do that and see if the videos still load for you?

@crssi
Copy link

crssi commented Jun 1, 2020

@crssi, did you set media.memory_caches_combined_limit_kb to something like 163840 (ie much smaller than the size of the videos) and test in a private window?
If not, can you please do that and see if the videos still load for you?

In this case the video stutters.
But if I go much lower, like 8192, the video doesn't load at all. I haven't try where is the "not loading" limit is.

@earthlng
Copy link
Contributor

earthlng commented Jun 2, 2020

Thanks @crssi

yeah the stuttering is probably because the media cache is too small. We'll likely increase that soon.

@WolfWhisperer videos not loading at all seems to be caused by something on your end. We're now 2 people who can't replicate that issue.

@Thorin-Oakenpants
Copy link
Contributor

@earthlng Do we want to fixup the kibibytes info and up the cache size etc for 78, given it's an ESR release. It doesn't have to hold up 78-beta, but the time til 78 final is not far off either.

@earthlng
Copy link
Contributor

yeah we can do that

@earthlng
Copy link
Contributor

earthlng commented Aug 3, 2020

problems with forceMediaMemoryCache (ie videos not loading) could be this (fixed in FF80).
Don't know if that's bad enough to disable it until FF80 is out. (?)

I committed the kibibytes fixup. What do you want to up the cache to? 5x default? 10x default?

@Thorin-Oakenpants
Copy link
Contributor

   8192 = default
 16384 = 2x (TB)
 32768 = 4x
 65536 = 8x
 81920 = 10x = WolfWhisperer

WolfWhisperer said

64-128 MiB seems to be sufficient for most reasonable cases.

I think we do either 4x or 8x: since this user.js is aimed at desktop, we should be fine with 64 - I'll do a commit

@earthlng Was there anything else in this PR you wanted?

Thorin-Oakenpants added a commit that referenced this pull request Aug 3, 2020
@GIPeon GIPeon mentioned this pull request Aug 4, 2020
@earthlng
Copy link
Contributor

Was there anything else in this PR you wanted?

No, I think it's too much and most people wouldn't know what to do with it.

@Thorin-Oakenpants
Copy link
Contributor

@earthlng

FYI: https://bugzilla.mozilla.org/show_bug.cgi?id=1650281 - fixed in FF80

  • Is this worth adding to the user.js for ESR78 users
    • reloading/refreshing the page breaks playback (see comment 3)

@earthlng
Copy link
Contributor

earthlng commented Sep 5, 2020

I linked to that ticket on August 3rd (see above). They added this pref specifically for TB so I'd hope that they'll backport that patch to ESR78. But yes, until they do that I think it's probably worth mentioning for ESR78 users.

@Thorin-Oakenpants
Copy link
Contributor

OK, I'll add it to the other patch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants