From: Michael R. <mr...@us...> - 2003-11-11 15:34:11
|
Hi Miguel, > > > I don't think that changing things like: - > > > xine_set_stream_info(..) > > > to > > > xine->set_stream_info(...) > > > > > > would be at all bad. > > > > I like that idea, too. I know that this means even more work, but I > > think some (not all) functions could indeed be moved into xine_t or > > xine_stream_t. Daniel? > > which functions do you have in mind? > > remember that function pointers in structs have their own advantages > and disadvantages. if it is something we may want to change on > runtime, for example, function pointer is the way to go. otoh > changing the struct (adding new fields) may require recompiling > everything. I just think the two files xine.c and xine_interface.c could need some cleanup, since at least I cannot see a convention which contains what. My idea would have been to move internal functions that clearly operate on a specific struct as a function pointer into this struct. Examples? Ok: * These could go into xine_stream_t: xine_set_speed_internal() xine_stop_internal() xine_close_internal() xine_open_internal() xine_play_internal() xine_get_current_position() xine_get_current_info() xine_get_stream_length() * These could go into extra_info_t: extra_info_reset() extra_info_merge() The convention could then be: xine.c contains the implementations of the struct member functions, similar to what all the other components like metronom, decoders, output loops do; and xine_interface.c contains all functions available in the public API. Helper functions for plugins should remain plain functions for the reasons you gave. But moving the above functions (and maybe some more I haven't found, but I guess these are the majority) into the structures looks much cleaner (and more object-oriented) to me. If we agree on this and noone else wants the job, I would volunteer to do this. Daniel, you can go ahead with your patch in the meantime. Michael -- /* Nobody will ever see this message :-) */ panic("Cannot initialize video hardware\n"); 2.0.38 /usr/src/linux/arch/m68k/atari/atafb.c |