[tor-bugs] #29120 [Applications/Tor Browser]: Default value of media.cache_size (0) causes some media to load extremely slowly or become unplayable

[Tor Bug Tracker & Wiki]

#29120: Default value of media.cache_size (0) causes some media to load extremely
slowly or become unplayable
-------------------------------------------------+-------------------------
 Reporter:  QZw2aBQoPyuEVXYVlBps                 |          Owner:  tbb-
                                                 |  team
     Type:  defect                               |         Status:  new
 Priority:  High                                 |      Milestone:
Component:  Applications/Tor Browser             |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  tbb-disk-leak, tbb-usability-        |  Actual Points:
  website, TorBrowserTeam201901                  |
Parent ID:                                       |         Points:
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------

Comment (by QZw2aBQoPyuEVXYVlBps):

 Replying to [comment:8 cypherpunks2]:
 > Replying to [comment:4 QZw2aBQoPyuEVXYVlBps]:
 > > {{{RefPtr<MediaBlockCacheBase> bc = new MemoryBlockCache(8192000);}}}
 > {{{RefPtr<MediaBlockCacheBase> bc = new MemoryBlockCache(0);}}}
 > https://hg.mozilla.org/releases/mozilla-
 esr60/file/256453759958ed9c2eb17a0764d2fcfd7f8e3323/dom/media/MemoryBlockCache.cpp#l182
 I don't think the code you linked does what you think it does. If you look
 right before that, you'll see an early return on {{{aContentLength ==
 0}}}. {{{initialLength}}} is based on the size of the internal buffer, so
 that code in the if statement will simply be executed on the first write
 to the cache rather than on initialization, it doesn't change the
 situation very much.

 Another option I thought of while reading the MediaCache source is that
 was can probably get by with just slightly modifying the current code that
 creates MemoryBlockCache objects.
 https://hg.mozilla.org/releases/mozilla-
 esr60/file/256453759958ed9c2eb17a0764d2fcfd7f8e3323/dom/media/MediaCache.cpp#l737

 If you would humor some code, this would be replacing the if statement
 from that linked line:
 {{{
 // The size we will initialize the cache to
 int64_t cacheBytes = 0;

 if (aContentLength <= 0) {
   // Unknown content length, give it a max sized buffer just to be sure.
   cacheBytes = int64_t(MediaPrefs::MediaMemoryCacheMaxSize()) * 1024
 } else {
   // If the content length is known, we'll try to initialize a cache that
 will hold it, but only up to the max cache size
   cacheBytes = std::min(aContentLength,
 int64_t(MediaPrefs::MediaMemoryCacheMaxSize()) * 1024)
 }

 RefPtr<MediaBlockCacheBase> bc = new MemoryBlockCache(cacheBytes);
 nsresult rv = bc->Init();
 if (NS_SUCCEEDED(rv)) {
   RefPtr<MediaCache> mc = new MediaCache(bc);
   LOG("GetMediaCache(%" PRIi64 ") -> Memory MediaCache %p",
 aContentLength,
     mc.get());
   return mc;
 }

 // Init() failed...
 // In the MediaCache source, there is a fallback to a file-backed cache
 beyond this point.
 // We don't want that, so something else will have to be done here.
 // In the origial code, the function will return a nullptr (gMediaCache)
 if both caches fail to initialize, but I'm not sure what the consequences
 of that are.
 }}}

 This code would create a new MemoryBlockCache for every request, doing
 away with the content size limit, and instead replacing it with a maximum
 cache size (based on the {{{media.memory_cache_max_size}}} preference).
 This should work since my previous test showed that an undersized cache
 will properly reuse memory as needed.
 I think it should also play nicely with whatever built in mechanism is in
 place to manage the total in-memory cache size, since it's pretty much
 using the original code with just minor modification.

 Let me know what you think.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/29120#comment:9>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online