Closed Bug 1010000 Opened 7 years ago Closed 7 years ago

update mTracksKnownTime when finishing a stream

Categories

(Core :: Audio/Video, defect)

31 Branch
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla32
Tracking Status
firefox31 --- verified
firefox32 --- verified
b2g-v2.0 --- fixed

People

(Reporter: karlt, Assigned: karlt)

References

Details

Attachments

(2 files)

A. A test in StreamBuffer::GetAllTracksEnd() [1] assumes that no tracks will
   be added if all ending tracks finish before mTracksKnownTime.

B. It also does not let the track end if any track ends after
   mTracksKnownTime.  This causes trouble with bug 999267 fixed since bug
   818822, because rounding with resampling means that a track end can be
   slightly after mTracksKnownTime.  When/if the resampling latency will be
   correctly handled, the track end will be even later.

Setting mTracksKnownTime to infinity when the stream ends (to indicate that no
futher tracks will be added) removes the issue in B, and allows the test to be
re-written to avoid the assumption in A.

[1] http://hg.mozilla.org/mozilla-central/rev/e526d37a9a19#l2.23
AdvanceKnownTracksTime() is no longer involved in the way described, since e526d37a9a19.
Attachment #8422159 - Flags: review?(roc)
and use this in StreamBuffer::GetAllTracksEnd() to detect when no more tracks
will be added.
Attachment #8422161 - Flags: review?(roc)
Blocks: 750258
https://hg.mozilla.org/integration/mozilla-inbound/rev/c79e3519bc93
https://hg.mozilla.org/integration/mozilla-inbound/rev/f6100b38ffd7

No direct tests, beyond some intermittents in bug 750258, but this will be in the testsuite when bug 999267 can land.
Flags: in-testsuite-
https://hg.mozilla.org/mozilla-central/rev/c79e3519bc93
https://hg.mozilla.org/mozilla-central/rev/f6100b38ffd7
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Comment on attachment 8422161 [details] [diff] [review]
update mTracksKnownTime when finishing a stream

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 943461
User impact if declined: Can't fix bug 999267, at least without test failures.
Testing completed (on m-c, etc.): on -m-c.
Risk to taking this patch (and alternatives if risky): 
Moderate risk, given some non-deterministic behavior observed in tests (even without this patch) such as bug 750258.  This patch is meant to remove some of the non-determinism, but there may be some unexpected assumptions made in dependent code.
String or IDL/UUID changes made by this patch: none.
Attachment #8422161 - Flags: approval-mozilla-beta?
Comment on attachment 8422161 [details] [diff] [review]
update mTracksKnownTime when finishing a stream

Approving this uplift because it had some testing with 3 weeks in m-c and I would like to see the audio issue fixed.
Attachment #8422161 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flagging for verification via the verification of the dependencies.
Keywords: verifyme
Set as verified since the dependencies are marked as verified too.
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.