android: Pass song information over the JNI bridge

[DDRBoxman]

[DDRBoxman]

I'm not quite sure what the calls to previous and next should look like. I tried making some locking versions like the pause function but seemed to be running into issues with that.

I also wasn't sure which of these would be preferred either.

partition.PlayNext();
partition.playlist.PlayNext(partition.pc);

[MaxKellermann]

This looks a bit overengineered. Does the Java part really need all the tag values? I thought it might be enough to just have a Bridge method:

public static native void onSongChange(String artist, String title, int durationMilliseconds);

I don't know because there's no code for actually using it, and that's one of the problems of this PR: it adds code for no visible purpose, the code is completely unused. I also don't like adding empty method declarations (seen in the first commit, "android: add next and previous track to the jni bridge"). This way of making incremental commits is useless - each commit must work, but your first commit doesn't work.

[DDRBoxman]

The MediaMetadata object that gets passed into the media session actually accepts a ton of metadata information and I think it would be great if we could fill that out as much as we can: https://developer.android.com/reference/android/media/MediaMetadata

What's the preferred way to route events back to the JNI bridge. Looks to me like the Partition internally creates a PlayerControl which then calls back to the Partition which emits events into the event loop which get invoked on the list of clients the partition has. Should there be an event callback class that can be passed into the partition or should the client be abstracted and allow passing in a new say jni bridge client?

I also wondered if you had any thoughts about having the JNI bridge make next and previous calls, the few things I tried ended up with the mpd process in what seemed like a deadlock.

[MaxKellermann]

Okay, so you want to pass something to Java that will be converted to MediaMetadata - since your C++ code is so complicated already, would it be useful to create MediaMetadata in C++, instead of this custom class? (btw. your code doesn't work well because it overwrites previous values if a song has multiple instances of a tag, e.g multiple artists.)

Should there be an event callback class that can be passed into the partition or should the client be abstracted and allow passing in a new say jni bridge client?

I don't quite get it - do you want to supported multiple partitions? In that case, I guess we should have a Java object for each partition; maybe Bridge, but maybe a separate class Partition - I'm not quite sure yet.

I also wondered if you had any thoughts about having the JNI bridge make next and previous calls, the few things I tried ended up with the mpd process in what seemed like a deadlock.

When you call native methods from Java, you're inside some random thread, from which you must not access most data structures or even invoke most methods. Most methods are only supposed to be called from MPD's main thread. But you can always inject calls into that thread via BlockingCall().

[DDRBoxman]

Building MediaMetadata on the native side is probably even more gross than the current code to build that new class since it uses the builder pattern. This ends up with a ton of calls to GetMethodID and then calling those methods. It probably makes sense to just turn that hash map into something like an array of tuples to correct the duplicate entry issue.

I guess I'm not quite clear on what multiple partitions are used for. If we only end up with one player on android having callbacks for say song change events from that sounds fine. I just wasn't sure what hooking that would look like.

[MaxKellermann]

I don't think building a MediaMetadata is really harder, and neither does the Builder pattern make it any harder. You only need to look up the Builder class and some of its methods. Then you have a TagType to string table, containing the documented values of MediaMetadata.METADATA_KEY_*. Your code iterates this table, looks up all values of a certain TagType, concatenates them (separated by comma or semicolon?), call build(), return it. (Does MediaMetadata support multiple values for one key or are applications supposed to concatenate them into one string?)

I guess I'm not quite clear on what multiple partitions are used for.

Then let's just forget that feature and only support the first partition. If somebody ever finds this feature useful on Android, you can add it, but don't think about the complexity right now.