diff --git a/taglib/flac/flacfile.cpp b/taglib/flac/flacfile.cpp index 017df40f..99acb62e 100644 --- a/taglib/flac/flacfile.cpp +++ b/taglib/flac/flacfile.cpp @@ -72,6 +72,8 @@ public: ByteVector xiphCommentData; String iXMLData; ByteVector bextData; + bool hasiXML { false }; + bool hasBEXT { false }; List blocks; offset_t flacStart { 0 }; @@ -279,6 +281,10 @@ bool FLAC::File::save() payload.append(ByteVector::fromUInt(xml.size(), false)); payload.append(xml); d->blocks.append(new UnknownMetadataBlock(MetadataBlock::Application, payload)); + d->hasiXML = true; + } + else { + d->hasiXML = false; } if(!d->bextData.isEmpty()) { ByteVector payload; @@ -287,6 +293,10 @@ bool FLAC::File::save() payload.append(ByteVector::fromUInt(d->bextData.size(), false)); payload.append(d->bextData); d->blocks.append(new UnknownMetadataBlock(MetadataBlock::Application, payload)); + d->hasBEXT = true; + } + else { + d->hasBEXT = false; } // Replace metadata blocks @@ -532,12 +542,12 @@ bool FLAC::File::hasID3v2Tag() const bool FLAC::File::hasiXMLData() const { - return !d->iXMLData.isEmpty(); + return d->hasiXML; } bool FLAC::File::hasBEXTData() const { - return !d->bextData.isEmpty(); + return d->hasBEXT; } //////////////////////////////////////////////////////////////////////////////// @@ -719,14 +729,18 @@ void FLAC::File::scan() } if(innerId == "iXML") { - if(d->iXMLData.isEmpty()) + if(!d->hasiXML) { + d->hasiXML = true; d->iXMLData = String(innerData, String::UTF8); + } else debug("FLAC::File::scan() -- multiple iXML blocks found, discarding"); } else if(innerId == "bext") { - if(d->bextData.isEmpty()) + if(!d->hasBEXT) { + d->hasBEXT = true; d->bextData = innerData; + } else debug("FLAC::File::scan() -- multiple BEXT blocks found, discarding"); } diff --git a/tests/test_flac.cpp b/tests/test_flac.cpp index 3541b3f5..791fc439 100644 --- a/tests/test_flac.cpp +++ b/tests/test_flac.cpp @@ -74,6 +74,7 @@ class TestFLAC : public CppUnit::TestFixture CPPUNIT_TEST(testReadBEXTRiffWrapped); CPPUNIT_TEST(testWriteiXMLAndBEXT); CPPUNIT_TEST(testWriteEmptyClearsiXMLAndBEXT); + CPPUNIT_TEST(testHasiXMLAndBEXTReflectFileState); CPPUNIT_TEST(testRoundTripPreservesUnknownApplicationBlock); CPPUNIT_TEST_SUITE_END(); @@ -833,6 +834,45 @@ public: } } + void testHasiXMLAndBEXTReflectFileState() + { + // hasiXMLData() / hasBEXTData() must report whether the *file* carries an + // iXML / bext APPLICATION block, not whether in-memory payload happens to + // be non-empty. Regression test for an issue where the accessors were + // implemented as !data.isEmpty() and so flipped on as soon as set*Data() + // was called, before save(), and missed a real-but-empty block. + ScopedFileCopy copy("silence-44-s", ".flac"); + const string newname = copy.fileName(); + + { + FLAC::File f(newname.c_str()); + CPPUNIT_ASSERT(!f.hasiXMLData()); + CPPUNIT_ASSERT(!f.hasBEXTData()); + f.setiXMLData(""); + f.setBEXTData(ByteVector("bext")); + // File hasn't been saved yet — file still has no blocks. + CPPUNIT_ASSERT(!f.hasiXMLData()); + CPPUNIT_ASSERT(!f.hasBEXTData()); + f.save(); + // After save the blocks are on disk. + CPPUNIT_ASSERT(f.hasiXMLData()); + CPPUNIT_ASSERT(f.hasBEXTData()); + } + { + FLAC::File f(newname.c_str()); + CPPUNIT_ASSERT(f.hasiXMLData()); + CPPUNIT_ASSERT(f.hasBEXTData()); + f.setiXMLData(String()); + f.setBEXTData(ByteVector()); + // In-memory payload now empty but the file still carries both blocks. + CPPUNIT_ASSERT(f.hasiXMLData()); + CPPUNIT_ASSERT(f.hasBEXTData()); + f.save(); + CPPUNIT_ASSERT(!f.hasiXMLData()); + CPPUNIT_ASSERT(!f.hasBEXTData()); + } + } + void testRoundTripPreservesUnknownApplicationBlock() { // Source: silence-44-s with an extra APPLICATION/"SMED" block injected