diff --git a/taglib/mp4/mp4chapterholder.h b/taglib/mp4/mp4chapterholder.h index 72752ba4..6a101f20 100644 --- a/taglib/mp4/mp4chapterholder.h +++ b/taglib/mp4/mp4chapterholder.h @@ -31,6 +31,7 @@ namespace TagLib { class File; namespace MP4 { + class Atoms; /*! * Base class to hold chapters and store modified state. */ @@ -71,13 +72,32 @@ namespace TagLib { template ChapterList getChaptersLazy(std::unique_ptr &holder, TagLib::File *file) { - if (!holder) { + if(!holder) { holder = std::make_unique(); holder->read(file); } return holder->chapters(); } + /*! + * Lazily fetch list of chapters from the already parsed \a atoms of \a file, + * instead of parsing them again. + * @tparam T class derived from ChapterHolder and implementing read(File *, const Atoms *) + * @param holder unique pointer to holder, initially null + * @param file file with chapters + * @param atoms parsed atoms of \a file, must be up to date + * @return list of chapters, empty if no chapters found. + */ + template + ChapterList getChaptersLazy(std::unique_ptr &holder, TagLib::File *file, const Atoms *atoms) + { + if(!holder) { + holder = std::make_unique(); + holder->read(file, atoms); + } + return holder->chapters(); + } + /*! * Lazily set a list of chapters. * @tparam T class derived from ChapterHolder @@ -87,7 +107,7 @@ namespace TagLib { template void setChaptersLazy(std::unique_ptr &holder, const ChapterList& chapters) { - if (!holder) { + if(!holder) { holder = std::make_unique(); // The chapters have not been read before, so we do not know their // current state and mark them as modified. Otherwise, the check below diff --git a/taglib/mp4/mp4file.cpp b/taglib/mp4/mp4file.cpp index 5f8e5bec..33e85137 100644 --- a/taglib/mp4/mp4file.cpp +++ b/taglib/mp4/mp4file.cpp @@ -52,6 +52,8 @@ public: std::unique_ptr properties; std::unique_ptr neroChapterList; std::unique_ptr qtChapterList; + // Saving can move atoms without updating all offsets of the parsed tree. + bool atomsStale = false; }; //////////////////////////////////////////////////////////////////////////////// @@ -117,7 +119,9 @@ MP4::Properties *MP4::File::audioProperties() const MP4::ChapterList MP4::File::neroChapters() { - return getChaptersLazy(d->neroChapterList, this); + if(d->atomsStale) + return getChaptersLazy(d->neroChapterList, this); + return getChaptersLazy(d->neroChapterList, this, d->atoms.get()); } void MP4::File::setNeroChapters(const ChapterList& chapters) @@ -127,7 +131,9 @@ void MP4::File::setNeroChapters(const ChapterList& chapters) MP4::ChapterList MP4::File::qtChapters() { - return getChaptersLazy(d->qtChapterList, this); + if(d->atomsStale) + return getChaptersLazy(d->qtChapterList, this); + return getChaptersLazy(d->qtChapterList, this, d->atoms.get()); } void MP4::File::setQtChapters(const ChapterList& chapters) @@ -172,6 +178,7 @@ MP4::File::save() return false; } + d->atomsStale = true; return d->tag->save() && saveChaptersIfModified(d->neroChapterList, this) && saveChaptersIfModified(d->qtChapterList, this); @@ -191,6 +198,7 @@ MP4::File::strip(int tags) } if(tags & MP4) { + d->atomsStale = true; return d->tag->strip(); } diff --git a/taglib/mp4/mp4nerochapterlist.cpp b/taglib/mp4/mp4nerochapterlist.cpp index 26058c3e..b6c926ff 100644 --- a/taglib/mp4/mp4nerochapterlist.cpp +++ b/taglib/mp4/mp4nerochapterlist.cpp @@ -221,8 +221,12 @@ namespace bool MP4::NeroChapterList::read(TagLib::File *file) { const Atoms atoms(file); + return read(file, &atoms); +} - const Atom *chpl = atoms.find("moov", "udta", "chpl"); +bool MP4::NeroChapterList::read(TagLib::File *file, const Atoms *atoms) +{ + const Atom *chpl = atoms->find("moov", "udta", "chpl"); modified = false; chapterList.clear(); if(chpl) { diff --git a/taglib/mp4/mp4nerochapterlist.h b/taglib/mp4/mp4nerochapterlist.h index 3d2ac4c2..44661b32 100644 --- a/taglib/mp4/mp4nerochapterlist.h +++ b/taglib/mp4/mp4nerochapterlist.h @@ -30,6 +30,7 @@ namespace TagLib { class File; namespace MP4 { + class Atoms; /*! * Reads, writes, and removes Nero-style chapter markers (chpl atom) @@ -45,6 +46,12 @@ namespace TagLib { */ bool read(TagLib::File *file); + /*! + * Same as read(TagLib::File *), using the already parsed \a atoms + * of \a file instead of parsing them again. + */ + bool read(TagLib::File *file, const Atoms *atoms); + /*! * Writes chapter markers to the already-opened \a file, * replacing any existing chpl atom. diff --git a/taglib/mp4/mp4qtchapterlist.cpp b/taglib/mp4/mp4qtchapterlist.cpp index 54bf4014..1cdd17e0 100644 --- a/taglib/mp4/mp4qtchapterlist.cpp +++ b/taglib/mp4/mp4qtchapterlist.cpp @@ -1231,14 +1231,19 @@ namespace bool MP4::QtChapterList::read(TagLib::File *file) { const Atoms atoms(file); + return read(file, &atoms); +} + +bool MP4::QtChapterList::read(TagLib::File *file, const Atoms *atoms) +{ modified = false; chapterList.clear(); - TrackInfo audio = findAudioTrack(file, &atoms); + TrackInfo audio = findAudioTrack(file, atoms); if(!audio.trak) return false; - Atom *chapterTrak = findChapterTrak(file, &atoms, audio.trak); + Atom *chapterTrak = findChapterTrak(file, atoms, audio.trak); if(!chapterTrak) return false; diff --git a/taglib/mp4/mp4qtchapterlist.h b/taglib/mp4/mp4qtchapterlist.h index c2900156..58e1c791 100644 --- a/taglib/mp4/mp4qtchapterlist.h +++ b/taglib/mp4/mp4qtchapterlist.h @@ -30,6 +30,7 @@ namespace TagLib { class File; namespace MP4 { + class Atoms; /*! * Reads, writes, and removes QuickTime-style chapter tracks from MP4 @@ -57,6 +58,12 @@ namespace TagLib { */ bool read(TagLib::File *file); + /*! + * Same as read(TagLib::File *), using the already parsed \a atoms + * of \a file instead of parsing them again. + */ + bool read(TagLib::File *file, const Atoms *atoms); + /*! * Writes chapter markers as a QuickTime chapter track to the * already-opened \a file, replacing any existing chapter track. diff --git a/tests/test_mp4.cpp b/tests/test_mp4.cpp index dfb90f0b..63ea5d71 100644 --- a/tests/test_mp4.cpp +++ b/tests/test_mp4.cpp @@ -179,6 +179,7 @@ class TestMP4 : public CppUnit::TestFixture CPPUNIT_TEST(testNeroAndQTChaptersAreIndependent); CPPUNIT_TEST(testNeroChaptersAloneWhenNoQT); CPPUNIT_TEST(testLazyReadingAndWritingChapters); + CPPUNIT_TEST(testReadingChaptersAfterSave); CPPUNIT_TEST_SUITE_END(); public: @@ -1992,6 +1993,37 @@ public: } } + void testReadingChaptersAfterSave() + { + ScopedFileCopy copy("has-tags", ".m4a"); + string filename = copy.fileName(); + + { + MP4::File f(filename.c_str()); + f.setNeroChapters(MP4::ChapterList{ + MP4::Chapter("Nero 1", 0), + MP4::Chapter("Nero 2", 10000LL) + }); + CPPUNIT_ASSERT(f.save()); + } + + // Writing QT chapters moves atoms, chapters first read afterwards must + // not use the atoms parsed when the file was opened. + { + MP4::File f(filename.c_str()); + f.setQtChapters(MP4::ChapterList{ + MP4::Chapter("QT 1", 0), + MP4::Chapter("QT 2", 20000LL) + }); + CPPUNIT_ASSERT(f.save()); + + const MP4::ChapterList nero = f.neroChapters(); + CPPUNIT_ASSERT_EQUAL(2U, nero.size()); + CPPUNIT_ASSERT_EQUAL(String("Nero 2"), nero[1].title()); + CPPUNIT_ASSERT_EQUAL(10000LL, nero[1].startTime()); + } + } + }; CPPUNIT_TEST_SUITE_REGISTRATION(TestMP4);