make ChannelMappingAudioProcessor, TrimmingAudioProcessor and ToFloatPcmAudioProcessor public UnstableApi instead of package-private

[nift4]

[REQUIRED] Use case description

I'm working on my own audio sink and started out by copy-pasting DefaultAudioSink into my project. After cleaning up some low-hanging fruits, I got quite a few errors because my sink can't access TrimmingAudioProcessor, ChannelMappingAudioProcessor and ToFloatPcmAudioProcessor; as they are package-private in another package. (I also got errors about AudioTrackPositionTracker and AudioDeviceInfoApi23, but those feel more like implementation details so no need to make them public.)

Proposed solution

Mark these classes as public @UnstableApi instead of package-private

Alternatives considered

I could of course copy paste them as well, but those are sufficiently abstracted and I'd never end up editing them, so that'd be suboptimal tbh.

[andrewlewis]

As a side note, please could you share some information about what prevents delegating to a DefaultAudioSink from your custom audio sink, or what customization points you are missing to use it directly? Thanks.

[nift4]

Hi @andrewlewis, I'm currently working on two different projects, somewhat related:

1. 24 bit and 32 bit int MediaCodec and AudioTrack output #1931 and Plans for Android 14 bit-perfect audio support #415, which requires Try input format PCM encoding first in MediaCodecAudioRenderer #2218 and expand format support for audio processors used by DefaultAudioSink #2259, and then editing DefaultAudioSink to accept int24/int32, and then editing it to call setPreferredMixerAttributes - I'm planning to upstream all of this work
2. some tricks for specific devices based on the C++ AudioTrack private API from libaudioclient (and libmedia). As such, I need to replace all usages of the java AudioTrack with my custom JNI-based wrapper. While Android 16 adds PCM offload support to the platform, Qualcomm already supported this since Android 4.x using AUDIO_OUTPUT_FLAG_DIRECT and/or a AUDIO_FORMAT_PCM_16_BIT_OFFLOAD; that can both save power by resampling on ADSP if needed and improve the output quality on devices like the LG V20 (where all non-direct mixPorts are limited to 48000kHz 16bit) - for this I want to use a custom audio sink

[tonihei]

I'm currently working on two different projects

We are still working on #1931 and #415 (despite the silence on these issues), so hopefully this need to fork the class will go away.

I'm planning to upstream all of this work

Thanks for wanting to contribute the changes back to the library! When you get to this point, before spending too much time preparing pull requests for this, it would be great if you could outline the implementation idea in a new issue (or on the existing ones if applicable), just so we can make sure the design makes sense for the library. As an app, it's often easier to take shortcuts or rely on side channels to communicate preferences and properties, but when integrating it for everyone, it often ends up requiring more changes than you may anticipate. And maybe we get around to submit the pending drafts for both integrations ourselves until then :)

TrimmingAudioProcessor, ChannelMappingAudioProcessor and ToFloatPcmAudioProcessor

Don't see why they have to be package-private. I will send a change to make them public, so they can be easily reused in other contexts as well.

[nift4]

Thanks @tonihei! I'll make sure to consider writing an outline first (depending on how much time preparing the PR needs).

One more question, considering the fast issue response times (which btw is very, very appreciated): are the PRs I submitted (the oldest one being 5 weeks old) not recieving feedback due to time constraints (in which case I totally understand that), did they fall off the radar or am I just too impatient? ;)

[nift4]

Oh, and could DefaultAudioTrackBufferSizeProvider.getMaximumEncodedRateBytesPerSecond possibly be made public as well? It's used in the audio sink since 2729dbb

[tonihei]

Could DefaultAudioTrackBufferSizeProvider.getMaximumEncodedRateBytesPerSecond possibly be made public as well?

We can move it to a shared util (like ExtractorUtil as this is where all the constants in there are defined anyway).

[nift4]

Thanks @tonihei :) closing this issue as resolved