Set MP4 chapters only if modified

An equality operator is added for the chapters. The chapters are
only written to the file if they were really modified, so just
reading the chapters without modifying them will not affect
the save operation.
This commit is contained in:
Urs Fleisch
2026-04-25 11:46:51 +02:00
parent 05c2c8671e
commit 497c040f04
6 changed files with 192 additions and 50 deletions

View File

@@ -377,7 +377,6 @@ if(WITH_MP4)
mp4/mp4stem.cpp
mp4/mp4itemfactory.cpp
mp4/mp4chapter.cpp
mp4/mp4chapterholder.cpp
mp4/mp4nerochapterlist.cpp
mp4/mp4qtchapterlist.cpp
)

View File

@@ -61,6 +61,16 @@ MP4::Chapter &MP4::Chapter::Chapter::operator=(const Chapter &other)
MP4::Chapter &MP4::Chapter::Chapter::operator=(
Chapter &&other) noexcept = default;
bool MP4::Chapter::operator==(const Chapter &other) const
{
return title() == other.title() && startTime() == other.startTime();
}
bool MP4::Chapter::operator!=(const Chapter &other) const
{
return !(*this == other);
}
void MP4::Chapter::swap(Chapter &other) noexcept
{
using std::swap;

View File

@@ -68,6 +68,16 @@ namespace TagLib {
*/
Chapter &operator=(Chapter &&other) noexcept;
/*!
* Returns \c true if the chapter and \a other contain the same data.
*/
bool operator==(const Chapter &other) const;
/*!
* Returns \c true if the chapter and \a other differ in data.
*/
bool operator!=(const Chapter &other) const;
/*!
* Exchanges the content of the object with the content of \a other.
*/

View File

@@ -1,44 +0,0 @@
/**************************************************************************
copyright : (C) 2006 by Urs Fleisch
email : ufleisch@users.sourceforge.net
**************************************************************************/
/***************************************************************************
* 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 "mp4chapterholder.h"
using namespace TagLib;
MP4::ChapterList MP4::ChapterHolder::chapters() const
{
return chapterList;
}
void MP4::ChapterHolder::setChapters(const ChapterList &chapters)
{
chapterList = chapters;
modified = true;
}
bool MP4::ChapterHolder::isModified() const
{
return modified;
}

View File

@@ -39,17 +39,22 @@ namespace TagLib {
/*!
* Get list of chapters.
*/
ChapterList chapters() const;
ChapterList chapters() const { return chapterList; }
/*!
* Set list of chapters.
*/
void setChapters(const ChapterList &chapters);
void setChapters(const ChapterList &chapters) { chapterList = chapters; }
/*!
* Returns \c true if the list of chapters has been modified.
*/
bool isModified() const;
bool isModified() const { return modified; }
/*!
* Set if the contained chapters are modified.
*/
void setModified(bool chaptersModified) { modified = chaptersModified; }
protected:
ChapterList chapterList;
@@ -84,8 +89,15 @@ namespace TagLib {
{
if (!holder) {
holder = std::make_unique<T>();
// The chapters have not been read before, so we do not know their
// current state and mark them as modified. Otherwise, the check below
// would not set the chapters if they are empty.
holder->setModified(true);
}
if(holder->isModified() || holder->chapters() != chapters) {
holder->setChapters(chapters);
holder->setModified(true);
}
holder->setChapters(chapters);
}
/*!
@@ -99,7 +111,11 @@ namespace TagLib {
bool saveChaptersIfModified(std::unique_ptr<T> &holder, TagLib::File *file)
{
if(holder && holder->isModified()) {
return holder->write(file);
if(holder->write(file)) {
holder->setModified(false);
return true;
}
return false;
}
return true;
}

View File

@@ -34,6 +34,7 @@
#include "mp4atom.h"
#include "mp4file.h"
#include "mp4itemfactory.h"
#include "mp4chapterholder.h"
#include "plainfile.h"
#include <cppunit/extensions/HelperMacros.h>
#include "utils.h"
@@ -69,6 +70,56 @@ namespace
};
CustomItemFactory CustomItemFactory::factory;
class MockChapterList : public MP4::ChapterHolder {
public:
static const MP4::ChapterList mockChapters;
bool read(TagLib::File *)
{
chapterList = mockChapters;
++readCount;
return true;
}
bool write(TagLib::File *)
{
++writeCount;
return true;
}
int readCount = 0;
int writeCount = 0;
};
const MP4::ChapterList MockChapterList::mockChapters = {
MP4::Chapter("Mock", 123)
};
class MockChapterFile : public PlainFile {
public:
explicit MockChapterFile(FileName name) : PlainFile(name)
{
}
MP4::ChapterList chapters()
{
return getChaptersLazy(chapterList, this);
}
void setChapters(const MP4::ChapterList& chapters)
{
setChaptersLazy(chapterList, chapters);
}
bool save() override
{
return MP4::saveChaptersIfModified(chapterList, this);
}
std::unique_ptr<MockChapterList> chapterList;
};
} // namespace
class TestMP4 : public CppUnit::TestFixture
@@ -121,6 +172,7 @@ class TestMP4 : public CppUnit::TestFixture
CPPUNIT_TEST(testQTChapterListSingleEmptyTitleNotStripped);
CPPUNIT_TEST(testNeroAndQTChaptersAreIndependent);
CPPUNIT_TEST(testNeroChaptersAloneWhenNoQT);
CPPUNIT_TEST(testLazyReadingAndWritingChapters);
CPPUNIT_TEST_SUITE_END();
public:
@@ -1634,6 +1686,105 @@ public:
}
}
void testLazyReadingAndWritingChapters()
{
// No reads or writes if chapters are not used
{
MockChapterFile f(TEST_FILE_PATH_C("no-tags.m4a"));
f.save();
CPPUNIT_ASSERT(!f.chapterList);
}
// Do not read if already read, do not write if not modified
{
MockChapterFile f(TEST_FILE_PATH_C("no-tags.m4a"));
auto chapters = f.chapters();
CPPUNIT_ASSERT(chapters == MockChapterList::mockChapters);
CPPUNIT_ASSERT(f.chapterList);
CPPUNIT_ASSERT_EQUAL(1, f.chapterList->readCount);
chapters = f.chapters();
CPPUNIT_ASSERT(chapters == MockChapterList::mockChapters);
CPPUNIT_ASSERT_EQUAL(1, f.chapterList->readCount);
f.save();
CPPUNIT_ASSERT_EQUAL(1, f.chapterList->readCount);
CPPUNIT_ASSERT_EQUAL(0, f.chapterList->writeCount);
}
// Do not write if not modified
{
MockChapterFile f(TEST_FILE_PATH_C("no-tags.m4a"));
auto chapters = f.chapters();
CPPUNIT_ASSERT(chapters == MockChapterList::mockChapters);
CPPUNIT_ASSERT(f.chapterList);
CPPUNIT_ASSERT_EQUAL(1, f.chapterList->readCount);
f.setChapters(MockChapterList::mockChapters);
f.save();
CPPUNIT_ASSERT_EQUAL(1, f.chapterList->readCount);
CPPUNIT_ASSERT_EQUAL(0, f.chapterList->writeCount);
}
// Write if set without being read before
{
MockChapterFile f(TEST_FILE_PATH_C("no-tags.m4a"));
f.setChapters(MP4::ChapterList());
f.save();
CPPUNIT_ASSERT(f.chapterList);
CPPUNIT_ASSERT_EQUAL(0, f.chapterList->readCount);
CPPUNIT_ASSERT_EQUAL(1, f.chapterList->writeCount);
}
// Do write if modified
{
MockChapterFile f(TEST_FILE_PATH_C("no-tags.m4a"));
CPPUNIT_ASSERT(f.chapters() == MockChapterList::mockChapters);
CPPUNIT_ASSERT(f.chapterList);
CPPUNIT_ASSERT_EQUAL(1, f.chapterList->readCount);
const auto chapters1 = MP4::ChapterList({
MP4::Chapter("Chapter 1", 0),
});
f.setChapters(chapters1);
CPPUNIT_ASSERT(f.chapters() == chapters1);
f.save();
CPPUNIT_ASSERT(f.chapters() == chapters1);
CPPUNIT_ASSERT_EQUAL(1, f.chapterList->readCount);
CPPUNIT_ASSERT_EQUAL(1, f.chapterList->writeCount);
f.setChapters(chapters1);
f.save();
CPPUNIT_ASSERT_EQUAL(1, f.chapterList->writeCount);
auto chapters2 = MP4::ChapterList({
MP4::Chapter("Chapter 1", 0),
MP4::Chapter("Chapter 2", 1),
});
f.setChapters(chapters2);
CPPUNIT_ASSERT(f.chapters() == chapters2);
f.save();
CPPUNIT_ASSERT(f.chapters() == chapters2);
CPPUNIT_ASSERT_EQUAL(2, f.chapterList->writeCount);
chapters2 = MP4::ChapterList({
MP4::Chapter("Chapter 1", 0),
MP4::Chapter("Chapter 2", 2),
});
f.setChapters(chapters2);
f.save();
CPPUNIT_ASSERT_EQUAL(3, f.chapterList->writeCount);
f.setChapters(chapters2);
CPPUNIT_ASSERT(f.chapters() == chapters2);
f.save();
CPPUNIT_ASSERT(f.chapters() == chapters2);
CPPUNIT_ASSERT_EQUAL(3, f.chapterList->writeCount);
const auto chapters3 = MP4::ChapterList({
MP4::Chapter("Chapter 1", 0),
MP4::Chapter("Chapter 3", 2),
});
f.setChapters(chapters3);
CPPUNIT_ASSERT(f.chapters() == chapters3);
f.save();
CPPUNIT_ASSERT(f.chapters() == chapters3);
CPPUNIT_ASSERT_EQUAL(4, f.chapterList->writeCount);
f.setChapters(MP4::ChapterList());
CPPUNIT_ASSERT(f.chapters().isEmpty());
f.save();
CPPUNIT_ASSERT(f.chapters().isEmpty());
CPPUNIT_ASSERT_EQUAL(5, f.chapterList->writeCount);
}
}
};
CPPUNIT_TEST_SUITE_REGISTRATION(TestMP4);