Closed Bug 1540573 Opened 6 years ago Closed 6 years ago

Use large values for MediaCache size/readahead/resume and preload action if we're on Wifi/Ethernet, small if we're on cellular

Categories

(Core :: Audio/Video: Playback, enhancement, P2)

Unspecified
Android
enhancement

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox66 --- wontfix
firefox67 --- wontfix
firefox68 --- fixed

People

(Reporter: cpearce, Assigned: cpearce)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [gvtv:p1] [media-q2])

Attachments

(8 files, 2 obsolete files)

(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details
(deleted), text/x-phabricator-request
Details

On Android, we unilaterally use small values for the media cache size, resume threshold, and readahead limit. We do this under the assumption that Android is likely to be running on a phone, and we assume we should conserve the user's cellular data allowance on a phone.

This is probably a reasonable thing to do while we're running on a cellular connection, but a phone may in fact have a WiFi connection (say while being used from the couch at home).

For running GeckoView on the FireTV Stick/Cube, these small values are annoying, as while running on these device we typically have either an Ethernet or WiFi connection.

Small values of these prefs mean we're more likely to have to rebuffer while playing HD content. If we have super high quality content, the default cache size of 32MB can actually be too small to reliably play (see bug 1538023, but note such content is only likely to be in src=url video for testing purposes).

We should also change the default preload action if we're running on what we assume is non-cellular connection; assuming preload:none is super annoying on FireTV devices.

Rank: 19
Priority: -- → P2
Whiteboard: [gvtv] → [gvtv] [media-q2]
Assignee: nobody → cpearce

67=fix-optional because FFTV will use some GV version > 67.

OS: Unspecified → Android
Whiteboard: [gvtv] [media-q2] → [gvtv:p1] [media-q2]

In GeckoView the nsINetworkLinkService doesn't work in the content process, as
we don't seem to have an AndroidBridge there, so just maintain the network
connection type on the ContentChild.

(I had considered keeping this on the NeckoChild, but the creation of that is
initiated from the content process side, and there's not an easy and clean way
to have the parent process send us the connection type after construction of
the NeckoParent, other than have the NeckoChild request it either
synchronously, or doing it async and hoping it's not asked for the value before
the response comes in.)

Depends on D26231

Normally when downloading media data we throttle the download only if we're
ahead of the read cursor more than the "readahead limit", and if we estimate
that the connection is fast enough that we'll be able to download at a rate
fast enough to playback in real time if we resume it later.

On mobile we additionally override this so that we always throttle the download
once we're ahead of the read cursor by the readahead limit. This is to save
data. I think we can relax this to only do this override if we're on a
cellular connection; if we're on WiFi we can assume data is cheap.

Depends on D26233

We're allowed to take some liberties as to what the default value and behaviour
we assume for the 'preload' attribute on HTMLMediaElement by the spec. On
desktop we assumed preload="metadata", while on mobile we assumed the default
of preload="none" to save data. On mobile we also assumed that preload="auto"
meant preload="metadata".

I think it makes sense to instead of always assuming that data on Android is
always expensive, we can instead detect if we're running on a cellular connection,
and preload frugally then, otherwise aggressively.

Depends on D26234

67=wontfix because there's no rush to uplift to Fennec 67 Beta if this fix only introduces new behavior for Android devices on wired networks. That's an uncommon configuration.

Blocks: 1545266

GeckoView can't necessarily assume that the android.app.Application instance
is a GeckoApplication, so we need to factor out the foreground/background
callbacks into a new interface which GeckoRuntime and GeckoApplication can
both implement.

Attachment #9055973 - Attachment description: Bug 1540573 - Send network type change notifications from GeckoView/Java to Gecko/C++. r?snorp → Bug 1540573 - P2. Have GeckoRuntime listen for network link changes when in the foreground. r?snorp
Attachment #9055974 - Attachment description: Bug 1540573 - Expose network link type on ContentChild for use in content process. r?snorp → Bug 1540573 - P3. Expose network link type on ContentChild for use in content process. r?snorp
Attachment #9055975 - Attachment description: Bug 1540573 - Use larger MediaCache sizes when on cellular connection. r?jya → Bug 1540573 - P4. Use larger MediaCache sizes when on cellular connection. r?jya
Attachment #9055976 - Attachment description: Bug 1540573 - Only "always throttle" media download to the readahead on cellular connections. r?jya → Bug 1540573 - P5. Only "always throttle" media download to the readahead on cellular connections. r?jya
Attachment #9055977 - Attachment description: Bug 1540573 - Use frugal preloading of media data when on cellular, otherwise aggressive. r?jya → Bug 1540573 - P6. Use frugal preloading of media data when on cellular, otherwise aggressive. r?jya

This allows Gecko to react to network link/status changes events as needed.

Depends on D28941

Attachment #9055973 - Attachment is obsolete: true
Attachment #9059177 - Attachment is obsolete: true
Pushed by cpearce@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b49cd2b191ae P1. Observe ProcessLifecycle events in GeckoRuntime. r=snorp https://hg.mozilla.org/integration/autoland/rev/2c4303b175ec P2. Have GeckoRuntime listen for network link changes when in the foreground. r=snorp https://hg.mozilla.org/integration/autoland/rev/3496ca48f6e0 P3. Expose network link type on ContentChild for use in content process. r=snorp https://hg.mozilla.org/integration/autoland/rev/814c94b26028 P4. Use larger MediaCache sizes when on cellular connection. r=jya https://hg.mozilla.org/integration/autoland/rev/bf725b7daa5b P5. Only "always throttle" media download to the readahead on cellular connections. r=jya https://hg.mozilla.org/integration/autoland/rev/599e6e06599d P6. Use frugal preloading of media data when on cellular, otherwise aggressive. r=jya

The Content Security Policy tests were handling the smaller android preload
values that were #defined on Android by setting media.preload.default=2. Now
that we're detecting whether we're on cellular or not, and the android
emulators that our tests run on emulate a cellular connection, just setting
media.preload.default is no longer enough.

So set media.preload.default.cellular=2 in the CSP mochitests instead of
media.preload.default, to make the CSP mochitests pass in the Android
emulators.

Depends on D26235

Pushed by cpearce@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4ff116b9d729 P1. Observe ProcessLifecycle events in GeckoRuntime. r=snorp https://hg.mozilla.org/integration/autoland/rev/1bde5042e507 P2. Have GeckoRuntime listen for network link changes when in the foreground. r=snorp https://hg.mozilla.org/integration/autoland/rev/fe196b2dfc62 P3. Expose network link type on ContentChild for use in content process. r=snorp https://hg.mozilla.org/integration/autoland/rev/3db059b34e40 P4. Use larger MediaCache sizes when on cellular connection. r=jya https://hg.mozilla.org/integration/autoland/rev/270a8917377f P5. Only "always throttle" media download to the readahead on cellular connections. r=jya https://hg.mozilla.org/integration/autoland/rev/b10d2cae45f2 P6. Use frugal preloading of media data when on cellular, otherwise aggressive. r=jya https://hg.mozilla.org/integration/autoland/rev/dd882b8cd73e p7. Modify CSP tests to use preload=2 on (emulated) cellular connections. r=jya

Backed out for causing 1548441

backout: https://hg.mozilla.org/integration/autoland/rev/0bb9da8fe6af

Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla68 → ---

I believe the problem with test_preload_suspend is caused by that test changing the media cache size via pref, and the there's nothing to cause MediaCache::UpdateGeometry() to run in response to the pref change.

Pushed by cpearce@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/6a17951bc4b9 P1. Observe ProcessLifecycle events in GeckoRuntime. r=snorp https://hg.mozilla.org/integration/autoland/rev/d029091d7fd8 P2. Have GeckoRuntime listen for network link changes when in the foreground. r=snorp https://hg.mozilla.org/integration/autoland/rev/b78948e4f480 P3. Expose network link type on ContentChild for use in content process. r=snorp https://hg.mozilla.org/integration/autoland/rev/bcbb8c779852 P4. Use larger MediaCache sizes when on cellular connection. r=jya https://hg.mozilla.org/integration/autoland/rev/07c6d54744f7 P5. Only "always throttle" media download to the readahead on cellular connections. r=jya https://hg.mozilla.org/integration/autoland/rev/0d175912734d P6. Use frugal preloading of media data when on cellular, otherwise aggressive. r=jya https://hg.mozilla.org/integration/autoland/rev/90ebd256a92b p7. Modify CSP tests to use preload=2 on (emulated) cellular connections. r=jya
Pushed by mozilla@buttercookie.de: https://hg.mozilla.org/integration/autoland/rev/9905f7082bed Followup - Don't double-start/stop NetworkManager in Fennec. r=snorp
Blocks: 1529812
Regressions: 1552145
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: