diff --git a/taglib/CMakeLists.txt b/taglib/CMakeLists.txt index 54905307..f2689a88 100644 --- a/taglib/CMakeLists.txt +++ b/taglib/CMakeLists.txt @@ -248,6 +248,7 @@ if(WITH_MATROSKA) ) set(tag_PRIVATE_HDRS ${tag_PRIVATE_HDRS} matroska/ebml/ebmlbinaryelement.h + matroska/ebml/ebmldeferredbinaryelement.h matroska/ebml/ebmlelement.h matroska/ebml/ebmlmasterelement.h matroska/ebml/ebmlmkattachments.h @@ -481,6 +482,7 @@ if(WITH_MATROSKA) set(ebml_SRCS matroska/ebml/ebmlbinaryelement.cpp + matroska/ebml/ebmldeferredbinaryelement.cpp matroska/ebml/ebmlelement.cpp matroska/ebml/ebmlmasterelement.cpp matroska/ebml/ebmlmkattachments.cpp diff --git a/taglib/matroska/ebml/ebmldeferredbinaryelement.cpp b/taglib/matroska/ebml/ebmldeferredbinaryelement.cpp new file mode 100644 index 00000000..2db8be73 --- /dev/null +++ b/taglib/matroska/ebml/ebmldeferredbinaryelement.cpp @@ -0,0 +1,59 @@ +/*************************************************************************** + * This library is free software; you can redistribute it and/or modify * + * it under the terms of the GNU Lesser General Public License version * + * 2.1 as published by the Free Software Foundation. * + * * + * This library is distributed in the hope that it will be useful, but * + * WITHOUT ANY WARRANTY; without even the implied warranty of * + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * + * Lesser General Public License for more details. * + * * + * You should have received a copy of the GNU Lesser General Public * + * License along with this library; if not, write to the Free Software * + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA * + * 02110-1301 USA * + * * + * Alternatively, this file is available under the Mozilla Public * + * License Version 1.1. You may obtain a copy of the License at * + * http://www.mozilla.org/MPL/ * + ***************************************************************************/ + +#include "ebmldeferredbinaryelement.h" +#include "tfile.h" + +using namespace TagLib; + +EBML::DeferredBinaryElement::DeferredBinaryElement(Id id, int sizeLength, offset_t dataSize): + BinaryElement(id, sizeLength, dataSize) +{ +} + +EBML::DeferredBinaryElement::DeferredBinaryElement(Id id, int sizeLength, offset_t dataSize, offset_t): + BinaryElement(id, sizeLength, dataSize) +{ +} + +EBML::DeferredBinaryElement::DeferredBinaryElement(Id id): + BinaryElement(id) +{ +} + +bool EBML::DeferredBinaryElement::read(File &file) +{ + // The file is positioned at the data of the element, which is all we have + // to remember in order to be able to read it later. + dataOffset = file.tell(); + deferred = true; + skipData(file); + return true; +} + +bool EBML::DeferredBinaryElement::isDeferred() const +{ + return deferred; +} + +offset_t EBML::DeferredBinaryElement::getDataOffset() const +{ + return dataOffset; +} diff --git a/taglib/matroska/ebml/ebmldeferredbinaryelement.h b/taglib/matroska/ebml/ebmldeferredbinaryelement.h new file mode 100644 index 00000000..a386fb88 --- /dev/null +++ b/taglib/matroska/ebml/ebmldeferredbinaryelement.h @@ -0,0 +1,74 @@ +/*************************************************************************** + * This library is free software; you can redistribute it and/or modify * + * it under the terms of the GNU Lesser General Public License version * + * 2.1 as published by the Free Software Foundation. * + * * + * This library is distributed in the hope that it will be useful, but * + * WITHOUT ANY WARRANTY; without even the implied warranty of * + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * + * Lesser General Public License for more details. * + * * + * You should have received a copy of the GNU Lesser General Public * + * License along with this library; if not, write to the Free Software * + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA * + * 02110-1301 USA * + * * + * Alternatively, this file is available under the Mozilla Public * + * License Version 1.1. You may obtain a copy of the License at * + * http://www.mozilla.org/MPL/ * + ***************************************************************************/ + +#ifndef TAGLIB_EBMLDEFERREDBINARYELEMENT_H +#define TAGLIB_EBMLDEFERREDBINARYELEMENT_H +#ifndef DO_NOT_DOCUMENT + +#include "ebmlbinaryelement.h" + +namespace TagLib { + class File; + + namespace EBML { + /*! + * A binary element whose data is not pulled into memory while the file is + * read. read() only registers the offset of the data in the file and + * skips over it, so that the payload can be loaded later, when it is + * really requested. This keeps reading the metadata of a file cheap even + * if it contains large attachments. + * + * Elements which are created to be rendered (i.e. not read from a file) + * behave exactly like a BinaryElement: setValue() makes the data + * available and isDeferred() stays false. + */ + class DeferredBinaryElement : public BinaryElement + { + public: + DeferredBinaryElement(Id id, int sizeLength, offset_t dataSize); + DeferredBinaryElement(Id id, int sizeLength, offset_t dataSize, offset_t); + explicit DeferredBinaryElement(Id id); + + /*! + * Registers the offset of the data in \a file and skips the data. + */ + bool read(File &file) override; + + /*! + * Returns \c true if the data has not been read into memory, i.e. if it + * has to be loaded from getDataOffset() to be available. + */ + bool isDeferred() const; + + /*! + * Returns the offset of the data inside the file, only valid if + * isDeferred() is \c true. + */ + offset_t getDataOffset() const; + + private: + offset_t dataOffset = 0; + bool deferred = false; + }; + } +} + +#endif +#endif diff --git a/taglib/matroska/ebml/ebmlelement.cpp b/taglib/matroska/ebml/ebmlelement.cpp index 27b43240..e15edc52 100644 --- a/taglib/matroska/ebml/ebmlelement.cpp +++ b/taglib/matroska/ebml/ebmlelement.cpp @@ -22,6 +22,7 @@ #include "ebmlvoidelement.h" #include "ebmlmasterelement.h" #include "ebmlbinaryelement.h" +#include "ebmldeferredbinaryelement.h" #include "ebmlfloatelement.h" #include "ebmlmkseekhead.h" #include "ebmlmksegment.h" diff --git a/taglib/matroska/ebml/ebmlelement.h b/taglib/matroska/ebml/ebmlelement.h index e6c4ded3..54776578 100644 --- a/taglib/matroska/ebml/ebmlelement.h +++ b/taglib/matroska/ebml/ebmlelement.h @@ -139,6 +139,7 @@ namespace TagLib class MasterElement; class UIntElement; class BinaryElement; + class DeferredBinaryElement; class FloatElement; class MkSegment; class MkInfo; @@ -201,7 +202,9 @@ namespace TagLib template <> struct GetElementTypeById { using type = UIntElement; }; template <> struct GetElementTypeById { using type = UIntElement; }; template <> struct GetElementTypeById { using type = UIntElement; }; - template <> struct GetElementTypeById { using type = BinaryElement; }; + // The data of an attached file is only loaded when it is requested, + // see Matroska::File::attachments(). + template <> struct GetElementTypeById { using type = DeferredBinaryElement; }; template <> struct GetElementTypeById { using type = BinaryElement; }; template <> struct GetElementTypeById { using type = BinaryElement; }; template <> struct GetElementTypeById { using type = BinaryElement; }; diff --git a/taglib/matroska/ebml/ebmlmkattachments.cpp b/taglib/matroska/ebml/ebmlmkattachments.cpp index c66fb660..e2e180dd 100644 --- a/taglib/matroska/ebml/ebmlmkattachments.cpp +++ b/taglib/matroska/ebml/ebmlmkattachments.cpp @@ -22,6 +22,7 @@ #include "ebmlstringelement.h" #include "ebmluintelement.h" #include "ebmlbinaryelement.h" +#include "ebmldeferredbinaryelement.h" #include "matroskaattachments.h" #include "matroskaattachedfile.h" @@ -55,14 +56,14 @@ std::unique_ptr EBML::MkAttachments::parse() const const String *filename = nullptr; const String *description = nullptr; const String *mediaType = nullptr; - const ByteVector *data = nullptr; + const DeferredBinaryElement *data = nullptr; Matroska::AttachedFile::UID uid = 0; const auto attachedFile = element_cast(element); for(const auto &attachedFileChild : *attachedFile) { if(const Id id = attachedFileChild->getId(); id == Id::MkAttachedFileName) filename = &element_cast(attachedFileChild)->getValue(); else if(id == Id::MkAttachedFileData) - data = &element_cast(attachedFileChild)->getValue(); + data = element_cast(attachedFileChild); else if(id == Id::MkAttachedFileDescription) description = &element_cast(attachedFileChild)->getValue(); else if(id == Id::MkAttachedFileMediaType) @@ -73,9 +74,19 @@ std::unique_ptr EBML::MkAttachments::parse() const if(!(filename && data)) continue; - attachments->addAttachedFile(Matroska::AttachedFile( - *data, *filename, mediaType ? *mediaType : String(), - uid, description ? *description : String())); + const String mediaTypeValue = mediaType ? *mediaType : String(); + const String descriptionValue = description ? *description : String(); + if(data->isDeferred()) { + // The data has been left in the file, it is loaded when the attachments + // are requested, see Matroska::File::attachments(). + attachments->addAttachedFile(Matroska::AttachedFile( + data->getDataOffset(), data->getDataSize(), *filename, mediaTypeValue, + uid, descriptionValue)); + } + else { + attachments->addAttachedFile(Matroska::AttachedFile( + data->getValue(), *filename, mediaTypeValue, uid, descriptionValue)); + } } return attachments; } diff --git a/taglib/matroska/matroskaattachedfile.cpp b/taglib/matroska/matroskaattachedfile.cpp index 7957e86e..7a4366c3 100644 --- a/taglib/matroska/matroskaattachedfile.cpp +++ b/taglib/matroska/matroskaattachedfile.cpp @@ -30,12 +30,21 @@ public: const String &mediaType, UID uid, const String &description) : fileName(fileName), description(description), mediaType(mediaType), data(data), uid(uid) {} + AttachedFilePrivate(offset_t dataOffset, offset_t dataSize, + const String &fileName, const String &mediaType, UID uid, + const String &description) : + fileName(fileName), description(description), mediaType(mediaType), + uid(uid), dataOffset(dataOffset), dataSize(dataSize), dataDeferred(true) {} ~AttachedFilePrivate() = default; String fileName; String description; String mediaType; ByteVector data; UID uid = 0; + // Position of the data in the file, valid while dataDeferred is true. + offset_t dataOffset = 0; + offset_t dataSize = 0; + bool dataDeferred = false; }; //////////////////////////////////////////////////////////////////////////////// @@ -49,6 +58,14 @@ Matroska::AttachedFile::AttachedFile(const ByteVector &data, { } +Matroska::AttachedFile::AttachedFile(offset_t dataOffset, offset_t dataSize, + const String &fileName, const String &mediaType, UID uid, + const String &description) : + d(std::make_unique(dataOffset, dataSize, fileName, + mediaType, uid, description)) +{ +} + Matroska::AttachedFile::AttachedFile(const AttachedFile &other) : d(std::make_unique(*other.d)) { @@ -97,3 +114,30 @@ Matroska::AttachedFile::UID Matroska::AttachedFile::uid() const { return d->uid; } + +//////////////////////////////////////////////////////////////////////////////// +// private members +//////////////////////////////////////////////////////////////////////////////// + +bool Matroska::AttachedFile::isDataDeferred() const +{ + return d->dataDeferred; +} + +offset_t Matroska::AttachedFile::deferredDataOffset() const +{ + return d->dataOffset; +} + +offset_t Matroska::AttachedFile::deferredDataSize() const +{ + return d->dataSize; +} + +void Matroska::AttachedFile::setLoadedData(const ByteVector &data) +{ + d->data = data; + d->dataDeferred = false; + d->dataOffset = 0; + d->dataSize = 0; +} diff --git a/taglib/matroska/matroskaattachedfile.h b/taglib/matroska/matroskaattachedfile.h index 2b7a2964..2758c3c5 100644 --- a/taglib/matroska/matroskaattachedfile.h +++ b/taglib/matroska/matroskaattachedfile.h @@ -22,6 +22,7 @@ #define TAGLIB_MATROSKAATTACHEDFILE_H #include +#include "taglib.h" #include "tstring.h" #include "taglib_export.h" @@ -29,7 +30,13 @@ namespace TagLib { class String; class ByteVector; + namespace EBML { + class MkAttachments; + } + namespace Matroska { + class File; + //! Attached file embedded into a Matroska file. class TAGLIB_EXPORT AttachedFile { @@ -91,6 +98,11 @@ namespace TagLib { /*! * Returns the data of the attached file. + * + * \note When the attached file was read from a file, its data is only + * loaded from the file when the attachments are requested using + * Matroska::File::attachments(). Objects obtained from there therefore + * always have their data available. */ const ByteVector &data() const; @@ -100,7 +112,31 @@ namespace TagLib { UID uid() const; private: + friend class EBML::MkAttachments; + friend class File; class AttachedFilePrivate; + + /*! + * Construct an attached file whose data is not loaded yet. The data is + * at \a dataOffset in the file and \a dataSize bytes long, it will be + * loaded by Matroska::File when the attachments are requested. + */ + AttachedFile(offset_t dataOffset, offset_t dataSize, + const String &fileName, const String &mediaType, UID uid, + const String &description); + + //! Returns \c true if the data still has to be loaded from the file. + bool isDataDeferred() const; + + //! Returns the offset of the not yet loaded data inside the file. + offset_t deferredDataOffset() const; + + //! Returns the size of the not yet loaded data. + offset_t deferredDataSize() const; + + //! Sets the data which has been loaded from the file. + void setLoadedData(const ByteVector &data); + TAGLIB_MSVC_SUPPRESS_WARNING_NEEDS_TO_HAVE_DLL_INTERFACE std::unique_ptr d; }; diff --git a/taglib/matroska/matroskaattachments.cpp b/taglib/matroska/matroskaattachments.cpp index f5dd1b29..3c97d799 100644 --- a/taglib/matroska/matroskaattachments.cpp +++ b/taglib/matroska/matroskaattachments.cpp @@ -27,6 +27,7 @@ #include "ebmlmasterelement.h" #include "ebmlstringelement.h" #include "ebmlbinaryelement.h" +#include "ebmldeferredbinaryelement.h" #include "ebmluintelement.h" #include "ebmlutils.h" #include "tlist.h" diff --git a/taglib/matroska/matroskafile.cpp b/taglib/matroska/matroskafile.cpp index 81742a94..f542ddbc 100644 --- a/taglib/matroska/matroskafile.cpp +++ b/taglib/matroska/matroskafile.cpp @@ -47,12 +47,15 @@ using namespace TagLib; class Matroska::File::FilePrivate { public: - FilePrivate() = default; + explicit FilePrivate(TagLib::File *file) : file(file) {} ~FilePrivate() = default; FilePrivate(const FilePrivate &) = delete; FilePrivate &operator=(const FilePrivate &) = delete; + // The file the elements have been read from, used to load the data of + // attached files on demand. + TagLib::File *file; std::unique_ptr tag; std::unique_ptr attachments; std::unique_ptr chapters; @@ -79,7 +82,7 @@ bool Matroska::File::isSupported(IOStream *stream) Matroska::File::File(FileName file, bool readProperties, Properties::ReadStyle readStyle) : TagLib::File(file), - d(std::make_unique()) + d(std::make_unique(this)) { if(!isOpen()) { debug("Failed to open matroska file"); @@ -92,7 +95,7 @@ Matroska::File::File(FileName file, bool readProperties, Matroska::File::File(IOStream *stream, bool readProperties, Properties::ReadStyle readStyle) : TagLib::File(stream), - d(std::make_unique()) + d(std::make_unique(this)) { if(!isOpen()) { debug("Failed to open matroska file"); @@ -246,6 +249,7 @@ List Matroska::File::complexProperties(const String &key) const } } if(d->attachments) { + loadAttachedFileData(); const auto &attachedFiles = d->attachments->attachedFileList(); for(const auto &attachedFile : attachedFiles) { if(keyMatchesAttachedFile(key, attachedFile)) { @@ -352,6 +356,7 @@ Matroska::Attachments *Matroska::File::attachments(bool create) const { if(!d->attachments && create) d->attachments = std::make_unique(); + loadAttachedFileData(); return d->attachments.get(); } @@ -362,6 +367,42 @@ Matroska::Chapters *Matroska::File::chapters(bool create) const return d->chapters.get(); } +void Matroska::File::loadAttachedFileData() const +{ + if(!d->attachments) + return; + + // Check with the const list first, getting the mutable list would mark the + // attachments as to be rendered again. + const auto &files = d->attachments->attachedFileList(); + if(std::none_of(files.begin(), files.end(), + [](const AttachedFile &file) { return file.isDataDeferred(); })) + return; + + bool positionSaved = false; + offset_t position = 0; + for(auto &attachedFile : d->attachments->attachedFiles()) { + if(!attachedFile.isDataDeferred()) + continue; + if(!positionSaved) { + position = d->file->tell(); + positionSaved = true; + } + const auto dataSize = attachedFile.deferredDataSize(); + d->file->seek(attachedFile.deferredDataOffset()); + ByteVector data = d->file->readBlock(dataSize); + if(static_cast(data.size()) != dataSize) { + debug("Failed to read data of attached file"); + } + // Mark the data as loaded even if it is incomplete, retrying would fail + // just the same and the file position is no longer known afterwards. + attachedFile.setLoadedData(data); + } + if(positionSaved) { + d->file->seek(position); + } +} + void Matroska::File::read(bool readProperties, Properties::ReadStyle readStyle) { const offset_t fileLength = length(); @@ -463,6 +504,11 @@ bool Matroska::File::save(WriteStyle writeStyle) return false; } + // The attachments are rendered from the attached files, so the data of + // attached files which have not been requested has to be loaded now, before + // the file is modified, otherwise it would be lost. + loadAttachedFileData(); + // Do not create new attachments, chapters or tags and corresponding // seek head entries if only empty objects were created. if(d->chapters && d->chapters->chapterEditionList().isEmpty() && diff --git a/taglib/matroska/matroskafile.h b/taglib/matroska/matroskafile.h index c3f146cd..4fb23293 100644 --- a/taglib/matroska/matroskafile.h +++ b/taglib/matroska/matroskafile.h @@ -160,6 +160,11 @@ namespace TagLib::Matroska { * attachments. * If \a create is \c true it will create attachments if none exist and * returns a valid pointer. + * + * The data of the attached files is read from the file here, reading the + * file itself only registers where the data is stored. Applications + * which are not interested in the attached files therefore do not have to + * pay for reading them. */ Attachments *attachments(bool create = false) const; @@ -184,6 +189,13 @@ namespace TagLib::Matroska { private: void read(bool readProperties, Properties::ReadStyle readStyle); + /*! + * Read the data of the attached files which has been left in the file + * while reading, see EBML::DeferredBinaryElement. This is done when the + * attachments are requested and before saving, so that an attachment + * which has never been requested is not lost when the file is written. + */ + void loadAttachedFileData() const; class FilePrivate; friend class Properties; TAGLIB_MSVC_SUPPRESS_WARNING_NEEDS_TO_HAVE_DLL_INTERFACE diff --git a/tests/test_matroska.cpp b/tests/test_matroska.cpp index 730d4a10..7057d52b 100644 --- a/tests/test_matroska.cpp +++ b/tests/test_matroska.cpp @@ -135,12 +135,33 @@ #include "matroskachapters.h" #include "matroskasimpletag.h" #include "plainfile.h" +#include "tfilestream.h" #include #include "utils.h" using namespace std; using namespace TagLib; +namespace { + + //! File stream counting the number of bytes read from the file. + class CountingFileStream : public FileStream + { + public: + explicit CountingFileStream(FileName name) : FileStream(name, true) {} + + ByteVector readBlock(size_t length) override + { + ByteVector data = FileStream::readBlock(length); + bytesRead += data.size(); + return data; + } + + size_t bytesRead = 0; + }; + +} + class TestMatroska : public CppUnit::TestFixture { CPPUNIT_TEST_SUITE(TestMatroska); @@ -162,6 +183,8 @@ class TestMatroska : public CppUnit::TestFixture CPPUNIT_TEST(testSaveTypesReclaimVoid); CPPUNIT_TEST(testUnknownSizeSegment); CPPUNIT_TEST(testFastReadStyleLargeSegment); + CPPUNIT_TEST(testAttachedFileDataReadOnDemand); + CPPUNIT_TEST(testSaveUnrequestedAttachedFileData); CPPUNIT_TEST_SUITE_END(); public: @@ -1816,6 +1839,75 @@ public: } } + void testAttachedFileDataReadOnDemand() + { + ScopedFileCopy copy("no-tags", ".mka"); + string newname = copy.fileName(); + + // The number of bytes read stays the same for any size of attached file. + for(const unsigned int dataSize : {64U * 1024U, 512U * 1024U}) { + const ByteVector attachmentData(dataSize, 'x'); + { + Matroska::File f(newname.c_str()); + CPPUNIT_ASSERT(f.isValid()); + auto attachments = f.attachments(true); + attachments->clear(); + attachments->addAttachedFile(Matroska::AttachedFile( + attachmentData, "cover.jpg", "image/jpeg", 0xa7ac, "Cover")); + CPPUNIT_ASSERT(f.save()); + } + + CountingFileStream stream(newname.c_str()); + Matroska::File f(&stream, true, AudioProperties::Fast); + CPPUNIT_ASSERT(f.isValid()); + // Only the elements around the data of the attached file were read. + CPPUNIT_ASSERT(stream.bytesRead < 4096); + + const size_t bytesReadWithoutData = stream.bytesRead; + auto attachments = f.attachments(false); + CPPUNIT_ASSERT(attachments); + const auto &attachedFiles = attachments->attachedFileList(); + CPPUNIT_ASSERT_EQUAL(1U, attachedFiles.size()); + CPPUNIT_ASSERT_EQUAL(String("cover.jpg"), attachedFiles.front().fileName()); + CPPUNIT_ASSERT_EQUAL(String("Cover"), attachedFiles.front().description()); + CPPUNIT_ASSERT_EQUAL(0xa7acULL, attachedFiles.front().uid()); + CPPUNIT_ASSERT_EQUAL(attachmentData, attachedFiles.front().data()); + CPPUNIT_ASSERT(stream.bytesRead >= bytesReadWithoutData + dataSize); + } + } + + void testSaveUnrequestedAttachedFileData() + { + ScopedFileCopy copy("no-tags", ".mka"); + string newname = copy.fileName(); + const ByteVector attachmentData(64 * 1024, 'x'); + { + Matroska::File f(newname.c_str()); + CPPUNIT_ASSERT(f.isValid()); + f.attachments(true)->addAttachedFile(Matroska::AttachedFile( + attachmentData, "cover.jpg", "image/jpeg", 0xa7ac, "Cover")); + CPPUNIT_ASSERT(f.save()); + } + { + // Save without ever requesting the attachments. + Matroska::File f(newname.c_str(), true, AudioProperties::Fast); + CPPUNIT_ASSERT(f.isValid()); + f.tag(true)->setTitle("Test title"); + CPPUNIT_ASSERT(f.save()); + } + { + Matroska::File f(newname.c_str(), true, AudioProperties::Accurate); + CPPUNIT_ASSERT(f.isValid()); + CPPUNIT_ASSERT_EQUAL(String("Test title"), f.tag(true)->title()); + auto attachments = f.attachments(false); + CPPUNIT_ASSERT(attachments); + const auto &attachedFiles = attachments->attachedFileList(); + CPPUNIT_ASSERT_EQUAL(1U, attachedFiles.size()); + CPPUNIT_ASSERT_EQUAL(String("cover.jpg"), attachedFiles.front().fileName()); + CPPUNIT_ASSERT_EQUAL(attachmentData, attachedFiles.front().data()); + } + } + void testUnknownSizeSegment() { ScopedFileCopy copy("no-tags", ".mka");