[mythtv] Ticket #3077: Default CD Device selection
devel at mrwire.co.uk
devel at mrwire.co.uk
Mon Feb 12 11:27:22 UTC 2007
> 1) I agree that having a default device selector is useful, and having the
> burning functions seperate is also good. Having four defaults seems
> overkill, though? Maybe just one DefaultBurner setting?
This patch actually only creates two defaults - one for DVD and one for CD drives. However, it may well make more sense to just have a single default?
> 2) Most of your code compares the *DeviceLocation setting to "default",
> and then looks up another setting. Why not just use the first setting?
> (''i.e.'' make the UI GetCDROMBlockDevices changes in
Not quite sure what you mean. By default plugins have their own setting for CD/DVD drive locations, if it is set to "default" only then will it look up the MythTV global default. This would require removing the plugin setting which I don't think people would like.
> 3) mythvideo/mythvideo/main.cpp adds "/2" onto the URL/URI/MRI/thingy.
> Why? Doesn't this always force one particular track?
Oops, that shouldn't have sneaked in there, I was trying to get Mythvideo to play different titles on a DVD. Patch updated to remove the offending line!
> 4) Prepending /dev into the paths in the GUI selectors is a bit wasteful,
> and won't work on Windows or Mac OS X. (Note, however, that I do something
> equally bad in mythvideo/main.cpp on OS X). Maybe a new method,
> MythMediaMonitor::GetCDMedias() that already prepends this for appropriate
That may well be better, that could internally call getCDROMBlockDevices and append /dev - getCDROMBlockDevices actually won't detect windows/mac drives anyway but the combobox remains editable for this situation unless anyone with a mac/windows machine can tell me how to detect drives on that platform.
> 5) I am wondering if the user really needs the /dev name in the GUI
> selector? Ideally, we would present the device name (''e.g.'' "PIONEER
> DVD-RW DVR-108" or "LITE-ON DVDRW SOHW-832S") parsed from
That would be nice, I did have some code which did that, but I felt it added unnecessary complexity to the initial patch, and again would only work on linux. It would be nice to add later though, I agree.
Thankyou for the feedback,
More information about the mythtv-dev