From: Stas S. <st...@ak...> - 2007-07-06 18:30:25
Attachments:
logo_lk.diff
|
Hello. The race condition I was chaising today, was in gui_display_logo(). If after xine_stop(), some other thread (stdctl one, in my case) starts the playback, it doesn't reset gGui->logo_mode, because gGui->logo_mode==2, and there is a check in _gui_xine_play() to not reset it when it is ==2. Then, because I use --no-logo, the logo is not played, gui_display_logo() sets gGui->logo_mode=1 and exits. Now the movie is playing with gGui->logo_mode==1, which should never happen. When movie finishes, the check in event_listener() thinks it was a logo movie, and prevents the finish event from being processed. The attached patch fixes the problem. At least, it covers the code path that my program uses, but there can be more instances of that race. Actually, there seem to be a lot of races in xine-ui, as if it was never developed with thread-safety in the mind. As a result, xine keeps crashing on me when I try to control it via stdctl. Let's see whether or not it will be possible for me to track down the necessary amount of races to at least make my simple program to work reliably. :) |
From: Richard A. <ri...@ys...> - 2007-07-06 18:39:02
|
On Fri, 2007-07-06 at 22:30 +0400, Stas Sergeev wrote: > Actually, there seem to be a lot of races in > xine-ui, as if it was never developed with > thread-safety in the mind. As a result, xine > keeps crashing on me when I try to control it > via stdctl. Let's see whether or not it will > be possible for me to track down the necessary > amount of races to at least make my simple program > to work reliably. :) YSTV (http://ystv.york.ac.uk/) went for the easy way out of this for full-screen video playback under remote control, and wrote a minimal, custom CLI to xine-lib. The source is GPL so I can dig out the link to it if you are interested. It's pretty much feature free (in the sense of, it does play, stop and query commands) but is generally very reliable. Richard Ash |
From: Stas S. <st...@ak...> - 2007-07-06 20:14:02
|
Hello. Richard Ash wrote: > YSTV (http://ystv.york.ac.uk/) went for the easy way out of this for > full-screen video playback under remote control, and wrote a minimal, > custom CLI to xine-lib. The source is GPL so I can dig out the link to > it if you are interested. Yes, I am very interested. It would be pity to move away from xine-ui now, when I got all the features I needed. But as long as it simply crashes, I probably don't have a lot of a choice... > It's pretty much feature free (in the sense > of, it does play, stop and query commands) but is generally very > reliable. As for the features - I really need only very few. I need a play-list, from which my prog can choice any video at any moment, and I need to always get the status (movie finished, user pressed a key, etc), and that's pretty much about it. Using xine-ui for just that, is probably an overkill from the very beginning, but I wasn't aware about an alternatives. |
From: Hans-Dieter K. <hd...@t-...> - 2007-07-07 00:14:01
|
Hi, Stas Sergeev wrote: > I got all the features I needed. But as long > as it simply crashes, I probably don't have > a lot of a choice... > Why, the hell, should we both together not be able to get it crash-free ;-) Hans-Dieter |
From: Richard A. <ri...@ys...> - 2007-07-09 11:34:07
|
On Sat, 2007-07-07 at 00:13 +0400, Stas Sergeev wrote: > Richard Ash wrote: > > YSTV (http://ystv.york.ac.uk/) went for the easy way out of this for > > full-screen video playback under remote control, and wrote a minimal, > > custom CLI to xine-lib. The source is GPL so I can dig out the link to > > it if you are interested. > Yes, I am very interested. It would be > pity to move away from xine-ui now, when > I got all the features I needed. But as long > as it simply crashes, I probably don't have > a lot of a choice... > > It's pretty much feature free (in the sense > > of, it does play, stop and query commands) but is generally very > > reliable. > As for the features - I really need only very > few. I need a play-list, from which my prog > can choice any video at any moment, and I need > to always get the status (movie finished, user > pressed a key, etc), and that's pretty much about > it. Using xine-ui for just that, is probably an > overkill from the very beginning, but I wasn't > aware about an alternatives. The code is in SVN at http://ystv.york.ac.uk/software/svn/software/xine-net/trunk/ (it's not mine to start with, but I seem to be looking after it at the moment). It doesn't do any keystroke handling or playlist stuff at all - it just opens a telnet socket through which you can send a few basic commands and get a message when a video starts and stops. You should be able to run a separate control application in the foreground to catch all the user's key strokes, and then just have that send the relevant commands over the socket to the back end as required, and listen to the socket for video end messages. Richard |
From: Hans-Dieter K. <hd...@t-...> - 2007-07-07 00:05:37
|
Hi, Stas Sergeev wrote: > > Actually, there seem to be a lot of races in > xine-ui, as if it was never developed with > thread-safety in the mind. As a result, xine > keeps crashing on me when I try to control it > via stdctl. Let's see whether or not it will > be possible for me to track down the necessary > amount of races to at least make my simple program > to work reliably. :) > I resolved some races in the past which I experienced myself. The problem is that you can't track them down if you don't experience them. So it would be great if you could help :-) Hans-Dieter |
From: Stas S. <st...@ak...> - 2007-07-07 11:14:30
|
Hi. Hans-Dieter Kosch wrote: > I resolved some races in the past which I experienced myself. The > problem is that you can't track them down if you don't experience them. > So it would be great if you could help :-) I am trying, but the deadline for my program is nearing with the great pace, and that makes me nervous. :) Next week I'll see about creating a simple test-case, and if that succeeds, then maybe I can demonstrate the problem to you. > Why, the hell, should we both together > not be able to get it crash-free ;-) Most likely because we both do not have enough time to work on that. :) Also, it doesn't look like the xine-ui code is properly structured, so even if you find the race, it is still difficult to fix it properly and completely. Re: [xine-devel] [patch][xine-ui] protect mmk with lock > Which other patch? I've lost the oversight at the moment ;-) The mmk patch depends on the SelectMrl patch. The SelectMrl patch cleans up and narrows the code in gui_playlist_start_next(), and after that cleanup, it became possible for me to protect that code path with the mutex. |
From: Stas S. <st...@ak...> - 2007-07-12 19:33:48
|
Hi. Hans-Dieter Kosch wrote: > I resolved some races in the past which I experienced myself. The > problem is that you can't track them down if you don't experience them. > So it would be great if you could help :-) OK. There is a race here, which I tracked but don't know how to solve. When xine reaches an end of the stream, it sends an event to xine-ui. In a mean time, the user (directly or via stdctl) changes the movie. The PLAYBACK_FINISHED event is not yet received, and the new playback is started. Then the PLAYBACK_FINISHED event is finally delivered, and it interrupts the just started playback! Something have to be protected with the mutex here, but what is to protect, if the event is generated by xine-lib, while the new playback is started by xine-ui? Should the xine-ui pass the mutex to xine-lib so that xine-lib can lock it, or what? I failed to find a clean solution to that, so I had to apply some hacks to work around. Do you have any ideas of how to fix this race? |
From: Hans-Dieter K. <hd...@t-...> - 2007-07-14 23:43:16
|
Hi, Stas Sergeev wrote: > Hi. > > Hans-Dieter Kosch wrote: > >> I resolved some races in the past which I experienced myself. The >> problem is that you can't track them down if you don't experience them. >> So it would be great if you could help :-) > > OK. There is a race here, which I tracked > but don't know how to solve. > When xine reaches an end of the stream, it > sends an event to xine-ui. In a mean time, > the user (directly or via stdctl) changes > the movie. The PLAYBACK_FINISHED event is not > yet received, and the new playback is started. > Then the PLAYBACK_FINISHED event is finally > delivered, and it interrupts the just started > playback! > Something have to be protected with the mutex > here, but what is to protect, if the event is > generated by xine-lib, while the new playback > is started by xine-ui? Should the xine-ui > pass the mutex to xine-lib so that xine-lib > can lock it, or what? I failed to find a clean > solution to that, so I had to apply some hacks > to work around. > Do you have any ideas of how to fix this race? > I wouldn't touch xine-lib as this is a UI problem. I have no clear idea at the moment. Maybe we can fix it by a counter in xine-ui "number of playback starts against number of playback finished events", but we must be sure that the counter doesn't get out of sync. Hans-Dieter |
From: Stas S. <st...@ak...> - 2007-07-15 13:07:36
|
Hi. Hans-Dieter Kosch wrote: >> Do you have any ideas of how to fix this race? > I wouldn't touch xine-lib as this is a UI problem. Yes, but only xine-lib knows that the stream have finished, so only it can lock the mutex at the right moment. It would be nice to get that solved on xine-ui level, but I think some help from xine-lib is unavoidable. > at the moment. Maybe we can fix it by a counter in xine-ui "number of > playback starts against number of playback finished events", but we must > be sure that the counter doesn't get out of sync. I don't know what to do with such a counter. You can't have more than one playback started, so what it to count? |
From: Thibaut M. <thi...@gm...> - 2007-07-16 09:01:05
|
Hi, On 7/15/07, Stas Sergeev <st...@ak...> wrote: > Hi. > > Hans-Dieter Kosch wrote: > >> Do you have any ideas of how to fix this race? > > I wouldn't touch xine-lib as this is a UI problem. > Yes, but only xine-lib knows that the stream > have finished, so only it can lock the mutex > at the right moment. It would be nice to get > that solved on xine-ui level, but I think some > help from xine-lib is unavoidable. I can't find the original message from Hans-Dieter, what race are you talking about ? > > at the moment. Maybe we can fix it by a counter in xine-ui "number of > > playback starts against number of playback finished events", but we must > > be sure that the counter doesn't get out of sync. > I don't know what to do with such a counter. > You can't have more than one playback started, > so what it to count? You can use more than one xine stream. IIRC xine-ui uses 2 or 3 xine streams (it was the case 4 years ago). Thibaut |
From: Stas S. <st...@ak...> - 2007-07-16 10:09:59
|
Hello. Thibaut Mattern wrote: > I can't find the original message from Hans-Dieter, > what race are you talking about ? Right, I have changed the subject, as we started to discuss the unrelated things under the same topic. Here is the original msg: http://sourceforge.net/mailarchive/message.php?msg_name=4696822D.2060308%40aknet.ru >> You can't have more than one playback started, >> so what it to count? > You can use more than one xine stream. IIRC xine-ui uses 2 or 3 xine > streams (it was the case 4 years ago). Ah, ok. Though looking at the code, it looks mostly like the old playback stops when the new stream opens, but I may be missing something. Still, I am not sure how such a counter can be used to avoid the aforementioned race. |
From: Hans-Dieter K. <hd...@t-...> - 2007-07-16 23:29:50
|
Hi, Stas Sergeev wrote: > >> at the moment. Maybe we can fix it by a counter in xine-ui "number of >> playback starts against number of playback finished events", but we >> must be sure that the counter doesn't get out of sync. > > I don't know what to do with such a counter. > You can't have more than one playback started, > so what it to count? > I thought about a use count: Increment on each playback start, decrement on each finished event, handle event when counter has dropped to zero. But maybe I'm totally wrong ... Hans-Dieter |
From: Stas S. <st...@ak...> - 2007-07-17 03:07:06
|
Hi. Hans-Dieter Kosch wrote: > I thought about a use count: Increment on each playback start, decrement > on each finished event, handle event when counter has dropped to zero. How can this ever work? You start the playback, counter increments. You start another playback, interrupting the previous one in the middle. Counter increments again. Now the counter==2. The playback stops - counter==1. It will never reach zero again. Because if you interrupt the playback without letting it to finish, the event is not generated. Or? |
From: Hans-Dieter K. <hd...@t-...> - 2007-07-19 23:54:19
|
Hi, Stas Sergeev wrote: > Hans-Dieter Kosch wrote: > >> I thought about a use count: Increment on each playback start, decrement >> on each finished event, handle event when counter has dropped to zero. > > How can this ever work? > You start the playback, counter increments. > You start another playback, interrupting > the previous one in the middle. Counter > increments again. Now the counter==2. The > playback stops - counter==1. It will never > reach zero again. Because if you interrupt > the playback without letting it to finish, > the event is not generated. > Or? OK, as I said, I may be wrong. If the counter gets out of sync, this idea will definitely not work. Hans-Dieter |
From: Thibaut M. <thi...@gm...> - 2007-07-17 10:27:11
|
On 7/17/07, Stas Sergeev <st...@ak...> wrote: > Hi. > > Hans-Dieter Kosch wrote: > > I thought about a use count: Increment on each playback start, decrement > > on each finished event, handle event when counter has dropped to zero. > How can this ever work? > You start the playback, counter increments. > You start another playback, interrupting > the previous one in the middle. Counter > increments again. Now the counter==2. The > playback stops - counter==1. It will never > reach zero again. Because if you interrupt > the playback without letting it to finish, > the event is not generated. > Or? That's right. The event is not generated if the playback is interrupted by the frontend. You've currently no way to know which stream events are related to, and no way to synchronize the event loop with the open or stop calls. There is several ways to fix the problem. Idea 1 : Create a new event and send it when a playback is interrupted by the frontend. This way you could implement your counter idea without breaking the current xine-lib API. Idea 1bis : Create a new event and send it when a xine_open call succeded. Idea 2 : Introduce the concept of stream id, which could be a counter incremented by the lib each time the frontend calls xine_open (the behavior of xine_open could be changed to return the id instead of 1 in case of success), then events could be tagged with this id. regards Thibaut |
From: Stas S. <st...@ak...> - 2007-07-17 12:02:07
|
Hi. Thibaut Mattern wrote: > That's right. The event is not generated if the playback is > interrupted by the frontend. > You've currently no way to know which stream events are related to, > and no way to synchronize the event loop with the open or stop calls. > There is several ways to fix the problem. > Idea 1 : Create a new event and send it when a playback is > Idea 2 : Introduce the concept of stream id, which could be a counter Oh, both ideas look nice. :) I even think the combined approach can be used: introduce an interruption event, as well as tag the events with the stream ID. But it may take some time before I start coding this, so if someone else wants to solve that, it would be brilliant. :) |
From: Hans-Dieter K. <hd...@t-...> - 2007-07-25 23:23:43
|
Hi again, Stas Sergeev wrote: > Hans-Dieter Kosch wrote: > >> I thought about a use count: Increment on each playback start, decrement >> on each finished event, handle event when counter has dropped to zero. > > How can this ever work? > You start the playback, counter increments. > You start another playback, interrupting > the previous one in the middle. Counter > increments again. Now the counter==2. The > playback stops - counter==1. It will never > reach zero again. Because if you interrupt > the playback without letting it to finish, > the event is not generated. > Or? (OK, as I said, I may be wrong. If the counter gets out of sync, this idea will definitely not work.) Just a new idea: The xine finished event carries a creation timestamp. We could create a xine-ui playback start timestamp and compare it against the finished timestamp. I suspect, in this race, the finished timestamp is earlier or only little later than the new start timestamp. As I suppose that video sequences are not indefinitely short, we could ignore the finished event during a well defined small guard time after playback start. Or am I again wrong? What do you think, would you give it a try? Hans-Dieter |
From: Stas S. <st...@ak...> - 2007-07-26 11:57:48
|
Hi. Hans-Dieter Kosch wrote: > The xine finished event carries a creation timestamp. We could create a > xine-ui playback start timestamp and compare it against the finished > timestamp. I suspect, in this race, the finished timestamp is earlier or > only little later than the new start timestamp. As I suppose that video > sequences are not indefinitely short, we could ignore the finished event > during a well defined small guard time after playback start. The video sequences are not indefinitely short, but there is some buffering involved, esp in a gapless mode, in which case, for the relatively short video, the finish event may follow the start event without a delay (entire video buffered). Or is this wrong? I haven't checked, just a guess. But in any case, this is a heuristic. How the "well defined small guard time" can really be well-defined? Could you please list an advantages of that approach over the previously discussed one? > Or am I again wrong? I don't think you were wrong with the counter, you just haven't provided enough details for me to understand it. :) Nevertheless, having the stream IDs makes the counter unnecessary too. > What do you think, would you give it a try? I am having a vacation now, my prog is unavailable to me, and it will remain so for a few weeks more. So if you really think this approach is worth trying, you can use that delay to explain me why so. :) |
From: Hans-Dieter K. <hd...@t-...> - 2007-07-27 01:17:38
|
Hi, Stas Sergeev wrote: > The video sequences are not indefinitely > short, but there is some buffering involved, > esp in a gapless mode, in which case, for > the relatively short video, the finish > event may follow the start event without > a delay (entire video buffered). Or is this > wrong? I haven't checked, just a guess. If such an immediate finish event would occur, we would never be able to see the video in xine-ui. The event occures when the buffer has played up to the end. > But in any case, this is a heuristic. How > the "well defined small guard time" can > really be well-defined? Of course it's heuristic, it should be evaluated by looking at the shortest usual playing times (I don't suppose you are using such psychodelic advertising sequences in fractions of a second). > Could you please list an advantages of that > approach over the previously discussed one? A lock would require deeper intrusion into xine-lib and the counter approach won't work (see below). > > I don't think you were wrong with the counter, > you just haven't provided enough details for me > to understand it. :) We discussed it already. My counter idea is wrong since we can't rely on a one-to-one matching of the number of start events and the number of finish events in order to cause the counter to reach zero at the right time. > Nevertheless, having the > stream IDs makes the counter unnecessary too. Do you mean gGui->stream? This is a channel established between lib and ui and won't change between the videos. I'm wondering if the "data" member of the xine_event_t struct keeps some useful information... > >> What do you think, would you give it a try? > > I am having a vacation now, my prog is unavailable > to me, and it will remain so for a few weeks more. > So if you really think this approach is worth > trying, you can use that delay to explain me why > so. :) > I simply think, a finish event related to the started stream cannot occur before the start time of the stream plus its duration. So, if we receive an event earlier, it can't belong to that stream (it must come from the previous one, what's exactly your race problem) and should be ignored. In order to avoid determining the duration of the actually played stream, I suggested to choose a "well defined" short time (e.g. 0.5sec) sufficient for at least most of the usual streams. There may be uncertainties (see heuristic above), but I think it could be better than nothing... Hans-Dieter |
From: Stas S. <st...@ak...> - 2007-07-27 10:21:33
|
Hi. Hans-Dieter Kosch wrote: > If such an immediate finish event would occur, we would never be able to > see the video in xine-ui. In a nearby thread I was raising a problem that in a gapless mode you can't switch a video quickly - you have to wait till the beffered data gets all displayed, which takes up to 20sec for me sometimes. During that period the xine-ui is "not responding". So no matter what xine-ui would do with that event, it can't interrupt the playback till the buffers are flushed. So yes, we would still be able to see the video. To me this is clearly a bug, but other people disagree. > The event occures when the buffer has played > up to the end. OK, I beleive. :) > A lock would require deeper intrusion into xine-lib and the counter > approach won't work (see below). A lock will basically not work, that's what I do agree with. >> I don't think you were wrong with the counter, >> you just haven't provided enough details for me >> to understand it. :) > We discussed it already. My counter idea is wrong since we can't rely on > a one-to-one matching of the number of start events and the number of > finish events in order to cause the counter to reach zero at the right time. We discussed it already too. :) You might have missed these postings, probably because I was changing a subject: http://sourceforge.net/mailarchive/forum.php?thread_name=20070717210528.11053.qmail%40ystv.york.ac.uk&forum_name=xine-devel (see the quoted message in the bottom) > Do you mean gGui->stream? No, I mean the "Idea 2" in the aforementioned message. > 0.5sec) sufficient for at least most of the usual streams. There may be > uncertainties (see heuristic above), but I think it could be better than > nothing... It may be better than nothing, but how about being better than what's suggested by an URL above? |
From: Hans-Dieter K. <hd...@t-...> - 2007-07-28 22:56:08
|
Hi, Stas Sergeev wrote: > Hans-Dieter Kosch wrote: > >> If such an immediate finish event would occur, we would never be able to >> see the video in xine-ui. > > In a nearby thread I was raising a problem > that in a gapless mode you can't switch a > video quickly - you have to wait till the > beffered data gets all displayed, which takes > up to 20sec for me sometimes. During that > period the xine-ui is "not responding". > So no matter what xine-ui would do with that > event, it can't interrupt the playback till > the buffers are flushed. > So yes, we would still be able to see the video. > To me this is clearly a bug, but other people > disagree. > What should I say? That seems to be an issue of xine-lib, I'm not involved. > > We discussed it already too. :) You might have > missed these postings, probably because > I was changing a subject: > http://sourceforge.net/mailarchive/forum.php?thread_name=20070717210528.11053.qmail%40ystv.york.ac.uk&forum_name=xine-devel > I didn't miss it, but forgot for the moment :-) > >> Do you mean gGui->stream? > > No, I mean the "Idea 2" in the aforementioned message. > OK, I see. >> 0.5sec) sufficient for at least most of the usual streams. There may be >> uncertainties (see heuristic above), but I think it could be better than >> nothing... > > It may be better than nothing, but how about > being better than what's suggested by an URL above? > I agree, but that's beyond my competence. We depend on the guys maintaining xine-lib. So, I can do nothing further besides asking: Hi, xine-lib people, can anybody contribute? Hans-Dieter |
From: Stas S. <st...@ak...> - 2007-07-11 18:03:45
Attachments:
logo_lk1.diff
|
Stas Sergeev wrote: > At least, it covers the code path that my program > uses, but ...breaks the rest, no fun. Well, as a minimal change, I can just make the mutex recursive. |
From: Hans-Dieter K. <hd...@t-...> - 2007-07-14 23:25:24
|
Hi, Stas Sergeev wrote: > Stas Sergeev wrote: > >> At least, it covers the code path that my program >> uses, but > > ...breaks the rest, no fun. > Well, as a minimal change, I can just make > the mutex recursive. > I already noticed that and committed your patch with the static initializer PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP ;-) But meanwhile I learned that's also not correct as it doesn't prevent premature unlocking. Your solution matches what I intend to do :-) However it seems to be enough to simply change the static initializer to PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP. Or did I miss something? If not, I'll commit this variant. Hans-Dieter |
From: Stas S. <st...@ak...> - 2007-07-15 13:01:05
|
Hi Hans, and thanks for your work on my patches. :) Hans-Dieter Kosch wrote: >> Well, as a minimal change, I can just make >> the mutex recursive. > Your solution matches what I intend to do :-) Except that I don't like the recursive mutexes, so a cleaner fix might be better. But as long as it works - oh well. :) > However it seems to be enough to simply change the static initializeHi Hansr to > PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP. > Or did I miss something? If not, I'll commit this variant. That only depends on where have you go it. :) I also found that in pthread.h, but it doesn't seem to be documented, it is defined under __USE_GNU, and the suffix "_NP" doesn't suggest like it was intended to be used directly. So except being undocumented, it also seems to be completely internal. So unless it is actually not, I'd rather not do the things like that. But of course it is not me to think about the xine-ui portability, so if you think that's right... |