1079695 - Improve audio clock calculation in MediaDecoderStateMachine

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

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

As bug 1038091 landed which ensures the position returned by AudioStream will not go backward, we can do some refactoring to:

1. remove workarounds at [1] since the clock time will never go backward.
2. improve const-correctness by removing writes to |mPlayDuration| and |mPlayStartTime| at [2].


[1] http://hg.mozilla.org/mozilla-central/file/25a98bac9264/content/media/MediaDecoderStateMachine.cpp#l2735
[2] http://hg.mozilla.org/mozilla-central/file/25a98bac9264/content/media/MediaDecoderStateMachine.cpp#l2607

Comment 1

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

Attachment: 1079695_part1_ensure_mono_increasing_of_audiosink_position.patch

Ensure position returned by AudioSink is mono-increasing. This prevents the state machine from switching back and forth between audio and system clocks when AudioSink::GetPosition() returns -1.

Comment 2

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

Attachment: 1079695_part2_remove_audioclock_workaround_in_MediaDecoderStateMachine.patch

Remove workaround that keeps re-synchronizing audio clock in MediaDecoderStateMachine::GetClock due to audio clock drifting.

Comment 3

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

Attachment: 1079695_part3_fix_const_correstness_of_MediaDecoderStateMachine.patch

Fix const-correctness.

Comment 4

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

Try: https://tbpl.mozilla.org/?tree=Try&rev=bd3052450f85
Most green.

The oranges in OS X 10.6 Debug are known issues.

Comment 5

[Matthew Gregan [:kinetik]]

Comment on attachment 8514774 [details] [diff] [review]
1079695_part2_remove_audioclock_workaround_in_MediaDecoderStateMachine.patch

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

::: dom/media/MediaDecoderStateMachine.cpp
@@ +2625,2 @@
>      }
> +    // FIXME: This asseriton should also apply the case of decoding to a stream.

Typo: assertion

Comment 6

[Matthew Gregan [:kinetik]]

Comment on attachment 8514774 [details] [diff] [review]
1079695_part2_remove_audioclock_workaround_in_MediaDecoderStateMachine.patch

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

::: dom/media/MediaDecoderStateMachine.cpp
@@ +2625,2 @@
>      }
> +    // FIXME: This asseriton should also apply the case of decoding to a stream.

Typo: assertion

Comment 7

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

Attachment: 1079695_part2_remove_audioclock_workaround_in_MediaDecoderStateMachine-v2.patch

fix typo.

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/79f524daf501
https://hg.mozilla.org/integration/mozilla-inbound/rev/1ba570e8ca42
https://hg.mozilla.org/integration/mozilla-inbound/rev/67ca0ab022e8

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/79f524daf501
https://hg.mozilla.org/mozilla-central/rev/1ba570e8ca42
https://hg.mozilla.org/mozilla-central/rev/67ca0ab022e8