Closed Bug 1346116 Opened 5 years ago Closed 5 years ago

Consider a video element is in tree or not to suspend its video decoder

Categories

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

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: kaku, Assigned: kaku)

References

(Blocks 2 open bugs)

Details

Attachments

(3 files)

In Bug 1295921, we tried very hard to resume a video decoder synchronously, but we are not able to achieve it at this moment. We have split Bug 1295921 into bug 1345403 and bug 1345404; the former one is for tracking a video element as tainted and never suspend it again, the later one is for synchronous decoder-resume. We're going to land bug 1345403 at this moment but leave the bug 1345404 until bug 1338218 is landed.

Without, synchronous decoder-resume, we might 

The Telemetry data (https://mzl.la/2ncoeIj) shows that more that 70% of video elements, which are used as the source of drawImage() API, are not in-tree.

So, this bug is going to prevent those video elements which are not in-tree form suspended, so that we could alleviate the pain of not able to resume video decoder synchronously.
Assignee: nobody → kaku
Status: NEW → ASSIGNED
Depends on: 1337301
Depends on: 1346498
Comment on attachment 8846407 [details]
Bug 1346116 part 1 - initialize MediaDecoder::mIsDocumentVisible and MediaDecoder::mIsElementVisible at HTMLMediaElement::FinishDecoderSetup();

https://reviewboard.mozilla.org/r/119452/#review121412

::: dom/html/HTMLMediaElement.cpp:4717
(Diff revision 2)
>      mDecoder->SetMinimizePrerollUntilPlaybackStarts();
>    }
>    // Notify the decoder of suspend taint.
>    mDecoder->SetSuspendTaint(mHasSuspendTaint);
> +  // Notify the decoder of the initial activity status.
> +  this->SetActiviyChangesToDecoder();

Remove "this->".
Attachment #8846407 - Flags: review?(jwwang) → review+
Comment on attachment 8846409 [details]
Bug 1346116 part 3 - a test case for not suspend not-in-tree videos;

https://reviewboard.mozilla.org/r/119456/#review121416
Attachment #8846409 - Flags: review?(jwwang) → review+
Comment on attachment 8846408 [details]
Bug 1346116 part 2 - consider a video is in-tree or not in the suspend-video-decoding policy;

https://reviewboard.mozilla.org/r/119454/#review121418
Attachment #8846408 - Flags: review?(jwwang) → review+
Blocks: 1346705
Try looks good, thanks for review!
Keywords: checkin-needed
Pushed by ihsiao@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/096a4818b8c7
part 1 - initialize MediaDecoder::mIsDocumentVisible and MediaDecoder::mIsElementVisible at HTMLMediaElement::FinishDecoderSetup(); r=jwwang
https://hg.mozilla.org/integration/autoland/rev/f7ca43d48a42
part 2 - consider a video is in-tree or not in the suspend-video-decoding policy; r=jwwang
https://hg.mozilla.org/integration/autoland/rev/c4688144319f
part 3 - a test case for not suspend not-in-tree videos; r=jwwang
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/096a4818b8c7
https://hg.mozilla.org/mozilla-central/rev/f7ca43d48a42
https://hg.mozilla.org/mozilla-central/rev/c4688144319f
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.