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. > - 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. I had to learn a lot of new (to me) technologies to get as far as I did, so it's no surprise that some things are less than perfect. If you're willing to patiently tell me what should be improved I'll be happy to do it. > - 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? 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. 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. -- Dave Abrahams Boost Consulting www.boost-consulting.com
_______________________________________________ mythtv-dev mailing list [email protected] http://mythtv.org/cgi-bin/mailman/listinfo/mythtv-dev
