Since the timestamps received by MDSM are always zero-based, we should also make mDecodedAudioEndTime zero-based. We will be able to remove checks like |mDecodedAudioEndTime == -1| to simplify the code.

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

Review commit: https://reviewboard.mozilla.org/r/32397/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/32397/

Note that while MSE per spec has a time range of [0, oo); it is not uncommon to have the first video frame have a negative time. This happens rather regularly with YouTube videos. We had to explicitly allow a leeway for negative values close to 0 for start time (as if we were to drop that frame (a keyframe) we would have to drop all frames to the next keyframe leaving a gap which would cause a stall. So it's important to be aware that negative frames are allowed even for MSE (which doesn't adjust the times to start at 0 unlike the other player.

https://dxr.mozilla.org/mozilla-central/source/dom/media/MediaDecoderStateMachine.cpp#900 We adjust the timestamps by the start time. So the timestamps are still zero-based.

not for MSE we don't. When using MSE, MediaFormatReated::ForceZeroStartTime() will return false. So times will not be adjusted (as with MSE, in particular with live stream, time will *not* start at 0 nor do we want them to)

Comment on attachment 8711998 [details] MozReview Request: Bug 1242841 - Make MDSM::mDecodedAudioEndTime zero-based. r=kikuo. https://reviewboard.mozilla.org/r/32397/#review29293

(In reply to Jean-Yves Avenard [:jya] from comment #5) > not for MSE we don't. > > When using MSE, MediaFormatReated::ForceZeroStartTime() will return false. > So times will not be adjusted (as with MSE, in particular with live stream, > time will *not* start at 0 nor do we want them to) I thought it is the other way around. MediaFormatReader::ForceZeroStartTime() returns |!mDemuxer->ShouldComputeStartTime()| and MediaSourceDemuxer::ShouldComputeStartTime() returns false. So MediaFormatReader::ForceZeroStartTime() should return true for MSE, right?

My bad, the value I put was reverted, but what I describe is still the case. ForceZeroStartTime is true, means it starts at 0 no matter the time of the first sample returned. That is, the time of the first sample is not to be used to determine the reference time frame. So if ForceZeroStartTime was false, every time will be adjusted according to the first sample's time so the first sample will appear as 0 no matter what. But with MSE this is not the case, so the MDSM can see negative timestamp (even though per spec we should drop those frames)

Negative timestamps are fine since we take the greater value when updating mDecodedAudioEndTime in OnAudioDecoded(). (mDecodedAudioEndTime = std::max(audio->GetEndTime(), mDecodedAudioEndTime)) Negative timestamps are ignored in that sense. So it still makes sense to initialize mDecodedAudioEndTime with 0 instead of -1.