diff --git a/taglib/ape/apefile.cpp b/taglib/ape/apefile.cpp index 41341b86..b3da1379 100644 --- a/taglib/ape/apefile.cpp +++ b/taglib/ape/apefile.cpp @@ -131,12 +131,9 @@ void APE::File::removeUnsupportedProperties(const StringList &properties) PropertyMap APE::File::setProperties(const PropertyMap &properties) { - if(d->hasAPE) - return d->tag.access(ApeAPEIndex, false)->setProperties(properties); - else if(d->hasID3v1) - return d->tag.access(ApeID3v1Index, false)->setProperties(properties); - else - return d->tag.access(ApeAPEIndex, true)->setProperties(properties); + if(d->hasID3v1) + d->tag.access(ApeID3v1Index, false)->setProperties(properties); + return d->tag.access(ApeAPEIndex, true)->setProperties(properties); } APE::Properties *APE::File::audioProperties() const diff --git a/taglib/ape/apefile.h b/taglib/ape/apefile.h index 162f980d..cfee50da 100644 --- a/taglib/ape/apefile.h +++ b/taglib/ape/apefile.h @@ -128,8 +128,8 @@ namespace TagLib { /*! * Implements the unified property interface -- import function. - * As for the export, only one tag is taken into account. If the file - * has no tag at all, APE will be created. + * Creates an APEv2 tag if necessary. A pontentially existing ID3v1 + * tag will be updated as well. */ PropertyMap setProperties(const PropertyMap &); diff --git a/taglib/flac/flacfile.cpp b/taglib/flac/flacfile.cpp index cb1cd5ce..e0faa18d 100644 --- a/taglib/flac/flacfile.cpp +++ b/taglib/flac/flacfile.cpp @@ -163,14 +163,7 @@ void FLAC::File::removeUnsupportedProperties(const StringList &unsupported) PropertyMap FLAC::File::setProperties(const PropertyMap &properties) { - if(d->hasXiphComment) - return d->tag.access(FlacXiphIndex, false)->setProperties(properties); - else if(d->hasID3v2) - return d->tag.access(FlacID3v2Index, false)->setProperties(properties); - else if(d->hasID3v1) - return d->tag.access(FlacID3v1Index, false)->setProperties(properties); - else - return d->tag.access(FlacXiphIndex, true)->setProperties(properties); + return d->tag.access(FlacXiphIndex, true)->setProperties(properties); } FLAC::Properties *FLAC::File::audioProperties() const diff --git a/taglib/flac/flacfile.h b/taglib/flac/flacfile.h index 3b17bc7f..cf91a21e 100644 --- a/taglib/flac/flacfile.h +++ b/taglib/flac/flacfile.h @@ -122,8 +122,10 @@ namespace TagLib { /*! * Implements the unified property interface -- import function. - * As with the export, only one tag is taken into account. If the file - * has no tag at all, a XiphComment will be created. + * This always creates a Xiph comment, if none exists. The return value + * relates to the Xiph comment only. + * 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 46fa1a2c..b27fb8d2 100644 --- a/taglib/mpc/mpcfile.cpp +++ b/taglib/mpc/mpcfile.cpp @@ -135,15 +135,11 @@ void MPC::File::removeUnsupportedProperties(const StringList &properties) PropertyMap MPC::File::setProperties(const PropertyMap &properties) { - if(d->hasAPE) - return d->tag.access(MPCAPEIndex, false)->setProperties(properties); - else if(d->hasID3v1) - return d->tag.access(MPCID3v1Index, false)->setProperties(properties); - else - return d->tag.access(APE, true)->setProperties(properties); + if(d->hasID3v1) + d->tag.access(MPCID3v1Index, false)->setProperties(properties); + return d->tag.access(MPCAPEIndex, true)->setProperties(properties); } - MPC::Properties *MPC::File::audioProperties() const { return d->properties; diff --git a/taglib/mpc/mpcfile.h b/taglib/mpc/mpcfile.h index f3f4ac7d..887aa46d 100644 --- a/taglib/mpc/mpcfile.h +++ b/taglib/mpc/mpcfile.h @@ -124,8 +124,8 @@ namespace TagLib { /*! * Implements the unified property interface -- import function. - * As with the export, only one tag is taken into account. If the file - * has no tag at all, an APE tag will be created. + * Affects only the APEv2 tag which will be created if necessary. + * 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 d2dce48a..1091ea14 100644 --- a/taglib/mpeg/mpegfile.cpp +++ b/taglib/mpeg/mpegfile.cpp @@ -156,16 +156,13 @@ void MPEG::File::removeUnsupportedProperties(const StringList &properties) else if(d->hasID3v1) d->tag.access(ID3v1Index, false)->removeUnsupportedProperties(properties); } + PropertyMap MPEG::File::setProperties(const PropertyMap &properties) { - if(d->hasID3v2) - return d->tag.access(ID3v2Index, false)->setProperties(properties); - else if(d->hasAPE) - return d->tag.access(APEIndex, false)->setProperties(properties); - else if(d->hasID3v1) - return d->tag.access(ID3v1Index, false)->setProperties(properties); - else - return d->tag.access(ID3v2Index, true)->setProperties(properties); + if(d->hasID3v1) + // update ID3v1 tag if it exists, but ignore the return value + d->tag.access(ID3v1Index, false)->setProperties(properties); + return d->tag.access(ID3v2Index, true)->setProperties(properties); } MPEG::Properties *MPEG::File::audioProperties() const diff --git a/taglib/mpeg/mpegfile.h b/taglib/mpeg/mpegfile.h index f6dc8d56..725dc19d 100644 --- a/taglib/mpeg/mpegfile.h +++ b/taglib/mpeg/mpegfile.h @@ -133,7 +133,7 @@ namespace TagLib { virtual Tag *tag() const; /*! - * Implements the unified property interface -- export function. + * Implements the reading part of the unified property interface. * If the file contains more than one tag, only the * first one (in the order ID3v2, APE, ID3v1) will be converted to the * PropertyMap. @@ -143,9 +143,12 @@ namespace TagLib { void removeUnsupportedProperties(const StringList &properties); /*! - * Implements the unified tag dictionary interface -- import function. - * As with the export, only one tag is taken into account. If the file - * has no tag at all, ID3v2 will be created. + * 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. + * 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. */ PropertyMap setProperties(const PropertyMap &); diff --git a/taglib/toolkit/tfile.h b/taglib/toolkit/tfile.h index 860a889a..750efdff 100644 --- a/taglib/toolkit/tfile.h +++ b/taglib/toolkit/tfile.h @@ -80,12 +80,14 @@ namespace TagLib { /*! * Exports the tags of the file as dictionary mapping (human readable) tag - * names (Strings) to StringLists of tag values. Calls the according specialization - * in the File subclasses. + * names (uppercase Strings) to StringLists of tag values. Calls the according + * specialization in the File subclasses. * For each metadata object of the file that could not be parsed into the PropertyMap * format, the returend map's unsupportedData() list will contain one entry identifying * that object (e.g. the frame type for ID3v2 tags). Use removeUnsupportedProperties() * to remove (a subset of) them. + * For files that contain more than one tag (e.g. an MP3 with both an ID3v2 and an ID3v2 + * tag) only the most "modern" one will be exported (ID3v2 in this case). */ virtual PropertyMap properties() const; @@ -103,9 +105,14 @@ namespace TagLib { * If some value(s) could not be written imported to the specific metadata format, * the returned PropertyMap will contain those value(s). Otherwise it will be empty, * indicating that no problems occured. + * With file types that support several tag formats (for instance, MP3 files can have + * ID3v1, ID3v2, and APEv2 tags), this function will create the most appropriate one + * (ID3v2 for MP3 files). Older formats will be updated as well, if they exist, but won't + * be taken into account for the return value of this function. + * See the documentation of the subclass implementations for detailed descriptions. */ virtual PropertyMap setProperties(const PropertyMap &properties); - + /*! * Returns a pointer to this file's audio properties. This should be * reimplemented in the concrete subclasses. If no audio properties were diff --git a/taglib/trueaudio/trueaudiofile.cpp b/taglib/trueaudio/trueaudiofile.cpp index 06879447..b584ce67 100644 --- a/taglib/trueaudio/trueaudiofile.cpp +++ b/taglib/trueaudio/trueaudiofile.cpp @@ -141,12 +141,9 @@ PropertyMap TrueAudio::File::properties() const PropertyMap TrueAudio::File::setProperties(const PropertyMap &properties) { - if(d->hasID3v2) - return d->tag.access(TrueAudioID3v2Index, false)->setProperties(properties); - else if(d->hasID3v1) - return d->tag.access(TrueAudioID3v1Index, false)->setProperties(properties); - else - return d->tag.access(TrueAudioID3v2Index, true)->setProperties(properties); + if(d->hasID3v1) + d->tag.access(TrueAudioID3v1Index, false)->setProperties(properties); + return d->tag.access(TrueAudioID3v2Index, true)->setProperties(properties); } TrueAudio::Properties *TrueAudio::File::audioProperties() const diff --git a/taglib/trueaudio/trueaudiofile.h b/taglib/trueaudio/trueaudiofile.h index 8b8b0c05..d3f51c93 100644 --- a/taglib/trueaudio/trueaudiofile.h +++ b/taglib/trueaudio/trueaudiofile.h @@ -139,8 +139,8 @@ namespace TagLib { /*! * Implements the unified property interface -- import function. - * As with the export, only one tag is taken into account. If the file - * has no tag at all, ID3v2 will be created. + * Creates in ID3v2 tag if necessary. If an ID3v1 tag exists, it will + * be updated as well, within the limitations of ID3v1. */ PropertyMap setProperties(const PropertyMap &); diff --git a/taglib/wavpack/wavpackfile.cpp b/taglib/wavpack/wavpackfile.cpp index 25495662..409ccabf 100644 --- a/taglib/wavpack/wavpackfile.cpp +++ b/taglib/wavpack/wavpackfile.cpp @@ -119,12 +119,9 @@ PropertyMap WavPack::File::properties() const PropertyMap WavPack::File::setProperties(const PropertyMap &properties) { - if(d->hasAPE) - return d->tag.access(WavAPEIndex, false)->setProperties(properties); - else if(d->hasID3v1) - return d->tag.access(WavID3v1Index, false)->setProperties(properties); - else - return d->tag.access(APE, true)->setProperties(properties); + if(d->hasID3v1) + d->tag.access(WavID3v1Index, false)->setProperties(properties); + return d->tag.access(WavAPEIndex, true)->setProperties(properties); } WavPack::Properties *WavPack::File::audioProperties() const diff --git a/taglib/wavpack/wavpackfile.h b/taglib/wavpack/wavpackfile.h index cb0dfab9..ca1c752a 100644 --- a/taglib/wavpack/wavpackfile.h +++ b/taglib/wavpack/wavpackfile.h @@ -118,8 +118,8 @@ namespace TagLib { /*! * Implements the unified property interface -- import function. - * As for the export, only one tag is taken into account. If the file - * has no tag at all, APE will be created. + * Creates an APE tag if it does not exists and calls setProperties() on + * that. Any existing ID3v1 tag will be updated as well. */ PropertyMap setProperties(const PropertyMap&); diff --git a/tests/test_flac.cpp b/tests/test_flac.cpp index 3d31f33c..7f40a382 100644 --- a/tests/test_flac.cpp +++ b/tests/test_flac.cpp @@ -24,6 +24,7 @@ class TestFLAC : public CppUnit::TestFixture CPPUNIT_TEST(testRepeatedSave); CPPUNIT_TEST(testSaveMultipleValues); CPPUNIT_TEST(testDict); + CPPUNIT_TEST(testInvalid); CPPUNIT_TEST_SUITE_END(); public: @@ -231,6 +232,17 @@ public: CPPUNIT_ASSERT_EQUAL(String("artöst 2"), dict["ARTIST"][1]); } + void testInvalid() + { + ScopedFileCopy copy("silence-44-s", ".flac"); + PropertyMap map; + map["HÄÖ"] = String("bla"); + FLAC::File f(copy.fileName().c_str()); + PropertyMap invalid = f.setProperties(map); + CPPUNIT_ASSERT_EQUAL(uint(1), invalid.size()); + CPPUNIT_ASSERT_EQUAL(uint(0), f.properties().size()); + } + }; CPPUNIT_TEST_SUITE_REGISTRATION(TestFLAC); diff --git a/tests/utils.h b/tests/utils.h index 39e15ce9..b69bfa50 100644 --- a/tests/utils.h +++ b/tests/utils.h @@ -7,6 +7,7 @@ #include #include #include +#include #endif #include #include