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.

diff --git a/taglib/ape/apefile.cpp b/taglib/ape/apefile.cpp
index 7cc1c54de67dfb928b40bbdd3d0342710b7d2cee..415eb17edaf808b5ff0526c06911fa2684e61dd1 100644 (file)
--- 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<ID3v1::Tag>(ApeID3v1Index, false)->setProperties(properties);
   return d->tag.access<APE::Tag>(ApeAPEIndex, true)->setProperties(properties);
 }
 

diff --git a/taglib/ape/apefile.h b/taglib/ape/apefile.h
index df335ec6e58154d7f16eee70b15d75b71194142e..95c607429d4cedefc68f89a69a47397446a631f2 100644 (file)
--- 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 93f8bd7bb7a131f2009d447df17cd073eae31bb3..972fe7287743526d9d985847a77bcb63f0c88bbe 100644 (file)
--- 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<ID3v1::Tag>(FlacID3v1Index, false)->setProperties(properties);
-  if(d->hasID3v2)
-      d->tag.access<ID3v2::Tag>(FlacID3v2Index, false)->setProperties(properties);
   return d->tag.access<Ogg::XiphComment>(FlacXiphIndex, true)->setProperties(properties);
 }
 

diff --git a/taglib/flac/flacfile.h b/taglib/flac/flacfile.h
index 76f2e2d3fca47a4545f8b3971e5848dd45e7e906..7423d8f1edd5c9610d68b44a1d35f49c6652a320 100644 (file)
--- 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 c6cb3a410636d7d7c4aef55d2809b9ea02290fbb..ee992ebc3a67882478bb2949dfb7ffbd388ac343 100644 (file)
--- 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<APE::Tag>(MPCID3v1Index, false)->setProperties(properties);
   return d->tag.access<APE::Tag>(MPCAPEIndex, true)->setProperties(properties);
 }
 

diff --git a/taglib/mpc/mpcfile.h b/taglib/mpc/mpcfile.h
index b484be708631c287b7b61aff05a055dacb3c2f23..59617074e5610dd1474bd8f99c7fe22ac4c7c6d4 100644 (file)
--- 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 fd7223c7cbff1dc1b882a5489de0d5cd8463e587..c73da41457c7b8bf53ce230235b548df62fb0148 100644 (file)
--- 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<ID3v1::Tag>(ID3v1Index, false)->setProperties(properties);

diff --git a/taglib/mpeg/mpegfile.h b/taglib/mpeg/mpegfile.h
index 08415d5503f3879ee6768c84890b011bdeaf6caa..1d99898ce31631bf14c59698d203f42987623955 100644 (file)
--- 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 67be997c4dd669a7d49a8e1a296678f9f48d9818..78df182d17be7d7ca7490bc079af480e4e15b273 100644 (file)
--- 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<ID3v1::Tag>(WavID3v1Index, false)->setProperties(properties);
   return d->tag.access<APE::Tag>(WavAPEIndex, true)->setProperties(properties);
 }
 

diff --git a/taglib/wavpack/wavpackfile.h b/taglib/wavpack/wavpackfile.h
index 3e0c36c487e3cd56deaf1669e0443fbca11d90e4..918edca8eff2c36b07139368185ef0ccf63cff17 100644 (file)
--- 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&);