Closed Bug 1371202 Opened 2 years ago Closed 2 years ago

Don't release decoders in CompletedState if looping is true

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: jwwang, Assigned: jwwang)

References

(Depends on 1 open bug)

Details

Attachments

(2 files)

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.
See Also: → 654787
Assignee: nobody → jwwang
Priority: -- → P3
Attachment #8875624 - Flags: review?(jyavenard)
Attachment #8875625 - Flags: review?(jyavenard)
This relies on a bug I can't see here (using MediaDecoderInit).. Can you set the dependency in bugzilla please?
Flags: needinfo?(jwwang)
Comment on attachment 8875624 [details]
Bug 1371202. P1 - plumb the 'loop' attribute into MDSM.

https://reviewboard.mozilla.org/r/147048/#review151422
Attachment #8875624 - Flags: review?(jyavenard) → review+
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 ?
Attachment #8875625 - Flags: review?(jyavenard) → review+
Depends on: 1371200
Flags: needinfo?(jwwang)
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 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?
Thanks for the review!
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)
Pushed by jwwang@mozilla.com:
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
https://hg.mozilla.org/mozilla-central/rev/9305945e748c
https://hg.mozilla.org/mozilla-central/rev/52b2148d954c
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.