Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support MediaCodec audio Float output #6749

Closed
krocard opened this issue Dec 11, 2019 · 6 comments
Closed

Support MediaCodec audio Float output #6749

krocard opened this issue Dec 11, 2019 · 6 comments
Assignees

Comments

@krocard
Copy link
Contributor

krocard commented Dec 11, 2019

Use case description

Exoplayer (using MediaCodec) always play compressed audio with a 16bit encoding, when it should play float if available
This has the following drawbacks:

And no advantages that I could identify.

Proposed solution

Use Media Codec float output for audio if supported (API>24).
https://developer.android.com/reference/android/media/MediaCodec#raw-audio-buffers
To do that, the MediaFormat#KEY_PCM_ENCODING has to be set to AudioFormat#ENCODING_PCM_FLOAT during before calling MediaCodec.configure.

Also DefaultAudioSink.enableConvertHighResIntPcmToFloat should probably default to true.

Alternatives considered

  • Use the ffmpeg extension to decode the stream to high res audio.
    This leads to bigger app size and does not use the platform codecs.
  • overridde MediaCodecAudioRenderer.getMediaFormat to configure the codec to output FLOAT.
    Drawback: Need to subclass MediaCodecAudioRenderer for one utility method. Might not be stable with respect to new releases.
ojw28 added a commit that referenced this issue Mar 25, 2020
This is less confusing than having audio processing functionality (e.g., playback
speed adjustment) just "not work" for some pieces of media.

If this change is merged, I will update #6749 to also track making DefaultAudioSink
intelligently enable/disable float output depending on how the audio processors are
configured.

Issue: #7134
PiperOrigin-RevId: 302871568
@ojw28
Copy link
Contributor

ojw28 commented Mar 25, 2020

Also DefaultAudioSink.enableConvertHighResIntPcmToFloat should probably default to true.

This is now called enableFloatOutput, and is still disabled by default. It can't be enabled by default in its current form, since this causes audio processing (e.g., playback speed adjustment) to be disabled. I think we should work toward being able to remove the option, by making it so that DefaultAudioSink can automatically configure resampling on both sides of the other audio processors to do the best thing given the sink's configuration. Which probably means:

  • On the input side, automatically enabling re-sampling to 16-bit integer PCM only if other audio processors in the chain actually need to do some processing.
  • On the output side, automatically enabling re-sampling 24-bit and 32-bit integer PCM to float PCM if the output supports it, and automatically enabling re-sampling to 16-bit integer PCM otherwise.

@krocard
Copy link
Contributor Author

krocard commented Mar 25, 2020

I think AudioProcessors should expose what input format they support. That would avoid having to re-sample to 16bit needlessly (which also decrease quality). Only if a processor does not support the inputFormat should the AudioSink automatically resampling to 16bit (as I guess all AudioProcessors are required to support it right now).
On the output side, I'm not sure to understand why resampling to Float from 24bit or 32bit PCM is required.
The platform probably support those formats if it supports Float, and the platform re-samplers are probably more performant and of better quality. Of course, as a fallback if the platform does not support the output format, then ExoPlayer must reformat to Float, but otherwise, I think it's better to let the platform take care of it (I doubt we will see significant device differences in the wild).

@ojw28
Copy link
Contributor

ojw28 commented Mar 25, 2020

I think AudioProcessors should expose what input format they support. That would avoid having to re-sample to 16bit needlessly (which also decrease quality). Only if a processor does not support the inputFormat should the AudioSink automatically resampling to 16bit (as I guess all AudioProcessors are required to support it right now).

Agreed. My first bullet point is conditional on the fact that all of the (non-resampling) audio processors only accept 16-bit integer PCM at the moment. In a future where we have audio processors that support additional input formats, it would be good if the logic could be clever enough to enable resampling only when necessary. I don't think we need to look at this as a priority though, given what our current audio processors support.

On the output side, I'm not sure to understand why resampling to Float from 24bit or 32bit PCM is required.

As discussed, it doesn't appear AudioTrack currently supports 24-bit and 32-bit integer PCM output. If all of the underlying logic to support that is implemented already in the platform, please file a feature request on the platform team to have them expose it. I agree that having the platform do this resampling is preferable to us doing it.

@krocard
Copy link
Contributor Author

krocard commented Mar 30, 2020

I asked the audio framework team why 24 and 32 bit linear PCM are not exposed in the SDK [internal: b/152481314].

ojw28 added a commit that referenced this issue Mar 30, 2020
This is less confusing than having audio processing functionality (e.g., playback
speed adjustment) just "not work" for some pieces of media.

If this change is merged, I will update #6749 to also track making DefaultAudioSink
intelligently enable/disable float output depending on how the audio processors are
configured.

Issue: #7134
PiperOrigin-RevId: 302871568
@krocard
Copy link
Contributor Author

krocard commented Jul 31, 2020

Starting with ec78bde, ExoPlayer will now decode audio to float (highest bit depth supported by Android) if supported. It will be enabled by default in the next major release: 2.12. It is not disabled by default. See next comment.

@krocard krocard closed this as completed Jul 31, 2020
@krocard
Copy link
Contributor Author

krocard commented Jul 31, 2020

Correction:
Float output will not be enabled by default in the next major version. It will stay be disable by default. Enable it using: DefaultRenderersFactory.setEnableAudioFloatOutput(true) or when constructing a DefaultAudioSink using the enableFloatOutput parameter.

@google google locked and limited conversation to collaborators Sep 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants