[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