1364341 - Fallback to software codec if the hardware codec configures fail

Status: RESOLVED FIXED

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

Description

[Alastor Wu [:alwu]]

If the video resolution isn't supported by hardware codec, we should fallback to software codec.

STR.
1. Run dom/media/test/test_resolution_change.html on Android

Expect.
2. The video can be playback

Actual.
2. The video can't be playback

Comment 1

[Alastor Wu [:alwu]]

Hi, John,
Could you help me to check this bug?
Thanks!

Comment 2

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

Attachment: bug 1364341 - part 1: release all resources used by existing codec when re-configure.

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

Comment 3

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

Attachment: bug 1364341 - part 2: try next codec when configuration fails.

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

Comment 4

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

Comment on attachment 8868011 [details]
bug 1364341 - part 1: release all resources used by existing codec when re-configure.

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

Comment 5

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

Comment on attachment 8868012 [details]
bug 1364341 - part 2: try next codec when configuration fails.

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

Comment 6

[Jean-Yves Avenard [:jya]]

Comment on attachment 8868011 [details]
bug 1364341 - part 1: release all resources used by existing codec when re-configure.

https://reviewboard.mozilla.org/r/139574/#review145020

::: commit-message-35a46:1
(Diff revision 2)
> +bug 1364341 - part 1: release all resources used by existing codec when re-configure. r?jya

what is re-configure here ?

if it's just a change of resolution. Shouldn't we wait for all the previous frames to have been returned and displayed first?

Following bug 1299105, there's no longer any wait when the resolution change.

Dropping all frames because a new resolution is seen will lead to be transitions.

Comment 7

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

Comment on attachment 8868011 [details]
bug 1364341 - part 1: release all resources used by existing codec when re-configure.

https://reviewboard.mozilla.org/r/139574/#review145020

> what is re-configure here ?
> 
> if it's just a change of resolution. Shouldn't we wait for all the previous frames to have been returned and displayed first?
> 
> Following bug 1299105, there's no longer any wait when the resolution change.
> 
> Dropping all frames because a new resolution is seen will lead to be transitions.

Re-configure here is lightweight remote codec recreation. With this a proxy can be reused for different decoding/encoding sessions.
Not currently in used, though.

Comment 8

[Jean-Yves Avenard [:jya]]

Comment on attachment 8868011 [details]
bug 1364341 - part 1: release all resources used by existing codec when re-configure.

https://reviewboard.mozilla.org/r/139574/#review145084

Comment 9

[Jean-Yves Avenard [:jya]]

Comment on attachment 8868012 [details]
bug 1364341 - part 2: try next codec when configuration fails.

https://reviewboard.mozilla.org/r/139576/#review145086

LGTM, thought I'm no expert in Java and MediaCodec

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1ffcbd8d55f6
part 1: release all resources used by existing codec when re-configure. r=jya
https://hg.mozilla.org/integration/autoland/rev/bc722bc488b5
part 2: try next codec when configuration fails. r=jya

Comment 11

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

Comment on attachment 8868011 [details]
bug 1364341 - part 1: release all resources used by existing codec when re-configure.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1350279
[User impact if declined]: some video (e.q. VP8 with resolution < 64x64) won't play
[Is this code covered by automated tests?]: yes (dom/media/test/test_resolution_change.html)
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: bug 1350279
[Is the change risky?]: low risk
[Why is the change risky/not risky?]: it falls back to SW decoder
[String changes made/needed]: none

Please uplift both patches.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1ffcbd8d55f6
https://hg.mozilla.org/mozilla-central/rev/bc722bc488b5

Comment 13

[Gerry Chang [:gchang]]

Comment on attachment 8868011 [details]
bug 1364341 - part 1: release all resources used by existing codec when re-configure.

OOP is enabled in 54 and fallback to software codec is important once the hardware resource is not enough. We need this in 54. Beta54+. Should be in 54 beta 12 for mobile.

Comment 14

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

Attachment: part 1 for beta(54)

Comment 15

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

Attachment: part 2 for beta(54)

Comment 16

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

Please use attachment 8871592 [details] [diff] [review] and attachment 8871593 [details] [diff] [review] for uplifting. Thanks a lot.

Comment 17

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

Attachment: part 1 for beta(54)

Comment 18

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

Attachment: part 2 for beta(54)

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/157988294fe9
https://hg.mozilla.org/releases/mozilla-beta/rev/6fb66467485a