1230141 - currentTime starts ticking although only segments for a moment far into the future are loaded

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P2)

Description

[svetlin.mladenov]

Attachment: ff-starts-ticking-after-append.zip

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Firefox/45.0
Build ID: 20151202030210

Steps to reproduce:

1. Download the attached .zip file and extract
2. Start an http server in the extracted directory (ff-starts-ticking-after-append/)
3. Open the index.html page using FF
   This page will crate a MediaSource and two buffer for audio and video and load a mp4 init segment and a short video/audio segment into the corresponding buffers. Each audio and video segment begins far into the future (1905584s). The .currentTime property is not set.


Actual results:

Under FF 44 and 45 the video playback starts. The 'playing' event is fired and currentTime starts ticking form 0 onwards (look at the controls being updated) but video stays frozen (the only loaded segments start at 1905584s!)


Expected results:

If the page is loaded using FF 42, 43 or Chrome playback does not begin, and the video element's currentTime stays at 0. This is the expected result because the loaded segments are for a point far into the future - 1905584s.

If the audio buffer is turned off (by editing index.html and setting the global audio variable to false) the behavior is as expected and this bug is not evident.

Comment 1

[svetlin.mladenov]

I forgot to mention that this bug was noticed soon after https://bugzilla.mozilla.org/show_bug.cgi?id=1188887 was fixed.

Comment 2

[Jean-Yves Avenard [:jya]]

Bug 1188887 has nothing to do with this (and I checked by reversing the two commits)

JW it appears to be related to the audio sink deciding to play content. 

Currently with MSE (and I'm in the opinion that it's not the right thing to do, but it's been like that since before I started at Mozilla), even if we don't have content at position 0, we do return the first audio and video frame to the MDSM as part of the decode first frame logic. This is to allow a frame to appear, even though the media won't play. 

For some reason something has changed recently, so that it actually is trying to play. 

Here the audio and video sample have greatly different timestamp

Comment 3

[JW Wang [:jwwang] [:jw_wang]]

some logs:
Decoder=7fc23b5c6840 OnNotDecoded (aType=0, aReason=2)
Decoder=7fc23b5c6840 EnsureAudioDecodeTaskQueued isDecoding=1 status=waiting
Decoder=7fc23b5c6840 EnsureAudioDecodeTaskQueued isDecoding=1 status=idle <- somehow the waiting promise is resolved and audio decoding starts
Decoder=7fc23b5c6840 OnAudioDecoded [1905582022000,1905582064666] disc=1 <- this is the 1st audio sample received by MDSM. The start time is quite large
Decoder=7fc23b5c6840 OnAudioDecoded [1905582065000,1905582107666] disc=0

The problem is why the waiting promise is resolved before the missing segments are appended.

Comment 4

[Jean-Yves Avenard [:jya]]

For the reason I listed above.
We want to draw a frame (whichever it is) (because this is what Chrome does I guess).

I will have a proper fix of course, but in the mean type, need to find out what caused the regression as the fix may introduce other regressions for sites or tests that rely on that behaviour (in particular we emit loadeddata immediately).

(In reply to JW Wang [:jwwang] from comment #3)
> some logs:
> Decoder=7fc23b5c6840 OnAudioDecoded [1905582022000,1905582064666] disc=1 <-
> this is the 1st audio sample received by MDSM. The start time is quite large
> Decoder=7fc23b5c6840 OnAudioDecoded [1905582065000,1905582107666] disc=0

Yes, but unlike with other readers, when using MSE the start time isn't adjusted. So it shouldn't be played !

Comment 5

[Ryan VanderMeulen [:RyanVM]]

[Tracking Requested - why for this release]: Playback regression in Fx44

Regression range: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=ce75fb8d5a8bde7590dfdc3ee2b65b8ca6200c8e&tochange=5e890d7ad1744e38b0d9de03696dfe09631c12ec

Caused by bug 1194918.

Comment 6

[Jean-Yves Avenard [:jya]]

jw: can we revert that change in 44? For 45 we can have the new MSE/readyState which fix all that.

Comment 7

[Kilik Kuo [:kikuo] (inactive)]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #5)
> [Tracking Requested - why for this release]: Playback regression in Fx44
> 
> Regression range:
> https://hg.mozilla.org/integration/mozilla-inbound/
> pushloghtml?fromchange=ce75fb8d5a8bde7590dfdc3ee2b65b8ca6200c8e&tochange=5e89
> 0d7ad1744e38b0d9de03696dfe09631c12ec
> 
> Caused by bug 1194918.

I'm not really sure that this modification make the regression
The playback started as |MediaDecoder::Play| being called in |HTMLMediaElement::CheckAutoplayDataReady|.

MDSM can set a frame to be drawn without starting playback, IMHO the point is why the criterion allows playback to start in this scenario (video frames available, big gap start time).  We've readyState = HAVE_CURRENT_DATA, nextFrameStatus = NEXT_FRAME_AVAILABLE for video.  A enhanced rule may be needed in HTMLMediaElement, but this's just my $0.02.

Bug 1194918 is landed for gecko44, because we want this architecture modification could be a reference pattern for future playback usage (3rd-party data consumer for partner, i.e. encrypted data consumer, renderer)

Comment 8

[JW Wang [:jwwang] [:jw_wang]]

It should be easier to just fix this bug than trying to roll back the changes.

Comment 9

[JW Wang [:jwwang] [:jw_wang]]

I mean to find some simple workarounds before new MSE/readyState is ready.

Comment 10

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

[e-mail], could you please verify this issue is fixed as expected on a latest nightly build? Thanks!

Comment 11

[svetlin.mladenov]

Yes, I confirm that the issue is fixed on the latest Firefox Nightly (46).

However the issue is still present on Beta (44). Will this fix make it to 44 or will Firefox 44 be released with this bug present? Thanks.

Comment 12

[Jean-Yves Avenard [:jya]]

the fix is in 45. It is unclear on the fix being uplifted to 44 at this stage.

Is this something that will break one of your service or it's just a UI issue?

Comment 13

[svetlin.mladenov]

It's not just a UI issue unfortunately.
I was hoping for a fix in 44 but from what I can conclude from all the comments, it would be too much work for you so I guess we can implement a workaround in our code for this issue.

Comment 14

[Liz Henry (:lizzard) (relman/hg->git project)]

Marking fixed in 45 and 46 from comment 12.  Yes, it is too late to get this work into 44 right now as we are quite late in beta. 
But, where was it fixed for 45? Another bug/patch?

Comment 15

[Jean-Yves Avenard [:jya]]

Because the commit fixing it went before central became 45.

It is not fixed in 44 however, not sure why you marked that as fixed.

http://hg.mozilla.org/releases/mozilla-aurora/rev/6b2990b893770edbd35a0347f2023a56cec28aab

maybe we could have just uplifted that commit now that I think of it... it's fairly straight forward... oh well :(

Comment 16

[Ryan VanderMeulen [:RyanVM]]

"fixed" is right above "wontfix" FWIW :)