From c21fd955ff51ae0c01c66cf182406535e818df67 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Luk=C3=A1=C5=A1=20Lalinsk=C3=BD?= Date: Sun, 13 Feb 2011 10:27:56 +0000 Subject: [PATCH] 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 git-svn-id: svn://anonsvn.kde.org/home/kde/trunk/kdesupport/taglib@1220223 283d02a7-25f6-0310-bc7c-ecb5cbfe19da --- taglib/riff/rifffile.cpp | 55 ++++++++++++++----- taglib/riff/rifffile.h | 13 ++++- tests/data/noise.aif | Bin 0 -> 4400 bytes tests/data/noise_odd.aif | Bin 0 -> 4399 bytes tests/test_riff.cpp | 116 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 170 insertions(+), 14 deletions(-) create mode 100644 tests/data/noise.aif create mode 100644 tests/data/noise_odd.aif diff --git a/taglib/riff/rifffile.cpp b/taglib/riff/rifffile.cpp index e69f4b4b..5fa8e212 100644 --- a/taglib/riff/rifffile.cpp +++ b/taglib/riff/rifffile.cpp @@ -74,6 +74,11 @@ RIFF::File::File(FileName file, Endianness endianness) : TagLib::File(file) read(); } +TagLib::uint RIFF::File::riffSize() const +{ + return d->size; +} + TagLib::uint RIFF::File::chunkCount() const { return d->chunkNames.size(); @@ -89,6 +94,11 @@ TagLib::uint RIFF::File::chunkOffset(uint i) const 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 @@ ByteVector RIFF::File::chunkData(uint i) 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 @@ void RIFF::File::setChunkData(const ByteVector &name, const ByteVector &data) 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,11 +155,30 @@ void RIFF::File::setChunkData(const ByteVector &name, const ByteVector &data) } } - // 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); } //////////////////////////////////////////////////////////////////////////////// @@ -203,13 +229,16 @@ void RIFF::File::read() } 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); diff --git a/taglib/riff/rifffile.h b/taglib/riff/rifffile.h index 5c8c7d17..99282564 100644 --- a/taglib/riff/rifffile.h +++ b/taglib/riff/rifffile.h @@ -57,6 +57,11 @@ namespace TagLib { File(FileName file, Endianness endianness); + /*! + * \return The size of the main RIFF chunk. + */ + uint riffSize() const; + /*! * \return The number of chunks in the file. */ @@ -72,6 +77,11 @@ namespace TagLib { */ 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 " */ @@ -99,7 +109,8 @@ namespace TagLib { 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; diff --git a/tests/data/noise.aif b/tests/data/noise.aif new file mode 100644 index 0000000000000000000000000000000000000000..310b995e3c64878ca14d096b8e219f59389e6ef9 GIT binary patch literal 4400 zcmZ?s5AtPT5L9>cbaQj|_XV;UgculsWGaJ%1K%1KAPWfGe0+i!82ArCc%$TK2#kin zXb6mkz-S1JhQMeDjE2By2#kinXb6mkz-S1JhQQzsf#6_2V4t4R9Og=v!*?MBu_cG%{JsSW!C zT}EbahC3BZKKwj0_&NXcGl(r^iH`c+{NdX@hCeqieQ&j4Ui^?7xLvG;;9x)r9 ztO_m#Z}I$guEn~%jO}di4~t7zaP9iQuHpT=j{nc^rECfh*#h{5PBQ$;KFoGim$AH^ zdD1j?&Zu8*TYf%S`b+Z?vr!b6l{aIl&5s+G{)O0l)|30SLhk33Ex&f@GBz${`}l!z z*N1P`6%3O6e^sV&>OSP+l@pvX?Z46`4woolcklnjx~%CBzp!O~dzSet@-T~~Cj$T^ C(vHsn literal 0 HcmV?d00001 diff --git a/tests/data/noise_odd.aif b/tests/data/noise_odd.aif new file mode 100644 index 0000000000000000000000000000000000000000..bccfd7283c4be54cb38d869161148a620cc0647b GIT binary patch literal 4399 zcmZ?s5AtPT5L9>cbaQj|_XV;UgculsWGaJ%1K%1KAPWfGe0+i!82ArCc%$TK2#kin zXb6mkz-S1JhQMeDjE2By2#kinXb6mkz-S1JhQQzsf#6_2V4t4R9Og=v!*?MBu_cG%{JsSW!C zT}EbahC3BZKKwj0_&NXcGl(r^iH`c+{NdX@hCeqieQ&j4Ui^?7xLvG;;9x)r9 ztO_m#Z}I$guEn~%jO}di4~t7zaP9iQuHpT=j{nc^rECfh*#h{5PBQ$;KFoGim$AH^ zdD1j?&Zu8*TYf%S`b+Z?vr!b6l{aIl&5s+G{)O0l)|30SLhk33Ex&f@GBz${`}l!z z*N1P`6%3O6e^sV&>OSP+l@pvX?Z46`4woolcklnjx~%CBzp!O~dzSet@-T~~Cjgc7 Bj?Vx9 literal 0 HcmV?d00001 diff --git a/tests/test_riff.cpp b/tests/test_riff.cpp index 20539ae1..43b2f8bb 100644 --- a/tests/test_riff.cpp +++ b/tests/test_riff.cpp @@ -13,8 +13,10 @@ class PublicRIFF : public RIFF::File { 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 @@ class TestRIFF : public CppUnit::TestFixture { 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 @@ public: 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); -- 2.40.0