[mythtv] [mythtv-commits] Ticket #2096: Mac OS X Plextor ConvertX USB Video Capture support
awk at awkward.org
Thu Sep 7 01:45:40 UTC 2006
> #2096: Mac OS X Plextor ConvertX USB Video Capture support
> Did a quick once-over of the patch:
> * I'm not very comfortable with the amount of ifdefs in there. Some
> refactoring into other classes would probably be significantly cleaner.
I can certainly do that - I was a little apprehensive about 'setting
precedent' though. So I only subclassed those classes which others had
subclassed before me (Channel for example). cardutil I can probably
subclass easily for my purposes, but tv_rec.cpp and videosource.cpp have
already got a lot of ifdefs for the other recorders (freebox, firewire
etc) does it really make sense to start subclassing there now ?
Also is subclassing NuppelVideoRecorder.cpp 'a good thing' ? That seemed
to me like a 'bold step' but if you're fine with it then I'm OK - though
of course there'll probably still need to be an ifdef somewhere else
when we decide what to instantiate - but that's in tv_rec.cpp anyway.
> * I'm even more unsure about including the driver & firmware in the
> mythtv source/build process. Seems rather odd to me.
Well... there's just simply no need on OS X for a separate process (much
less any kernel extension/driver) to do this work - it's all user level
stuff. If you place it in a separate process you just have to add a
whole bunch of code to deal with interapplication sharing of memory
buffers, that seems like a lot of work. I can certainly investigate
adding a separate app to the package to handle the initial firmware boot
process of the Plextor boxes, but really I was looking for the 'install
and go' approach.
The 'truly' OS X approach would be to place the firmware files inside
the Resources folder of the OS X bundle structure that's being created
for the mythbackend.app - that way there's never any need to worry about
where these files are. I'll investigate that change - the code's not a
problem it's just convincing qmake and the makefile of what you want to do.
> * Please don't make tarballs that extract into the current directory.
No problem - I'll change the tarball with my next updates so that it
extracts from 'TOP' (ie libs/libmythtv/blah.cpp)
> * svn diff will create diffs for all changed files, you don't have to
> make separate patches for each file.
'Tis changed too - I'll update it with the next round of changes.
Thanks for your comments
More information about the mythtv-dev