[mythtv] [RFC] MythMusic: use libcdio to play & rip CDs (Win32 & MacOSX too)
Lawrence Rust
lvr at softsystem.co.uk
Wed Dec 1 17:05:07 UTC 2010
On Wed, 2010-12-01 at 15:32 +0000, Paul Harrison wrote:
[snip]
> Thanks for the patch.
>
> Is there any reason we couldn't just make libcdio a compulsory
> dependency and remove cdparanoia and libcdaudio all together?
Absolutely no problem doing that. The new code is a complete
replacement and runs on all the existing platforms. Perhaps some kind
soul can try on MacOSX?
> A few minor things I spotted while browsing through the patch :-
>
> httpcomms is going to be removed use
> GetMythDownloadManager()->download() instead.
OK, wiil do.
> The description in CdDecoderFactory::description() doesn't look correct :)
No, a bit ambitious. I just cloned that bit from the current cddecoder.
Will fix.
> Our coding standards (http://www.mythtv.org/wiki/Coding_Standards) say
> we should use m_someVariable rather than this->someVariable to identify
> class member variables.
Hmm, that's going to be a problem to maintain compatibility with the
existing cddecooder.h where there's no m_ prefixes. I just chose to
maintain the status quo.
As for cddb.h, the structs Match, Matches, Track & Album are simple POD
types, albeit with the simplest of constructors, and are intended to be
used like C structs. Hence the absence of the 'm_' wart.
I have no axe to grind here, but prefixing class member data with this->
causes no overhead and makes the intention clearer. I have no
reservation adding a 'hungarian' prefix and often use this in more
complex classes.
-- Lawrence
More information about the mythtv-dev
mailing list