1383610 - [ShutdownDecoder] Turn off preference on FF55 desktop version.

Status: RESOLVED FIXED

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

Description

[Tzuhao Kuo [:kaku]]

Attachment: Mozilla Mail - Update: Media Shutdown Decoder for Firefox:Fennec 55 Release.pdf

According to PM and EM's decision, let's turn off the feature on FF55 desktop version and keep the Feneec ON.

Comment 1

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1383610 - turn off preference of ShutdownDecoder feature on desktop version release and beta channel but keep the Fennec on;

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

Comment 2

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

Comment on attachment 8889270 [details]
Bug 1383610 - turn off preference of ShutdownDecoder feature on desktop version release and beta channel but keep the Fennec on;

https://reviewboard.mozilla.org/r/160328/#review166114

Per bug 1380584 comment 4.

Comment 3

[Tzuhao Kuo [:kaku]]

Thanks!

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c48bb95530b1
turn off preference on desktop version release channel but keep the Fennec on; r=bwu

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c48bb95530b1

Comment 6

[Jean-Yves Avenard [:jya]]

This is a very unfortunate decision... How is that for desktop?

as far as I can tell, mobile PC/Mac device will now be affected.

Why this change when it will have serious consequence on battery life of affected device?

We should back this out IMHO

Comment 7

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

(In reply to Jean-Yves Avenard [:jya] from comment #6)
> as far as I can tell, mobile PC/Mac device will now be affected.

In this context desktop means Windows, Mac, Linux, etc.

Comment 8

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

(In reply to Jean-Yves Avenard [:jya] from comment #6)
> This is a very unfortunate decision... How is that for desktop?
> 
> as far as I can tell, mobile PC/Mac device will now be affected.
> 
> Why this change when it will have serious consequence on battery life of
> affected device?
> 
> We should back this out IMHO
I love this feature and really like to back out or re-enable it soon. Per the discussions in the mail, we just need to see some shield studies to make sure if there is any thing we should be aware of. Besides, we have a great enhancement on bug 1274919 on the way.

Comment 9

[Bobby Chien]

Blake and Jean-Yves. Please help to work on this. This is the dependence of starting the shield study. Many thanks.

Comment 10

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

Bobby, 
We already landed the patch to turn the pref off in comment 5.

Comment 11

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Please make sure your commit message specific enough so it will make sense to others. Thanks.

Comment 12

[Bobby Chien]

Kaku and Blake, 

As Julien's response in intent-to-ship mail, please help to make sure this change apply to mozilla-beta. So we could ride to release. Thanks.

Comment 13

[Julien Cristau [:jcristau]]

FWIW the patch looks wrong to me, AFAIK we don't have a RELEASE macro defined anywhere.  You probably want RELEASE_OR_BETA instead.

Comment 14

[Tzuhao Kuo [:kaku]]

(In reply to Julien Cristau [:jcristau] from comment #13)
> FWIW the patch looks wrong to me, AFAIK we don't have a RELEASE macro
> defined anywhere.  You probably want RELEASE_OR_BETA instead.

Neither do I know where is the definition of RELEASE, but looks like I am not the only occurrence.
http://searchfox.org/mozilla-central/rev/ad093e98f42338effe2e2513e26c3a311dd96422/modules/libpref/init/all.js#5741-5751

I will confirm the usage is correct or not and uplift/modify accordingly.

Comment 15

[Tzuhao Kuo [:kaku]]

Comment on attachment 8889270 [details]
Bug 1383610 - turn off preference of ShutdownDecoder feature on desktop version release and beta channel but keep the Fennec on;

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

Comment 16

[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 6fd0bb2d8bec -d ac1b19c47aac: rebasing 409710:6fd0bb2d8bec "Bug 1383610 - turn off preference of ShutdownDecoder feature on desktop version release and beta channel but keep the Fennec on; r=bwu" (tip)
merging modules/libpref/init/all.js
warning: conflicts while merging modules/libpref/init/all.js! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 17

[Tzuhao Kuo [:kaku]]

Comment on attachment 8889270 [details]
Bug 1383610 - turn off preference of ShutdownDecoder feature on desktop version release and beta channel but keep the Fennec on;

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

Comment 18

[Pulsebot]

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

Comment 19

[Tzuhao Kuo [:kaku]]

Attachment: bug-1383610-beta-uplift.patch

Approval Request Comment
[Feature/Bug causing the regression]: bug 1383610
[User impact if declined]: we'll release the ShutdownDecoder feature which is not ready to be released.
[Is this code covered by automated tests?]: this patch only turn off the preference, nothing to do with the feature's code. (The feature is well tested.)
[Has the fix been verified in Nightly?]: we want to keep the preference ON at Nightly.
[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?]: NO.
[Why is the change risky/not risky?]: It's not because we're turning off the feature.
[String changes made/needed]: NO.

Comment 20

[Julien Cristau [:jcristau]]

Comment on attachment 8890766 [details] [diff] [review]
bug-1383610-beta-uplift.patch

pref off "shutdown decoder" feature on desktop for 55 release, beta55+

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/2686c54f0fe8

Comment 22

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/c95cc38d6065

Comment 23

[Andrei Vaida [:avaida]]

(In reply to Tzuhao Kuo [:kaku] (PTO 7/26) from comment #19)
> [Is this code covered by automated tests?]: this patch only turn off the
> preference, nothing to do with the feature's code. (The feature is well
> tested.)
> [Has the fix been verified in Nightly?]: we want to keep the preference ON
> at Nightly.
> [Needs manual test from QE? If yes, steps to reproduce]: NO.

Setting qe-verify- based on Tzuhao Kuo's assessment on manual testing needs.

Comment 24

[Julien Cristau [:jcristau]]

FWIW I checked that media.suspend-bkgnd-video.enabled is false in 55.0b13 (linux64).