1093399 - canplaythrough should fire before playback reaches the end of a video, but it doesn't

Status: VERIFIED FIXED

(Core :: Audio/Video, defect)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Steps to reproduce:
1) Visit http://people.mozilla.org/~roc/autoplay.html
2) Wait for video to start playing.
It should start before we reach the end of the video. If the network is reasonably fast, it should start playing almost immediately.
Currently, on trunk, for me, the video doesn't start playing until it's fully downloaded.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This is difficult to write an automated test for since under test conditions, a "canplaythrough" event could easily be delayed until playback has ended due to factors beyond our control.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I wonder what regressed this, but it's difficult to tell by looking at the code. Alice, would you be able to find out?

Comment 4

[Alice0775 White]

[Tracking Requested - why for this release]:

[Tracking Requested - why for this release]:

Regression window(m-i)
Good:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3d9ca37ae70f
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:35.0) Gecko/20100101 Firefox/35.0 ID:20140928182757
Bad:
https://hg.mozilla.org/integration/mozilla-inbound/rev/99b68a13246b
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:35.0) Gecko/20100101 Firefox/35.0 ID:20140929001902
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=3d9ca37ae70f&tochange=99b68a13246b

Regressed by Bug 883731

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Amazing, thanks!!!

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

JWWang, any suggestions for writing a test here? I don't have any good ideas.

Comment 7

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

Are we gonna test if autopaly kicks off after buffering enough?
Can we check in the 'progress' handler to see if playback has started when buffer range is large enough? This test should be immune from the download speed.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to JW Wang [:jwwang] from comment #7)
> Are we gonna test if autopaly kicks off after buffering enough?

We'd like to.

> Can we check in the 'progress' handler to see if playback has started when
> buffer range is large enough? This test should be immune from the download
> speed.

I don't think it is. autoplay kicks in when either a) the estimated download data rate is greater than the estimated playback data rate or b) we estimate that we can download the rest of the video data in less time than it will take to play to the end. Both of those depend on the download rate.

Comment 9

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

http://hg.mozilla.org/mozilla-central/file/a458860efad7/dom/media/MediaDecoder.cpp#l1625

Don't know if I read MediaDecoder::CanPlayThrough correctly.
MediaDecoder::CanPlayThrough return true when:
1. all bytes are downloaded (stats.mTotalBytes == stats.mDownloadPosition)
2. download position is ahead of playback position by some amount (stats.mDownloadPosition > stats.mPlaybackPosition + readAheadMargin)

Since playback position doesn't change, download position will outplay playback position sooner or later.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Don't forget
  if (timeToDownload > timeToPlay) {
above.

There's also the reliability checks:
if (!stats.mDownloadRateReliable || !stats.mPlaybackRateReliable)

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3dc4a906df95

Comment 12

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

How expecting a 'canplaythrough' event when download speed is fast enough? The download speed can be measured by the buffer range and we should get a decent download speed most of the time when running the test. If download is stalled for some reason, we can finish the test silently.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3dc4a906df95

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 8516379 [details] [diff] [review]
fix

Approval Request Comment
[Feature/regressing bug #]: 883731
[User impact if declined]: autoplay videos won't start
[Describe test coverage new/current, TBPL]: no tests specifically for this issue
[Risks and why]: Pretty low risk, we just fire extra notifications, and it restores code we removed in bug 883731
[String/UUID change made/needed]: none

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/46d0ef8ca0da

Comment 16

[Mayur Patil]

[bugday-20150114]
Name: Firefox
Version: 36.0b1
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:36.0) Gecko/20100101 Firefox/36.0
Multiprocess Windows: 0/1


The suggested method worked for me.

Actual result:
Video starts playing before fully loaded.

Expected result
Video starts playing before fully loaded.

status:  Fixed -> Verified

Comment 17

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Marking as verified based on comment 16.