Closed Bug 1179110 Opened 4 years ago Closed 4 years ago

twit.tv mp4 video would not autostart

Categories

(Core :: Audio/Video, defect)

41 Branch
Unspecified
Windows 7
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla42
Tracking Status
firefox40 --- unaffected
firefox41 + verified
firefox42 + verified

People

(Reporter: alice0775, Assigned: jya)

References

Details

(Keywords: regression, reproducible)

Attachments

(3 files)

[Tracking Requested - why for this release]:

Reproduced : always

Steps to reproduce:
1. Open direct link of mp4, http://www.podtrac.com/pts/redirect.mp4/twit.cachefly.net/video/sn/sn0512/sn0512_h264m_864x480_500.mp4
  (or original link https://twit.tv/shows/security-now/episodes/513?autostart=true )
2. Wait for 10-20 sec (depended on network speed), the video should auto start

Actual results:
video would not autostart


Expected results:
The video should auto start after for a while(10-20 sec)
(Although Firefox stutter badly. due to Bug 1178063)


Regression window:
m-c
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1fb3ebf68777&tochange=efe86609e776

m-i
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=ce11a9ce7600&tochange=a5eb0b1fcf39

Regressed by: Bug 1172264
Flags: needinfo?(bobbyholley)
Attached file media logging
set NSPR_LOG_MODULES=timestamp,MediaDecoder:5,MediaSource:5,MediaPromise:5,MP4Demuxer:5,nsMediaElement:5,nsMediaElementEvents:5

wait about 50-60sec, but the video did not start.and then I quit browser.
Thanks for the awesome bug report Alice!
Assignee: nobody → bobbyholley
Flags: needinfo?(bobbyholley)
This allows us to properly support videos with a negative start time, which
is happening here.
Attachment #8628553 - Flags: review?(jyavenard)
Comment on attachment 8628553 [details] [diff] [review]
Part 1 - Use a Maybe<> to store start time, rather than using -1 as a sentinel. v1

Review of attachment 8628553 [details] [diff] [review]:
-----------------------------------------------------------------

Excellent, this had been on my todo list for a while... (bug 1131305)
Attachment #8628553 - Flags: review?(jyavenard) → review+
Duplicate of this bug: 1131305
Comment on attachment 8628554 [details] [diff] [review]
Part 2 - Fix ComputePlaybackRate. v1

Review of attachment 8628554 [details] [diff] [review]:
-----------------------------------------------------------------

Doh!
Attachment #8628554 - Flags: review?(jwwang) → review+
this needs to be uplifted to 41.
Comment on attachment 8628554 [details] [diff] [review]
Part 2 - Fix ComputePlaybackRate. v1

Request applies to both patches in the bug. Part 2 fixes the specific regression in this bug, but Part 1 is necessary to make the testcase run properly on my machine, so I think we should uplift it too.

Approval Request Comment
[Feature/regressing bug #]: bug 1172264
[User impact if declined]: Playback issues
[Describe test coverage new/current, TreeHerder]: None
[Risks and why]: Low risk.
[String/UUID change made/needed]: None
Attachment #8628554 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/e4aa3b58f6a6
https://hg.mozilla.org/mozilla-central/rev/be64b22e2d97
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
WOW!
This also fixes Bug 1178063, see Bug 1178063 Comment 8.
Can you uplift this to 40beta and 38ESR?
I don't believe this will be possible. There's a lot that happened in the background in 41. Uplifting to 40 would be significant effort (and risk). 

The handling of negative start time may be enough though.
(In reply to Jean-Yves Avenard [:jya] from comment #14)
> The handling of negative start time may be enough though.

Yes, that part could be backportable, though it would take a bit of work.
(To be clear, I'm not working on media stuff anymore, so I don't have the cycles to do this backport myself)
Blocks: 1178063
jya, do you have the bandwidth to uplift part 2?
Flags: needinfo?(jyavenard)
I can once approved... The bug is only in 41 though...
(In reply to Jean-Yves Avenard [:jya] from comment #18)
> I can once approved... The bug is only in 41 though...

Yes, but bug 1178063 goes further back, and Alice says this push fixed that - presumably because of the negative start time handling.
Tracking for 41, 42 because affected. Please keep posted on potential uplift to 41 Aurora.
Comment on attachment 8628554 [details] [diff] [review]
Part 2 - Fix ComputePlaybackRate. v1

Approving for uplift to Aurora based on my discussion with Sylvestre.
Attachment #8628554 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Requesting QE team to verify this fix.
Flags: qe-verify+
Blocks: 1183888
Considering the lack of traction on bug 1183888 I think we may need to back this out.
(In reply to Eric Rahm [:erahm] from comment #24)
> Considering the lack of traction on bug 1183888 I think we may need to back
> this out.

That would be hard and pretty silly. r=me on a patch to replace the NS_ENSURE with an |if|.

(Sorry for not doing it myself - I'm not working on media anymore, and am about to head out on PTO).
Assignee: bobbyholley → jyavenard
Flags: needinfo?(jyavenard)
Reproduced with 41.0a2 from 2015-06-30, under Windows 7 64-bit.
Verified fixed with 41.0b2 (Build ID: 20150817163452) and latest Developer Edition 42 (from 2015-08-17), under Windows 7 64-bit and Windows 10 32-bit; confirming that bug 1178063 is fixed too, as Alice also mentioned in comment 13.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.