1010000 - update mTracksKnownTime when finishing a stream

Status: VERIFIED FIXED

(Core :: Audio/Video, defect)

Description

[Karl Tomlinson (:karlt)]

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

Comment 1

[Karl Tomlinson (:karlt)]

Attachment: update SourceMediaStream::Finish() doc

AdvanceKnownTracksTime() is no longer involved in the way described, since e526d37a9a19.

Comment 2

[Karl Tomlinson (:karlt)]

Attachment: update mTracksKnownTime when finishing a stream

and use this in StreamBuffer::GetAllTracksEnd() to detect when no more tracks
will be added.

Comment 3

[Karl Tomlinson (:karlt)]

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.

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/c79e3519bc93
https://hg.mozilla.org/mozilla-central/rev/f6100b38ffd7

Comment 5

[Karl Tomlinson (:karlt)]

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.

Comment 6

[Sylvestre Ledru [:Sylvestre]]

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.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/ba42d247a943
https://hg.mozilla.org/releases/mozilla-beta/rev/a655b771fe10

Comment 8

[u279076]

Flagging for verification via the verification of the dependencies.

Comment 9

[Petruta Horea [:phorea], Desktop QA]

Set as verified since the dependencies are marked as verified too.