Daniel Kristjansson <[EMAIL PROTECTED]> writes: > On Mon, 2006-01-09 at 08:50 -0500, David Abrahams wrote: >> As I do my refactoring, it begins to look to me as though the original >> FirewireChannel code has a few problems. Close() doesn't seem to do >> anything, for example. Also, if the channel failed to open, >> SetChannelByString generates an error message about having the wrong >> kind of device and then returns true. > > The FirewireRecorder code is not our finest example of a recorder, > it was written by one guy who then disappeared. It was actually > impressive how quickly he got it together, but it needed on-going > care to iron out the oddities... > > Look at ChannelBase, and Channel to figure out what channel should do. > If neither applies the DVBChannel class is probably doing the right > thing too, but not always. As for close it is possible that > StartRecording() is closing the file handle, this is not so nice.
I've already gotten as far as possible by looking through the existing source; I really need to know the *intention* of the author of the base classes in order to do the right thing. That intention is neither documented nor evident from the code. >> There are no documented requirements for SetChannelByString, so I >> could be wrong about what it's supposed to do, of course. Maybe >> returning true in that case is just fine. > > It should only be returning true when it changes the channel > successfully. *that's* the sort of answer I'm looking for. So does it matter that Close() doesn't do anything and all the real action happens in the Channel's destructor? >> And maybe it doesn't matter >> if Close() doesn't release the firewire handle allocated in the >> channel's Open() call. > > Close should probably release the handle if it is still being held. > It looks like this is being done in the destructor instead. Yes. > Since none of the active developers are using the FirewireRecorder > we can't really change this since the assumption is that whatever is > in there now at least works to some degree. Ouch. So my refactoring, which is pulling lots of common source out of firewirerecorder/firewirechannel into firewirerecorderbase/firewirechannelbase so that darwinfirewirerecorder/darwinfirewirechannel can use it too, can't be tested? That's a major problem. It seems to me that the only alternative is copy-and-paste programming: copy firewirerecorder/firewirechannel into darwinfirewirerecorder/darwinfirewirechannel, remove all the non-darwin stuff, and then revert firewirerecorder/firewirechannel. There will be *lots* of code duplication. Is that the only acceptable course of action? >> Also, is the Channel always allocated first? > > I'm pretty sure the answer is yes. Yeah, it is, AFAICT. >> Is the Recorder supposed >> to work if the Channel couldn't be allocated or opened? > > It is not required to. We used to try to do this, but with some > types of recorders this would be very difficult to do. Okay, thanks. -- Dave Abrahams Boost Consulting www.boost-consulting.com
_______________________________________________ mythtv-dev mailing list mythtv-dev@mythtv.org http://mythtv.org/cgi-bin/mailman/listinfo/mythtv-dev
