1317239 - Config the codec if it supports adaptive playback feature for seamless change in resolution.

Status: RESOLVED FIXED

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

Description

[James Cheng[:JamesCheng]]

By document [1], we should ask if the codec supports this feature then set the MAX width height to MediaFormat. 

First we need an API to ask this feature then set the value.

Take [2] for reference.


[1] https://developer.android.com/about/versions/android-4.4.html#Multimedia
[2] http://androidxref.com/5.1.1_r6/xref/external/chromium_org/media/base/android/java/src/org/chromium/media/MediaCodecBridge.java#509

Comment 1

[James Cheng[:JamesCheng]]

Once we decide to change the behavior "do not recreate decoder when resolution change", I will start to work on this bug. Pause here.

Comment 2

[James Cheng[:JamesCheng]]

Attachment: Bug 1317239 - Part1 - Add an API to ask if the codec support adaptive playback.

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

Comment 3

[James Cheng[:JamesCheng]]

Attachment: Bug 1317239 - Part2 - Config the video decoder with adaptive playback feature if it is supported.

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

Comment 4

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

Comment on attachment 8813511 [details]
Bug 1317239 - Part1 - Add an API to ask if the codec support adaptive playback.

https://reviewboard.mozilla.org/r/94940/#review95170

::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/HardwareCodecCapabilityUtils.java:57
(Diff revision 1)
>      }
>      return false;
>    }
>  
> +  @WrapForJNI
> +  public static boolean checkSupportsAdaptivePlayback(String aMimeType) {

There could be more than one decoder component that support given MIME-type.
I think it's better to check the feature after decoder creation using its component name rather than just MIME-type.

Comment 5

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

Comment on attachment 8813511 [details]
Bug 1317239 - Part1 - Add an API to ask if the codec support adaptive playback.

https://reviewboard.mozilla.org/r/94940/#review95174

::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/HardwareCodecCapabilityUtils.java:57
(Diff revision 1)
>      }
>      return false;
>    }
>  
> +  @WrapForJNI
> +  public static boolean checkSupportsAdaptivePlayback(String aMimeType) {

Sorry I meant using component name **and** MIME-type.

Comment 6

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

Comment on attachment 8813512 [details]
Bug 1317239 - Part2 - Config the video decoder with adaptive playback feature if it is supported.

https://reviewboard.mozilla.org/r/94942/#review95172

::: dom/media/platforms/android/MediaCodecDataDecoder.cpp:82
(Diff revision 1)
>      if (!mSurfaceTexture) {
>        NS_WARNING("Failed to create SurfaceTexture for video decode\n");
>        return InitPromise::CreateAndReject(NS_ERROR_DOM_MEDIA_FATAL_ERR, __func__);
>      }
>  
> +    // Check if the codec supports adaptive playback or not.

Suggest to check the feature after `InitDecoder` (where `MediaCodec` is created).
Maybe introduce a boolean data member to store it for later?

Comment 7

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

Comment on attachment 8813511 [details]
Bug 1317239 - Part1 - Add an API to ask if the codec support adaptive playback.

https://reviewboard.mozilla.org/r/94940/#review95176

Comment 8

[James Cheng[:JamesCheng]]

Comment on attachment 8813511 [details]
Bug 1317239 - Part1 - Add an API to ask if the codec support adaptive playback.

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

Comment 9

[James Cheng[:JamesCheng]]

Comment on attachment 8813512 [details]
Bug 1317239 - Part2 - Config the video decoder with adaptive playback feature if it is supported.

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

Comment 10

[James Cheng[:JamesCheng]]

Thank you for your suggestion.

Addressed, please review it again.

Thanks.

Comment 11

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

Comment on attachment 8813511 [details]
Bug 1317239 - Part1 - Add an API to ask if the codec support adaptive playback.

https://reviewboard.mozilla.org/r/94940/#review95190

::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/HardwareCodecCapabilityUtils.java:66
(Diff revision 2)
> +          return false;
> +      }
> +
> +      try {
> +          MediaCodecInfo info = aCodec.getCodecInfo();
> +          if (info.isEncoder()) {

Nit: remove this check. In theory an encoder won't be passes as aCodec. If it were, cannot have this feature anyway.

Comment 12

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

Comment on attachment 8813512 [details]
Bug 1317239 - Part2 - Config the video decoder with adaptive playback feature if it is supported.

https://reviewboard.mozilla.org/r/94942/#review95194

LGTM.

Comment 13

[James Cheng[:JamesCheng]]

Comment on attachment 8813511 [details]
Bug 1317239 - Part1 - Add an API to ask if the codec support adaptive playback.

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

Comment 14

[James Cheng[:JamesCheng]]

Comment on attachment 8813512 [details]
Bug 1317239 - Part2 - Config the video decoder with adaptive playback feature if it is supported.

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

Comment 15

[James Cheng[:JamesCheng]]

Thank you for the quick review~~

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4d45db027c09
Part1 - Add an API to ask if the codec support adaptive playback. r=jolin
https://hg.mozilla.org/integration/autoland/rev/2e9dbc8426fb
Part2 - Config the video decoder with adaptive playback feature if it is supported. r=jolin

Comment 17

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

https://hg.mozilla.org/mozilla-central/rev/4d45db027c09
https://hg.mozilla.org/mozilla-central/rev/2e9dbc8426fb

Comment 18

[Ryan VanderMeulen [:RyanVM]]

Uplifted to Aurora as part of the roll-up patch in bug 1299105.
https://hg.mozilla.org/releases/mozilla-aurora/rev/783ca8448b09

Comment 19

[Ryan VanderMeulen [:RyanVM]]

This was backed out from 52 in bug 1340480.
https://hg.mozilla.org/releases/mozilla-beta/rev/b70b7650fa1d