Garteal / MPDN_Extensions

Media Player .Net (MPDN) Open Source Extensions
Other
0 stars 0 forks source link

Error Message when playing/pausing + crash when loading file #19

Closed Belphemur closed 9 years ago

Belphemur commented 9 years ago

Hello @Garteal,

Since the merge of your branch there is an error message that appear everytime I play/pause an MKV file. I debugged it and found the culprit:

https://github.com/Garteal/MPDN_Extensions/blob/master/Extensions/PlayerExtensions/PlaylistForm.cs#L1870-L1875

This is throwing an InvalidOperationException that you pass to the Player.HandleException. The problem is ... the player don't really handle it but show an Error Message (popup) which is pretty annoying.

Could you make that exception as Trace.write() instead of Player.HandleException until there is a better mechanism to track and log exceptions ?

Garteal commented 9 years ago

Odd, I haven't encountered this and I cannot reproduce this. I can't see any error messages either and I have the debug option enabled. I've also tried to have it show a MessageBox, but none were shown after opening a file (any file really, including MKVs).

As far as I can tell the Player.HandleException seems to close down the player whenever an exception shows up (or maybe certain type of exceptions), so I'm not sure if that part is the issue.

If you get a error box, do you also get to see the stacktrace? If so, it would be nice if you could paste that here. I'd love to fix this if there's an issue.

Belphemur commented 9 years ago

Okay this is very very interesting. If I drag and drop the file into the player (x64), I have no error message on play/pause. If I open the file from the explorer (since mkv is assigned to it) I get the error message Like This This is a weird error message since it's the usual with stack trace. It look like a debugging/warning message. (Since the player don't crash).

Then if I use the AnyCPU version (the one I used in development), if I drag & drop, it just crash (with the line I gave you).

Okay It seems the problem with the MessageBox Error has nothing to do with those line ... I'm going to try with the x64 in debug mode to see if I can find where does that error is coming from.

Belphemur commented 9 years ago

Okay I found where the error message is coming from:

https://github.com/Garteal/MPDN_Extensions/blob/master/Extensions/PlayerExtensions/PlaylistForm.cs#L1134

Error:

NullPointerException {"Object reference not set to an instance of an object."} dgv_PlayList.CurrentCell is NULL

Stacktrace:

   at Mpdn.Extensions.PlayerExtensions.Playlist.PlaylistForm.HandleContextMenu() in d:\VS\MPDN_Extensions\Extensions\PlayerExtensions\PlaylistForm.cs:line 1134
   at Mpdn.Extensions.PlayerExtensions.Playlist.PlaylistForm.PlayerStateChanged(Object sender, PlayerStateEventArgs e) in d:\VS\MPDN_Extensions\Extensions\PlayerExtensions\PlaylistForm.cs:line 1361
   at System.EventHandler`1.Invoke(Object sender, TEventArgs e)
   at Mpdn.PlayerControl.OnPlayerStateChanged(PlayerState playerState, PlayerState oldState)
   at MediaPlayerDotNet.MainForm.<>c__DisplayClass3d.<SetPlayerState>b__3c()
   at Mpdn.Utilities.ControlHelpers.HandleAsync()

Btw I do get the crash also with the x64 in debug mode when it reach the Player.HandleException for the previous piece of code.

Garteal commented 9 years ago

Nice debugging. Thanks for doing that. Should be a quick fix. Interesting how this never surfaced until now.

Belphemur commented 9 years ago

Yeah I have really no idea how MPDN was doing before with a raised Exception. And why this doesn't happen on drag & drop... seems MPDN is loading the file differently than with path.

Sure no problem, in fact I love debugging !

Garteal commented 9 years ago

Just pushed to my fork, can you check if you still get any (other) crashes? Btw does MPDN handle the Trace.Write call? And if so, where does it output?

Belphemur commented 9 years ago

AnyCPU -> OK x64 -> Drag & Drop = Crash on Player.HandleException(ex); https://github.com/Garteal/MPDN_Extensions/blob/master/Extensions/PlayerExtensions/PlaylistForm.cs#L1870-L1875

Belphemur commented 9 years ago

Forget the previous message. I had a problem with the branch and my clone of your github repo.

Now that I tested it with the right branch. I'm still getting an Exception triggered by this: https://github.com/Garteal/MPDN_Extensions/blob/master/Extensions/PlayerExtensions/PlaylistForm.cs#L1870-L1875

It only happen when launching MPDN from command line (or from the windows explorer on a file).

It's a thread problem I'm sure of that, replace that code by this:

                GuiThread.DoAsync(() =>
                {
                    if (dgv_PlayList.Rows.Count < 1) return;
                    dgv_PlayList.Rows[currentPlayIndex].Cells["Duration"].Value = time.ToString(@"hh\:mm\:ss");
                    dgv_PlayList.InvalidateRow(currentPlayIndex);
                });

And the problem is resolved. This way you are sure this code is always runned by the GuiThread.

Garteal commented 9 years ago

It only happen when launching MPDN from command line (or from the windows explorer on a file).

Yeah they should execute the same piece of code, though I'm not doing anything with that except for when it's a playlist file.

It's a thread problem I'm sure of that, replace that code by this:

Well let's find out. I can't do the durations for all media async though as things will break and I'm not sure if there will still be an issue if I don't change it, but let's see.

Still can't reproduce and I'm running everything from VS2013 with 64-bit MPDN. Made MPDN the default player of MKVs and tried it standalone aswell. No dice.

Belphemur commented 9 years ago

I tried with GuiThread.Do and it works (also put a breakpoint into the runned code, it reached it without problem). I think you reach a race condition since the previous error was: {"Invoke or BeginInvoke cannot be called on a control until the window handle has been created."}

When using the GuiThread (Proxy for PlayerControl.VideoPanel.VideoBox) it seems to wait for the creation of the window handle and avoid the race condition.

In case of, the playlist is not visible for me, I let it with the default settings. I had to configure VS 2013 to catch the System.InvalidOperationException (ctrl + alt + e -> System -> System.InvalidOperationException -> Check Thrown).

Garteal commented 9 years ago

Cool, I think using the GuiThread proxy is the proper way to do this anyway. Wasn't aware of it until now. I've pushed to my fork again changing both functions to use the GuiThread.

In case of, the playlist is not visible for me, I let it with the default settings.

Well it's not visible by default. Need to check an option in the settings to have it show on start-up if you don't want to manually open it each time.

Belphemur commented 9 years ago

Just reported it in case it affect the bug or not :)

Yup it's the proper way, when I was using Invoke before (for OpenSubtitles), @zachsaw corrected my code by using the PlayerControl.VideoPanel that got replaced by Gui and GuiThread.

I confirm all the issues are now resolved. Thanks for the quick reply :)

zachsaw commented 9 years ago

Fantastic stuff. With so many changes I'll be doing a release soon but I'm away at the moment so haven't been particularly active here or in the forum. There's also a few MPDN bugs I need to attend to when I get back so we may not get a new release until I get some time to work on it.

Garteal commented 9 years ago

We've noticed ;p. Yeah some have been reported in the thread already, but there's one more I'd like to report but your inbox is full. Would you like me to just put it in the thread? It's related to dev so hence why I didn't.

zachsaw commented 9 years ago

Oh didn't even realise my inbox is full. I'll clear it in just a tick.

EDIT: Done.