From b8dc105ae3ae5948ac6c7e3a45d5e4d6d846bd75 Mon Sep 17 00:00:00 2001 From: Scott Wheeler Date: Tue, 10 Sep 2019 15:27:44 +0200 Subject: Deprecate calls to MPEG::File::save(...) that use boolean params This uses explicit enums for e.g. the ID3v2 version, making calls more readable: file.save(ID3v1 | ID3v2, StripOthers, ID3v2::v4, Duplicate); Instead of: file.save(ID3v1 | ID3v2, true, 4, true); Needs to be ported to other types, per #922 --- taglib/CMakeLists.txt | 1 + taglib/mpeg/id3v2/id3v2header.h | 1 + taglib/mpeg/id3v2/id3v2tag.cpp | 16 +++++++------- taglib/mpeg/id3v2/id3v2tag.h | 22 ++++++++----------- taglib/mpeg/mpegfile.cpp | 26 ++++++++++++++-------- taglib/mpeg/mpegfile.h | 48 +++++++++++++++++------------------------ taglib/toolkit/tfile.h | 18 ++++++++++++++++ tests/test_id3v2.cpp | 10 ++++----- tests/test_mpeg.cpp | 10 ++++----- 9 files changed, 84 insertions(+), 68 deletions(-) diff --git a/taglib/CMakeLists.txt b/taglib/CMakeLists.txt index ec30e14c..f252256c 100644 --- a/taglib/CMakeLists.txt +++ b/taglib/CMakeLists.txt @@ -63,6 +63,7 @@ set(tag_HDRS mpeg/xingheader.h mpeg/id3v1/id3v1tag.h mpeg/id3v1/id3v1genres.h + mpeg/id3v2/id3v2.h mpeg/id3v2/id3v2extendedheader.h mpeg/id3v2/id3v2frame.h mpeg/id3v2/id3v2header.h diff --git a/taglib/mpeg/id3v2/id3v2header.h b/taglib/mpeg/id3v2/id3v2header.h index 12fb6d02..a1f2985e 100644 --- a/taglib/mpeg/id3v2/id3v2header.h +++ b/taglib/mpeg/id3v2/id3v2header.h @@ -28,6 +28,7 @@ #include "tbytevector.h" #include "taglib_export.h" +#include "id3v2.h" namespace TagLib { diff --git a/taglib/mpeg/id3v2/id3v2tag.cpp b/taglib/mpeg/id3v2/id3v2tag.cpp index 525e88b6..cf7f191a 100644 --- a/taglib/mpeg/id3v2/id3v2tag.cpp +++ b/taglib/mpeg/id3v2/id3v2tag.cpp @@ -462,7 +462,7 @@ PropertyMap ID3v2::Tag::setProperties(const PropertyMap &origProps) ByteVector ID3v2::Tag::render() const { - return render(4); + return render(ID3v2::v4); } void ID3v2::Tag::downgradeFrames(FrameList *frames, FrameList *newFrames) const @@ -568,17 +568,17 @@ void ID3v2::Tag::downgradeFrames(FrameList *frames, FrameList *newFrames) const } ByteVector ID3v2::Tag::render(int version) const +{ + return render(version == 3 ? v3 : v4); +} + +ByteVector ID3v2::Tag::render(Version version) const { // We need to render the "tag data" first so that we have to correct size to // render in the tag's header. The "tag data" -- everything that is included // in ID3v2::Header::tagSize() -- includes the extended header, frames and // padding, but does not include the tag's header or footer. - if(version != 3 && version != 4) { - debug("Unknown ID3v2 version, using ID3v2.4"); - version = 4; - } - // TODO: Render the extended header. // Downgrade the frames that ID3v2.3 doesn't support. @@ -587,7 +587,7 @@ ByteVector ID3v2::Tag::render(int version) const newFrames.setAutoDelete(true); FrameList frameList; - if(version == 4) { + if(version == v4) { frameList = d->frameList; } else { @@ -601,7 +601,7 @@ ByteVector ID3v2::Tag::render(int version) const // Loop through the frames rendering them and adding them to the tagData. for(FrameList::ConstIterator it = frameList.begin(); it != frameList.end(); it++) { - (*it)->header()->setVersion(version); + (*it)->header()->setVersion(version == v3 ? 3 : 4); if((*it)->header()->frameID().size() != 4) { debug("An ID3v2 frame of unsupported or unknown type \'" + String((*it)->header()->frameID()) + "\' has been discarded"); diff --git a/taglib/mpeg/id3v2/id3v2tag.h b/taglib/mpeg/id3v2/id3v2tag.h index 5811a5f1..96b6fb4a 100644 --- a/taglib/mpeg/id3v2/id3v2tag.h +++ b/taglib/mpeg/id3v2/id3v2tag.h @@ -33,21 +33,13 @@ #include "tmap.h" #include "taglib_export.h" +#include "id3v2.h" #include "id3v2framefactory.h" namespace TagLib { class File; - //! An ID3v2 implementation - - /*! - * This is a relatively complete and flexible framework for working with ID3v2 - * tags. - * - * \see ID3v2::Tag - */ - namespace ID3v2 { class Header; @@ -345,14 +337,18 @@ namespace TagLib { */ ByteVector render() const; + /*! + * \deprecated + */ + ByteVector render(int version) const; + /*! * Render the tag back to binary data, suitable to be written to disk. * - * The \a version parameter specifies the version of the rendered - * ID3v2 tag. It can be either 4 or 3. + * The \a version parameter specifies whether ID3v2.4 (default) or ID3v2.3 + * should be used. */ - // BIC: combine with the above method - ByteVector render(int version) const; + ByteVector render(Version version) const; /*! * Gets the current string handler that decides how the "Latin-1" data diff --git a/taglib/mpeg/mpegfile.cpp b/taglib/mpeg/mpegfile.cpp index 29a527e8..ddfe7f9e 100644 --- a/taglib/mpeg/mpegfile.cpp +++ b/taglib/mpeg/mpegfile.cpp @@ -214,6 +214,14 @@ bool MPEG::File::save(int tags, bool stripOthers, int id3v2Version) } bool MPEG::File::save(int tags, bool stripOthers, int id3v2Version, bool duplicateTags) +{ + return save(tags, + stripOthers ? StripOthers : StripNone, + id3v2Version == 3 ? ID3v2::v3 : ID3v2::v4, + duplicateTags ? Duplicate : DoNotDuplicate); +} + +bool MPEG::File::save(int tags, StripTags strip, ID3v2::Version version, DuplicateTags duplicate) { if(readOnly()) { debug("MPEG::File::save() -- File is read only."); @@ -222,22 +230,22 @@ bool MPEG::File::save(int tags, bool stripOthers, int id3v2Version, bool duplica // Create the tags if we've been asked to. - if(duplicateTags) { + if(duplicate == Duplicate) { // Copy the values from the tag that does exist into the new tag, // except if the existing tag is to be stripped. - if((tags & ID3v2) && ID3v1Tag() && !(stripOthers && !(tags & ID3v1))) + if((tags & ID3v2) && ID3v1Tag() && !(strip == StripOthers && !(tags & ID3v1))) Tag::duplicate(ID3v1Tag(), ID3v2Tag(true), false); - if((tags & ID3v1) && d->tag[ID3v2Index] && !(stripOthers && !(tags & ID3v2))) + if((tags & ID3v1) && d->tag[ID3v2Index] && !(strip == StripOthers && !(tags & ID3v2))) Tag::duplicate(ID3v2Tag(), ID3v1Tag(true), false); } // Remove all the tags not going to be saved. - if(stripOthers) - strip(~tags, false); + if(strip == StripOthers) + File::strip(~tags, false); if(ID3v2 & tags) { @@ -248,7 +256,7 @@ bool MPEG::File::save(int tags, bool stripOthers, int id3v2Version, bool duplica if(d->ID3v2Location < 0) d->ID3v2Location = 0; - const ByteVector data = ID3v2Tag()->render(id3v2Version); + const ByteVector data = ID3v2Tag()->render(version); insert(data, d->ID3v2Location, d->ID3v2OriginalSize); if(d->APELocation >= 0) @@ -263,7 +271,7 @@ bool MPEG::File::save(int tags, bool stripOthers, int id3v2Version, bool duplica // ID3v2 tag is empty. Remove the old one. - strip(ID3v2, false); + File::strip(ID3v2, false); } } @@ -287,7 +295,7 @@ bool MPEG::File::save(int tags, bool stripOthers, int id3v2Version, bool duplica // ID3v1 tag is empty. Remove the old one. - strip(ID3v1, false); + File::strip(ID3v1, false); } } @@ -316,7 +324,7 @@ bool MPEG::File::save(int tags, bool stripOthers, int id3v2Version, bool duplica // APE tag is empty. Remove the old one. - strip(APE, false); + File::strip(APE, false); } } diff --git a/taglib/mpeg/mpegfile.h b/taglib/mpeg/mpegfile.h index 2d2dff00..40beefc0 100644 --- a/taglib/mpeg/mpegfile.h +++ b/taglib/mpeg/mpegfile.h @@ -32,6 +32,8 @@ #include "mpegproperties.h" +#include "id3v2.h" + namespace TagLib { namespace ID3v2 { class Tag; class FrameFactory; } @@ -191,49 +193,39 @@ namespace TagLib { bool save(int tags); /*! - * Save the file. This will attempt to save all of the tag types that are - * specified by OR-ing together TagTypes values. The save() method above - * uses AllTags. This returns true if saving was successful. - * - * If \a stripOthers is true this strips all tags not included in the mask, - * but does not modify them in memory, so later calls to save() which make - * use of these tags will remain valid. This also strips empty tags. + * \deprecated */ // BIC: combine with the above method bool save(int tags, bool stripOthers); /*! - * Save the file. This will attempt to save all of the tag types that are - * specified by OR-ing together TagTypes values. The save() method above - * uses AllTags. This returns true if saving was successful. - * - * If \a stripOthers is true this strips all tags not included in the mask, - * but does not modify them in memory, so later calls to save() which make - * use of these tags will remain valid. This also strips empty tags. - * - * The \a id3v2Version parameter specifies the version of the saved - * ID3v2 tag. It can be either 4 or 3. + * \deprecated */ // BIC: combine with the above method bool save(int tags, bool stripOthers, int id3v2Version); + /*! + * \deprecated + */ + // BIC: combine with the above method + bool save(int tags, bool stripOthers, int id3v2Version, bool duplicateTags); + /*! * Save the file. This will attempt to save all of the tag types that are - * specified by OR-ing together TagTypes values. The save() method above - * uses AllTags. This returns true if saving was successful. + * specified by OR-ing together TagTypes values. * - * If \a stripOthers is true this strips all tags not included in the mask, - * but does not modify them in memory, so later calls to save() which make - * use of these tags will remain valid. This also strips empty tags. + * \a strip can be set to strip all tags except those in \a tags. Those + * tags will not be modified in memory, and thus remain valid. * - * The \a id3v2Version parameter specifies the version of the saved - * ID3v2 tag. It can be either 4 or 3. + * \a version specifies the ID3v2 version to be used for writing tags. By + * default, the latest standard, ID3v2.4 is used. * - * If \a duplicateTags is true and at least one tag -- ID3v1 or ID3v2 -- - * exists this will duplicate its content into the other tag. + * If \a duplicate is set to DuplicateTags and at least one tag -- ID3v1 + * or ID3v2 -- exists this will duplicate its content into the other tag. */ - // BIC: combine with the above method - bool save(int tags, bool stripOthers, int id3v2Version, bool duplicateTags); + bool save(int tags, StripTags strip, + ID3v2::Version version = ID3v2::v4, + DuplicateTags duplicate = Duplicate); /*! * Returns a pointer to the ID3v2 tag of the file. diff --git a/taglib/toolkit/tfile.h b/taglib/toolkit/tfile.h index 55f53a52..c9a9d37e 100644 --- a/taglib/toolkit/tfile.h +++ b/taglib/toolkit/tfile.h @@ -62,6 +62,24 @@ namespace TagLib { End }; + /*! + * Specify which tags to strip either explicitly, or on save. + */ + enum StripTags { + StripNone, //addFrame(f); - file.save(MPEG::File::ID3v2, true, 3); + file.save(MPEG::File::ID3v2, File::StripOthers, ID3v2::v3); CPPUNIT_ASSERT_EQUAL(true, file.hasID3v2Tag()); ByteVector data = f->render(); @@ -166,7 +166,7 @@ public: MPEG::File file(copy.fileName().c_str()); file.ID3v2Tag(true)->addFrame(f); - file.save(MPEG::File::ID3v2, true, 3); + file.save(MPEG::File::ID3v2, File::StripOthers, ID3v2::v3); CPPUNIT_ASSERT(file.hasID3v2Tag()); ByteVector data = f->render(); @@ -756,7 +756,7 @@ public: foo.ID3v2Tag()->addFrame(new ID3v2::TextIdentificationFrame("TSOT", String::Latin1)); foo.ID3v2Tag()->addFrame(new ID3v2::TextIdentificationFrame("TSST", String::Latin1)); foo.ID3v2Tag()->addFrame(new ID3v2::TextIdentificationFrame("TSOP", String::Latin1)); - foo.save(MPEG::File::AllTags, true, 3); + foo.save(MPEG::File::AllTags, File::StripOthers, ID3v2::v3); } { MPEG::File bar(newname.c_str()); @@ -1029,7 +1029,7 @@ public: MPEG::File bar(newname.c_str()); bar.ID3v2Tag()->removeFrames("TPE1"); // Should strip ID3v1 here and not add old values to ID3v2 again - bar.save(MPEG::File::ID3v2, true); + bar.save(MPEG::File::ID3v2, File::StripOthers); } MPEG::File f(newname.c_str()); @@ -1275,7 +1275,7 @@ public: CPPUNIT_ASSERT_EQUAL(44100, f.audioProperties()->sampleRate()); f.ID3v2Tag()->setArtist("Artist A"); - f.save(MPEG::File::ID3v2, true); + f.save(MPEG::File::ID3v2, File::StripOthers); } { MPEG::File f(copy.fileName().c_str()); diff --git a/tests/test_mpeg.cpp b/tests/test_mpeg.cpp index fd46f9ed..45d60ece 100644 --- a/tests/test_mpeg.cpp +++ b/tests/test_mpeg.cpp @@ -187,7 +187,7 @@ public: f.tag()->setTitle(xxx); f.tag()->setArtist("Artist A"); - f.save(MPEG::File::AllTags, true, 4); + f.save(MPEG::File::AllTags, File::StripOthers, ID3v2::v4); CPPUNIT_ASSERT_EQUAL(true, f.hasID3v2Tag()); } { @@ -230,7 +230,7 @@ public: f.tag()->setTitle(xxx); f.tag()->setArtist("Artist A"); - f.save(MPEG::File::AllTags, true, 3); + f.save(MPEG::File::AllTags, File::StripOthers, ID3v2::v3); CPPUNIT_ASSERT_EQUAL(true, f.hasID3v2Tag()); } { @@ -369,7 +369,7 @@ public: { MPEG::File f(copy.fileName().c_str()); f.ID3v2Tag(true)->setTitle(""); - f.save(MPEG::File::ID3v2, false); + f.save(MPEG::File::ID3v2, File::StripNone); } { MPEG::File f(copy.fileName().c_str()); @@ -389,7 +389,7 @@ public: { MPEG::File f(copy.fileName().c_str()); f.ID3v1Tag(true)->setTitle(""); - f.save(MPEG::File::ID3v1, false); + f.save(MPEG::File::ID3v1, File::StripNone); } { MPEG::File f(copy.fileName().c_str()); @@ -409,7 +409,7 @@ public: { MPEG::File f(copy.fileName().c_str()); f.APETag(true)->setTitle(""); - f.save(MPEG::File::APE, false); + f.save(MPEG::File::APE, File::StripNone); } { MPEG::File f(copy.fileName().c_str()); -- cgit v1.2.3