Implemented SetMouseInput and SetKeyInput #424
Conversation
I implemented the delegates for libvlc_video_set_mouse_input and libvlc_video_set_key_input. These values are true by default in libvlc. I chose to make them default to false for the vlcControl. libvlc implements these functions in a non-intuitive way. They only take effect when the next video plays. Also, for mouse inputs to work BOTH libvlc_video_set_mouse_input and libvlc_video_set_key_input have to be set to false.
|
Thanks a lot for that, I will need to have a look at this, don't hesitate to ping me in a few days if I forget (quite busy right now) I'm not sure if we should change the default however. If libvlc sets this to false, so should we. There should be a reason... |
Changed the default values for libvlc_video_set_mouse_input and libvlc_video_set_ky_input to true. I also updated the WinForms.Advanced sample to reflect the change.
| @@ -46,5 +48,15 @@ public string Deinterlace | |||
| public IMarqueeManagement Marquee { get; private set; } | |||
| public ILogoManagement Logo { get; private set; } | |||
| public IAdjustmentsManagement Adjustments { get; private set; } | |||
|
|
|||
| public bool IsMouseInputDisabled | |||
There was a problem hiding this comment.
I'd prefer "positive" naming here, and everywhere else. Please use IsMouseInputEnabled and IsKeyInputEnabled (This avoids double negation confusion)
| { | ||
| public sealed partial class VlcManager | ||
| { | ||
| public void SetKeyInput(VlcMediaPlayerInstance mediaPlayerInstance, bool status) |
There was a problem hiding this comment.
rename "status" to "enabled" or "on" (to make it on par with vlc's naming)
There was a problem hiding this comment.
In my experiments, those methods have no effect once the video has started playing. Do you think we should make that clear? (A comment above saying : "Must be called before the stream has started playing"). What do you think about that?
| { | ||
| if (mediaPlayerInstance == IntPtr.Zero) | ||
| throw new ArgumentException("Media player instance is not initialized."); | ||
| GetInteropDelegate<SetKeyInput>().Invoke(mediaPlayerInstance, Convert.ToUInt32(status ? 1 : 0)); |
There was a problem hiding this comment.
You can use 1u / 0u to avoid the conversion
| { | ||
| public sealed partial class VlcManager | ||
| { | ||
| public void SetMouseInput(VlcMediaPlayerInstance mediaPlayerInstance, bool status) |
There was a problem hiding this comment.
same comments apply here
|
|
||
| public bool IsMouseInputDisabled | ||
| { | ||
| set { myManager.SetMouseInput(myMediaPlayer, value); } |
There was a problem hiding this comment.
Please check the logic here. If "disabled" is true, then "state" in SetMouseInput is true and 1 is passed, which means on. Unless I'm missing something, that logic is broken, but will be OK once you have performed the renaming above.
There was a problem hiding this comment.
Please note that "enabled" means "enable handling by libvlc"
| @@ -16,6 +16,8 @@ public VideoManagement(VlcManager manager, VlcMediaPlayerInstance mediaPlayerIns | |||
| Marquee = new MarqueeManagement(manager, mediaPlayerInstance); | |||
| Logo = new LogoManagement(manager, mediaPlayerInstance); | |||
| Adjustments = new AdjustmentsManagement(manager, mediaPlayerInstance); | |||
| IsMouseInputDisabled = true; | |||
There was a problem hiding this comment.
You can remove the initialization, it's the default
…cation to the perspective of the libvlc. Switched the Enable/Disabled naming from the perspective of the application to the perspective of the libvlc.
|
I have no strong opinions about any of the changes you requested. I think I have made all of the changes you have requested. Thanks for being very specific about the changes you wanted. Have a nice day. |
Oops looks Like a I missed a small problem with references.
|
I'm not sure how to resolve this errors here. I don't want to mess up your repository so I am going to just close this pull request. Sorry to have wasted your time on this one. |
This reverts commit bea4b83.
|
Ok, I have reverted that change to the references. I'm going to try and reopen this pull request. |
|
Otherwise that's fine, thanks for your PR |
|
I forgot : could you also update the changelog.md with a link to this PR? |
|
I merged this manually, thanks for your contribution! |
New to the project I hope I put stuff in the right spot. Feel free to tell me what I did wrong.
I implemented the delegates for libvlc_video_set_mouse_input and libvlc_video_set_key_input. These values are true by default in libvlc. I chose to make them default to false for the vlcControl.
I also left in the changes I made to the WinForms.Advanced Sample that I used for testing.
libvlc implements these functions in a non-intuitive way. They only take effect when the next video plays. Also, for mouse inputs to work BOTH libvlc_video_set_mouse_input and libvlc_video_set_key_input have to be set to false.