[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    kdesupport/taglib
From:       Lukáš Lalinský <lalinsky () gmail ! com>
Date:       2011-02-13 10:27:56
Message-ID: 20110213102756.DC85BAC8D0 () svn ! kde ! org
[Download RAW message or body]

SVN commit 1220223 by lalinsky:

Fix writing of new RIFF chunks at even positions

If the last chunk had an odd size, the new chunk would have been written at
odd position, which is incorrect.

This is based on the patch by Jens Dyffort, but I ended up changing the
implementation to correctly handle subsequential updates to the file.

The whole RIFF code really needs to be rewritten in a different way...

BUG:243954


 M  +42 -13    taglib/riff/rifffile.cpp  
 M  +12 -1     taglib/riff/rifffile.h  
 AM            tests/data/noise.aif  
 AM            tests/data/noise_odd.aif  
 M  +116 -0    tests/test_riff.cpp  


--- trunk/kdesupport/taglib/taglib/riff/rifffile.cpp #1220222:1220223
@@ -74,6 +74,11 @@
     read();
 }
 
+TagLib::uint RIFF::File::riffSize() const
+{
+  return d->size;
+}
+
 TagLib::uint RIFF::File::chunkCount() const
 {
   return d->chunkNames.size();
@@ -89,6 +94,11 @@
   return d->chunkOffsets[i];
 }
 
+TagLib::uint RIFF::File::chunkPadding(uint i) const
+{
+  return d->chunkPadding[i];
+}
+
 ByteVector RIFF::File::chunkName(uint i) const
 {
   if(i >= chunkCount())
@@ -116,8 +126,7 @@
 
 void RIFF::File::setChunkData(const ByteVector &name, const ByteVector &data)
 {
-  if(d->chunkNames.size() == 0)
-  {
+  if(d->chunkNames.size() == 0) {
     debug("RIFF::File::setChunkData - No valid chunks found.");
     return;
   }
@@ -125,12 +134,10 @@
   for(uint i = 0; i < d->chunkNames.size(); i++) {
     if(d->chunkNames[i] == name) {
 
-      int sizeDifference = data.size() - d->chunkSizes[i];
-
       // First we update the global size
 
-      insert(ByteVector::fromUInt(d->size + sizeDifference,
-                                  d->endianness == BigEndian), 4, 4);
+      d->size += ((data.size() + 1) & ~1) - (d->chunkSizes[i] + d->chunkPadding[i]);
+      insert(ByteVector::fromUInt(d->size, d->endianness == BigEndian), 4, 4);
 
       // Now update the specific chunk
 
@@ -148,12 +155,31 @@
     }
   }
 
-  // Couldn't find an existing chunk, so let's create a new one.  First update
-  // the global size:
+  // Couldn't find an existing chunk, so let's create a new one.
 
-  insert(ByteVector::fromUInt(d->size + data.size() + 8, d->endianness == BigEndian), 4, 4);
-  writeChunk(name, data, d->chunkOffsets.back() + d->chunkSizes.back());
+  uint i = d->chunkNames.size() - 1;
+  ulong offset = d->chunkOffsets[i] + d->chunkSizes[i];
+
+  // First we update the global size
+
+  d->size += (offset & 1) + data.size() + 8;
+  insert(ByteVector::fromUInt(d->size, d->endianness == BigEndian), 4, 4);
+
+  // Now add the chunk to the file
+
+  writeChunk(name, data, offset, std::max(ulong(0), length() - offset), (offset & 1) ? 1 : 0);
+
+  // And update our internal structure
+
+  if (offset & 1) {
+    d->chunkPadding[i] = 1;
+    offset++;
 }
+  d->chunkNames.push_back(name);
+  d->chunkSizes.push_back(data.size());
+  d->chunkOffsets.push_back(offset + 8);
+  d->chunkPadding.push_back((data.size() & 0x01) ? 1 : 0);
+}
 
 ////////////////////////////////////////////////////////////////////////////////
 // private members
@@ -203,13 +229,16 @@
 }
 
 void RIFF::File::writeChunk(const ByteVector &name, const ByteVector &data,
-                            ulong offset, ulong replace)
+                            ulong offset, ulong replace, uint leadingPadding)
 {
-  ByteVector combined = name;
+  ByteVector combined;
+  if(leadingPadding) {
+    combined.append(ByteVector(leadingPadding, '\x00'));
+  }
+  combined.append(name);
   combined.append(ByteVector::fromUInt(data.size(), d->endianness == BigEndian));
   combined.append(data);
   if((data.size() & 0x01) != 0) {
-    // padding
     combined.append('\x00');
   }
   insert(combined, offset, replace);
--- trunk/kdesupport/taglib/taglib/riff/rifffile.h #1220222:1220223
@@ -58,6 +58,11 @@
       File(FileName file, Endianness endianness);
 
       /*!
+       * \return The size of the main RIFF chunk.
+       */
+      uint riffSize() const;
+
+      /*!
        * \return The number of chunks in the file.
        */
       uint chunkCount() const;
@@ -73,6 +78,11 @@
       uint chunkDataSize(uint i) const;
 
       /*!
+       * \return The size of the padding after the chunk (can be either 0 or 1).
+       */
+      uint chunkPadding(uint i) const;
+
+      /*!
        * \return The name of the specified chunk, for instance, "COMM" or "ID3 "
        */
       ByteVector chunkName(uint i) const;
@@ -99,7 +109,8 @@
 
       void read();
       void writeChunk(const ByteVector &name, const ByteVector &data,
-                      ulong offset, ulong replace = 0);
+                      ulong offset, ulong replace = 0,
+                      uint leadingPadding = 0);
 
       class FilePrivate;
       FilePrivate *d;
--- trunk/kdesupport/taglib/tests/test_riff.cpp #1220222:1220223
@@ -13,8 +13,10 @@
 {
 public:
   PublicRIFF(FileName file) : RIFF::File(file, BigEndian) {};
+  TagLib::uint riffSize() { return RIFF::File::riffSize(); };
   TagLib::uint chunkCount() { return RIFF::File::chunkCount(); };
   TagLib::uint chunkOffset(TagLib::uint i) { return RIFF::File::chunkOffset(i); };
+  TagLib::uint chunkPadding(TagLib::uint i) { return RIFF::File::chunkPadding(i); };
   TagLib::uint chunkDataSize(TagLib::uint i) { return RIFF::File::chunkDataSize(i); };
   ByteVector chunkName(TagLib::uint i) { return RIFF::File::chunkName(i); };
   ByteVector chunkData(TagLib::uint i) { return RIFF::File::chunkData(i); };
@@ -30,6 +32,9 @@
 {
   CPPUNIT_TEST_SUITE(TestRIFF);
   CPPUNIT_TEST(testPadding);
+  CPPUNIT_TEST(testLastChunkAtEvenPosition);
+  CPPUNIT_TEST(testLastChunkAtEvenPosition2);
+  CPPUNIT_TEST(testLastChunkAtEvenPosition3);
   CPPUNIT_TEST_SUITE_END();
 
 public:
@@ -77,6 +82,117 @@
     CPPUNIT_ASSERT_EQUAL(ByteVector("foo"), f->chunkData(2));
   }
 
+  void testLastChunkAtEvenPosition()
+  {
+    ScopedFileCopy copy("noise", ".aif");
+    string filename = copy.fileName();
+
+    PublicRIFF *f = new PublicRIFF(filename.c_str());
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(0xff0 + 8), f->chunkOffset(2));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(311), f->chunkDataSize(2));
+    CPPUNIT_ASSERT_EQUAL(ByteVector("SSND"), f->chunkName(2));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(1), f->chunkPadding(2));
+    CPPUNIT_ASSERT_EQUAL(long(4400), f->length());
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(4399 - 8), f->riffSize());
+    f->setChunkData("TEST", "abcd");
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(4088), f->chunkOffset(2));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(311), f->chunkDataSize(2));
+    CPPUNIT_ASSERT_EQUAL(ByteVector("SSND"), f->chunkName(2));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(1), f->chunkPadding(2));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(4408), f->chunkOffset(3));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(4), f->chunkDataSize(3));
+    CPPUNIT_ASSERT_EQUAL(ByteVector("TEST"), f->chunkName(3));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(0), f->chunkPadding(3));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(4412 - 8), f->riffSize());
+    delete f;
+
+    f = new PublicRIFF(filename.c_str());
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(4088), f->chunkOffset(2));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(311), f->chunkDataSize(2));
+    CPPUNIT_ASSERT_EQUAL(ByteVector("SSND"), f->chunkName(2));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(1), f->chunkPadding(2));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(4408), f->chunkOffset(3));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(4), f->chunkDataSize(3));
+    CPPUNIT_ASSERT_EQUAL(ByteVector("TEST"), f->chunkName(3));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(0), f->chunkPadding(3));
+    CPPUNIT_ASSERT_EQUAL(long(4412), f->length());
+    delete f;
+  }
+
+  void testLastChunkAtEvenPosition2()
+  {
+    ScopedFileCopy copy("noise_odd", ".aif");
+    string filename = copy.fileName();
+
+    PublicRIFF *f = new PublicRIFF(filename.c_str());
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(0xff0 + 8), f->chunkOffset(2));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(311), f->chunkDataSize(2));
+    CPPUNIT_ASSERT_EQUAL(ByteVector("SSND"), f->chunkName(2));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(0), f->chunkPadding(2));
+    CPPUNIT_ASSERT_EQUAL(long(4399), f->length());
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(4399 - 8), f->riffSize());
+    f->setChunkData("TEST", "abcd");
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(4088), f->chunkOffset(2));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(311), f->chunkDataSize(2));
+    CPPUNIT_ASSERT_EQUAL(ByteVector("SSND"), f->chunkName(2));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(1), f->chunkPadding(2));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(4408), f->chunkOffset(3));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(4), f->chunkDataSize(3));
+    CPPUNIT_ASSERT_EQUAL(ByteVector("TEST"), f->chunkName(3));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(0), f->chunkPadding(3));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(4412 - 8), f->riffSize());
+    delete f;
+
+    f = new PublicRIFF(filename.c_str());
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(4088), f->chunkOffset(2));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(311), f->chunkDataSize(2));
+    CPPUNIT_ASSERT_EQUAL(ByteVector("SSND"), f->chunkName(2));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(1), f->chunkPadding(2));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(4408), f->chunkOffset(3));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(4), f->chunkDataSize(3));
+    CPPUNIT_ASSERT_EQUAL(ByteVector("TEST"), f->chunkName(3));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(0), f->chunkPadding(3));
+    CPPUNIT_ASSERT_EQUAL(long(4412), f->length());
+    delete f;
+  }
+
+  void testLastChunkAtEvenPosition3()
+  {
+    ScopedFileCopy copy("noise_odd", ".aif");
+    string filename = copy.fileName();
+
+    PublicRIFF *f = new PublicRIFF(filename.c_str());
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(0xff0 + 8), f->chunkOffset(2));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(311), f->chunkDataSize(2));
+    CPPUNIT_ASSERT_EQUAL(ByteVector("SSND"), f->chunkName(2));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(0), f->chunkPadding(2));
+    CPPUNIT_ASSERT_EQUAL(long(4399), f->length());
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(4399 - 8), f->riffSize());
+    f->setChunkData("TEST", "abc");
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(4088), f->chunkOffset(2));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(311), f->chunkDataSize(2));
+    CPPUNIT_ASSERT_EQUAL(ByteVector("SSND"), f->chunkName(2));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(1), f->chunkPadding(2));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(4408), f->chunkOffset(3));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(3), f->chunkDataSize(3));
+    CPPUNIT_ASSERT_EQUAL(ByteVector("TEST"), f->chunkName(3));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(1), f->chunkPadding(3));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(4411 - 8), f->riffSize());
+    delete f;
+
+    f = new PublicRIFF(filename.c_str());
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(4088), f->chunkOffset(2));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(311), f->chunkDataSize(2));
+    CPPUNIT_ASSERT_EQUAL(ByteVector("SSND"), f->chunkName(2));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(1), f->chunkPadding(2));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(4408), f->chunkOffset(3));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(3), f->chunkDataSize(3));
+    CPPUNIT_ASSERT_EQUAL(ByteVector("TEST"), f->chunkName(3));
+    CPPUNIT_ASSERT_EQUAL(TagLib::uint(1), f->chunkPadding(3));
+    CPPUNIT_ASSERT_EQUAL(long(4412), f->length());
+    delete f;
+  }
+
 };
 
 CPPUNIT_TEST_SUITE_REGISTRATION(TestRIFF);
[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic