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