Intermittent dom/media/test/test_background_video_suspend.html | Assertion count 115 is greater than expected range 0-0 assertions from "ASSERTION: Clock should go forwards"

RESOLVED FIXED in Firefox 52

Status

()

Core
Audio/Video: Playback
P5
normal
Rank:
55
RESOLVED FIXED
a year ago
4 months ago

People

(Reporter: Treeherder Bug Filer, Assigned: jwwang)

Tracking

(Blocks: 1 bug, {intermittent-failure})

unspecified
mozilla52
intermittent-failure
Points:
---

Firefox Tracking Flags

(firefox50 unaffected, firefox51 unaffected, firefox52 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

Priority: -- → P5

Updated

a year ago
Rank: 55
(Assignee)

Updated

a year ago
Assignee: nobody → jwwang
Component: Audio/Video → Audio/Video: Playback
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Attachment #8800486 - Flags: review?(kaku)
(Assignee)

Updated

a year ago
Blocks: 1276556

Comment 2

a year ago
mozreview-review
Comment on attachment 8800486 [details]
Bug 1306551 - don't update playback position for video-only seek when seek is completed.

https://reviewboard.mozilla.org/r/85398/#review84028

::: dom/media/MediaDecoderStateMachine.cpp:967
(Diff revision 1)
> +      // Don't update playback position for video-only seek.
> +      // Otherwise we might have |newCurrentTime > mMediaSink->GetPosition()|
> +      // and fail the assertion in GetClock() since we didn't stop MediaSink.

Does this happen due to fast seek which might seek to somewhere after the seek target?
(Assignee)

Comment 3

a year ago
mozreview-review-reply
Comment on attachment 8800486 [details]
Bug 1306551 - don't update playback position for video-only seek when seek is completed.

https://reviewboard.mozilla.org/r/85398/#review84028

> Does this happen due to fast seek which might seek to somewhere after the seek target?

Since we pass GetMediaTime() to MediaSink::Start(), the clock time returned by the media sink is always >= GetMediaTime(). However, we don't restart the media sink for the video-only seek, the invariant doesn't necessarily hold. We could have newCurrentTime > mMediaSink->GetPosition() when seek is completed.

Comment 4

a year ago
mozreview-review-reply
Comment on attachment 8800486 [details]
Bug 1306551 - don't update playback position for video-only seek when seek is completed.

https://reviewboard.mozilla.org/r/85398/#review84028

> Since we pass GetMediaTime() to MediaSink::Start(), the clock time returned by the media sink is always >= GetMediaTime(). However, we don't restart the media sink for the video-only seek, the invariant doesn't necessarily hold. We could have newCurrentTime > mMediaSink->GetPosition() when seek is completed.

But, you see, if we trigger the video-only-seek at media time _m_, clock time _c_ ( _m_ <= _c_) and the media seek keeps going. 
After time _t_, the seek is resolved and the media time becomes _m_ + _t_ and the clock time becomes _c_ + _t_. 
The _newCurrentTime_ could be set to:
(1) the original seek-target, which is smaller than the current clock.
(2) the start time of the 1st audio sample in the audio queue, which follows the current clock since we do not do audio seek.
(3) the start time of the 1st video sample in the video queue, which might be larger than the current clock if fast seek was applied.

Not sure if the above analysis is right or not...

Whatever, I think it's just right to not update the media time here if we did not stop the playback.

Comment 5

a year ago
mozreview-review
Comment on attachment 8800486 [details]
Bug 1306551 - don't update playback position for video-only seek when seek is completed.

https://reviewboard.mozilla.org/r/85398/#review84074
Attachment #8800486 - Flags: review?(kaku) → review+
(Assignee)

Comment 6

a year ago
Thanks for the review!

Comment 7

a year ago
Pushed by jwwang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4e2c6be60379
don't update playback position for video-only seek when seek is completed. r=kaku

Comment 8

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/4e2c6be60379
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
status-firefox50: --- → unaffected
status-firefox51: --- → unaffected

Comment 9

7 months ago
1 failures in 822 pushes (0.001 failures/push) were associated with this bug in the last 7 days.   

Repository breakdown:
* mozilla-esr52: 1

Platform breakdown:
* windows7-32: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1306551&startday=2017-07-17&endday=2017-07-23&tree=all

Comment 10

4 months ago
1 failures in 947 pushes (0.001 failures/push) were associated with this bug in the last 7 days.    

Repository breakdown:
* mozilla-esr52: 1

Platform breakdown:
* windows7-32: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1306551&startday=2017-10-09&endday=2017-10-15&tree=all

Comment 11

4 months ago
1 failures in 912 pushes (0.001 failures/push) were associated with this bug in the last 7 days.    

Repository breakdown:
* mozilla-esr52: 1

Platform breakdown:
* windows7-32: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1306551&startday=2017-10-23&endday=2017-10-29&tree=all
You need to log in before you can comment on or make changes to this bug.