[mythtv] [mythtv-commits] Ticket #2096: Mac OS X Plextor ConvertX USB Video Capture support

[Andrew Kimpton]

MythTV wrote:
> #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

Andrew 8-)