Dialog

[mfkl]

Tried to follow on your ideas about the design. Few remarks before this can be merged.

• Still wondering if having a CancellationToken as the only way to close a dialog is a good idea.
• Some stuff needs to be ran on the UI thread I think. Maybe the user callback.
• Not sure about my pointer free/allocation which is why the next point is ->
• How do I test this? Guess I need to trigger a login dialog from the core of VLC somehow.

[jeremyVignelles]

A few things about what I've seen from my phone:

• User-provided callbacks should return a Task. Our code have the ownership of that task and is responsible to cancel it. once the task is finished, clean the dictionary and make the action in libvlc (dismiss, login...). this is a bit different from libvlc itself, but it is better for a .net developper. -Rather than doing just a Register with several functions, I would have done a class with callback properties that could be registered in your instance.

To answer your questions:

• In libvlc, pf_cancel is used to cancel a dialog. In our code, this maps to a cancellation token. It's then up to the user to cancel the dialog when the token is cancelled. The end user will still have the cancel button on the dialog.
• For things to be run on the main thread, this is a UI consideration. we can't assume that our code will be run on main thread, so how could we do that, while still being compatible with all frameworks? I think it's the role of the implementor to switch threads in its callbacks with Dispatcher.RunAsync() -Regarding memory allocation, having a dialog manager might help to clean things up. when you replace your manager, you Dispose() the old one -About unit tests, good question. how do they tests other libvlc bindings?

[mfkl]

User-provided callbacks should return a Task.

Done.

Rather than doing just a Register with several functions, I would have done a class with callback properties that could be registered in your instance.

There is already a struct that holds the libvlc-facing callbacks. I think its fine that way, the user can still create a class on its side when building the callbacks if they want though.

In libvlc, pf_cancel is used to cancel a dialog. In our code, this maps to a cancellation token. It's then up to the user to cancel the dialog when the token is cancelled.

Fine by me.

For things to be run on the main thread, this is a UI consideration. we can't assume that our code will be run on main thread, so how could we do that, while still being compatible with all frameworks?

There are unofficial ways to do this but I agree, it is up to the caller to do this. We should add a comment.

Regarding memory allocation, having a dialog manager might help to clean things up. when you replace your manager, you Dispose() the old one

Not 100% clear on the lifecycle of all involved objects yet, have to think about this.

About unit tests, good question. how do they tests other libvlc bindings?

Not sure they do test :-P Looking into this.

[mfkl]

Rebased on master. Still unsure about the cancellation code paths. Who is responsible for cancelling the token? Cancel can come from VLC AND from the user I think. This part is not clear to me at all.

Managed to trigger a login callback. Need to figure out how to trigger action/cancel callbacks.

[jeremyVignelles]

Who is responsible for cancelling the token?

If we have this architecture:

libvlc have been registered a dialogs_callbacks, it calls it. Turns out it is
    libvlcsharp's callback. It handles the dialogs instances currently opened, as well as cancellationTokenSources.
        - create a cancellation token source
        - store it within a ConcurrentDictionary or something, keyed by the dialog id
        - launch user's code (an async task), passing the token with the arguments.
            UserCallback's code shows the window
               User selects its option
            UserCallback Task completes with the dialog results
        libvlcsharp's callback is happy, it has the result and can call libvlc results accordingly (and cleanup dictionary)

Then, there are two things:

• libvlc cancels the dialog (through pf_cancel). Lookup in the dictionary, cancel the task. UserCallback aborts, throws a TaskCancelledException
• User wants to dismiss the dialog : This is a normal result (something like return null and libvlcsharp knows how to handle this (call libvlc_dialogs_dismiss or something)

[mfkl]

So the token can be cancelled by both libvlc AND the user (when clicking "cancel")? Still feel like I'm missing something here. He can't call .Cancel() on a CancellationToken unless I give it the CancellationTokenSource. I'm trying to understand how the libvlc dialog API works at the same time so yeah..

[jeremyVignelles]

If the user dismisses the dialog, then a value indicating that action could be returned from the Task<...>. This could be null for example. Another approach could be throw new TaskCanceledException(); inside user code, but there would then be no way to distinguish between "user has clicked on cancel" and "libvlc asked the window to cancel"

[jeremyVignelles]

Hi, you can find what I mean here, in my implementation for Vlc.DotNet : https://github.com/ZeBobo5/Vlc.DotNet/pull/373 . Sample codes are upcoming.

[mfkl]

Closed in favor of https://github.com/mfkl/LibVLCSharp/commit/f2bd07148db85c95729457fc77bc6481aa9833b6