Thread: [Mplayerplug-in-devel] A Question and some bug reports
Brought to you by:
kdekorte
From: Chris D. <c.d...@cd...> - 2004-01-23 11:05:37
|
Hi there, I've had a couple of problems with using mplayerplug-in out of the box. Firstly, when it comes to detecting running instances of mplayer, the pipeline often as not picks up the process id of itself, then tries to kill that process (which has since terminated) leading to an almost infinite loop of attempting to kill random PIDs. This problem is solvable, the simplest way would be to add "grep -v grep" to the pipelines in function DestroyCB. But see below, as to my later question. Second problem is that in function NPP_Destroy, sometimes "This" is NULL, but the code goes on to attempt to obtain a lock on a mutex that is obtained by dereferencing This. Obviously, if This is NULL, the entire browser dies, which is not exactly helpful :) Ironically, This is checked for NULL immediately after the lock is obtained.... So, my question. I'm wondering whether the elaborate PID determining code in DestroyCB is really nessercary. Could I just rewrite PlayPlaylist to use fork() and record the PIDs of instances of mplayer as we launch them. Then when we need to get rid of them later, just kill them. Would this be worth my time to write? Cheers, C.Davies |
From: Alexandre P. da S. <ale...@po...> - 2004-01-23 14:03:11
|
I already wrote that. I am just testing my code, as there is still others bugs around. My code already gets the right pid. I changed from popen to manual pipe, fork, execv and fdopen. This also removes the need to escape the url. It still do not works correctly, the browser locks after you close the video. I think its because of pclose call. I'll correct this latter. Patch attached. Em Fri, Jan 23, 2004 at 09:16:49AM +0000, Chris Davies escreveu: > Hi there, > > I've had a couple of problems with using mplayerplug-in out of the box. > > Firstly, when it comes to detecting running instances of mplayer, the pipeline often as not picks > up the process id of itself, then tries to kill that process (which has since terminated) leading > to an almost infinite loop of attempting to kill random PIDs. > > This problem is solvable, the simplest way would be to add "grep -v grep" to the pipelines in > function DestroyCB. But see below, as to my later question. > > Second problem is that in function NPP_Destroy, sometimes "This" is NULL, but the code > goes on to attempt to obtain a lock on a mutex that is obtained by dereferencing This. > Obviously, if This is NULL, the entire browser dies, which is not exactly helpful :) > Ironically, This is checked for NULL immediately after the lock is obtained.... > > So, my question. I'm wondering whether the elaborate PID determining code in DestroyCB is really > nessercary. Could I just rewrite PlayPlaylist to use fork() and record the PIDs of instances of > mplayer as we launch them. Then when we need to get rid of them later, just kill them. > > Would this be worth my time to write? > > Cheers, > C.Davies > > > ------------------------------------------------------- > The SF.Net email is sponsored by EclipseCon 2004 > Premiere Conference on Open Tools Development and Integration > See the breadth of Eclipse activity. February 3-5 in Anaheim, CA. > http://www.eclipsecon.org/osdn > _______________________________________________ > Mplayerplug-in-devel mailing list > Mpl...@li... > https://lists.sourceforge.net/lists/listinfo/mplayerplug-in-devel -- Alexandre Pereira da Silva http://aletes.is-a-geek.net:2000/ Tels: (11) 93830810(cel), 37271018(casa) |
From: Kevin D. <kde...@ya...> - 2004-01-23 14:11:30
|
Chris, Ok, the kill code is only entered after the attempts to shutdown mplayer gracefully have failed. I have added the grep -v grep to that, but I have never seen it get into the loop you describe. In NPP_Destroy I have added the check for the This==NULL and exit out if it is. I have not seen the code crash there either, but the safety check is good to have. As for replacing the popen with fork.. no don't want to do that. The code used to work that way and was extra complicated. For a couple of reasons. 1. We needed to do a fork inside of the fork in order to keep the browser responding. 2. Needed to used shared memory blocks to pass PID's around between the parent and the childs child. You can look at 0.90 of the plugin to see the fork code. 3. I'm not sure how well some things like fifo's and such work over forks. Because the fork closes file descriptors for the child. 4. X synchronization, sometimes updated the GUI from the fork would cause bad crashes for no reason. I found that since X is picky about forks it is better not to update X from within a child process. 5. I actually looked pretty hard at phreads because they don't have some of these issues, but the popen seemed to be the better solution. Kevin On Friday 23 January 2004 2:16 am, Chris Davies wrote: > Hi there, > > I've had a couple of problems with using mplayerplug-in out of the box. > > Firstly, when it comes to detecting running instances of mplayer, the > pipeline often as not picks up the process id of itself, then tries to kill > that process (which has since terminated) leading to an almost infinite > loop of attempting to kill random PIDs. > > This problem is solvable, the simplest way would be to add "grep -v grep" > to the pipelines in function DestroyCB. But see below, as to my later > question. > > Second problem is that in function NPP_Destroy, sometimes "This" is NULL, > but the code goes on to attempt to obtain a lock on a mutex that is > obtained by dereferencing This. Obviously, if This is NULL, the entire > browser dies, which is not exactly helpful :) Ironically, This is checked > for NULL immediately after the lock is obtained.... > > So, my question. I'm wondering whether the elaborate PID determining code > in DestroyCB is really nessercary. Could I just rewrite PlayPlaylist to use > fork() and record the PIDs of instances of mplayer as we launch them. Then > when we need to get rid of them later, just kill them. > > Would this be worth my time to write? > > Cheers, > C.Davies > > > ------------------------------------------------------- > The SF.Net email is sponsored by EclipseCon 2004 > Premiere Conference on Open Tools Development and Integration > See the breadth of Eclipse activity. February 3-5 in Anaheim, CA. > http://www.eclipsecon.org/osdn > _______________________________________________ > Mplayerplug-in-devel mailing list > Mpl...@li... > https://lists.sourceforge.net/lists/listinfo/mplayerplug-in-devel |
From: Alexandre P. da S. <ale...@po...> - 2004-01-29 15:02:01
Attachments:
mplayerplug-in.patch
|
Em Fri, Jan 23, 2004 at 07:10:06AM -0700, Kevin DeKorte escreveu: > Chris, > > Ok, the kill code is only entered after the attempts to shutdown mplayer > gracefully have failed. I have added the grep -v grep to that, but I have > never seen it get into the loop you describe. > > In NPP_Destroy I have added the check for the This==NULL and exit out if it > is. I have not seen the code crash there either, but the safety check is good > to have. > > As for replacing the popen with fork.. no don't want to do that. The code used > to work that way and was extra complicated. For a couple of reasons. > > 1. We needed to do a fork inside of the fork in order to keep the browser > responding. We dont, my code dont need that. I think that setsid and the like solved that. The code is not complicated, and you dont need shm as before because you dont fork 2 times. > 2. Needed to used shared memory blocks to pass PID's around between the parent > and the childs child. You can look at 0.90 of the plugin to see the fork > code. > > 3. I'm not sure how well some things like fifo's and such work over forks. > Because the fork closes file descriptors for the child. > > 4. X synchronization, sometimes updated the GUI from the fork would cause bad > crashes for no reason. I found that since X is picky about forks it is better > not to update X from within a child process. But we dont do that. Mplayer does, but its paiting on a swallowed window. > 5. I actually looked pretty hard at phreads because they don't have some of > these issues, but the popen seemed to be the better solution. > > Kevin One thing I didnt like is that I had to remove the sendCommand("quit") to make it work. But kill does the same job anyway. This code also fixes that bug that I told you kevin. When there is no window, the only place to kill mplayer is inside NPP_Destroy. Patch attached. -- Alexandre Pereira da Silva http://aletes.is-a-geek.net:2000/ |