1371202 - Don't release decoders in CompletedState if looping is true

Status: RESOLVED FIXED

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

Description

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

When looping is true, playback will start again after reaching the end. We can reduce the latency by not releasing the decoders which will be slightly helpful for seamless audio playback.

Comment 1

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

Attachment: Bug 1371202. P1 - plumb the 'loop' attribute into MDSM.

Review commit: https://reviewboard.mozilla.org/r/147048/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/147048/

Comment 2

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

Attachment: Bug 1371202. P2 - don't release decoders if we are looping since we will need them soon.

Review commit: https://reviewboard.mozilla.org/r/147050/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/147050/

Comment 3

[Jean-Yves Avenard [:jya]]

This relies on a bug I can't see here (using MediaDecoderInit).. Can you set the dependency in bugzilla please?

Comment 4

[Jean-Yves Avenard [:jya]]

Comment on attachment 8875624 [details]
Bug 1371202. P1 - plumb the 'loop' attribute into MDSM.

https://reviewboard.mozilla.org/r/147048/#review151422

Comment 5

[Jean-Yves Avenard [:jya]]

Comment on attachment 8875625 [details]
Bug 1371202. P2 - don't release decoders if we are looping since we will need them soon.

https://reviewboard.mozilla.org/r/147050/#review151432

::: dom/media/MediaDecoderStateMachine.cpp:1912
(Diff revision 1)
>    void Enter()
>    {
>      // TODO : use more approriate way to decide whether need to release
>      // resource in bug1367983.
>  #ifndef MOZ_WIDGET_ANDROID
> -    // We've decoded all samples. We don't need decoders anymore.
> +    if (!mMaster->mLooping) {

shouldn't we only do so if looping is true and we've reached eos ?

Comment 6

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

Comment on attachment 8875625 [details]
Bug 1371202. P2 - don't release decoders if we are looping since we will need them soon.

https://reviewboard.mozilla.org/r/147050/#review151432

> shouldn't we only do so if looping is true and we've reached eos ?

CompletedState implies EOS. We transition to this state only after decoding all samples.

Comment 7

[Jean-Yves Avenard [:jya]]

Comment on attachment 8875625 [details]
Bug 1371202. P2 - don't release decoders if we are looping since we will need them soon.

https://reviewboard.mozilla.org/r/147050/#review151432

> CompletedState implies EOS. We transition to this state only after decoding all samples.

if so , why the check right after to see if we have a next frame?

Comment 8

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

Thanks for the review!

Comment 9

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 0e39a5c82024 -d 1ce78807187a: rebasing 401236:0e39a5c82024 "Bug 1371202. P1 - plumb the 'loop' attribute into MDSM. r=jya"
merging dom/html/HTMLMediaElement.cpp
merging dom/media/MediaDecoderStateMachine.cpp
warning: conflicts while merging dom/html/HTMLMediaElement.cpp! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 10

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

Comment on attachment 8875624 [details]
Bug 1371202. P1 - plumb the 'loop' attribute into MDSM.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/147048/diff/1-2/

Comment 11

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

Comment on attachment 8875625 [details]
Bug 1371202. P2 - don't release decoders if we are looping since we will need them soon.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/147050/diff/1-2/

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9305945e748c
P1 - plumb the 'loop' attribute into MDSM. r=jya
https://hg.mozilla.org/integration/autoland/rev/52b2148d954c
P2 - don't release decoders if we are looping since we will need them soon. r=jya

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9305945e748c
https://hg.mozilla.org/mozilla-central/rev/52b2148d954c