1132825 - Evicting data can leave data in unplayable state

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Jean-Yves Avenard [:jya]]

I was investigating a stall in YouTube that occurred once I lowered the eviction threshold to a very low value (1MB).

This caused the TrackBuffer to attempt to evict data very quickly (within 10s).

When this happened
about:media would show me video buffer still containing about 800KB of data, yet with an empty buffered range. how bizarre?!

The eviction code does:
      double time = aPlaybackTime - MSE_EVICT_THRESHOLD_TIME;
      int64_t playbackOffset = decoders[i]->ConvertToByteOffset(time);
      MSE_DEBUG("evicting some bufferedEnd=%f"
                "aPlaybackTime=%f time=%f, playbackOffset=%lld size=%lld",
                buffered->GetEndTime(), aPlaybackTime, time,
                playbackOffset, decoders[i]->GetResource()->GetSize());
      if (playbackOffset > 0) {
        toEvict -= decoders[i]->GetResource()->EvictData(playbackOffset,
                                                         toEvict);

So, to be sure, I added the following assertion:
        nsRefPtr<dom::TimeRanges> buffered2 = new dom::TimeRanges();
        decoders[i]->GetBuffered(buffered2);
        if (buffered->GetEndTime() >= aPlaybackTime && buffered2->Length()) {
          MOZ_ASSERT(false);
        }

basically, if we add a buffered range that contained our current playback time it would cause an assert if following the SourceBufferResource->EvictData we ended up with no buffered range (which should never happen).

The log was:
TrackBuffer(141b780f0:video/mp4)::EvictData: evicting some bufferedEnd=59.400000 aPlaybackTime=32.821405 time=30.821405, playbackOffset=371571 size=817489

toEvict is 48392.

Prior the call to SourceBufferResource->EvictData, the range was [0, 59.399999999999999].

After the call, it is empty.

My guess is that as we only partially evict up to "toEvict" value, we shorten the data and leave it in an undemuxable state.

Comment 1

[Jean-Yves Avenard [:jya]]

Attachment: Don't evict partial segments

Don't evict partial segment

Comment 2

[cajbir (:cajbir)]

Link to try run please.

Comment 3

[cajbir (:cajbir)]

Comment on attachment 8564850 [details] [diff] [review]
Don't evict partial segments

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

r+ with greenish-try run.

Comment 4

[Jean-Yves Avenard [:jya]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e9b7c6dfb66f

Comment 5

[Jean-Yves Avenard [:jya]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/355f5e2dee58

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/355f5e2dee58

Comment 7

[Ralph Giles (:rillian)]

Comment on attachment 8564850 [details] [diff] [review]
Don't evict partial segments

Approval Request Comment
[Feature/regressing bug #]: MSE
[User impact if declined]: Possible stalls with MSE video playback.
[Describe test coverage new/current, TreeHerder]: Landed on m-c.
[Risks and why]: MSE-specific small change. Risk is low.
[String/UUID change made/needed]: None.

Comment 8

[Ralph Giles (:rillian)]

Pushed to aurora with pre-approval from lmandel.

https://hg.mozilla.org/releases/mozilla-aurora/rev/c106faf8f0b9

Comment 9

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8564850 [details] [diff] [review]
Don't evict partial segments

As stated, this bug was pre-approved to land with a set of changes for MSE. Marking the approval after the fact.

Comment 11

[Jean-Yves Avenard [:jya]]

need beta uplift

Comment 12

[Ralph Giles (:rillian)]

This landed on 37 aurora and accidentally reverted by bug 1132796, so it was merged into 37 beta but didn't make the 37b1 build. That's resolved. We're good on all three branches now.

Comment 13

[advcomp2019]

I am wondering about something. Could this bug cause my issue in Firefox 36: https://bugzilla.mozilla.org/show_bug.cgi?id=1138140 ?

My issue gets bad with really long videos.

Comment 14

[Jean-Yves Avenard [:jya]]

No. This bug only affects MSE

Comment 15

[advcomp2019]

(In reply to Jean-Yves Avenard [:jya] from comment #14)
> No. This bug only affects MSE

Well, With my bug, I think I am starting to get this bug without MSE now because it goes on but then a few minute it gets buffered. Then a few minutes later, the buffer is down to back to where is progress bar is at.

Comment 16

[advcomp2019]

Oops.. Wrong bug