From: Christoph P. <chr...@gm...> - 2006-10-13 13:19:09
|
Hi xine-lib's current xlib approach (having a seperate x thread) causes problems e.g when trying to use it in browsers or embedding into other apps. Currently the only solution is to create a new process and using xine-lib in there. To solve that problem, I'd propose to extend x11_visual_t by two callbacks (like lock_display and unlock_display). Those would be used in the output plugin instead of XLockDisplay and XUnlockDisplay. Would such a solution (at least the general idea) be accepted to go into xine-lib's repo? Below some notes about the concept I've in mind. a) effect to apps which don't want to use that They can set the callbacks to null, so that xine internally uses X*LockDisplay and apps continue working without the need to change much. Sample code for that: ... plugin_open (...) { ... if (visual->lock_display) { internal->lock_display = visual->lock_display; internal->user_data = visual->user_data; } else { internal->lock_display = &XLockDisplay; internal->user_data = visual->display; } ... } b) binary compatibility Adding members to x11_visual_t isn't binary incompatible, however the callback pointers may be unitialised. To avoid that I'd suggest to introduce e. g. XINE_VISUAL_TYPE_X11_V2. Only with that visual type the new callbacks are used, otherwise fallback to the old method (like above), so binary compatibility shouldn't be a problem. Other ideas are of course welcome too :) Christoph |
From: Christophe T. <hf...@fr...> - 2006-10-13 13:42:58
|
Le Vendredi 13 Octobre 2006 15:19, Christoph Pfister a =E9crit=A0: > Hi > > xine-lib's current xlib approach (having a seperate x thread) causes > problems e.g when trying to use it in browsers or embedding into other > apps. Currently the only solution is to create a new process and using > xine-lib in there. > > To solve that problem, I'd propose to extend x11_visual_t by two callbacks > (like lock_display and unlock_display). Those would be used in the output > plugin instead of XLockDisplay and XUnlockDisplay. Would such a solution > (at least the general idea) be accepted to go into xine-lib's repo? Below > some notes about the concept I've in mind. > > a) effect to apps which don't want to use that > > They can set the callbacks to null, so that xine internally uses > X*LockDisplay and apps continue working without the need to change much. > Sample code for that: > > ... plugin_open (...) > { > ... > if (visual->lock_display) { > internal->lock_display =3D visual->lock_display; > internal->user_data =3D visual->user_data; > } else { > internal->lock_display =3D &XLockDisplay; > internal->user_data =3D visual->display; > } > ... > } > > b) binary compatibility > > Adding members to x11_visual_t isn't binary incompatible, however the > callback pointers may be unitialised. To avoid that I'd suggest to > introduce e. g. XINE_VISUAL_TYPE_X11_V2. Only with that visual type the n= ew > callbacks are used, otherwise fallback to the old method (like above), so > binary compatibility shouldn't be a problem. > > Other ideas are of course welcome too :) > > Christoph I second that request. It would not only be a solution for existing applications, but would also=20 allow xine-lib to become one of the best choice for KDE4 phonon (a=20 replacement for arts, http://phonon.kde.org ) =2D-=20 Christophe Thommeret |
From: Miguel F. <mfr...@gm...> - 2006-10-13 22:20:43
|
Hi Christoph, On 10/13/06, Christoph Pfister <chr...@gm...> wrote: > To solve that problem, I'd propose to extend x11_visual_t by two callbacks > (like lock_display and unlock_display). Those would be used in the output > plugin instead of XLockDisplay and XUnlockDisplay. Would such a solution (at > least the general idea) be accepted to go into xine-lib's repo? Below some > notes about the concept I've in mind. I've just searched my mail: this same idea was suggested 2 years ago by kxine author, but the thread died (apparently he found another way of doing it). > b) binary compatibility > > Adding members to x11_visual_t isn't binary incompatible, however the callback > pointers may be unitialised. To avoid that I'd suggest to introduce e. g. > XINE_VISUAL_TYPE_X11_V2. Only with that visual type the new callbacks are > used, otherwise fallback to the old method (like above), so binary > compatibility shouldn't be a problem. yes, binary compatibility is definitely a must. we just need to find the easiest/cleanest way of doing it, but i don't see any problem with the idea per se... Miguel |
From: Christoph P. <chr...@gm...> - 2006-10-14 09:15:32
|
Hi Miguel Am Samstag, 14. Oktober 2006 00:20 schrieb Miguel Freitas: > Hi Christoph, > > On 10/13/06, Christoph Pfister <chr...@gm...> wrote: > > To solve that problem, I'd propose to extend x11_visual_t by two > > callbacks (like lock_display and unlock_display). Those would be used in > > the output plugin instead of XLockDisplay and XUnlockDisplay. Would such > > a solution (at least the general idea) be accepted to go into xine-lib's > > repo? Below some notes about the concept I've in mind. > > I've just searched my mail: this same idea was suggested 2 years ago > by kxine author, but the thread died (apparently he found another way > of doing it). First, kxine is a standalone application (not embedded). So it's possible to call XInitThreads() _before_ the x connection is created (but they do it after the x connection is created -> that leads to a crash at exit [1]). And they use a real big qt hack which isn't necessary (they could have created a second x connection). For standalone apps the XInitThreads() thing is ok, but obviously you can't use it when embedding xine somewhere [1]. > > b) binary compatibility > > > > Adding members to x11_visual_t isn't binary incompatible, however the > > callback pointers may be unitialised. To avoid that I'd suggest to > > introduce e. g. XINE_VISUAL_TYPE_X11_V2. Only with that visual type the > > new callbacks are used, otherwise fallback to the old method (like > > above), so binary compatibility shouldn't be a problem. > > yes, binary compatibility is definitely a must. > > we just need to find the easiest/cleanest way of doing it, but i don't > see any problem with the idea per se... Ok. I'll create an experimental patch (already have some hacks to confirm that the idea works). > Miguel Thanks for having a look at this! Christoph [1] You need to call XInitThreads() even if you have two seperate connections to the x server (because there are some global vars in that **** lib). When you create a XrmDatabase (implicitly done by e.g. by qt) without having called XInitThreads(), the mutex in there isn't initialised. When you call XCloseDisplay() after having called XInitThreads(), it tries to unlock that mutex -> I always get seg faults, but in general it's unpredictable. Couldn't compile kxine (because of too much trouble in there), but I'm pretty sure of my analysis ;-) |
From: Diego 'F. <fla...@ge...> - 2006-10-14 09:26:05
|
On Saturday 14 October 2006 11:15, Christoph Pfister wrote: > Couldn't compile kxine (because of too much trouble in there), but I'm > pretty sure of my analysis ;-) I can confirm with Kaffeine's Konqueror plugin: it makes konqueror segfault= =20 every time the plugin is closed. =2D-=20 Diego "Flameeyes" Petten=F2 - http://farragut.flameeyes.is-a-geek.org/ Gentoo/Alt lead, Gentoo/FreeBSD, Video, Sound, ALSA, PAM, KDE, CJK, Ruby ... |
From: Christoph P. <chr...@gm...> - 2006-10-14 10:14:31
Attachments:
sync_xlib.diff
|
Hi again Here is the experimental patch (neither compile nor really tested - shame on me :-) It contains the needed modifications for the engine and one of the output plugins. Christoph Am Samstag, 14. Oktober 2006 11:15 schrieb Christoph Pfister: > Hi Miguel > > Am Samstag, 14. Oktober 2006 00:20 schrieb Miguel Freitas: > > Hi Christoph, > > > > On 10/13/06, Christoph Pfister <chr...@gm...> wrote: > > > To solve that problem, I'd propose to extend x11_visual_t by two > > > callbacks (like lock_display and unlock_display). Those would be used > > > in the output plugin instead of XLockDisplay and XUnlockDisplay. Would > > > such a solution (at least the general idea) be accepted to go into > > > xine-lib's repo? Below some notes about the concept I've in mind. > > > > I've just searched my mail: this same idea was suggested 2 years ago > > by kxine author, but the thread died (apparently he found another way > > of doing it). > > First, kxine is a standalone application (not embedded). So it's possible > to call XInitThreads() _before_ the x connection is created (but they do it > after the x connection is created -> that leads to a crash at exit [1]). > And they use a real big qt hack which isn't necessary (they could have > created a second x connection). > For standalone apps the XInitThreads() thing is ok, but obviously you can't > use it when embedding xine somewhere [1]. > > > > b) binary compatibility > > > > > > Adding members to x11_visual_t isn't binary incompatible, however the > > > callback pointers may be unitialised. To avoid that I'd suggest to > > > introduce e. g. XINE_VISUAL_TYPE_X11_V2. Only with that visual type the > > > new callbacks are used, otherwise fallback to the old method (like > > > above), so binary compatibility shouldn't be a problem. > > > > yes, binary compatibility is definitely a must. > > > > we just need to find the easiest/cleanest way of doing it, but i don't > > see any problem with the idea per se... > > Ok. I'll create an experimental patch (already have some hacks to confirm > that the idea works). > > > Miguel > > Thanks for having a look at this! > > Christoph > > > [1] > > You need to call XInitThreads() even if you have two seperate connections > to the x server (because there are some global vars in that **** lib). > > When you create a XrmDatabase (implicitly done by e.g. by qt) without > having called XInitThreads(), the mutex in there isn't initialised. When > you call XCloseDisplay() after having called XInitThreads(), it tries to > unlock that mutex -> I always get seg faults, but in general it's > unpredictable. > > Couldn't compile kxine (because of too much trouble in there), but I'm > pretty sure of my analysis ;-) |
From: Christoph P. <chr...@gm...> - 2006-10-15 11:33:27
Attachments:
sync_xlib.diff
|
Hi This time I have successfully tested it. Attached is the patch against cvs head. Again, only one of the output plugins is modified. Christoph Am Samstag, 14. Oktober 2006 12:14 schrieb Christoph Pfister: > Hi again > > Here is the experimental patch (neither compile nor really tested - shame > on me :-) It contains the needed modifications for the engine and one of > the output plugins. > > Christoph > > Am Samstag, 14. Oktober 2006 11:15 schrieb Christoph Pfister: > > Hi Miguel > > > > Am Samstag, 14. Oktober 2006 00:20 schrieb Miguel Freitas: > > > Hi Christoph, > > > > > > On 10/13/06, Christoph Pfister <chr...@gm...> wrote: > > > > To solve that problem, I'd propose to extend x11_visual_t by two > > > > callbacks (like lock_display and unlock_display). Those would be used > > > > in the output plugin instead of XLockDisplay and XUnlockDisplay. > > > > Would such a solution (at least the general idea) be accepted to go > > > > into xine-lib's repo? Below some notes about the concept I've in > > > > mind. > > > > > > I've just searched my mail: this same idea was suggested 2 years ago > > > by kxine author, but the thread died (apparently he found another way > > > of doing it). > > > > First, kxine is a standalone application (not embedded). So it's possible > > to call XInitThreads() _before_ the x connection is created (but they do > > it after the x connection is created -> that leads to a crash at exit > > [1]). And they use a real big qt hack which isn't necessary (they could > > have created a second x connection). > > For standalone apps the XInitThreads() thing is ok, but obviously you > > can't use it when embedding xine somewhere [1]. > > > > > > b) binary compatibility > > > > > > > > Adding members to x11_visual_t isn't binary incompatible, however the > > > > callback pointers may be unitialised. To avoid that I'd suggest to > > > > introduce e. g. XINE_VISUAL_TYPE_X11_V2. Only with that visual type > > > > the new callbacks are used, otherwise fallback to the old method > > > > (like above), so binary compatibility shouldn't be a problem. > > > > > > yes, binary compatibility is definitely a must. > > > > > > we just need to find the easiest/cleanest way of doing it, but i don't > > > see any problem with the idea per se... > > > > Ok. I'll create an experimental patch (already have some hacks to confirm > > that the idea works). > > > > > Miguel > > > > Thanks for having a look at this! > > > > Christoph > > > > > > [1] > > > > You need to call XInitThreads() even if you have two seperate connections > > to the x server (because there are some global vars in that **** lib). > > > > When you create a XrmDatabase (implicitly done by e.g. by qt) without > > having called XInitThreads(), the mutex in there isn't initialised. When > > you call XCloseDisplay() after having called XInitThreads(), it tries to > > unlock that mutex -> I always get seg faults, but in general it's > > unpredictable. > > > > Couldn't compile kxine (because of too much trouble in there), but I'm > > pretty sure of my analysis ;-) |
From: Miguel F. <mfr...@gm...> - 2006-10-21 22:00:46
|
Hi Christoph, On 10/15/06, Christoph Pfister <chr...@gm...> wrote: > Hi > > This time I have successfully tested it. Attached is the patch against cvs > head. Again, only one of the output plugins is modified. I have a few comments about it: - lock_display has a different declaration from XLockDisplay. of course we can get rid by casting it, but it doesn't look right to me. i think a better idea would be to use a macro to check if lock_display == NULL and them use XLockDisplay instead. - i would prefer to not change load_plugins.c with such specific plugin code. instead, each plugin supporting the new struct would need to provide two visual types. i will keep looking in this issue, i'm favorable of including this idea just not exactly in this form ;-) Miguel |
From: Hannes J. <han...@go...> - 2006-10-13 18:51:15
|
2006/10/13, Christoph Pfister <chr...@gm...>: > > > b) binary compatibility > > Adding members to x11_visual_t isn't binary incompatible, however the > callback > pointers may be unitialised. To avoid that I'd suggest to introduce e. g. > XINE_VISUAL_TYPE_X11_V2. Only with that visual type the new callbacks are > used, otherwise fallback to the old method (like above), so binary > compatibility shouldn't be a problem. > > Other ideas are of course welcome too :) i don=B4t get what exactly your plan is, tho i=B4m new to playing with xi= ne=B4s output plugins. it is possible to pass pointers to functions with a struct that you pass as user_data. -- Hannes Christoph > > ------------------------------------------------------------------------- > Using Tomcat but need to do more? Need to support web services, security? > Get stuff done quickly with pre-integrated technology to make your job > easier > Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronim= o > > http://sel.as-us.falkag.net/sel?cmd=3Dlnk&kid=3D120709&bid=3D263057&dat= =3D121642 > _______________________________________________ > xine-devel mailing list > xin...@li... > https://lists.sourceforge.net/lists/listinfo/xine-devel > |
From: Christoph P. <chr...@gm...> - 2006-10-20 18:32:36
|
Hi Am Sonntag, 15. Oktober 2006 13:33 schrieb Christoph Pfister: > Hi > > This time I have successfully tested it. Attached is the patch against cvs > head. Again, only one of the output plugins is modified. > > Christoph Bah, I'll give that up (after >70kb patch) and let xine run in a seperate process. The biggest problem I see is that you nearly nowhere have to call X*LockDisplay directly (because most x functions call it internally - using a recursive mutex). Especially for xvmc there were too much lock / unlock missing for my taste. So I can just hope that there will be a better xlib mt concept in the future ... Christoph |
From: Diego 'F. <fla...@ge...> - 2006-10-20 18:49:03
|
On Friday 20 October 2006 20:32, Christoph Pfister wrote: > So I can just hope that there will be a better xlib mt concept in the > future ... XCB might help. =2D-=20 Diego "Flameeyes" Petten=F2 - http://farragut.flameeyes.is-a-geek.org/ Gentoo/Alt lead, Gentoo/FreeBSD, Video, Sound, ALSA, PAM, KDE, CJK, Ruby ... |
From: Miguel F. <mfr...@gm...> - 2006-10-28 17:05:54
|
On 10/15/06, Christoph Pfister <chr...@gm...> wrote: > Hi > > This time I have successfully tested it. Attached is the patch against cvs > head. Again, only one of the output plugins is modified. Christoph, i have just commited a XINE_VISUAL_TYPE_X11_2 patch based on yours but incorporating that two issues i wrote about in my previous message. please give it a try so we may port the other plugins as well (hint: help is appreciated ;-) Miguel |
From: Christoph P. <chr...@gm...> - 2006-10-28 18:34:19
Attachments:
christoph.diff
|
Hi Am Samstag, 28. Oktober 2006 19:05 schrieb Miguel Freitas: > On 10/15/06, Christoph Pfister <chr...@gm...> wrote: > > Hi > > > > This time I have successfully tested it. Attached is the patch against > > cvs head. Again, only one of the output plugins is modified. > > Christoph, i have just commited a XINE_VISUAL_TYPE_X11_2 patch based > on yours but incorporating that two issues i wrote about in my > previous message. > > please give it a try so we may port the other plugins as well (hint: > help is appreciated ;-) > > Miguel Bah, your mail box really seems to ignore certain mails ;) Fix comment in xine.h, add missing locking in xshm output plugin and porting xv. Not even compile tested. Gonna make some tests now ... Christoph |
From: Miguel F. <mfr...@gm...> - 2006-10-28 19:03:27
|
On 10/28/06, Christoph Pfister <chr...@gm...> wrote: > Bah, your mail box really seems to ignore certain mails ;) huh? > Fix comment in xine.h, add missing locking in xshm output plugin and porting > xv. Not even compile tested. Gonna make some tests now ... you're fast ;-) compiled and tested here (using old visual type). commited, thanks. Miguel |
From: Christophe T. <hf...@fr...> - 2006-10-28 19:07:28
|
Le Samedi 28 Octobre 2006 21:03, Miguel Freitas a =E9crit=A0: > On 10/28/06, Christoph Pfister <chr...@gm...> wrote: > > Bah, your mail box really seems to ignore certain mails ;) > > huh? Quoting Christoph: Bah, I'll give that up (after >70kb patch) and let xine run in a seperate=20 process. The biggest problem I see is that you nearly nowhere have to call=20 X*LockDisplay directly (because most x functions call it internally - using= a=20 recursive mutex). Especially for xvmc there were too much lock / unlock=20 missing for my taste. So I can just hope that there will be a better xlib mt concept in the=20 future ... =2D-=20 Christophe Thommeret |
From: Miguel F. <mfr...@gm...> - 2006-10-28 19:16:02
|
On 10/28/06, Christophe Thommeret <hf...@fr...> wrote: > Le Samedi 28 Octobre 2006 21:03, Miguel Freitas a =E9crit: > > On 10/28/06, Christoph Pfister <chr...@gm...> wrote: > > > Bah, your mail box really seems to ignore certain mails ;) > > > > huh? > > Quoting Christoph: > > Bah, I'll give that up (after >70kb patch) and let xine run in a seperate > process. > The biggest problem I see is that you nearly nowhere have to call > X*LockDisplay directly (because most x functions call it internally - usi= ng a > recursive mutex). Especially for xvmc there were too much lock / unlock > missing for my taste. > So I can just hope that there will be a better xlib mt concept in the > future ... i saw that message, but i guess i misunderstood it: i thought he gave up because the amount of changes (but what i commited must be less than 20kb). anyway, patch is now on cvs and will be included in 1.1.3. Miguel |
From: Christoph P. <chr...@gm...> - 2006-10-29 09:06:16
|
Am Samstag, 28. Oktober 2006 21:15 schrieb Miguel Freitas: > On 10/28/06, Christophe Thommeret <hf...@fr...> wrote: > > Le Samedi 28 Octobre 2006 21:03, Miguel Freitas a =E9crit: > > > On 10/28/06, Christoph Pfister <chr...@gm...> wrote: > > > > Bah, your mail box really seems to ignore certain mails ;) > > > > > > huh? > > > > Quoting Christoph: > > > > Bah, I'll give that up (after >70kb patch) and let xine run in a sepera= te > > process. > > The biggest problem I see is that you nearly nowhere have to call > > X*LockDisplay directly (because most x functions call it internally - > > using a recursive mutex). Especially for xvmc there were too much lock / > > unlock missing for my taste. > > So I can just hope that there will be a better xlib mt concept in the > > future ... > > i saw that message, but i guess i misunderstood it: i thought he gave > up because the amount of changes (but what i commited must be less > than 20kb). 70kb included most output plugins. > anyway, patch is now on cvs and will be included in 1.1.3. > > Miguel It won't be that easy for all plugins - e.g. for xvmc i expect some=20 trouble ... but now when this solution seems tangible i must admit that i=20 consider it as less effort than to let xine run in a seperate process ... Christoph |
From: Christoph P. <chr...@gm...> - 2006-10-31 20:07:55
|
Hi, Am Sonntag, 29. Oktober 2006 09:06 schrieb Christoph Pfister: > Am Samstag, 28. Oktober 2006 21:15 schrieb Miguel Freitas: > > On 10/28/06, Christophe Thommeret <hf...@fr...> wrote: > > > Le Samedi 28 Octobre 2006 21:03, Miguel Freitas a =E9crit: > > > > On 10/28/06, Christoph Pfister <chr...@gm...> wrote: > > > > > Bah, your mail box really seems to ignore certain mails ;) > > > > > > > > huh? > > > > > > Quoting Christoph: > > > > > > Bah, I'll give that up (after >70kb patch) and let xine run in a > > > seperate process. > > > The biggest problem I see is that you nearly nowhere have to call > > > X*LockDisplay directly (because most x functions call it internally - > > > using a recursive mutex). Especially for xvmc there were too much lock > > > / unlock missing for my taste. > > > So I can just hope that there will be a better xlib mt concept in the > > > future ... > > > > i saw that message, but i guess i misunderstood it: i thought he gave > > up because the amount of changes (but what i commited must be less > > than 20kb). > > 70kb included most output plugins. > > > anyway, patch is now on cvs and will be included in 1.1.3. > > > > Miguel > > It won't be that easy for all plugins - e.g. for xvmc i expect some > trouble ... but now when this solution seems tangible i must admit that i > consider it as less effort than to let xine run in a seperate process ... > > Christoph Hmmm ... i'm not very convinced of this whole action anymore. You have to avoid all kind of deadlocks in the main app (e.g. xine_play=20 waiting to have painted first picture) and code gets generally ugly. So it= =20 seems morely half-baken to me ... :-( There's also a problem with your current code which is fixable but i dunno = if=20 it's worth the effort ... atm i would prefer to have a full-baken solution= =20 e.g. in direction of xcb. Christoph Wrong init_class is called (for "visual_type=3D10" #2 isn't correct): #0 0xa6633445 in XLockDisplay () from /usr/lib/libX11.so.6 #1 0xa4318f79 in open_plugin_2 (class_gen=3D0x87ffc00, visual_gen=3D0xafb2= 8c48)=20 at video_out_xv.c:1280 #2 0xa431a3f0 in open_plugin_old (class_gen=3D0x87ffc00, visual_gen=3D0x86= 3a248)=20 at video_out_xv.c:1599 #3 0xa5604314 in _load_video_driver (this=3D<value optimized out>,=20 node=3D0x87cf940, data=3D0x863a248) at load_plugins.c:1506 #4 0xa560440e in _x_load_video_output_plugin (this=3D0x85c0bb8, id=3D0x0,= =20 visual_type=3D10, visual=3D0x863a248) at load_plugins.c:1548 #5 0xa5605cfa in xine_open_video_driver (this=3D0x85c0bb8, id=3D0x87ffb50 = "auto",=20 visual_type=3D10, visual=3D0x863a248) at load_plugins.c:1570 |