Replacing SonicAudioProcessor for an interface

[brAzzi64]

Hi,

I'm migrating my app from ExoPlayer r1.5.10 the the latest 2.X, and I'm excited to see that pitch-shifting and time-stretching has been incorporated to the library formally, and in the AudioTrack wrapper object, with logic to enable draining of current transformed data when changing playback parameters and such.

According to its author, Sonic however is mostly aimed at time-stretching and pitch-shifting of speech, and yields pretty poor quality for music. Because of this, it'd be great for the AudioTrack object to treat SonicAudioProcessor as an abstraction (some kind of TimeStretchingPitchShiftingAudioProcessor interface) that we could replace with our own implementations that use different algorithms.

For what I've seen, the only extra methods that AudioTrack is calling on the instance (apart from those from the AudioProcessor interface), are setPitch / setSpeed / getOutputByteCount / getInputByteCount, which would be pretty generic to any other implementation.

Would it make sense to provide this functionality? I could come up with an RB that lets users of the library to pass the specific time-stretching instance to use through the MediaCodecAudioRenderer constructor.

Thanks!

Bruno

[brAzzi64]

I found a workaround that doesn't require re-compiling, but is definitely a bit hacky.

I pass my own MyAudioProcessor instance (that does pitch shifting & time stretching) when I construct the MediaCodecAudioRenderer. I make the setPitch/setSpeed calls to that object myself when I want to change the parameters, but I also have the draining logic kick-in by calling MediaCodecAudioRenderer::setPlaybackParameters twice in a row without having the inner SonicAudioProcessor actually modify the audio:

audioRenderer.setPlaybackParameters(new PlaybackParameters(2, 1));
audioRenderer.setPlaybackParameters(new PlaybackParameters(1, 1));

It seems to be working well, sounds just fine and it's not producing any strange logging output, even though the getCurrentPositionUs / applySpeedup logic might not be actually running.

The approach is working for me now, but it'd be nice to have proper hooks to allow for this kind of customization. Thoughts?

Thanks!

[andrewlewis]

This is on our roadmap.

[brAzzi64]

Thanks for getting back to me. Any hints on the timeline for this?

As an update, with the approach above the sound was just fine but the reported positions where indeed wrong (because of applySpeedup not doing what it was supposed to) so it wasn't a good workaround.

In the end I forked and applied the generalization I mentioned originally, and I'm currently depending on this on my app. Here's the diff as a reference (wanted to avoid changes with the constructors, so the way to use it is a bit tricky, you can see it in the comments).

This is working great right now, even with the ability to hot-swap the audio processor without re-building the player (re-preparing though).

[brAzzi64]

That said, I think that a better way to abstract this would be to just add the getInputByteCount / getOutputByteCount methods to the AudioProcessor interface, and try to apply this position compensation generically, taking into account every AudioProcessor; the implementation would probably not be super straightforward though.

[brAzzi64]

Hi guys. I wanted to know if there's any chance this could be considered soon, since my patch has been quite time consuming to maintain. Updated here:

brAzzi64@019095f

It's pretty straightforward; you'd probably just have to figure out a better name for PSTSAudioProcessor (pitch-shifting, time-stretching) and come up with maybe a better API to "override" the default SonicAudioProcessor instance.

Thanks!

[andrewlewis]

@brAzzi64 There will be a way to provide a custom implementation of the playback parameters audio processors next time we push the dev branch, which is likely to be over the next week or so. This issue will be updated automatically when that happens. Thanks for your patience!