On Sunday 08 January 2006 19:05, David Abrahams wrote: > Isaac Richards <[EMAIL PROTECTED]> writes: > > On Saturday 07 January 2006 21:56, David Abrahams wrote: > >> Okay, I got it working, as far as I can tell; the patch against this > >> morning's source (Version 8516) is enclosed. > > > > Comments: > > > > - The amount of ifdefs you've added are not good. Create new classes > > when the code is significantly different, as it is in this case. > > Sure, I'll be happy to refactor it; I just wanted to get something > working first, and I expected more code to be shared than actually is.
Would've expected it would have been easier separated out to begin with, but however you want to do things is fine. > > - I assume that this should be compile-time optional, and it's not? > > I don't know what you mean, exactly. It might be very obvious to > everyone else what you mean by "this" and "compile-time optional," but > being new to MythTV culture I need a little more explanation. Are you > saying that > configure --disable-firewire > > should cause all of this new code not to be included? I admit I > haven't yet considered whether configure is as fine-grained as > possible. Yes. Additional unnecessary dependencies should be always be optional. > > - I'd prefer not to have more device-specific code in tv_rec than there > > already is. > > I suppose you're referring to the inclusion of the > AVCDeviceController* in that class? Yes. > If so, I understand; that class > is already looking pretty scary. I actually started without it in > there, to avoid intruding. Here's the problem: the Channel changer > and the Recorder must share the same device controller. Otherwise, > the OS tells you that you're trying to open a device twice with > exclusive access and nothing gets recorded. Where would you like to > see the shared data stored? I could keep it in the Channel, but then > I'd need to expose some way to get the channel from the TVRec, which I > wasn't sure would be well received either -- and it would require a > downcast to get it from the ChannelBase to FirewireChannel, which ends > up making some assumptions about the operation of TVRec (e.g. that the > Channel is initialized first), which I didn't like much either. Recorder specific data structures belong in the recorder specific code - the other recorders already share data between the recorder and channel classes. Shouldn't be difficult to copy that. > Actually, from my limited perspective the independence of the Channel > and Recorder classes seems a bit conflicted, since at least in some > situations tuning and video streaming happen on the same device. They're not completely independent. Isaac
_______________________________________________ mythtv-dev mailing list [email protected] http://mythtv.org/cgi-bin/mailman/listinfo/mythtv-dev
