overloads for read, write, remove

Changes made

mp4chapterlist.h
• Added (​MP4::​File*) overloads for read, write, remove
• Replaced broken class ​File; forward declaration with #include "mp4file​.h" (fixed a subtle C++ name-resolution linker bug where Atoms(​File*) resolved to MP4::​File* instead of Tag​Lib::​File*)

mp4chapterlist.cpp
• Refactored: path-based overloads are now thin wrappers that delegate to file-based overloads
• File-based overloads construct Atoms locally — no Atoms* in the public API
• Removed chpl​Header​Size = 9 constant; replaced the minimum-size guard in parse​Chpl​Data with a correct 5-byte check (the old constant was version-1 specific and would reject valid version-0 atoms)

mp4qtchapterlist.h
• Added (​MP4::​File*) overloads for read, write, remove
• Removed Atoms* parameters entirely from the public API

mp4qtchapterlist.cpp
• Same refactor: path-based overloads delegate; file-based overloads construct Atoms locally
• Added empty-chapter guard: write(​MP4::​File*, {}) delegates to remove(file) instead of writing a 0-sample chapter track

tests/test_mp4.cpp
• Added test​Chapter​List​File​API and test​QTChapter​List​File​API — exercise the full write/read/remove cycle via the file-based API
• Updated test bodies to use the simplified (​MP4::​File*) API (no MP4::​Atoms construction in test code)
This commit is contained in:
Ryan Francesconi
2026-04-14 14:23:35 -07:00
parent 4a73d73b20
commit c5ea13bb34
5 changed files with 246 additions and 60 deletions

View File

@@ -34,9 +34,6 @@ using namespace TagLib;
namespace
{
// Nero chpl version 1 header: version(1) + flags(3) + reserved(4) + count(1) = 9 bytes
constexpr int chplHeaderSize = 9;
ByteVector renderAtom(const ByteVector &name, const ByteVector &data)
{
return ByteVector::fromUInt(static_cast<unsigned int>(data.size() + 8)) + name + data;
@@ -171,7 +168,8 @@ namespace
{
MP4::ChapterList chapters;
if(data.size() < static_cast<unsigned int>(chplHeaderSize))
// Minimum: version(1) + flags(3) + count(1) = 5 bytes (version 0 layout)
if(data.size() < 5)
return chapters;
unsigned int pos = 0;
@@ -225,15 +223,21 @@ MP4::MP4ChapterList::read(const char *path)
return ChapterList();
}
Atoms atoms(&file);
return read(&file);
}
MP4::ChapterList
MP4::MP4ChapterList::read(MP4::File *file)
{
Atoms atoms(file);
Atom *chpl = atoms.find("moov", "udta", "chpl");
if(!chpl)
return ChapterList();
// Read the atom content (skip 8-byte atom header)
file.seek(chpl->offset() + 8);
ByteVector data = file.readBlock(chpl->length() - 8);
file->seek(chpl->offset() + 8);
ByteVector data = file->readBlock(chpl->length() - 8);
return parseChplData(data);
}
@@ -247,7 +251,13 @@ MP4::MP4ChapterList::write(const char *path, const ChapterList &chapters)
return false;
}
Atoms atoms(&file);
return write(&file, chapters);
}
bool
MP4::MP4ChapterList::write(MP4::File *file, const ChapterList &chapters)
{
Atoms atoms(file);
if(!atoms.find("moov")) {
debug("MP4ChapterList::write() -- No moov atom found");
@@ -265,13 +275,13 @@ MP4::MP4ChapterList::write(const char *path, const ChapterList &chapters)
offset_t oldLength = existingChpl->length();
offset_t delta = static_cast<offset_t>(chplAtom.size()) - oldLength;
file.insert(chplAtom, offset, oldLength);
file->insert(chplAtom, offset, oldLength);
if(delta != 0) {
// Update parent sizes: moov and udta
AtomList parentPath = atoms.path("moov", "udta", "chpl");
updateParentSizes(&file, parentPath, delta, 1); // ignore chpl itself
updateChunkOffsets(&file, &atoms, delta, offset);
updateParentSizes(file, parentPath, delta, 1); // ignore chpl itself
updateChunkOffsets(file, &atoms, delta, offset);
}
}
else {
@@ -281,10 +291,10 @@ MP4::MP4ChapterList::write(const char *path, const ChapterList &chapters)
if(udtaPath.size() == 2) {
// udta exists -- insert chpl at the beginning of udta's content
offset_t insertOffset = udtaPath.back()->offset() + 8;
file.insert(chplAtom, insertOffset, 0);
file->insert(chplAtom, insertOffset, 0);
updateParentSizes(&file, udtaPath, chplAtom.size());
updateChunkOffsets(&file, &atoms, chplAtom.size(), insertOffset);
updateParentSizes(file, udtaPath, chplAtom.size());
updateChunkOffsets(file, &atoms, chplAtom.size(), insertOffset);
}
else {
// No udta -- insert udta + chpl at the beginning of moov's content
@@ -297,10 +307,10 @@ MP4::MP4ChapterList::write(const char *path, const ChapterList &chapters)
}
offset_t insertOffset = moovPath.back()->offset() + 8;
file.insert(udtaAtom, insertOffset, 0);
file->insert(udtaAtom, insertOffset, 0);
updateParentSizes(&file, moovPath, udtaAtom.size());
updateChunkOffsets(&file, &atoms, udtaAtom.size(), insertOffset);
updateParentSizes(file, moovPath, udtaAtom.size());
updateChunkOffsets(file, &atoms, udtaAtom.size(), insertOffset);
}
}
@@ -316,7 +326,13 @@ MP4::MP4ChapterList::remove(const char *path)
return false;
}
Atoms atoms(&file);
return remove(&file);
}
bool
MP4::MP4ChapterList::remove(MP4::File *file)
{
Atoms atoms(file);
Atom *chpl = atoms.find("moov", "udta", "chpl");
if(!chpl) {
@@ -327,12 +343,12 @@ MP4::MP4ChapterList::remove(const char *path)
offset_t offset = chpl->offset();
offset_t length = chpl->length();
file.removeBlock(offset, length);
file->removeBlock(offset, length);
// Update parent sizes with negative delta
AtomList parentPath = atoms.path("moov", "udta", "chpl");
updateParentSizes(&file, parentPath, -length, 1); // ignore chpl itself
updateChunkOffsets(&file, &atoms, -length, offset);
updateParentSizes(file, parentPath, -length, 1); // ignore chpl itself
updateChunkOffsets(file, &atoms, -length, offset);
return true;
}

View File

@@ -28,6 +28,7 @@
#include "tlist.h"
#include "tstring.h"
#include "taglib_export.h"
#include "mp4file.h"
namespace TagLib {
namespace MP4 {
@@ -56,6 +57,13 @@ namespace TagLib {
*/
static ChapterList read(const char *path);
/*!
* Reads chapter markers from the already-opened \a file.
* Avoids a second open when the caller already has the file open.
* Returns an empty list if the file has no chpl atom.
*/
static ChapterList read(MP4::File *file);
/*!
* Writes chapter markers to the MP4 file at \a path,
* replacing any existing chpl atom. The chapter count is
@@ -64,11 +72,25 @@ namespace TagLib {
*/
static bool write(const char *path, const ChapterList &chapters);
/*!
* Writes chapter markers to the already-opened \a file,
* replacing any existing chpl atom.
* The chapter count is capped at 255 (Nero format limit).
* Returns \c true on success.
*/
static bool write(MP4::File *file, const ChapterList &chapters);
/*!
* Removes the chpl atom from the MP4 file at \a path.
* Returns \c true on success, or if no chpl atom exists.
*/
static bool remove(const char *path);
/*!
* Removes the chpl atom from the already-opened \a file.
* Returns \c true on success, or if no chpl atom exists.
*/
static bool remove(MP4::File *file);
};
} // namespace MP4

View File

@@ -950,23 +950,29 @@ MP4::MP4QTChapterList::read(const char *path)
if(!file.isOpen() || !file.isValid())
return ChapterList();
Atoms atoms(&file);
return read(&file);
}
TrackInfo audio = findAudioTrack(&file, &atoms);
MP4::ChapterList
MP4::MP4QTChapterList::read(MP4::File *file)
{
Atoms atoms(file);
TrackInfo audio = findAudioTrack(file, &atoms);
if(!audio.trak)
return ChapterList();
Atom *chapterTrak = findChapterTrak(&file, &atoms, audio.trak);
Atom *chapterTrak = findChapterTrak(file, &atoms, audio.trak);
if(!chapterTrak)
return ChapterList();
ChapterTrackInfo trackInfo = readChapterTrackInfo(&file, chapterTrak);
ChapterTrackInfo trackInfo = readChapterTrackInfo(file, chapterTrak);
if(trackInfo.timescale == 0)
return ChapterList();
std::vector<SttsEntry> sttsEntries = readStts(&file, chapterTrak);
SampleSizeInfo sizeInfo = readStsz(&file, chapterTrak);
std::vector<unsigned int> offsets = resolveSampleOffsets(&file, chapterTrak, sizeInfo);
std::vector<SttsEntry> sttsEntries = readStts(file, chapterTrak);
SampleSizeInfo sizeInfo = readStsz(file, chapterTrak);
std::vector<unsigned int> offsets = resolveSampleOffsets(file, chapterTrak, sizeInfo);
if(offsets.empty())
return ChapterList();
@@ -984,7 +990,7 @@ MP4::MP4QTChapterList::read(const char *path)
if(sampleSize == 0 && sampleIndex < sizeInfo.perSampleSizes.size())
sampleSize = sizeInfo.perSampleSizes[sampleIndex];
String title = readTextSample(&file, offsets[sampleIndex], sampleSize);
String title = readTextSample(file, offsets[sampleIndex], sampleSize);
long long startTime100ns = static_cast<long long>(
static_cast<double>(currentTime) * 10000000.0 /
@@ -1021,24 +1027,33 @@ MP4::MP4QTChapterList::write(const char *path, const ChapterList &chapters)
return false;
}
return write(&file, chapters);
}
bool
MP4::MP4QTChapterList::write(MP4::File *file, const ChapterList &chapters)
{
// Writing an empty list is equivalent to removing the chapter track.
if(chapters.isEmpty())
return remove(file);
// ---- Phase 1: Parse and gather info ----
Atoms atoms(&file);
Atoms atoms(file);
Atom *moov = atoms.find("moov");
if(!moov) {
debug("MP4QTChapterList::write() -- No moov atom found");
return false;
}
MovieInfo movieInfo = readMovieInfo(&file, &atoms);
MovieInfo movieInfo = readMovieInfo(file, &atoms);
if(movieInfo.durationMs <= 0) {
debug("MP4QTChapterList::write() -- Could not determine file duration");
return false;
}
long long durationMs = movieInfo.durationMs;
TrackInfo audio = findAudioTrack(&file, &atoms);
TrackInfo audio = findAudioTrack(file, &atoms);
if(!audio.trak) {
debug("MP4QTChapterList::write() -- No audio track found");
return false;
@@ -1052,7 +1067,7 @@ MP4::MP4QTChapterList::write(const char *path, const ChapterList &chapters)
// Optional second parse -- only constructed when replacing existing chapters.
std::unique_ptr<Atoms> cleanAtoms;
Atom *existingChapter = findChapterTrak(&file, &atoms, audio.trak);
Atom *existingChapter = findChapterTrak(file, &atoms, audio.trak);
if(existingChapter) {
// Remove chapter trak FIRST (higher offset in file).
offset_t chapterOff = existingChapter->offset();
@@ -1062,17 +1077,17 @@ MP4::MP4QTChapterList::write(const char *path, const ChapterList &chapters)
moov->removeChild(existingChapter);
delete existingChapter;
file.removeBlock(chapterOff, chapterLen);
file->removeBlock(chapterOff, chapterLen);
AtomList moovPath = atoms.path("moov");
updateParentSizes(&file, moovPath, -chapterLen);
updateChunkOffsets(&file, &atoms, -chapterLen, chapterOff);
updateParentSizes(file, moovPath, -chapterLen);
updateChunkOffsets(file, &atoms, -chapterLen, chapterOff);
// Remove tref from audio trak (lower offset, still valid).
removeAudioTref(&file, &atoms, audio.trak);
removeAudioTref(file, &atoms, audio.trak);
// Re-parse to get clean state after removals.
cleanAtoms = std::make_unique<Atoms>(&file);
cleanAtoms = std::make_unique<Atoms>(file);
activeAtoms = cleanAtoms.get();
moov = activeAtoms->find("moov");
@@ -1080,7 +1095,7 @@ MP4::MP4QTChapterList::write(const char *path, const ChapterList &chapters)
debug("MP4QTChapterList::write() -- moov disappeared after cleanup");
return false;
}
audio = findAudioTrack(&file, activeAtoms);
audio = findAudioTrack(file, activeAtoms);
if(!audio.trak) {
debug("MP4QTChapterList::write() -- No audio track after cleanup");
return false;
@@ -1100,7 +1115,7 @@ MP4::MP4QTChapterList::write(const char *path, const ChapterList &chapters)
workingChapters.prepend(dummy);
}
unsigned int nextId = getNextTrackId(&file, activeAtoms);
unsigned int nextId = getNextTrackId(file, activeAtoms);
unsigned int chapterTrackId = nextId > 0 ? nextId : audio.trackId + 1;
constexpr unsigned int timescale = 1000;
std::vector<unsigned int> sampleSizes = calculateSampleSizes(workingChapters);
@@ -1114,7 +1129,7 @@ MP4::MP4QTChapterList::write(const char *path, const ChapterList &chapters)
movieInfo.duration);
offset_t totalInsert = static_cast<offset_t>(trefAtom.size() + trakMeasure.size());
// Text samples go inside an mdat atom at EOF. stco offsets point past the 8-byte mdat header.
offset_t textDataOffset = file.length() + totalInsert + 8;
offset_t textDataOffset = file->length() + totalInsert + 8;
// Build final trak with correct stco offsets pointing to where text data will land.
ByteVector trakAtom = buildChapterTrak(
@@ -1129,22 +1144,22 @@ MP4::MP4QTChapterList::write(const char *path, const ChapterList &chapters)
// tref is logically inside audio trak; chapter trak is logically after it.
offset_t insertOffset = audio.trak->offset() + audio.trak->length();
file.insert(combinedPayload, insertOffset, 0);
file->insert(combinedPayload, insertOffset, 0);
// Fix audio trak size on disk -- only tref goes inside
file.seek(audio.trak->offset());
unsigned int audioTrakSize = file.readBlock(4).toUInt();
file->seek(audio.trak->offset());
unsigned int audioTrakSize = file->readBlock(4).toUInt();
unsigned int newAudioTrakSize = static_cast<unsigned int>(audioTrakSize + trefAtom.size());
file.seek(audio.trak->offset());
file.writeBlock(ByteVector::fromUInt(newAudioTrakSize));
file->seek(audio.trak->offset());
file->writeBlock(ByteVector::fromUInt(newAudioTrakSize));
// Fix moov size -- both tref and chapter trak are inside moov
AtomList moovPath = activeAtoms->path("moov");
updateParentSizes(&file, moovPath, combinedPayload.size());
updateParentSizes(file, moovPath, combinedPayload.size());
// Fix existing chunk offsets -- only the ORIGINAL atom tree is iterated,
// so the new chapter trak's stco (which already has correct offsets) is untouched.
updateChunkOffsets(&file, activeAtoms, combinedPayload.size(), insertOffset);
updateChunkOffsets(file, activeAtoms, combinedPayload.size(), insertOffset);
// ---- Phase 4: Append text samples in mdat at EOF ----
@@ -1155,15 +1170,15 @@ MP4::MP4QTChapterList::write(const char *path, const ChapterList &chapters)
// Wrap text samples in an mdat atom so players can find them.
ByteVector mdatAtom = renderAtom("mdat", textSamples);
file.seek(0, TagLib::File::End);
file.writeBlock(mdatAtom);
file->seek(0, TagLib::File::End);
file->writeBlock(mdatAtom);
// ---- Phase 5: Update mvhd next_track_ID ----
// mvhd is before insertOffset, so its offset is unchanged.
unsigned int currentNextId = getNextTrackId(&file, activeAtoms);
unsigned int currentNextId = getNextTrackId(file, activeAtoms);
if(chapterTrackId >= currentNextId) {
setNextTrackId(&file, activeAtoms, chapterTrackId + 1);
setNextTrackId(file, activeAtoms, chapterTrackId + 1);
}
return true;
@@ -1178,13 +1193,19 @@ MP4::MP4QTChapterList::remove(const char *path)
return false;
}
Atoms atoms(&file);
return remove(&file);
}
TrackInfo audio = findAudioTrack(&file, &atoms);
bool
MP4::MP4QTChapterList::remove(MP4::File *file)
{
Atoms atoms(file);
TrackInfo audio = findAudioTrack(file, &atoms);
if(!audio.trak)
return true; // No audio track -- nothing to do
Atom *chapterTrak = findChapterTrak(&file, &atoms, audio.trak);
Atom *chapterTrak = findChapterTrak(file, &atoms, audio.trak);
if(!chapterTrak)
return true; // No chapter track -- nothing to do
@@ -1200,14 +1221,14 @@ MP4::MP4QTChapterList::remove(const char *path)
moov->removeChild(chapterTrak);
delete chapterTrak;
file.removeBlock(chapterOff, chapterLen);
file->removeBlock(chapterOff, chapterLen);
AtomList moovPath = atoms.path("moov");
updateParentSizes(&file, moovPath, -chapterLen);
updateChunkOffsets(&file, &atoms, -chapterLen, chapterOff);
updateParentSizes(file, moovPath, -chapterLen);
updateChunkOffsets(file, &atoms, -chapterLen, chapterOff);
// Remove tref from audio trak (lower offset, still valid after chapter trak removal).
removeAudioTref(&file, &atoms, audio.trak);
removeAudioTref(file, &atoms, audio.trak);
return true;
}

View File

@@ -56,6 +56,14 @@ namespace TagLib {
*/
static ChapterList read(const char *path);
/*!
* Reads chapter markers from the QuickTime chapter track in the
* already-opened \a file. Avoids a second open when the caller
* already has the file open.
* Returns an empty list if the file has no chapter track.
*/
static ChapterList read(MP4::File *file);
/*!
* Writes chapter markers as a QuickTime chapter track to the MP4
* file at \a path, replacing any existing chapter track. The
@@ -64,12 +72,26 @@ namespace TagLib {
*/
static bool write(const char *path, const ChapterList &chapters);
/*!
* Writes chapter markers as a QuickTime chapter track to the
* already-opened \a file, replacing any existing chapter track.
* Returns \c true on success.
*/
static bool write(MP4::File *file, const ChapterList &chapters);
/*!
* Removes the QuickTime chapter track and its \c tref/chap
* reference from the MP4 file at \a path.
* Returns \c true on success, or if no chapter track exists.
*/
static bool remove(const char *path);
/*!
* Removes the QuickTime chapter track and its \c tref/chap
* reference from the already-opened \a file.
* Returns \c true on success, or if no chapter track exists.
*/
static bool remove(MP4::File *file);
};
} // namespace MP4

View File

@@ -115,6 +115,8 @@ class TestMP4 : public CppUnit::TestFixture
CPPUNIT_TEST(testQTChapterListOverwrite);
CPPUNIT_TEST(testQTChapterListTimestampPrecision);
CPPUNIT_TEST(testQTChapterListNonZeroFirstChapter);
CPPUNIT_TEST(testChapterListFileAPI);
CPPUNIT_TEST(testQTChapterListFileAPI);
CPPUNIT_TEST_SUITE_END();
public:
@@ -1313,6 +1315,109 @@ public:
CPPUNIT_ASSERT_EQUAL(String("Three"), chapters[2].title);
}
}
void testChapterListFileAPI()
{
ScopedFileCopy copy("no-tags", ".m4a");
string filename = copy.fileName();
// Write chapters via the file-based API
{
MP4::File file(filename.c_str(), false);
CPPUNIT_ASSERT(file.isOpen() && file.isValid() && !file.readOnly());
MP4::ChapterList chapters;
MP4::Chapter ch1;
ch1.startTime = 0;
ch1.title = "Alpha";
chapters.append(ch1);
MP4::Chapter ch2;
ch2.startTime = 200000000LL; // 20 seconds
ch2.title = "Beta";
chapters.append(ch2);
CPPUNIT_ASSERT(MP4::MP4ChapterList::write(&file, chapters));
}
// Read back via the file-based API
{
MP4::File file(filename.c_str(), false);
CPPUNIT_ASSERT(file.isOpen() && file.isValid());
MP4::ChapterList chapters = MP4::MP4ChapterList::read(&file);
CPPUNIT_ASSERT_EQUAL(2U, chapters.size());
CPPUNIT_ASSERT_EQUAL(0LL, chapters[0].startTime);
CPPUNIT_ASSERT_EQUAL(String("Alpha"), chapters[0].title);
CPPUNIT_ASSERT_EQUAL(200000000LL, chapters[1].startTime);
CPPUNIT_ASSERT_EQUAL(String("Beta"), chapters[1].title);
}
// Remove via the file-based API
{
MP4::File file(filename.c_str(), false);
CPPUNIT_ASSERT(file.isOpen() && file.isValid() && !file.readOnly());
CPPUNIT_ASSERT(MP4::MP4ChapterList::remove(&file));
}
// Verify removed
{
MP4::ChapterList chapters = MP4::MP4ChapterList::read(filename.c_str());
CPPUNIT_ASSERT(chapters.isEmpty());
}
}
void testQTChapterListFileAPI()
{
ScopedFileCopy copy("no-tags", ".m4a");
string filename = copy.fileName();
// Write chapters via the file-based API
{
MP4::File file(filename.c_str(), false);
CPPUNIT_ASSERT(file.isOpen() && file.isValid() && !file.readOnly());
MP4::ChapterList chapters;
MP4::Chapter ch1;
ch1.startTime = 0;
ch1.title = "Alpha";
chapters.append(ch1);
MP4::Chapter ch2;
ch2.startTime = 200000000LL; // 20 seconds
ch2.title = "Beta";
chapters.append(ch2);
CPPUNIT_ASSERT(MP4::MP4QTChapterList::write(&file, chapters));
}
// Read back via the file-based API
{
MP4::File file(filename.c_str(), false);
CPPUNIT_ASSERT(file.isOpen() && file.isValid());
MP4::ChapterList chapters = MP4::MP4QTChapterList::read(&file);
CPPUNIT_ASSERT_EQUAL(2U, chapters.size());
CPPUNIT_ASSERT_EQUAL(0LL, chapters[0].startTime);
CPPUNIT_ASSERT_EQUAL(String("Alpha"), chapters[0].title);
CPPUNIT_ASSERT_EQUAL(200000000LL, chapters[1].startTime);
CPPUNIT_ASSERT_EQUAL(String("Beta"), chapters[1].title);
}
// Remove via the file-based API
{
MP4::File file(filename.c_str(), false);
CPPUNIT_ASSERT(file.isOpen() && file.isValid() && !file.readOnly());
CPPUNIT_ASSERT(MP4::MP4QTChapterList::remove(&file));
}
// Verify removed
{
MP4::ChapterList chapters = MP4::MP4QTChapterList::read(filename.c_str());
CPPUNIT_ASSERT(chapters.isEmpty());
}
}
};
CPPUNIT_TEST_SUITE_REGISTRATION(TestMP4);