INPC and/or EventManager

[mfkl]

So the current libvlcpp API for events is about one EventManager abstract class and several specialized implementation for each core objects. This approach is fine by itself, but we're thinking using INotifyPropertyChanged could be more .NET-ty and easier to integrate with native views on each platform.

The MediaPlayer seems to be a good candidate to discuss this.

Links of interest: C# MediaPlayer https://github.com/mfkl/LibVLCSharp/blob/master/LibVlcSharp/MediaPlayer.cs C++ MediaPlayer https://code.videolan.org/videolan/libvlcpp/blob/master/vlcpp/MediaPlayer.hpp C# EventManagers https://github.com/mfkl/LibVLCSharp/blob/master/LibVlcSharp/EventManager.cs C++ EventManagers: https://code.videolan.org/videolan/libvlcpp/blob/master/vlcpp/EventManager.hpp C# Events https://github.com/mfkl/LibVLCSharp/blob/master/LibVlcSharp/Events/LibVLCEvent.cs

I think we should choose one approach or the other, but not both. Less API surface == less confused users.

INPC (+)

• Familiarity to .NET devs.
• Easier to integrate with UIs (views and view models).

INPC (-)

• Harder to implement?
• Different from libvlcpp's approach.

EventManager (+)

• Already implemented.
• On par with libvlcpp API

EventManager (-)

• Not .NET-ty

I think we should try to implement it all with INPC. But... if we have a C# getter that directly calls the C getter and returns the value with no backing field, what do we do with the event data when for example ChapterChanged is fired?

/cc @jeremyVignelles

[jeremyVignelles]

My two cents in this discussion:

INPC +

As said, using UI constructs like <ProgressBar MinValue="0" MaxValue="{Binding Length}" Value="{Binding Position}" /> out of the box could be really useful for developpers using XAML, It feels natural.

Easy to implement in my opinion:

public float Position {
   get => libvlc_media_player_get_position();
   set => libvlc_media_player_set_position(value);
}

PositionChanged += (...) => {this.OnPropertyChanged(nameof(Position));};

INPC -

Differs from libvlcpp approach : Not only is the surface API different, but event handlers will always be registered, even if nothing is listening.

Using only INPC will probably be complex to use if someone wants to listen to events from code. I'm thinking about those who want things "The Reactive Way". Using an event is pretty easy with things like FromEventPattern, but using INPC as an event source requires a lot of if(propertyName == nameof(...)) { ... }

EventManager +

The points that you mention, plus:

• Data is passed with the event, without needing to query the instance.
• Will probably look familiar to MVC users (controller that refreshes the view on event.

EventManager -

• Requires extra work to use it in UI frameworks based on INPC

Questions:

In the .net world, are All UI frameworks using INotifyPropertyChanged?

• WPF yes
• WinForms no (but the INPC class is still present, just unused)
• UWP yes
• Xamarin.Forms yes
• Please fill me!

Do we even need to choose one?

I'm in favor of having both INPC and events, but discussion is still open

How will it be layered? Which part of the library does the event management?

We could have another class that is just there to expose INPC, based on a "core" class, which is aligned with libvlcpp. I think that adding layers could be hard to use, and I suggest merging it in the "core". again, discussion is still open

[mfkl]

I'm in favor of having both INPC and events.

OK. I'm usually in favor of restricting the API surface as much as possible, but in this case I don't think we have a choice if we want it all.

We could have another class that is just there to expose INPC, based on a "core" class, which is aligned with libvlcpp. I think that adding layers could be hard to use, and I suggest merging it in the "core". again, discussion is still open.

This part is not really clear to me.

But if I understand well what you're saying, we could do:

• Keep LibVLCSharp API as is, similar to libvlcpp with the EventManagers.
• Create a new project, LibVLCSharp.UI.Core (or something), which would basically be the MediaElementBase mentioned in #19. It would hold the INPC code and other crossplatform code.

Are we on the same page?

[mfkl]

Initial experiment https://github.com/mfkl/LibVLCSharp/blob/wpf/LibVLCSharp/MediaElementCore.cs