From f1d723077f2ee346182982d89febee0494a5b89e Mon Sep 17 00:00:00 2001 From: Michael Helmling Date: Wed, 26 Dec 2012 22:46:37 +0100 Subject: [PATCH] Consistently handle invalid and deprecated tags in setProperties() This commit reverts the use of strip() in setProperties() because the latter function should not change the file before save() is called. Instead, the following policy is now consistently applied for file formats with multiple tag types: - the recommended tag type is created, if it does not exist - deprecated tags are updated, if they exist, but not created - illegal tag types are ignored by setProperties(), but used in properties() if no others exist. The only tag types considered "illegal" so far are APEv2 in MPEG and ID3 in FLAC. --- taglib/ape/apefile.cpp | 2 +- taglib/ape/apefile.h | 3 +-- taglib/flac/flacfile.cpp | 4 ---- taglib/flac/flacfile.h | 3 ++- taglib/mpc/mpcfile.cpp | 2 +- taglib/mpc/mpcfile.h | 2 +- taglib/mpeg/mpegfile.cpp | 2 -- taglib/mpeg/mpegfile.h | 4 +--- taglib/wavpack/wavpackfile.cpp | 2 +- taglib/wavpack/wavpackfile.h | 2 +- 10 files changed, 9 insertions(+), 17 deletions(-) diff --git a/taglib/ape/apefile.cpp b/taglib/ape/apefile.cpp index 7cc1c54d..415eb17e 100644 --- a/taglib/ape/apefile.cpp +++ b/taglib/ape/apefile.cpp @@ -132,7 +132,7 @@ void APE::File::removeUnsupportedProperties(const StringList &properties) PropertyMap APE::File::setProperties(const PropertyMap &properties) { if(d->hasID3v1) - strip(ID3v1); + d->tag.access(ApeID3v1Index, false)->setProperties(properties); return d->tag.access(ApeAPEIndex, true)->setProperties(properties); } diff --git a/taglib/ape/apefile.h b/taglib/ape/apefile.h index df335ec6..95c60742 100644 --- a/taglib/ape/apefile.h +++ b/taglib/ape/apefile.h @@ -129,8 +129,7 @@ namespace TagLib { /*! * Implements the unified property interface -- import function. * Creates an APEv2 tag if necessary. A pontentially existing ID3v1 - * tag is considered deprecated and will be removed, invalidating all - * pointers to that tag. + * tag will be updated as well. */ PropertyMap setProperties(const PropertyMap &); diff --git a/taglib/flac/flacfile.cpp b/taglib/flac/flacfile.cpp index 93f8bd7b..972fe728 100644 --- a/taglib/flac/flacfile.cpp +++ b/taglib/flac/flacfile.cpp @@ -168,10 +168,6 @@ void FLAC::File::removeUnsupportedProperties(const StringList &unsupported) PropertyMap FLAC::File::setProperties(const PropertyMap &properties) { - if(d->hasID3v1) - d->tag.access(FlacID3v1Index, false)->setProperties(properties); - if(d->hasID3v2) - d->tag.access(FlacID3v2Index, false)->setProperties(properties); return d->tag.access(FlacXiphIndex, true)->setProperties(properties); } diff --git a/taglib/flac/flacfile.h b/taglib/flac/flacfile.h index 76f2e2d3..7423d8f1 100644 --- a/taglib/flac/flacfile.h +++ b/taglib/flac/flacfile.h @@ -135,7 +135,8 @@ namespace TagLib { * Implements the unified property interface -- import function. * This always creates a Xiph comment, if none exists. The return value * relates to the Xiph comment only. - * Potential ID3v1 and ID3v2 tags will also be updated. + * Ignores any changes to ID3v1 or ID3v2 comments since they are not allowed + * in the FLAC specification. */ PropertyMap setProperties(const PropertyMap &); diff --git a/taglib/mpc/mpcfile.cpp b/taglib/mpc/mpcfile.cpp index c6cb3a41..ee992ebc 100644 --- a/taglib/mpc/mpcfile.cpp +++ b/taglib/mpc/mpcfile.cpp @@ -136,7 +136,7 @@ void MPC::File::removeUnsupportedProperties(const StringList &properties) PropertyMap MPC::File::setProperties(const PropertyMap &properties) { if(d->hasID3v1) - strip(ID3v1); + d->tag.access(MPCID3v1Index, false)->setProperties(properties); return d->tag.access(MPCAPEIndex, true)->setProperties(properties); } diff --git a/taglib/mpc/mpcfile.h b/taglib/mpc/mpcfile.h index b484be70..59617074 100644 --- a/taglib/mpc/mpcfile.h +++ b/taglib/mpc/mpcfile.h @@ -125,7 +125,7 @@ namespace TagLib { /*! * Implements the unified property interface -- import function. * Affects only the APEv2 tag which will be created if necessary. - * If an ID3v1 tag exists, it will be stripped from the file. + * If an ID3v1 tag exists, it will be updated as well. */ PropertyMap setProperties(const PropertyMap &); diff --git a/taglib/mpeg/mpegfile.cpp b/taglib/mpeg/mpegfile.cpp index fd7223c7..c73da414 100644 --- a/taglib/mpeg/mpegfile.cpp +++ b/taglib/mpeg/mpegfile.cpp @@ -159,8 +159,6 @@ void MPEG::File::removeUnsupportedProperties(const StringList &properties) PropertyMap MPEG::File::setProperties(const PropertyMap &properties) { - if(d->hasAPE) - strip(APE, true); if(d->hasID3v1) // update ID3v1 tag if it exists, but ignore the return value d->tag.access(ID3v1Index, false)->setProperties(properties); diff --git a/taglib/mpeg/mpegfile.h b/taglib/mpeg/mpegfile.h index 08415d55..1d99898c 100644 --- a/taglib/mpeg/mpegfile.h +++ b/taglib/mpeg/mpegfile.h @@ -145,9 +145,7 @@ namespace TagLib { /*! * Implements the writing part of the unified tag dictionary interface. * In order to avoid problems with deprecated tag formats, this method - * always creates an ID3v2 tag if necessary, and removes potential APEv2 - * tags (also invalidating all pointers to the APE tag) which are - * considered bad practice in MP3 files. + * always creates an ID3v2 tag if necessary. * If an ID3v1 tag exists, it will be updated as well, within the * limitations of that format. * The returned PropertyMap refers to the ID3v2 tag only. diff --git a/taglib/wavpack/wavpackfile.cpp b/taglib/wavpack/wavpackfile.cpp index 67be997c..78df182d 100644 --- a/taglib/wavpack/wavpackfile.cpp +++ b/taglib/wavpack/wavpackfile.cpp @@ -120,7 +120,7 @@ PropertyMap WavPack::File::properties() const PropertyMap WavPack::File::setProperties(const PropertyMap &properties) { if(d->hasID3v1) - strip(ID3v1); + d->tag.access(WavID3v1Index, false)->setProperties(properties); return d->tag.access(WavAPEIndex, true)->setProperties(properties); } diff --git a/taglib/wavpack/wavpackfile.h b/taglib/wavpack/wavpackfile.h index 3e0c36c4..918edca8 100644 --- a/taglib/wavpack/wavpackfile.h +++ b/taglib/wavpack/wavpackfile.h @@ -119,7 +119,7 @@ namespace TagLib { /*! * Implements the unified property interface -- import function. * Creates an APE tag if it does not exists and calls setProperties() on - * that. Any existing ID3v1 tag will be removed. + * that. Any existing ID3v1 tag will be updated as well. */ PropertyMap setProperties(const PropertyMap&);