Remove unnecessary private data members from TrueAudio::File.

diff --git a/taglib/trueaudio/trueaudiofile.cpp b/taglib/trueaudio/trueaudiofile.cpp
index 7910c4857889788ec9a71701f9a839ea608eed85..27dc606d885c3346f73c3eecf4a3896cb06aecf9 100644 (file)
--- 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;
-      d->ID3v2OriginalSize = 0;
-    }
-    ByteVector data = ID3v2Tag()->render();
+
+    const ByteVector data = ID3v2Tag()->render();
     insert(data, d->ID3v2Location, d->ID3v2OriginalSize);
-    d->ID3v1Location -= d->ID3v2OriginalSize - data.size();
+
+    if(d->ID3v1Location >= 0)
+      d->ID3v1Location += (data.size() - d->ID3v2OriginalSize);
+
     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;
+  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;
+    }
   }
 
   // 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 8468e0ec3d6724e18d3dbfe2ba34a9b9c3a43115..c960f5f77313ff4e16654ce36ae2bb1550e00b63 100644 (file)
--- 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);