1283489 - "ASSERTION: Clock should go forwards" changing playbackRate

Status: RESOLVED FIXED

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

Description

[Jesse Ruderman]

Attachment: testcase

Comment 1

[Jesse Ruderman]

Attachment: stack

Comment 2

[Blake Wu [:bwu][:blakewu]]

Farmer, 
This bug should be suitable to you. 
Please have a look.
Thanks!

Comment 3

[Farmer Tseng[:fatseng](inactive)]

I will check it.

Comment 4

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Mass change P2 -> P3

Comment 5

[Farmer Tseng[:fatseng](inactive)]

http://searchfox.org/mozilla-central/source/dom/media/mediasink/AudioSinkWrapper.cpp#125
mParams.mPlaybackRate should be updated after call GetVideoPosition().

Comment 6

[Farmer Tseng[:fatseng](inactive)]

I will prepare patch later.

Comment 7

[Farmer Tseng[:fatseng](inactive)]

Attachment: mParams.mPlaybackRate should be updated after store mPlayDuration

Here is try server result.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=796bd18fa38e

Comment 8

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

Comment on attachment 8769527 [details] [diff] [review]
mParams.mPlaybackRate should be updated after store mPlayDuration

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

I can't see how that will make a difference. Can you elaborate it?

Comment 9

[Farmer Tseng[:fatseng](inactive)]

http://searchfox.org/mozilla-central/source/dom/media/mediasink/AudioSinkWrapper.cpp#133
GetVideoPosition(now) generate timestamp and store in mPlayDuration;

http://searchfox.org/mozilla-central/source/dom/media/mediasink/AudioSinkWrapper.cpp#67
but GetVideoPosition() use mParams.mPlaybackRate to calculate it.
Therefore, we should use old playback rate to generate it.

Comment 10

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

Comment on attachment 8769527 [details] [diff] [review]
mParams.mPlaybackRate should be updated after store mPlayDuration

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

Please add comments why mParams.mPlaybackRate should be updated after the call to GetVideoPosition().

Comment 11

[Farmer Tseng[:fatseng](inactive)]

Attachment: 0001-Bug-1283489-mParams.mPlaybackRate-should-be-updated-.patch

Carry on Comment 10

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ed9cd1e7f95a
mParams.mPlaybackRate should be updated after call GetVideoPosition(). r=jwwang

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ed9cd1e7f95a