Closed Bug 1364341 Opened 3 years ago Closed 2 years ago

Fallback to software codec if the hardware codec configures fail

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 55
Tracking Status
firefox54 --- fixed
firefox55 --- fixed

People

(Reporter: alwu, Assigned: jhlin)

References

Details

Attachments

(4 files, 2 obsolete files)

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
Hi, John,
Could you help me to check this bug?
Thanks!
Blocks: 1364340
Flags: needinfo?(jolin)
Assignee: nobody → jolin
Flags: needinfo?(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

::: 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 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 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
Attachment #8868011 - Flags: review?(jyavenard) → review+
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
Attachment #8868012 - Flags: review?(jyavenard) → review+
Pushed by jolin@mozilla.com:
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 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.
Attachment #8868011 - Flags: approval-mozilla-beta?
https://hg.mozilla.org/mozilla-central/rev/1ffcbd8d55f6
https://hg.mozilla.org/mozilla-central/rev/bc722bc488b5
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
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.
Attachment #8868011 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attached patch part 1 for beta(54) (obsolete) — Splinter Review
Attached patch part 2 for beta(54) (obsolete) — Splinter Review
Attachment #8871592 - Attachment is obsolete: true
Attachment #8871593 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.