1057199 - [B2G debug] Intermittent test_playback.html | ASSERTION: Clock should go forwards if the playback rate is > 0.: 'mCurrentFrameTime <= clock_time || mPlaybackRate <= 0', file ../../../gecko/content/media/MediaDecoderStateMachine.cpp, line 2585

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

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

https://tbpl.mozilla.org/php/getParsedLog.php?id=46476959&tree=Try&full=1#error0

Found this issue while running media tests on B2G emulator debug build.

INFO -  1248 INFO [Child 738] ###!!! ASSERTION: Clock should go forwards if the playback rate is > 0.: 'mCurrentFrameTime <= clock_time || mPlaybackRate <= 0', file ../../../gecko/content/media/MediaDecoderStateMachine.cpp, line 2585

Comment 1

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

Attachment: 1057199_fix_opensl_stream_get_position-v1.patch

Since |msec| and |mixer_latency| are both unsigned integers, |compensated_position = samplerate * (msec - mixer_latency) / 1000| will always be positive even if |compensated_position| is signed.

1. When |msec| is smaller than |mixer_latency|, |compensated_position| will be a large number which will be clamped to |maximum_position|.
2. Later when |msec| becomes larger than |mixer_latency|, |compensated_position| will be smaller than |maximum_position| and return a position which seems to go backward in time and causes the assertion.

Comment 2

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

Try: https://tbpl.mozilla.org/?tree=Try&rev=67a69ceb7138
The status of B2G ICS Emulator Opt is bad. Most of them are bug 1059120. So I guess my patch is fine then.

Comment 3

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/566bbeb07fb3

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/566bbeb07fb3

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Do we need to backport this anywhere?

Comment 6

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

Let me check Aurora and Beta and request uplift if necessary.

Comment 7

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

Since Beta can't apply this patch cleanly, I will uplift to Aurora only.

Comment 8

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

Comment on attachment 8479732 [details] [diff] [review]
1057199_fix_opensl_stream_get_position-v1.patch

Approval Request Comment
[Feature/regressing bug #]:unknow
[User impact if declined]:this patch fixes test case failures which affect
our test coverage which in the end impacts the software quality
[Describe test coverage new/current, TBPL]:
Try: https://tbpl.mozilla.org/?tree=Try&rev=fded56ef1b17
[Risks and why]: Low. The change is simple and has been tested on central.
[String/UUID change made/needed]: none

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/216944609df3

Comment 10

[Benjamin Chen [:bechen]]

Attachment: bug-1057199.b2g32v2_0.patch

Comment 11

[Benjamin Chen [:bechen]]

Comment on attachment 8490538 [details] [diff] [review]
bug-1057199.b2g32v2_0.patch

Want to uplift this patch to B2G 2.0 .

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 1056526
User impact if declined: When starting video playback, Video frame was stopped for a moment.
Testing completed: manual test. bug 1056526 comment 24, bug 1056526 comment 25
Risk to taking this patch (and alternatives if risky): Low. The change is simple and has been tested on central.
String or UUID changes made by this patch: none

Comment 12

[StevenLee[:slee]]

[Blocking Requested - why for this release]:
bug 1056526 needs this patch. can someone land this patch to 2.0?

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/070a89a40abd

Comment 14

[Kai-Zhen Li [:seinlin][:kli]]

http://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/070a89a40abd