From bdfeb82cbe2fa16fbb4c8d4a012030e2b4a21b46 Mon Sep 17 00:00:00 2001 From: Ryan Francesconi <2917795+ryanfrancesconi@users.noreply.github.com> Date: Wed, 28 Oct 2026 09:37:46 -0700 Subject: [PATCH] MP4: Cover the leading QuickTime placeholder chapter with an empty edit (#1473) QuickTime chapter samples run back to back from media time 0, so when the first chapter starts after 0, QtChapterList::write() prepends an untitled placeholder sample to keep the absolute start times. TagLib's own reader strips it again, but the chapter track's edit list played the whole media from time 0, so AVFoundation and ffmpeg listed the placeholder as an untitled first chapter. The edit list now begins with an empty edit spanning the placeholder, followed by the media from the first written chapter. The placeholder sample is kept, so readers which ignore edit lists see the same chapters as before. A list starting at 0 still gets a single edit. testQTChapterListNonZeroFirstChapterEditList writes chapters at 1 s and 2 s and checks the chapter track's elst: an empty edit (media time -1) of one second in the mvhd timescale, then the remaining duration from media time one second. It also checks that a list starting at 0 gets a single edit from media time 0, and that qtChapters() reads back exactly the written chapters in both cases. --- taglib/mp4/mp4qtchapterlist.cpp | 43 ++++++++--- tests/test_mp4.cpp | 133 ++++++++++++++++++++++++++++++++ 2 files changed, 167 insertions(+), 9 deletions(-) diff --git a/taglib/mp4/mp4qtchapterlist.cpp b/taglib/mp4/mp4qtchapterlist.cpp index 1cdd17e0..154574af 100644 --- a/taglib/mp4/mp4qtchapterlist.cpp +++ b/taglib/mp4/mp4qtchapterlist.cpp @@ -547,12 +547,17 @@ namespace //! \a textDataOffset is where the text samples will start in the file. //! \a sampleSizes contains per-sample sizes for the stsz table. //! \a movieDuration is the movie-level duration in mvhd timescale units (for edts/elst). + //! \a movieTimescale is the mvhd timescale. + //! \a leadMs is the start time of the first real chapter when a placeholder + //! chapter has been prepended at time 0, otherwise 0. ByteVector buildChapterTrak(unsigned int trackId, unsigned int timescale, long long durationMs, const MP4::ChapterList &chapters, const std::vector &sampleSizes, offset_t textDataOffset, - unsigned int movieDuration) + unsigned int movieDuration, + unsigned int movieTimescale, + long long leadMs) { unsigned int count = chapters.size(); auto totalDuration = static_cast( @@ -684,13 +689,31 @@ namespace ByteVector mdia = renderAtom("mdia", mdiaContent); // -- edts / elst (edit list) -- - // AVFoundation requires an edit list for the chapter track. - // Single entry: play the whole media from time 0, at normal rate. + // AVFoundation requires an edit list for the chapter track. The media is + // played from time 0 at normal rate, except that a leading placeholder + // chapter is covered by an empty edit, so that players honoring the edit + // list do not present it as an untitled first chapter. The placeholder + // sample stays in the media for players which ignore edit lists. + const auto leadDuration = static_cast( + static_cast(leadMs) * static_cast(movieTimescale) / 1000.0 + 0.5); + const bool hasEmptyEdit = leadDuration > 0 && leadDuration < movieDuration; + const auto leadMediaTime = static_cast( + static_cast(leadMs) * static_cast(timescale) / 1000.0 + 0.5); + ByteVector elstData; - elstData.append(ByteVector::fromUInt(1)); // entry count - elstData.append(ByteVector::fromUInt(movieDuration)); // segment duration (mvhd timescale) - elstData.append(ByteVector::fromUInt(0)); // media time = 0 - elstData.append(ByteVector::fromUInt(0x00010000)); // media rate = 1.0 (fixed point) + elstData.append(ByteVector::fromUInt(hasEmptyEdit ? 2 : 1)); // entry count + if(hasEmptyEdit) { + elstData.append(ByteVector::fromUInt(leadDuration)); // segment duration (mvhd timescale) + elstData.append(ByteVector::fromUInt(0xFFFFFFFF)); // media time = -1 (empty edit) + elstData.append(ByteVector::fromUInt(0x00010000)); // media rate = 1.0 (fixed point) + elstData.append(ByteVector::fromUInt(movieDuration - leadDuration)); + elstData.append(ByteVector::fromUInt(leadMediaTime)); // media time of first chapter + } + else { + elstData.append(ByteVector::fromUInt(movieDuration)); // segment duration (mvhd timescale) + elstData.append(ByteVector::fromUInt(0)); // media time = 0 + } + elstData.append(ByteVector::fromUInt(0x00010000)); // media rate = 1.0 (fixed point) ByteVector elst = renderFullBox("elst", 0, 0, elstData); ByteVector edts = renderAtom("edts", elst); @@ -1359,7 +1382,9 @@ bool MP4::QtChapterList::write(TagLib::File *file) // non-zero start time, prepend a dummy chapter at time 0 with an empty title // so the absolute positions are preserved as stts durations. ChapterList workingChapters(chapterList); + long long leadMs = 0; if(!workingChapters.isEmpty() && workingChapters.front().startTime() > 0) { + leadMs = workingChapters.front().startTime(); workingChapters.prepend(Chapter(String(), 0)); } @@ -1381,7 +1406,7 @@ bool MP4::QtChapterList::write(TagLib::File *file) // Two-pass build for chapter trak: first to measure size, then with correct stco offsets. const ByteVector trakMeasure = buildChapterTrak( chapterTrackId, timescale, durationMs, workingChapters, sampleSizes, 0, - movieInfo.duration); + movieInfo.duration, movieInfo.timescale, leadMs); const auto totalInsert = static_cast(refPayload.size() + trakMeasure.size()); // Text samples go inside an mdat atom at EOF. stco offsets point past the 8-byte mdat header. const offset_t textDataOffset = file->length() + totalInsert + 8; @@ -1389,7 +1414,7 @@ bool MP4::QtChapterList::write(TagLib::File *file) // Build final trak with correct stco offsets pointing to where text data will land. const ByteVector trakAtom = buildChapterTrak( chapterTrackId, timescale, durationMs, workingChapters, sampleSizes, textDataOffset, - movieInfo.duration); + movieInfo.duration, movieInfo.timescale, leadMs); // The chapter trak is a moov sibling placed after the audio trak; the reference // goes inside the audio trak, at or before that boundary. Insert the higher offset diff --git a/tests/test_mp4.cpp b/tests/test_mp4.cpp index 63ea5d71..7fae62ad 100644 --- a/tests/test_mp4.cpp +++ b/tests/test_mp4.cpp @@ -169,6 +169,7 @@ class TestMP4 : public CppUnit::TestFixture CPPUNIT_TEST(testQTChapterListOverwrite); CPPUNIT_TEST(testQTChapterListTimestampPrecision); CPPUNIT_TEST(testQTChapterListNonZeroFirstChapter); + CPPUNIT_TEST(testQTChapterListNonZeroFirstChapterEditList); CPPUNIT_TEST(testQTChapterListNoOrphanedMdat); CPPUNIT_TEST(testQTChapterListSharedTref); CPPUNIT_TEST(testQTChapterListSharedMdatPreservesAudio); @@ -1405,6 +1406,138 @@ public: } } + // A chapter track's samples run back to back from media time 0, so a list + // whose first chapter starts later is written with an untitled placeholder + // sample before it. The edit list covers that placeholder with an empty edit, + // so that players which honor edit lists start at the first written chapter. + void testQTChapterListNonZeroFirstChapterEditList() + { + ScopedFileCopy copy("no-tags", ".m4a"); + string filename = copy.fileName(); + + struct EditListEntry { + unsigned int segmentDuration; + int mediaTime; + }; + struct ChapterTrackTiming { + unsigned int movieTimescale = 0; + unsigned int movieDuration = 0; + unsigned int mediaTimescale = 0; + std::vector edits; + }; + + // The mvhd timing, and the chapter (text) track's mdhd timescale and + // version 0 edit list entries. + auto chapterTrackTiming = [&]() { + ChapterTrackTiming timing; + PlainFile pf(filename.c_str()); + MP4::Atoms atoms(&pf); + MP4::Atom *moov = atoms.find("moov"); + if(!moov) + return timing; + + if(const MP4::Atom *mvhd = moov->find("mvhd")) { + pf.seek(mvhd->offset()); + const ByteVector data = pf.readBlock(mvhd->length()); + if(data.size() >= 28 && data[8] == 0) { + timing.movieTimescale = data.toUInt(20U); + timing.movieDuration = data.toUInt(24U); + } + } + + for(auto *trak : moov->findall("trak")) { + const MP4::Atom *hdlr = trak->find("mdia", "hdlr"); + if(!hdlr) + continue; + pf.seek(hdlr->offset()); + if(!pf.readBlock(hdlr->length()).containsAt("text", 16)) + continue; + + if(const MP4::Atom *mdhd = trak->find("mdia", "mdhd")) { + pf.seek(mdhd->offset()); + const ByteVector data = pf.readBlock(mdhd->length()); + if(data.size() >= 24 && data[8] == 0) + timing.mediaTimescale = data.toUInt(20U); + } + + // edts is not parsed as a container, so its elst is read from its data. + if(const MP4::Atom *edts = trak->find("edts")) { + pf.seek(edts->offset()); + const ByteVector data = pf.readBlock(edts->length()); + if(data.size() >= 24 && data.containsAt("elst", 12) && data[16] == 0) { + const unsigned int count = data.toUInt(20U); + for(unsigned int i = 0; i < count && 24 + (i + 1) * 12 <= data.size(); ++i) { + const unsigned int pos = 24 + i * 12; + timing.edits.push_back({ + data.toUInt(pos), + static_cast(data.toUInt(pos + 4)) + }); + } + } + } + break; + } + return timing; + }; + + // First chapter after 0: an empty edit spans the gap, then the media plays + // from the first written chapter. + { + MP4::File f(filename.c_str()); + f.setQtChapters(MP4::ChapterList{ + MP4::Chapter("One", 1000LL), + MP4::Chapter("Two", 2000LL) + }); + CPPUNIT_ASSERT(f.save()); + } + { + const ChapterTrackTiming timing = chapterTrackTiming(); + CPPUNIT_ASSERT(timing.movieTimescale > 0); + CPPUNIT_ASSERT(timing.mediaTimescale > 0); + CPPUNIT_ASSERT_EQUAL(static_cast(2), timing.edits.size()); + + const unsigned int lead = timing.movieTimescale; + CPPUNIT_ASSERT_EQUAL(lead, timing.edits[0].segmentDuration); + CPPUNIT_ASSERT_EQUAL(-1, timing.edits[0].mediaTime); + CPPUNIT_ASSERT_EQUAL(timing.movieDuration - lead, timing.edits[1].segmentDuration); + CPPUNIT_ASSERT_EQUAL(static_cast(timing.mediaTimescale), timing.edits[1].mediaTime); + } + { + MP4::File f(filename.c_str()); + const MP4::ChapterList chapters = f.qtChapters(); + CPPUNIT_ASSERT_EQUAL(2U, chapters.size()); + CPPUNIT_ASSERT_EQUAL(String("One"), chapters[0].title()); + CPPUNIT_ASSERT_EQUAL(1000LL, chapters[0].startTime()); + CPPUNIT_ASSERT_EQUAL(String("Two"), chapters[1].title()); + CPPUNIT_ASSERT_EQUAL(2000LL, chapters[1].startTime()); + } + + // First chapter at 0: a single edit plays the whole media. + { + MP4::File f(filename.c_str()); + f.setQtChapters(MP4::ChapterList{ + MP4::Chapter("One", 0), + MP4::Chapter("Two", 2000LL) + }); + CPPUNIT_ASSERT(f.save()); + } + { + const ChapterTrackTiming timing = chapterTrackTiming(); + CPPUNIT_ASSERT_EQUAL(static_cast(1), timing.edits.size()); + CPPUNIT_ASSERT_EQUAL(timing.movieDuration, timing.edits[0].segmentDuration); + CPPUNIT_ASSERT_EQUAL(0, timing.edits[0].mediaTime); + } + { + MP4::File f(filename.c_str()); + const MP4::ChapterList chapters = f.qtChapters(); + CPPUNIT_ASSERT_EQUAL(2U, chapters.size()); + CPPUNIT_ASSERT_EQUAL(String("One"), chapters[0].title()); + CPPUNIT_ASSERT_EQUAL(0LL, chapters[0].startTime()); + CPPUNIT_ASSERT_EQUAL(String("Two"), chapters[1].title()); + CPPUNIT_ASSERT_EQUAL(2000LL, chapters[1].startTime()); + } + } + // Regression test for the orphaned-mdat bug reported in PR #1325 by ufleisch. // Each add/remove cycle must leave the file's mdat count unchanged. Before // the fix, the chapter mdat appended by write() was never removed, so three