Make Sink work

[mitschwimmer]

Hey @devgianlu, I finally found some time to make this work. I have a couple of questions and would like your review which is why I am not just merging this.
I will place my Implementation questions/remarks as comments on the PR.
Regarding testing, I want to transform the MainActivity of the app-module into a couple of emulator/device Integration Tests which will be expecting credentials as environment variables. Would you like to keep the app-module as an implementation example?

[mitschwimmer]

track.stop() in AdnroidSinkOutput does not immediately stop but will play the remaining buffer first. If we want it to stop right away we need to implement .stop() by calling .pause() and .flush() in the method. What would you expect .stop() to do?
While we are on the topic of expactations, what would you expoect in .close() over .release()?

[devgianlu]

track.stop() in AdnroidSinkOutput does not immediately stop but will play the remaining buffer first. If we want it to stop right away we need to implement .stop() by calling .pause() and .flush() in the method. What would you expect .stop() to do?
While we are on the topic of expactations, what would you expoect in .close() over .release()?

These methods have the same functionality as the ones in javax.sound.sampled.DataLine. Precisely:

• stop() is called when the playback pauses so it should stop immediately, but not discard the buffer data
• close() should stop the playback, free the line and associated resources
• release() could be a no-op in this case because it is called 20 seconds after the line has been paused to allow other programs to use it on systems that support one line only. I have just realized that in the MixerOutput implementation it has the same function as close(), but it may be worth keeping

[devgianlu]

Regarding testing, I want to transform the MainActivity of the app-module into a couple of emulator/device Integration Tests which will be expecting credentials as environment variables. Would you like to keep the app-module as an implementation example?

I think having an example activity that one can just run on its device would be a nice proof of concept, possibly adding some UI to play/pause (I can do that). Having tests would be an additional bonus since it is easy to break this if non-Android classes are used in the librespot-java source code.

[devgianlu]

@mitschwimmer What's your opinion on #1? I think it could be really useful, I am in the process of merging it

I have made it the default one on master (hence the merge conflicts).

[mitschwimmer]

@mitschwimmer What's your opinion on #1? I think it could be really useful, I am in the process of merging it

I have made it the default one on master (hence the merge conflicts).

I am full of respect for someone using the NDK :-)
However, I can't say much regarding the actual implementation, more so on a meta level. Sparing cpu cycles is great, therefore I am all in on a more efficient decoder implementation. I was just wondering; can't we even use one of Android's own decoder facilities?

[devgianlu]

can't we even use one of Android's own decoder facilities?

Something like https://developer.android.com/reference/android/media/MediaCodec ?

[mitschwimmer]

Yes, that should decode Ogg Vorbis (among others) and it offers buffers to work with.

[devgianlu]

We can definitely have another module for that one too. Giving multiple options won't hurt. We can eventually make some benchmarks (?).

Let me know if you want to take care of it or I'll do it.

[mitschwimmer]

Please go ahead 😁
Are you fine with me opening issues for (even small) todos? It will make discussions and coordination easier.

[devgianlu]

Absolutely, I love organization.

[funtax]

Hey there, I am looking forward to your ideas & implementations. I darkly remember that I had implemented (or at least tried to) Vorbis-decoding via Android's native MediaCodec years ago (~2016). But either it didn't work, the minimum compile level didn't match at this time or something else. Would be interesting if this could be used now down to this project's current minimum API-level.