Closed Bug 1093399 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: Audio/Video, defect)

x86_64
All
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla36
Tracking Status
firefox34 --- unaffected
firefox35 + fixed
firefox36 + verified
b2g-v2.1 --- unaffected
b2g-v2.2 --- fixed

People

(Reporter: roc, Assigned: roc)

References

Details

(Keywords: regression)

Attachments

(1 file)

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.
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.
I wonder what regressed this, but it's difficult to tell by looking at the code. Alice, would you be able to find out?
Flags: needinfo?(alice0775)
[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
Blocks: 883731
Flags: needinfo?(alice0775)
Keywords: regression
OS: Linux → All
JWWang, any suggestions for writing a test here? I don't have any good ideas.
Flags: needinfo?(jwwang)
Attachment #8516379 - Flags: review?(cpearce) → review+
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.
Flags: needinfo?(jwwang)
(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.
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.
Don't forget
  if (timeToDownload > timeToPlay) {
above.

There's also the reliability checks:
if (!stats.mDownloadRateReliable || !stats.mPlaybackRateReliable)
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.
https://hg.mozilla.org/mozilla-central/rev/3dc4a906df95
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
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
Attachment #8516379 - Flags: approval-mozilla-aurora?
Attachment #8516379 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
QA Whiteboard: [good first verify]
[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
Marking as verified based on comment 16.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.