diff --git a/taglib/matroska/ebml/ebmlmksegment.cpp b/taglib/matroska/ebml/ebmlmksegment.cpp index 88fcdd4c..79de5ca5 100644 --- a/taglib/matroska/ebml/ebmlmksegment.cpp +++ b/taglib/matroska/ebml/ebmlmksegment.cpp @@ -95,13 +95,13 @@ bool EBML::MkSegment::readLimited(File &file, offset_t scanLimit) MasterElement *pendingPaddingTarget = nullptr; offset_t accumulatedPadding = 0; std::unique_ptr element; - while((element = findNextElement(file, maxScanOffset))) { + while((element = findNextElement(file, maxOffset, maxScanOffset))) { if(const Id id = element->getId(); id == Id::MkSeekHead) { seekHead = element_cast(std::move(element)); if(!seekHead->read(file)) return false; // We have a seek head, let's use it for faster access to the other elements - if(const auto elementAfterSeekHead = findNextElement(file, maxScanOffset); + if(const auto elementAfterSeekHead = findNextElement(file, maxOffset, maxScanOffset); elementAfterSeekHead && elementAfterSeekHead->getId() == Id::VoidElement) seekHead->setPadding(elementAfterSeekHead->getSize()); const offset_t segDataOffset = segmentDataOffset(); diff --git a/taglib/matroska/ebml/ebmlutils.cpp b/taglib/matroska/ebml/ebmlutils.cpp index b76cdf2c..a150c78a 100644 --- a/taglib/matroska/ebml/ebmlutils.cpp +++ b/taglib/matroska/ebml/ebmlutils.cpp @@ -30,10 +30,13 @@ using namespace TagLib; -std::unique_ptr EBML::findElement(File &file, Element::Id id, offset_t maxOffset) +std::unique_ptr EBML::findElement( + File &file, Element::Id id, offset_t maxOffset, offset_t maxScanOffset) { + if(maxScanOffset < 0) + maxScanOffset = maxOffset; std::unique_ptr element; - while(file.tell() < maxOffset) { + while(file.tell() < maxScanOffset) { element = Element::factory(file, maxOffset); if(!element || element->getId() == id) return element; @@ -43,9 +46,12 @@ std::unique_ptr EBML::findElement(File &file, Element::Id id, off return element; } -std::unique_ptr EBML::findNextElement(File &file, offset_t maxOffset) +std::unique_ptr EBML::findNextElement( + File &file, offset_t maxOffset, offset_t maxScanOffset) { - return file.tell() < maxOffset ? Element::factory(file, maxOffset) : nullptr; + if(maxScanOffset < 0) + maxScanOffset = maxOffset; + return file.tell() < maxScanOffset ? Element::factory(file, maxOffset) : nullptr; } template diff --git a/taglib/matroska/ebml/ebmlutils.h b/taglib/matroska/ebml/ebmlutils.h index f35228dd..abbec1a5 100644 --- a/taglib/matroska/ebml/ebmlutils.h +++ b/taglib/matroska/ebml/ebmlutils.h @@ -31,8 +31,22 @@ namespace TagLib { class ByteVector; namespace EBML { - std::unique_ptr findElement(File &file, Element::Id id, offset_t maxOffset); - std::unique_ptr findNextElement(File &file, offset_t maxOffset); + /*! + * Find element with \a id in \a file starting at current file offset. + * The end of the element must be before \a maxOffset. + * The begin of the element must not be after \a maxScanOffset + * (\a maxOffset if not given). + */ + std::unique_ptr findElement( + File &file, Element::Id id, offset_t maxOffset, offset_t maxScanOffset = -1); + /*! + * Find element in \a file at current file offset. + * The end of the element must be before \a maxOffset. + * The begin of the element must not be after \a maxScanOffset + * (\a maxOffset if not given). + */ + std::unique_ptr findNextElement( + File &file, offset_t maxOffset, offset_t maxScanOffset = -1); template unsigned int VINTSizeLength(uint8_t firstByte); diff --git a/taglib/matroska/matroskafile.cpp b/taglib/matroska/matroskafile.cpp index 7ff001bd..81742a94 100644 --- a/taglib/matroska/matroskafile.cpp +++ b/taglib/matroska/matroskafile.cpp @@ -381,15 +381,15 @@ void Matroska::File::read(bool readProperties, Properties::ReadStyle readStyle) head->skipData(*this); } - offset_t maxOffset = fileLength; - if (readStyle == Properties::ReadStyle::Fast && maxOffset > FAST_SCAN_LIMIT) { - maxOffset = FAST_SCAN_LIMIT; + offset_t maxScanOffset = fileLength; + if(readStyle == Properties::ReadStyle::Fast && maxScanOffset > FAST_SCAN_LIMIT) { + maxScanOffset = FAST_SCAN_LIMIT; } - // Find the Matroska segment in the file + // Find the Matroska segment in the file. const std::unique_ptr segment( EBML::element_cast( - EBML::findElement(*this, EBML::Element::Id::MkSegment, maxOffset) + EBML::findElement(*this, EBML::Element::Id::MkSegment, fileLength, maxScanOffset) ) ); if(!segment) { @@ -400,11 +400,11 @@ void Matroska::File::read(bool readProperties, Properties::ReadStyle readStyle) // Read the segment into memory from file d->segment = segment->parseSegment(); - maxOffset = segment->getDataSize(); - if (readStyle == Properties::ReadStyle::Fast && maxOffset > FAST_SCAN_LIMIT) { - maxOffset = FAST_SCAN_LIMIT; + offset_t scanLimit = segment->getDataSize(); + if(readStyle == Properties::ReadStyle::Fast && scanLimit > FAST_SCAN_LIMIT) { + scanLimit = FAST_SCAN_LIMIT; } - if(!segment->readLimited(*this, maxOffset)) { + if(!segment->readLimited(*this, scanLimit)) { debug("Failed to read segment"); setValid(false); return; diff --git a/tests/test_matroska.cpp b/tests/test_matroska.cpp index 5ec0e79e..730d4a10 100644 --- a/tests/test_matroska.cpp +++ b/tests/test_matroska.cpp @@ -161,6 +161,7 @@ class TestMatroska : public CppUnit::TestFixture CPPUNIT_TEST(testSaveTypesNoTrailingVoid); CPPUNIT_TEST(testSaveTypesReclaimVoid); CPPUNIT_TEST(testUnknownSizeSegment); + CPPUNIT_TEST(testFastReadStyleLargeSegment); CPPUNIT_TEST_SUITE_END(); public: @@ -1778,6 +1779,43 @@ public: } } + void testFastReadStyleLargeSegment() + { + ScopedFileCopy copy("tags-before-cues", ".mkv"); + string newname = copy.fileName(); + + // Grow the segment past FAST_SCAN_LIMIT, as every real world file is. + { + PlainFile file(newname.c_str()); + ByteVector fileData = file.readAll(); + CPPUNIT_ASSERT_EQUAL(3412U, fileData.size()); + + // Void element, appended to the segment so the existing seek positions still hold. + const unsigned int voidDataSize = 1024 * 1024; + ByteVector voidElement("\xec", 1); + voidElement.append(ByteVector::fromULongLong(0x0100000000000000ULL | voidDataSize)); + voidElement.append(ByteVector(voidDataSize, '\0')); + + // Segment: 4 byte ID at 0x28, 8 byte size VINT at 0x2c, data from 0x34 to EOF. + ByteVector newData = fileData.mid(0, 0x2c); + newData.append(ByteVector::fromULongLong( + 0x0100000000000000ULL | (fileData.size() - 0x34 + voidElement.size()))); + newData.append(fileData.mid(0x34)); + newData.append(voidElement); + + file.seek(0); + file.writeBlock(newData); + } + + for(auto readStyle : {AudioProperties::Fast, AudioProperties::Average, + AudioProperties::Accurate}) { + Matroska::File f(newname.c_str(), true, readStyle); + CPPUNIT_ASSERT(f.isValid()); + CPPUNIT_ASSERT(f.tag(false)); + CPPUNIT_ASSERT_EQUAL(String("handbrake"), f.tag()->title()); + } + } + void testUnknownSizeSegment() { ScopedFileCopy copy("no-tags", ".mka");