[mythtv] [PATCH] FAO Thor (mainly) MythMusic Metadata restructuring...

[thor]

On Sunday 26 September 2004 11:33 am, Colin Guthrie wrote:

> Well, I've taken things a bit further, and abstracted things a bit more....

	Cool. And sorry it has taken me a few days to get back to you on this.

> tag.h/cpp Abstract Base Class and some methods.
> id3v2.h/cpp Derived Class for ID3v2 reading/writing.

	Makes perfect sense to me.

> I know the removal for the getMetadatafromfilename() from decoder.cpp
> stops other music formats working, but as I'm sure you'll realise, I
> intend to implement the equivelant of tagid3v2.cpp for Vorbis
> Comment/Ogg/Flac etc., thus removing the need for it.

	Yup.

>
> Basically, all I want is a "good structure, keep going" type reply at
> this stage. Obviously, you may say, "that's stupid, stop now", which is
> really why I'm posting this message, just in case I go off and spend
> ages writing it only for it not to be applied.

	All seems quite sensible. One thought ... it would be really sweet to have a 
metadata field (in the database at least, if not in the tag) with the 
original cddb id (that MD5 hash thing that is calculated for lookups). If 
that were in the db (and used as an index), you could add some logic (at some 
point) to see if the user already has that CD (on a track by track basis?) in 
their collection.

	Doesn't have to work straight away, but would be nice to get to at some 
point. That means tag classes have to have somewhere to store it, and the 
ripping/tagging process has to be aware of the possibility.

	Other than that (somewhat "out there" thought), sounds great to me.


>
> I've got some doxygen style comments in the patch, but if desired these
> can be removed prior to application to CVS etc. I just add them out of
> habbit and to keep myself sane!

	I defer to Isaac on commenting style. I think the basic rule is doxygen 
comments are just fine as long as they're not cluttering up perfectly obvious 
functions with lines and lines of redundant verbiage. 

>
> I've tried to match some coding styles, but not sure about member
> variable vs. local variable naming.... All of this can be changed easily
> enough, tho', so I'm not too worried about that at this stage, more the
> overall structure.
>

	Nothing hard and fast here. I tend to do: MyObject, MyObject::someFunction(), 
and have long and descriptive member variable names. Isaac mostly does: 
MyObject, MyObject::SomeFunction(), and terser member variable names but 
often with m_ as a prefix to keep things clear. To the extant that there's a 
rule, try and make the code look like the code around it (which you seem to 
be doing already).

	If you do if (foo)'s (with a space), you get extra brownie points for 
recognizing that if is not a function.    :-)

- thor