Closed
Bug 1317239
Opened 8 years ago
Closed 8 years ago
Config the codec if it supports adaptive playback feature for seamless change in resolution.
Categories
(Firefox for Android Graveyard :: Audio/Video, defect, P1)
Firefox for Android Graveyard
Audio/Video
Tracking
(firefox52 wontfix, firefox53 fixed)
RESOLVED
FIXED
Firefox 53
People
(Reporter: JamesCheng, Assigned: JamesCheng)
References
Details
Attachments
(2 files)
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
Assignee | ||
Comment 1•8 years ago
|
||
Once we decide to change the behavior "do not recreate decoder when resolution change", I will start to work on this bug. Pause here.
Blocks: 1299105
Updated•8 years ago
|
Priority: -- → P1
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•8 years ago
|
Attachment #8813511 -
Flags: review?(jolin)
Attachment #8813512 -
Flags: review?(jolin)
Comment 4•8 years ago
|
||
mozreview-review |
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•8 years ago
|
||
mozreview-review |
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•8 years ago
|
||
mozreview-review |
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?
Attachment #8813512 -
Flags: review?(jolin) → review-
Comment 7•8 years ago
|
||
mozreview-review |
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
Attachment #8813511 -
Flags: review?(jolin) → review-
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 10•8 years ago
|
||
Thank you for your suggestion. Addressed, please review it again. Thanks.
Comment 11•8 years ago
|
||
mozreview-review |
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.
Attachment #8813511 -
Flags: review?(jolin) → review+
Comment 12•8 years ago
|
||
mozreview-review |
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.
Attachment #8813512 -
Flags: review?(jolin) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 16•8 years ago
|
||
Pushed by cbook@mozilla.com: 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
Keywords: checkin-needed
Comment 17•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/4d45db027c09 https://hg.mozilla.org/mozilla-central/rev/2e9dbc8426fb
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox53:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Comment 18•7 years ago
|
||
bugherder uplift |
Uplifted to Aurora as part of the roll-up patch in bug 1299105. https://hg.mozilla.org/releases/mozilla-aurora/rev/783ca8448b09
status-firefox52:
--- → fixed
Comment 19•7 years ago
|
||
This was backed out from 52 in bug 1340480. https://hg.mozilla.org/releases/mozilla-beta/rev/b70b7650fa1d
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•