Closed Bug 1132825 Opened 5 years ago Closed 5 years ago

Evicting data can leave data in unplayable state

Categories

(Core :: Audio/Video, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla38
Tracking Status
firefox37 --- fixed
firefox38 --- fixed

People

(Reporter: jya, Assigned: jya)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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.
Don't evict partial segment
Attachment #8564850 - Flags: review?(cajbir.bugzilla)
Assignee: nobody → jyavenard
Status: NEW → ASSIGNED
Link to try run please.
Comment on attachment 8564850 [details] [diff] [review]
Don't evict partial segments

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

r+ with greenish-try run.
Attachment #8564850 - Flags: review?(cajbir.bugzilla) → review+
https://hg.mozilla.org/mozilla-central/rev/355f5e2dee58
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
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.
Attachment #8564850 - Flags: approval-mozilla-aurora?
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.
Attachment #8564850 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Duplicate of this bug: 1136138
need beta uplift
Flags: needinfo?(giles)
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.
Flags: needinfo?(giles)
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.
No. This bug only affects MSE
(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.
Oops.. Wrong bug
You need to log in before you can comment on or make changes to this bug.