1401909 - [ShutdownDecoder] Turn on preference on desktop version again.

Status: RESOLVED FIXED

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

Description

[Tzuhao Kuo [:kaku]]

The Shield study (bug 1380584) result is positive, so let's enable this feature.

Comment 1

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1401909 - turn on preference of ShutdownDecoder feature on desktop version release and beta channel;

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

Comment 2

[Blake Wu [:bwu][:blakewu]]

Comment on attachment 8910680 [details]
Bug 1401909 - turn on preference of ShutdownDecoder feature on desktop version release and beta channel;

https://reviewboard.mozilla.org/r/182134/#review188158

Bummer. My bad. I thought we should have enabled it on 56.

Comment 3

[Tzuhao Kuo [:kaku]]

(In reply to Blake Wu [:bwu][:blakewu] from comment #2)
> Comment on attachment 8910680 [details]
> Bug 1401909 - turn on preference of ShutdownDecoder feature on desktop
> version release and beta channel;
> 
> https://reviewboard.mozilla.org/r/182134/#review188158
> 
> Bummer. My bad. I thought we should have enabled it on 56.

Uplift?

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/27953b6cbc9b
turn on preference of ShutdownDecoder feature on desktop version release and beta channel; r=bwu

Comment 5

[Blake Wu [:bwu][:blakewu]]

(In reply to Tzuhao Kuo [:kaku] from comment #3)
> (In reply to Blake Wu [:bwu][:blakewu] from comment #2)
> > Comment on attachment 8910680 [details]
> > Bug 1401909 - turn on preference of ShutdownDecoder feature on desktop
> > version release and beta channel;
> > 
> > https://reviewboard.mozilla.org/r/182134/#review188158
> > 
> > Bummer. My bad. I thought we should have enabled it on 56.
> 
> Uplift?
Yes. Let's uplift to 57. Although this one missed the 56 train, I don't think it is a good idea to uplift 56.

Comment 6

[Tzuhao Kuo [:kaku]]

Comment on attachment 8910680 [details]
Bug 1401909 - turn on preference of ShutdownDecoder feature on desktop version release and beta channel;

Approval Request Comment
[Feature/Bug causing the regression]:not really a regression, but waiting for Shield study result, bug 1380584.
[User impact if declined]: users are not able to benefits from Shutdown Decoder at FF57.
[Is this code covered by automated tests?]: yes.
[Has the fix been verified in Nightly?]: this is not a fix, however, we just enable the preference and this feature has been in Nightly from FF55.
[Needs manual test from QE? If yes, steps to reproduce]: no.
[List of other uplifts needed for the feature/fix]: no.
[Is the change risky?]: the risk should be low. 
[Why is the change risky/not risky?]: because the feature is enabled from FF55 and covered with auto tests. The feature also got pre-release signoff reports from Softvision.
[String changes made/needed]: we might need to include this feature to FF57 release note.

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/27953b6cbc9b

Comment 8

[Sylvestre Ledru [:Sylvestre]]

Please provide a better wording for the release notes :)

Release Note Request (optional, but appreciated)
[Why is this notable]: 
[Affects Firefox for Android]: No?
[Suggested wording]: Video decoding is disabled when not visible, providing both performance and limited battery usage
[Links (documentation, blog post, etc)]:

Comment 9

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8910680 [details]
Bug 1401909 - turn on preference of ShutdownDecoder feature on desktop version release and beta channel;

Good improvement, seems under control, taking it!
Should be in 57b4 (gtb tomorrow Thursday)

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/01cad263a605

Comment 11

[Andrei Vaida [:avaida]]

(In reply to Tzuhao Kuo [:kaku] (PTO 9/27) from comment #6)
> [Is this code covered by automated tests?]: yes.
> [Has the fix been verified in Nightly?]: this is not a fix, however, we just
> enable the preference and this feature has been in Nightly from FF55.
> [Needs manual test from QE? If yes, steps to reproduce]: no.

Setting qe-verify- based on Tzuhao Kuo's assessment on manual testing needs and the fact that this fix has automated coverage.

Comment 12

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

This was added to 58 beta release notes. I am not sure if this should go into 57 release notes or not. Can someone please confirm and needinfo me if that is true. Thanks!

Comment 13

[Blake Wu [:bwu][:blakewu]]

Ritu,
We should put this into our release notes. Shutdown video decoders when users cannot see them. It can save CPU/GPU resource to save power and let others have more CPU/GPU resource to do their jobs which could improve the performance of foreground tab. In summary, it can reduce memory, resource, and power usage.

Comment 14

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Thanks Blake for your feedback. This relnote was reworded based on your suggestion on Nov 21st.