From: Thibaut M. <tma...@no...> - 2003-11-15 13:40:31
|
Hi, On Sat, 2003-11-15 at 00:11, Miguel Freitas wrote: > Hi Michael, > > On Thu, 2003-11-13 at 19:57, Michael Roitzsch wrote: > > Hi again, > > > > > > That could mean quite some work, since a good deal of these members > > > > are used from outside. If we really want to do this before 1.0, I > > > > think we should do it before rc3 (at least that's what the TODO > > > > says). Next issue would be: xine_stream_t is not the only struct > > > > exposing its members... > > > > > > I thought of something when i was coming to work today much less > > > intrusive than creating two structs. I'm attaching the patch so you > > > can get the idea. it would enforce the public/private separation > > > without requiring new structs and thousands of typecastings. > > > > > > Several variables that are on the public part shouldn't be there > > > (comments added). moving them will cause some plugin compilation to > > > fail and therefore it would be easier to fix those. > > > > Nice idea, ... but ( <- I guess you expected that :) ) I would prefer > > the way with the two structs. > > well, i really tried converting a couple of functions in xine.c to use > the two structs approach just to see how much work it would take... i > must say i didn't liked much the result. it seemed a bit overkill for > the sake of cleaning up internal interfaces. some observations: > > - it would add a lot of castings from public struct to private one. that > is, stream = (xine_stream_internal_t *)stream_gen; in several functions. > > - some lines start having so many levels of indirection, it looks a bit > ridiculous imho... > stream->public_part.input_plugin->input_class->get_description(stream->public_part.input_plugin->input_class->get_description); > > - moving variables between public and private sections requires a lot of > work (renaming variables, adding/removing prefixes all around). this > step will be needed for sure, unless all plugins are fixed at the same > time to not use public variables anymore. > > so, if nothing else, the #ifdef idea might provide a migration path: we > could use it to cleanup the interface, hiding private data and moving > the remaining variables while we fix the plugins... i like this idea. > > One reason is that during my last engine modification (vo_frame->stream > > now allowed to be NULL), an idea crossed my mind that a very complex > > post plugin (whatever it might be doing) could actually implement its > > own xine_stream_t interface and have vo_frame->stream of the frames it > > emits point to this. Output layer could then access parts of this > > (metronom for example). This is just an idea, nothing substantial, but > > it would be more clear with separated public/private structs of > > xine_stream_t. > > a plugin providing it's own engine? sounds odd. besides, the plugin > would have to rewrite everything (video_out and audio_out for example, > since they are compiled to use the private elements of xine_stream_t) > > definitely 2.0 material imho. > > > The other reason is my feeling again. We are using the object oriented C > > approach everywhere else in xine. Moving away from this paradigm for > > one of the most basic structs feels ungood to me. > > > > But I agree with the separation you suggest. (Although as said eariler, > > I would make the additional helper functions member pointers...) > > okay, so pluggable engine goes to 2.0? ;-) > > for now i believe having global helper functions is better... any other > opinions about that? my opinion, i don't like to use function pointers when it's not needed, so i prefer the global helper functions approach. Michael, you can see our helper functions as static methods of a xine engine utility class. ;) > > regards, > > Miguel cheers, Thibaut |