media icon indicating copy to clipboard operation
media copied to clipboard

Try input format PCM encoding first in MediaCodecAudioRenderer

Open nift4 opened this issue 9 months ago • 9 comments

...and remove hard requirement for 16-bit PCM support in the audio sink even if that's not needed to playback.

If the audio file is using 24-bit or 32-bit (int) PCM, MediaCodecAudioRenderer tries to output float, and if that is not supported, falls back to 16-bit PCM. This commit changes this behavior to first trying the media file format, then any close high-resolution audio formats (such as float) and then fall back to 16-bit PCM. The behaviour with DefaultAudioSink does not change, but if an audio sink supports any more optimal formats, they will now be used with MediaCodecAudioRenderer.

nift4 avatar Mar 08 '25 12:03 nift4

~Sadly forgot to add this to the commit message, but~ force pushed that into the commit message (I'll stop force pushing now), it's a first step towards #1931

nift4 avatar Mar 08 '25 13:03 nift4

@microkatz sorry to bump this but it's been a while, any chance you could take a look at this PR? thanks!

nift4 avatar Mar 31 '25 13:03 nift4

@microkatz @icbaker hi, please could you review this PR? it's been waiting for 2 months, and I'm somewhat blocked by this being reviewed/merged before upstreaming more things. thanks in advance!

nift4 avatar Apr 30 '25 10:04 nift4

@microkatz @marcbaechinger Hi, could one of you please help me get this merged? It's now been 3 months without review. Thanks!

nift4 avatar May 27 '25 16:05 nift4

@microkatz Hi, can you please take a look at this PR, as it has been a while? Thanks in advance!

nift4 avatar Jul 01 '25 17:07 nift4

Hi @microkatz @tonihei, is there any chance to get this PR reviewed, please?

nift4 avatar Oct 18 '25 14:10 nift4

@nift4

Thank you for your patience. Can you describe in greater detail what your goal is?

To note, the supportsFormat method in MediaCodecAudioRenderer method just checks format support by the audio sink. At default 16-bit PCM is the default and therefore that should all be needed to check baseline track support. Do you have a use-case where the audio sink is distinctly not support 16-bit PCM nor FLOAT(which is preferred to 24)?

Also as it stands, I don't think your code change will work as expected. If you checked further through to AudioTrackAudioOutputProvider.java then you would see that your PR needs additional code to check other encoding support.

microkatz avatar Oct 21 '25 11:10 microkatz

Can you describe in greater detail what your goal is?

If file is 16-bit, I want to output 16 bit and if file is 24-bit, I want to output 24 bit. The rationale is that I want audio sink to get unconverted data if possible, not converted by MediaCodec. It's also not a problem for performance because AudioTrack can do all conversions (like 24->16 or 24->float or whatever the device uses) natively in platform code. I'm trying to avoid additional effort of going 24 bit (file)->float (MediaCodec output)->24 bit again if file is 24bit in the first place.

Do you have a use-case where the audio sink is distinctly not support 16-bit PCM nor FLOAT(which is preferred to 24)?

No, but I have use-case where FLOAT is not supported. Some Qualcomm devices have unofficial API for PCM offload which supports only 16 and 24 bit.

Also as it stands, I don't think your code change will work as expected. If you checked further through to AudioTrackAudioOutputProvider.java then you would see that your PR needs additional code to check other encoding support.

I have a custom audio sink in my app currently, which is patched for supporting this. The relevant code change for upstreaming is #2445. As you can see it currently conflicts because I did not get to rebasing (I'll do it soon), but it should outline the general idea and it has the code which you identified as missing here. I chose to make two seperate PRs because they work independently (although there is no benefit unless you make a custom audio sink with only this PR, when both are merged, the benefit will appear). As the other PR seems to be blocked by some internal draft it would be nice if maybe this one can get merged first, before the other one is done.

nift4 avatar Oct 21 '25 12:10 nift4

Ah now I see what you were thinking about,

To note, the supportsFormat method in MediaCodecAudioRenderer method just checks format support by the audio sink. At default 16-bit PCM is the default and therefore that should all be needed to check baseline track support. Do you have a use-case where the audio sink is distinctly not support 16-bit PCM nor FLOAT(which is preferred to 24)?

I do not have a usecase where the explicit check is absolutely needed, I just thought it would be nice to not hardcode assumptions in the code.

nift4 avatar Oct 21 '25 12:10 nift4