Closed Bug 1583997 Opened 11 months ago Closed 10 months ago

Add pref to disallow mp3 decoding in AndroidDecoderModule so ffmpeg's decoder will be used

Categories

(Core :: Audio/Video: Playback, enhancement, P2)

Unspecified
Android
enhancement

Tracking

()

RESOLVED FIXED
mozilla71
Tracking Status
firefox-esr60 --- wontfix
firefox-esr68 --- wontfix
firefox69 --- wontfix
firefox70 --- wontfix
firefox71 --- fixed

People

(Reporter: mjf, Assigned: mjf)

References

Details

Attachments

(1 file)

At the moment, media.android-media-codec.preferred defaults to true. This causes the AndroidDecoderModule to be created[1] and inserted at the beginning of the list of PDMs here[2]. We need to add a pref and a bit of code to cause AndroidDecoderModule to return false if mp3, probably here[3], if we want to use the ffmpeg mp3 decoder.

[1] https://searchfox.org/mozilla-central/source/dom/media/platforms/PDMFactory.cpp#388-391
[2] https://searchfox.org/mozilla-central/source/dom/media/platforms/PDMFactory.cpp#414
[3] https://searchfox.org/mozilla-central/source/dom/media/platforms/android/AndroidDecoderModule.cpp#93-98

Assignee: nobody → mfroman
Priority: -- → P2
Depends on: 1582271
Depends on: 1583986
Blocks: 1528341

Under what circumstances would we prefer ffmpeg to the Android decoder?

Flags: needinfo?(mfroman)

As noted in https://bugzilla.mozilla.org/show_bug.cgi?id=1528341#c16, the native Android decoder is optimised for low decoder CPU usage rather than speed. Perhaps this was to satisfy the needs of the music player on Android, but its not suitable for web use. In the wild mp3 is heavily used as a format for interactive web and use of the native decoder leads to horribly long decode times. Chromium on Android uses software decoding.

It sounds like we should just use the software decoder all the time or add some kind of heuristic. A pref for unbreaking the browser is not ideal. Assuming this is for FxR, we really do not want a pref, since GeckoView apps aren't supposed to be able to modify those.

I don't have all the details, but Bug 1528341 hits most of what I've heard. The current decoder, IIUC, is decoding in roughly 1x speed, and that isn't always desirable. :drno, can you fill in any additional details?

Flags: needinfo?(mfroman) → needinfo?(drno)

I agree that the software decoder would be preferable in almost all cases. From examining the ffmpeg codebase it looks like pretty well-optimised code for most mobile hardware.

Perhaps we can make the default for the pref true, but continue with the pref until we're satisfied that there are no issues w/ battery life.

Attachment #9097479 - Attachment description: Bug 1583997 - add pref to prefer ffvpx mp3 decoding on android. r?jya! → Bug 1583997 - prefer ffvpx mp3 decoding on android. r?jya!

I'm not sure if we have a good understanding of how often MP3 are suppose to be decoded in real time because of someone just listening to music (vs the scenario of decoding a whole big chunk of MP3 at once like in the FxR case). But if GV and FxR both want to go for the software decoder, lets do that now. If we later learn that there is still a use case for using the hardware at 1x speed we can try to address it then.

Flags: needinfo?(drno)
Pushed by mfroman@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/437a24df0eef
prefer ffvpx mp3 decoding on android. r=jya

I assume we don't need to uplift this ffmp3 change to GeckoView Beta (70) or Fennec ESR 68. The Android mp3 decoder has worked well enough that there's probably no need to rush this change.

(In reply to Chris Peterson [:cpeterson] from comment #10)

I assume we don't need to uplift this ffmp3 change to GeckoView Beta (70) or Fennec ESR 68. The Android mp3 decoder has worked well enough that there's probably no need to rush this change.

Correct, I do not think this is a good candidate for uplift. It would require this and Bug 1582271 (and 1583986) which is not a small change.

Status: NEW → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71

(In reply to Nils Ohlmeier [:drno] from comment #8)

I'm not sure if we have a good understanding of how often MP3 are suppose to be decoded in real time because of someone just listening to music (vs the scenario of decoding a whole big chunk of MP3 at once like in the FxR case). But if GV and FxR both want to go for the software decoder, lets do that now. If we later learn that there is still a use case for using the hardware at 1x speed we can try to address it then.

One obvious case to me would be when we're decoding an audio track from a video. It's unlikely you need > 1x speed in that case. That's also where you would want to get the battery savings as well.

You need to log in before you can comment on or make changes to this bug.