1365505 - [Fennec][HLS] Turn on HLS native support not only for Nightly

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Audio/Video, defect)

Description

[James Cheng[:JamesCheng]]

We intend to enable HLS support only for nightly.

Three parts need to take care(should remove the '''nightly only''' flag in the future)
1.
Bug 1341990 - Part 0: Add a feature flag to control source code build time coverage for ExoPlayer.
2.
Bug 1350246 - [Part7] Define MOZ_ANDROID_HLS_SUPPORT macro and add the native code into source tree.
3.
Bug 1350246 - [Part4] Add a feature preference for HLS, default on for Fennec.

Comment 1

[James Cheng[:JamesCheng]]

AppConstants.NIGHTLY_BUILD || AppConstants.DEBUG_BUILD should remove AppConstants.NIGHTLY_BUILD when we support HLS not only on Nightly build.

Comment 2

[Kilik Kuo [:kikuo] (inactive)]

According to the SV's pre-beta sign-off report.

I think we could let this feature ride on FX 56, is that ok ?

Comment 3

[Bobby Chien]

To comment 2, Yes. Attached Pre-Beta Sign-off.

[Fennec] Make Gecko native support HLS on Fennec
Feature sign off report, Manual QA
Targeted GA: Firefox 56 - September 26, 2017
Sign off phase: Pre-Beta (Fx56).

Manual QA'S ASSESSMENT & RECOMMENDATION
Feature status:GREEN

    Reason: There are no blocking issues nor crashes, but there are still some bugs open .

    Proposed course of action: let this feature ride FX56 and fix open bugs during the beta cycle. 


MANUAL TESTING SUMMARY
Testing status: COMPLETED (100%)
  20 passed (83%), 0 blocked (0%), 4 failed with known bugs (17%), 0 failed with new bugs (0%)

    Test report: on TestRail
    New bugs uncovered during sign off: None
    Known bugs encountered during sign off:
         1377873, 1377891

Comment 4

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

(In reply to Kilik Kuo [:kikuo] from comment #2)
> According to the SV's pre-beta sign-off report.
> 
> I think we could let this feature ride on FX 56, is that ok ?
Good job! Let's ship it!
Please create a patch to turn on it.

Comment 5

[James Cheng[:JamesCheng]]

Attachment: Bug 1365505 - Part1-Make the feature flag be exposed not only on nightly.

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

Comment 6

[James Cheng[:JamesCheng]]

Attachment: Bug 1365505 - Part2-Turn on the pref for hls not only on nightly.

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

Comment 7

[James Cheng[:JamesCheng]]

Please see the patch for turning on this feature.

Comment 8

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

Comment on attachment 8892381 [details]
Bug 1365505 - Part2-Turn on the pref for hls not only on nightly.

https://reviewboard.mozilla.org/r/163336/#review168716

Ship it!

Comment 9

[Kilik Kuo [:kikuo] (inactive)]

Comment on attachment 8892380 [details]
Bug 1365505 - Part1-Make the feature flag be exposed not only on nightly.

https://reviewboard.mozilla.org/r/163334/#review168758

Comment 10

[Mozilla Autoland]

hg error in cmd: hg push -r tip ssh://hg.mozilla.org/integration/autoland: pushing to ssh://hg.mozilla.org/integration/autoland
searching for changes
remote: repository is read only
remote: Repo closed per glandium's request; sheriff not around to mark it closed, so deploying the read-only hammer.
remote: refusing to add changesets
remote: prechangegroup.readonly hook failed
abort: push failed on remote

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d5210a2cb19a
Part1-Make the feature flag be exposed not only on nightly. r=kikuo
https://hg.mozilla.org/integration/autoland/rev/80491c51a8e7
Part2-Turn on the pref for hls not only on nightly. r=bwu

Comment 12

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

https://hg.mozilla.org/mozilla-central/rev/d5210a2cb19a
https://hg.mozilla.org/mozilla-central/rev/80491c51a8e7

Comment 13

[James Cheng[:JamesCheng]]

Comment on attachment 8892380 [details]
Bug 1365505 - Part1-Make the feature flag be exposed not only on nightly.

Approval Request Comment
[Feature/Bug causing the regression]:Feature bug 1345752 for HLS playback on Fennec
[User impact if declined]: Cannot play video using HLS
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: SV has already tested and signed off.
[List of other uplifts needed for the feature/fix]: Part2 https://reviewboard.mozilla.org/r/163336/diff/1#index_header
[Is the change risky?]: na
[Why is the change risky/not risky?]: has already been tested through Nightly. 
[String changes made/needed]: na

Comment 14

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

Can you help on comment 13?

Comment 15

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8892380 [details]
Bug 1365505 - Part1-Make the feature flag be exposed not only on nightly.

Has QE signoff, let's uplift for 56 beta 1.

Comment 16

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8892381 [details]
Bug 1365505 - Part2-Turn on the pref for hls not only on nightly.

[Triage Comment]

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/e6eb9596f218
https://hg.mozilla.org/releases/mozilla-beta/rev/525891935c52

Comment 18

[Ryan VanderMeulen [:RyanVM]]

This was backed out from 56 and 57 due to bug 1389990.

https://hg.mozilla.org/mozilla-central/rev/b5fa1d07ad05
https://hg.mozilla.org/mozilla-central/rev/3bf1126bddd8

https://hg.mozilla.org/releases/mozilla-beta/rev/f53fa9ca89b9
https://hg.mozilla.org/releases/mozilla-beta/rev/dc22f5250239

Comment 19

[Marco Castelluccio [:marco]]

Release Note Request (optional, but appreciated)
[Why is this notable]: It has been disabled in 56, so the release note should be removed from https://www.mozilla.org/en-US/firefox/android/56.0beta/releasenotes/.
[Affects Firefox for Android]: Yes, only Firefox for Android

Comment 20

[Wesly Huang (EPM)]

James, I learned from Max that even there are some minor remaining work about Gradle, it's now ready for you to bring HLS back since Fennec 59. Please go ahead and you may consult Max shall you have any question about the setup w/ Gradle. Thanks.

Comment 21

[James Cheng[:JamesCheng]]

Thanks, I replied in Bug 1389990 comment 29

Comment 22

[Kilik Kuo [:kikuo] (inactive)]

Attachment: Bug 1365505 - [P1][Fennec] Enable HLS support by default for all brandings.

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

Comment 23

[John Lin [:jhlin][:jolin]]

Comment on attachment 8930872 [details]
Bug 1365505 - [P1][Fennec] Enable HLS support by default for all brandings.

https://reviewboard.mozilla.org/r/201972/#review207646

Comment 24

[Kilik Kuo [:kikuo] (inactive)]

Attachment: Bug 1365505 - [P2][Fennec] Only include exoplayer & HLSDecoder related components when building --with-gradle.

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

Comment 25

[Kilik Kuo [:kikuo] (inactive)]

Comment on attachment 8930872 [details]
Bug 1365505 - [P1][Fennec] Enable HLS support by default for all brandings.

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

Comment 26

[Kilik Kuo [:kikuo] (inactive)]

Comment on attachment 8933242 [details]
Bug 1365505 - [P2][Fennec] Only include exoplayer & HLSDecoder related components when building --with-gradle.

https://reviewboard.mozilla.org/r/204180/#review209712

Per Bug 1389990 Comment 42, I disable hls when building fennec without gradle.

Comment 27

[Max Liu [:maliu]]

Comment on attachment 8933242 [details]
Bug 1365505 - [P2][Fennec] Only include exoplayer & HLSDecoder related components when building --with-gradle.

https://reviewboard.mozilla.org/r/204180/#review210034

Comment 28

[Kilik Kuo [:kikuo] (inactive)]

Try run - https://treeherder.mozilla.org/#/jobs?repo=try&revision=f233974b4a5e
Try run for (Bng) - https://treeherder.mozilla.org/#/jobs?repo=try&revision=ac54f0149b56

Looks good.

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9545b9df0c6d
[P1][Fennec] Enable HLS support by default for all brandings. r=jolin
https://hg.mozilla.org/integration/autoland/rev/ce2c43ec79ae
[P2][Fennec] Only include exoplayer & HLSDecoder related components when building --with-gradle. r=maliu

Comment 30

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/9545b9df0c6d
https://hg.mozilla.org/mozilla-central/rev/ce2c43ec79ae

Comment 31

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

This was in the 56 beta notes as "Improved support for HLS (HTTP Live Streaming) playback". 

For now, I will add the same note back in for 59.0b1. 
What do you think? Is there anything more specific that you'd like to be in the note?

Comment 32

[Wesly Huang (EPM)]

Thanks for the reminder, Liz! Yes the support was re-enabled in 59 Nightly and got pre-beta sign-off on Dec. 20, so should be included in the 59 release note instead, and I think the existing description holds well. Once the release note is public for comment I'll also ping media team and PM to see if they would like to refine anything specific.

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #31)
> This was in the 56 beta notes as "Improved support for HLS (HTTP Live
> Streaming) playback". 
> 
> For now, I will add the same note back in for 59.0b1. 
> What do you think? Is there anything more specific that you'd like to be in
> the note?