extragear/multimedia/amarok/src

Jeff Mitchell kde-dev at emailgoeshere.com
Fri Jul 14 00:17:15 UTC 2006


Guys, please don't get overzealous in doing ATF checking.  If you see a 
function that doesn't do a check, it's usually either specifically by design, 
or because it will be checked for soon down the line anyways.  The core 
functions all check for it before anything (files or the database) is 
modified, so at best you save maybe a millisecond or two of processing time 
and at worst you change the functionality in unintended ways.  And functions 
that use the unique ids that don't do a check will just get QString::null 
anyways, and will just revert to normal behavior.

That having been said, most of this commit is incorrect or not needed, so I'm 
going to revert a lot of it.

Btw, sorry about all the ;s, either it was compiling fine for me anyways, or I 
made some last minute changes without realizing I hadn't compile-tested.

>  void MetaBundle::setUniqueId( TagLib::FileRef &fileref, bool recreate,
> bool strip ) {
> +    if( !AmarokConfig::advancedTagFeatures() )
> +        return;

This was right before, and now will behave incorrectly.  ATF being turned on 
is checked for below, and parts of this function must still be able to run 
even when ATF is off (see the "strip" bool).

> +
> +    if( !(url().protocol() == "file") )
> +    {
> +        debug() << "no file protocol url" << endl;
> +        return;
> +    }

It checks for it being a stream below, etc.  What if there are non "file" 
protocols that can handle metadata?  We check to see if TagLib can get a 
fileref and if the file isWritable according to TagLib anyways, so let TagLib 
figure it out.

>  void
>  MetaBundle::newUniqueId()
>  {
> +    if( !AmarokConfig::advancedTagFeatures() )
> +        return;
> +

Again, not necessary.  I'll leave it for now but at some point it may go away 
in the future so people can assign unique IDs even when ATF isn't actually 
enabled.

>  void MetaBundle::removeUniqueId()
>  {
> +    if( !AmarokConfig::advancedTagFeatures() )
> +        return;
> +

This defeats the functionality, since we want people to be able to remove UIDs 
even if ATF is off.  This is by design.  See the strip bool that is passed to 
setUniqueId.



More information about the Amarok mailing list