Add full macOS support

[dm3ch]

/fix #10

Finally, I found what has been missing in your macOS example - without registered handlers, macOS doesn't render playback info widget.

With the code in this PR, I was able to see test data in playback widget and get debug messages on commandHandler invocation.

Unfortunately, current code is just an example with low code quality and I'm still a noob in Rust, so I'm not sure if I would be able to finish this contribution. :(

[NovusTheory]

Hey, that's awesome. I'll look into this and see how it goes

[dm3ch]

@NovusTheory I have pushed my more recent commits.
What have been done:

• Added setting of different playbackinfo strings
• Experiment with reading artist info
• Experiment with registering callback

What should be added/fixed functional:

• Implement callback part
• My experiment with reading artist info always returns me string that have been wrote during init (but in OS widget I can see fresh data)
• See if it's possible to integrate Thumbnails (in macOS it should have some complex type not string)

And current code is awfull with a lot of hacks and unsafe everywhere, it would be better to try to refactor it when we would understand how all things could be implemented

[dm3ch]

FYI: Latest commits are broken, but c0b137c should be latest working for now

[dm3ch]

Actually right now following things works:

• Setting all song metadata except the picture.
• Passing media commands

Last 3 days I've been fighting with the function setting of the song picture.
Actually, it seems that it needs to: create NSString with URL, call NSImage initialization with URL, and then init MPMediaItemArtwork with NSImage.
Unfortunately, my rustlang and bindgen knowledge seems to be too low and I can't figure what am I doing wrong.

@NovusTheory maybe you can take a look on latest commits and try to point me how to achieve that.

[dm3ch]

@NovusTheory had you got a chance to look into this?

[NovusTheory]

@NovusTheory had you got a chance to look into this?

I have not had the chance yet

[dm3ch]

@NovusTheory I finally was able to make function seting thumbnail work. Also, I've implemented get functions too.

I have tested ytmdesktop with your xosms branch locally using my PR version of lib and it seems that all is fine:

• I'm able to see playback info
• I'm able to play, pause, change item via playback bubble and using media keys on keyboard or headphones (airpods works too)

I would be happy if you would be able to find some time to test this PR and review it?

There're 3 things that I believe needs your attention in this PR:

• I'm a newbie both in Rust and in ObjC runtime, so I'm afraid that there could be memory leakage. :(
• I've been using fruity::foundation lib just for to_str and from_str functions. Maybe we need to replace these functions cause it doesn't seems that fruity could feet our needs for all bindgened classes
• I'm not sure how to use tests you wrote in this repo the right way

P.S. It seems that this PR contains the most painful 200 lines of code in my life. :)

[dm3ch]

@NovusTheory Friendly reminder.

[NovusTheory]

Hey just want to give an update. I am still aware of this but just haven't gotten around to it yet. I'll see if I can do it soon

[dm3ch]

Friendly ping :)

[NovusTheory]

Okay so I've had some time to test and review this a bit. This isn't going to be a very thorough review but just highlighting some things I've noticed and want to provide feedback on. If I notice anything else I'll provide some more feedback.

I have tested ytmdesktop with your xosms branch

I tested ytmdesktop using my xosms branch with your PR and can reproduce results of showing media info.

I'm a newbie both in Rust and in ObjC runtime, so I'm afraid that there could be memory leakage. :(

I'm not worried about this as of right now in this review but I will look into this later.

I've been using fruity::foundation lib just for to_str and from_str functions. Maybe we need to replace these functions cause it doesn't seems that fruity could feet our needs for all bindgened classes

I'll take this into consideration but not as of right now in this review.

I'm not sure how to use tests you wrote in this repo the right way

The tests aren't exactly written in a well defined and meaningful way and were just tossed up to ensure that the API works without ever throwing errors.

Additionally since I can't review comment them. Some API's aren't being set up like get_playback_status

The tests work on every other OS however when I tested this in macOS there was some issues.
Segfaults occur when reading all of these when not initially set:

• Artist
• Album Artist
• Album Title
• Title
• TrackId

I presume the segfault is because the value is not initialized and is trying to be read on first glance. Set the values to an empty string on initialization and it shouldn't segfault(?)

Now for the interesting part. The tests I have should when I test them, show media information in the native service and operate as if an actual app (like ytmdesktop) were using it, however, it does not show anything. I'm unsure if macOS has some other requirements before the media information is shown for a program.

[dm3ch]

@NovusTheory Thank you for the initial review.

I have removed version changes and x86_64 target change.
I just have fixed segfault during the binding test by returning an empty string if MacOS runtime hasn't returned me data field.

Now for the interesting part. The tests I have should when I test them, show media information in the native service and operate as if an actual app (like ytmdesktop) were using it, however, it does not show anything. I'm unsure if macOS has some other requirements before the media information is shown for a program.

About this interesting case, I also realized that. As far as I understood MacOS will show playback widget only if this API is used from Application (in XCode terms). If we have a Console Application (in XCode terms) then even official Apple examples on ObjC or Swift wouldn't work. So I assume that to use playback API efficiently - your program should have a window (or should be shown in Dock)

[NovusTheory]

Updated review with further testing

It seems like what you were talking about with the console app/app is what determines if the media info is shown or not.

Regarding memory leaks, I've done testing and it almost seems like memory rises indefinitely and doesn't seem to fall. It's a hard thing to debug and I'm not even sure if memory leaks even apply to anywhere else in xosms either so it could be a bit of a mute point trying to solve it in this PR. I feel like this PR just adding support is good enough for now to get the support added and performance/optimization can be done at a later point in time considering xosms is meant to be unstable right now and I'm not too worried about it being in the best condition possible.

Before I merge this PR though do you want to finish up some of the APIs that don't have anything on them or should I? They currently have the unsupported filler in them. If the native API for macOS doesn't specifically support the API you can return what you think suits best like e.g. if you can't toggle something just always return true for it and ignore sets on it.

• is_enabled -> always false
• set_is_enabled -> empty block
• is_play_enabled -> always false
• set_is_play_enabled -> empty block
• is_pause_enabled -> always false
• set_is_pause_enabled -> empty block
• is_previous_enabled -> always false
• set_is_previous_enabled -> empty block
• is_next_enabled -> always false
• set_is_next_enabled -> empty block
• get_media_type -> always -1
• set_media_type -> empty block
• get_playback_status -> always -1 (this actually can be filled as set_playback_status is used)

[dm3ch]

@NovusTheory TY, for review.

I would like to finish the API, but unfortunately, I don't understand how to do it correctly for most of things you wrote (except just returning true and ignoring sets), cause I see no function for enablement separate features in https://developer.apple.com/documentation/mediaplayer/mpnowplayinginfocenter?language=objc .
It seems it's possible to implement some of them by unregistering some of handlers or ignoring some events, but I'm not sure if it worth it and I need to understand desired behaviour of this functions to emulate it in macos code.

What do you think about following plan?:

• Implement get_media_type set_media_type get_playback_status
• Ignore setting of others and always return true

[NovusTheory]

I see no function for enablement

You should be able to set MPRemoteCommandCenter.xCommand().enabled https://developer.apple.com/documentation/mediaplayer/mpremotecommandcenter since I had this when I was doing testing.

Otherwise if not then I'm fine with implementing what you can and ignoring set and always returning true 👍

[dm3ch]

https://developer.apple.com/documentation/mediaplayer/mpremotecommandcenter

Ok, I would try.

But what is_enabled/set_is_enabled should do? Should it disable all commands or what?

[NovusTheory]

But what is_enabled/set_is_enabled should do? Should it disable all commands or what?

On windows it's an actual thing which disables presenting the media information in the service. The linux side makes use of the CanControl property and advertises false to clients if the service is disabled which hints consumers to disable presentation and controls. I'm not sure what you can do for macOS outside disabling all the buttons but it still leaves the media shown which is not the idea of the is_enabled property. If you can't reliably figure something out just always return true and ignore sets on it.

[dm3ch]

@NovusTheory TY for comments they helped a lot.
Now all except is_enabled/set_is_enabled/remove_button_presed_callback is implemented.

Regarding memory leakage - right now I have only one hypothesis. I believe when I'm setting metadata (strings and Thumbnail) I always creates new ObjC object and set the dictionary field to it.

So i believe it would be better to create a separate issue for this problem.
In the best case it would be great if someone who is good in rust would help to wrap bindgenerated objects/procedures to work idiomatic rust way, that would allow to drop most of unsafe blocks and allow borrow-checker to make his work.

But we could at least create some test script that sets info in a loop and analyze behavior

[NovusTheory]

Everything looks good to me now after testing it all.

I'm not too worried about the remove_button_presed_callback not being implemented as not even the linux side is programmed for it and it's got a typo which has to be fixed in other places too. It's also just a bit of a mess right now too because it's setup to be event based but it's just a single function callback right now so the API for it is a bit ugly and I want to improve and change upon that.

If you have nothing else to add/comment on then I'll go ahead and merge this PR in.

[dm3ch]

Everything looks good to me now after testing it all.

I'm not too worried about the remove_button_presed_callback not being implemented as not even the linux side is programmed for it and it's got a typo which has to be fixed in other places too. It's also just a bit of a mess right now too because it's setup to be event based but it's just a single function callback right now so the API for it is a bit ugly and I want to improve and change upon that.

If you have nothing else to add/comment on then I'll go ahead and merge this PR in.

Yes, let's merge and proceed with memory leak research in separate issue.

[dm3ch]

P.S. Issue for memory-leak research #14