From 862952bdccd588ca4b262f94d0971bf82bf1c0bb Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Thu, 17 Dec 2015 11:43:11 +0900 Subject: [PATCH] Remove unnecessary private data members from TrueAudio::File. --- taglib/trueaudio/trueaudiofile.cpp | 103 ++++++++++++++--------------- tests/test_trueaudio.cpp | 27 ++++++++ 2 files changed, 78 insertions(+), 52 deletions(-) diff --git a/taglib/trueaudio/trueaudiofile.cpp b/taglib/trueaudio/trueaudiofile.cpp index 7910c485..27dc606d 100644 --- a/taglib/trueaudio/trueaudiofile.cpp +++ b/taglib/trueaudio/trueaudiofile.cpp @@ -55,9 +55,7 @@ public: ID3v2Location(-1), ID3v2OriginalSize(0), ID3v1Location(-1), - properties(0), - hasID3v1(false), - hasID3v2(false) {} + properties(0) {} ~FilePrivate() { @@ -73,12 +71,6 @@ public: TagUnion tag; Properties *properties; - - // These indicate whether the file *on disk* has these tags, not if - // this data structure does. This is used in computing offsets. - - bool hasID3v1; - bool hasID3v2; }; //////////////////////////////////////////////////////////////////////////////// @@ -167,40 +159,59 @@ bool TrueAudio::File::save() // Update ID3v2 tag if(ID3v2Tag() && !ID3v2Tag()->isEmpty()) { - if(!d->hasID3v2) { + + // ID3v2 tag is not empty. Update the old one or create a new one. + + if(d->ID3v2Location < 0) d->ID3v2Location = 0; + + const ByteVector data = ID3v2Tag()->render(); + insert(data, d->ID3v2Location, d->ID3v2OriginalSize); + + if(d->ID3v1Location >= 0) + d->ID3v1Location += (data.size() - d->ID3v2OriginalSize); + + d->ID3v2OriginalSize = data.size(); + } + else { + + // ID3v2 tag is empty. Remove the old one. + + if(d->ID3v2Location >= 0) { + removeBlock(d->ID3v2Location, d->ID3v2OriginalSize); + + if(d->ID3v1Location >= 0) + d->ID3v1Location -= d->ID3v2OriginalSize; + + d->ID3v2Location = -1; d->ID3v2OriginalSize = 0; } - ByteVector data = ID3v2Tag()->render(); - insert(data, d->ID3v2Location, d->ID3v2OriginalSize); - d->ID3v1Location -= d->ID3v2OriginalSize - data.size(); - d->ID3v2OriginalSize = data.size(); - d->hasID3v2 = true; - } - else if(d->hasID3v2) { - removeBlock(d->ID3v2Location, d->ID3v2OriginalSize); - d->ID3v1Location -= d->ID3v2OriginalSize; - d->ID3v2Location = -1; - d->ID3v2OriginalSize = 0; - d->hasID3v2 = false; } // Update ID3v1 tag if(ID3v1Tag() && !ID3v1Tag()->isEmpty()) { - if(!d->hasID3v1) { + + // ID3v1 tag is not empty. Update the old one or create a new one. + + if(d->ID3v1Location >= 0) { + seek(d->ID3v1Location); + } + else { seek(0, End); d->ID3v1Location = tell(); } - else - seek(d->ID3v1Location); + writeBlock(ID3v1Tag()->render()); - d->hasID3v1 = true; } - else if(d->hasID3v1) { - removeBlock(d->ID3v1Location, 128); - d->ID3v1Location = -1; - d->hasID3v1 = false; + else { + + // ID3v1 tag is empty. Remove the old one. + + if(d->ID3v1Location >= 0) { + truncate(d->ID3v1Location); + d->ID3v1Location = -1; + } } return true; @@ -218,27 +229,24 @@ ID3v2::Tag *TrueAudio::File::ID3v2Tag(bool create) void TrueAudio::File::strip(int tags) { - if(tags & ID3v1) { + if(tags & ID3v1) d->tag.set(TrueAudioID3v1Index, 0); - ID3v2Tag(true); - } - if(tags & ID3v2) { + if(tags & ID3v2) d->tag.set(TrueAudioID3v2Index, 0); - if(!ID3v1Tag()) - ID3v2Tag(true); - } + if(!ID3v1Tag()) + ID3v2Tag(true); } bool TrueAudio::File::hasID3v1Tag() const { - return d->hasID3v1; + return (d->ID3v1Location >= 0); } bool TrueAudio::File::hasID3v2Tag() const { - return d->hasID3v2; + return (d->ID3v2Location >= 0); } //////////////////////////////////////////////////////////////////////////////// @@ -252,27 +260,18 @@ void TrueAudio::File::read(bool readProperties) d->ID3v2Location = Utils::findID3v2(this); if(d->ID3v2Location >= 0) { - d->tag.set(TrueAudioID3v2Index, new ID3v2::Tag(this, d->ID3v2Location, d->ID3v2FrameFactory)); - d->ID3v2OriginalSize = ID3v2Tag()->header()->completeTagSize(); - - if(ID3v2Tag()->header()->tagSize() <= 0) - d->tag.set(TrueAudioID3v2Index, 0); - else - d->hasID3v2 = true; } // Look for an ID3v1 tag d->ID3v1Location = Utils::findID3v1(this); - if(d->ID3v1Location >= 0) { + if(d->ID3v1Location >= 0) d->tag.set(TrueAudioID3v1Index, new ID3v1::Tag(this, d->ID3v1Location)); - d->hasID3v1 = true; - } - if(!d->hasID3v1) + if(d->ID3v1Location < 0) ID3v2Tag(true); // Look for TrueAudio metadata @@ -281,12 +280,12 @@ void TrueAudio::File::read(bool readProperties) long streamLength; - if(d->hasID3v1) + if(d->ID3v1Location >= 0) streamLength = d->ID3v1Location; else streamLength = length(); - if(d->hasID3v2) { + if(d->ID3v2Location >= 0) { seek(d->ID3v2Location + d->ID3v2OriginalSize); streamLength -= (d->ID3v2Location + d->ID3v2OriginalSize); } diff --git a/tests/test_trueaudio.cpp b/tests/test_trueaudio.cpp index 8468e0ec..c960f5f7 100644 --- a/tests/test_trueaudio.cpp +++ b/tests/test_trueaudio.cpp @@ -16,6 +16,7 @@ class TestTrueAudio : public CppUnit::TestFixture CPPUNIT_TEST(testReadPropertiesWithoutID3v2); CPPUNIT_TEST(testReadPropertiesWithTags); CPPUNIT_TEST(testStripAndProperties); + CPPUNIT_TEST(testRepeatedSave); CPPUNIT_TEST_SUITE_END(); public: @@ -70,6 +71,32 @@ public: } } + void testRepeatedSave() + { + ScopedFileCopy copy("empty", ".tta"); + + { + TrueAudio::File f(copy.fileName().c_str()); + CPPUNIT_ASSERT(!f.hasID3v2Tag()); + CPPUNIT_ASSERT(!f.hasID3v1Tag()); + + f.ID3v2Tag(true)->setTitle("01234 56789 ABCDE FGHIJ"); + f.save(); + + f.ID3v2Tag()->setTitle("0"); + f.save(); + + f.ID3v1Tag(true)->setTitle("01234 56789 ABCDE FGHIJ"); + f.ID3v2Tag()->setTitle("01234 56789 ABCDE FGHIJ 01234 56789 ABCDE FGHIJ 01234 56789"); + f.save(); + } + { + TrueAudio::File f(copy.fileName().c_str()); + CPPUNIT_ASSERT(f.hasID3v2Tag()); + CPPUNIT_ASSERT(f.hasID3v1Tag()); + } + } + }; CPPUNIT_TEST_SUITE_REGISTRATION(TestTrueAudio);