1108917 - Allow eviction of entire decoders that are ahead of the current play position

Status: RESOLVED FIXED

Description

[Matt Woodrow (:mattwoodrow)]

Currently we only allow eviction of data that is behind the current play position.

It's possible to accumulate considerable amounts of data ahead of the current position (usually by seeking backwards in small increments), which won't be evicted by the current code.

Seeking backwards will results in new decoders for each seek, so we can evict entire decoders and prevent this from using excessive amounts of memory.

Comment 1

[Matt Woodrow (:mattwoodrow)]

Created attachment 8533510 [details] [diff] [review]
Part 1: Attempt to evict data from decoders in start-time order

We prefer to evict data as far away from the current playback position as possible, and this will help with that (in some cases).

Comment 2

[Matt Woodrow (:mattwoodrow)]

Created attachment 8533511 [details] [diff] [review]
Part 2: Allow eviction of decoders that are ahead of the current playback position

Comment 3

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Comment on attachment 8533510 [details] [diff] [review]
Part 1: Attempt to evict data from decoders in start-time order

Review of attachment 8533510 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/media/mediasource/TrackBuffer.cpp
@@ +239,5 @@
> +        decoders[i] != mCurrentDecoder) {
> +      RemoveDecoder(decoders[i]);
> +    }
> +    if (toEvict <= 0 ||
> +        decoders[i] == mCurrentDecoder) {

nit: line break

Comment 4

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5c24dfe0cf87
https://hg.mozilla.org/integration/mozilla-inbound/rev/e278ad9face8

Comment 5

[Wes Kocher (:KWierso)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/b2f813477747 for mochitest-2 bustage:

https://treeherder.mozilla.org/ui/logviewer.html#?job_id=4520195&repo=mozilla-inbound

Comment 6

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0f9b7c71ba3d

Comment 7

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/99d8d92a319b

Comment 8

[Wes Kocher (:KWierso)]

https://hg.mozilla.org/mozilla-central/rev/0f9b7c71ba3d
https://hg.mozilla.org/mozilla-central/rev/99d8d92a319b

Comment 9

[Ralph Giles (:rillian) | needinfo me]

Comment on attachment 8533511 [details] [diff] [review]
Part 2: Allow eviction of decoders that are ahead of the current playback position

Approval Request Comment
[Feature/regressing bug #]: MSE
[User impact if declined]: Less consistent testing, sites more likely to serve Flash video.
[Describe test coverage new/current, TBPL]: Landed on m-c.
[Risks and why]: Low, MSE-specific code.
[String/UUID change made/needed]: None.

Comment 10

[Ralph Giles (:rillian) | needinfo me]

Comment on attachment 8533510 [details] [diff] [review]
Part 1: Attempt to evict data from decoders in start-time order

Same request applies to all patches on this bug.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/72f6dba74653
https://hg.mozilla.org/releases/mozilla-aurora/rev/d64cfe41fa74