From a97cc6ab511663bbda9932efa2b993ee11d42d3c Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Sat, 3 Oct 2026 22:52:00 -0500 Subject: [PATCH] fix: append to the HLS manifest instead of rewriting it per fragment writeM3U8 truncated and rewrote the whole playlist every time a fragment completed. That is O(fragments) of writing per fragment, so an event pays O(fragments squared) overall. For the ten minute events a section length produces the manifest is about 55KB and the cost is invisible. It stops being invisible when an event does not close. On a box here, four monitors stopped closing their events after a reboot and ran for 51 hours. Their manifests reached 17MB and 470,000 lines, and strace showed where the writes were going: in one window the mp4 took 3 writes while index.m3u8 took 195, opened O_WRONLY|O_CREAT|O_TRUNC. The cameras produced 0.4MB/s of video between them and the disk was absorbing 24MB/s at 100% utilisation, 106ms average write latency. That is a loop rather than just waste. One rewrite took 3.48s of wall clock against a fragment arriving every 1.2s, so the event thread could never catch up, the packet queue stayed full ("Analysis is not keeping up" every three seconds), and the analysis thread is where the section length check that would have closed the event lives. The growth starved the only thing that could stop it. An EVENT playlist is append only: a fragment's three lines never change once written, and only the header depends on anything global. So write the new fragments to the end, and fall back to a full rewrite when something above them would differ -- a changed target duration or init segment end, the different url the close path passes, a different path, or the closing ENDLIST. The remembered byte count is checked against the file before appending, so a manifest that something else has truncated, replaced or removed is rebuilt rather than appended to; any failure zeroes the state, which makes a rewrite the answer to anything unexpected. The header and fragment text are split into m3u8Header, m3u8Fragment and m3u8TargetDuration so the property that matters can be tested without standing up a VideoStore: appending one fragment at a time produces a byte identical manifest to writing the whole thing at once. Co-Authored-By: Claude Opus 5 (1M context) --- src/zm_videostore.cpp | 134 ++++++++++++++++++++++++++----- src/zm_videostore.h | 27 +++++++ tests/CMakeLists.txt | 1 + tests/zm_videostore_m3u8.cpp | 150 +++++++++++++++++++++++++++++++++++ 4 files changed, 290 insertions(+), 22 deletions(-) create mode 100644 tests/zm_videostore_m3u8.cpp diff --git a/src/zm_videostore.cpp b/src/zm_videostore.cpp index 87464da9a..d94582964 100644 --- a/src/zm_videostore.cpp +++ b/src/zm_videostore.cpp @@ -25,6 +25,7 @@ #include "zm_mp4_sidx.h" #include "zm_signal.h" #include "zm_time.h" +#include "zm_utils.h" extern "C" { #include @@ -34,6 +35,7 @@ extern "C" { #include #include #include +#include VideoStore::VideoStore( const char *filename_in, @@ -83,7 +85,11 @@ VideoStore::VideoStore( init_segment_end_(0), sidx_region_offset_(-1), sidx_region_size_(0), - finalized_(false) { + finalized_(false), + m3u8_fragments_written_(0), + m3u8_target_duration_(0), + m3u8_init_segment_end_(-1), + m3u8_bytes_written_(0) { FFMPEGInit(); swscale.init(); opkt = av_packet_ptr{av_packet_alloc()}; @@ -1781,42 +1787,126 @@ void VideoStore::finalize() { } } -void VideoStore::writeM3U8(const std::string &m3u8_path, const std::string &video_url, bool is_complete) { - if (fragments_.empty()) return; - - // Calculate max duration for EXT-X-TARGETDURATION (must be integer, rounded up) +int VideoStore::m3u8TargetDuration(const std::vector &frags) { double max_duration = 0; - for (const auto &frag : fragments_) { + for (const auto &frag : frags) { if (frag.duration > max_duration) max_duration = frag.duration; } int target_duration = static_cast(ceil(max_duration)); - if (target_duration < 1) target_duration = 1; + return (target_duration < 1) ? 1 : target_duration; +} - FILE *fp = fopen(m3u8_path.c_str(), "w"); +std::string VideoStore::m3u8Header(int target_duration, + const std::string &video_url, + int64_t init_segment_end, + bool is_complete) { + return stringtf("#EXTM3U\n" + "#EXT-X-VERSION:7\n" + "#EXT-X-TARGETDURATION:%d\n" + "#EXT-X-MEDIA-SEQUENCE:0\n" + "#EXT-X-PLAYLIST-TYPE:%s\n" + "#EXT-X-MAP:URI=\"%s\",BYTERANGE=\"%" PRId64 "@0\"\n", + target_duration, + is_complete ? "VOD" : "EVENT", + video_url.c_str(), + init_segment_end); +} + +std::string VideoStore::m3u8Fragment(const Fragment &frag, const std::string &video_url) { + return stringtf("#EXTINF:%.3f,\n" + "#EXT-X-BYTERANGE:%" PRId64 "@%" PRId64 "\n" + "%s\n", + frag.duration, frag.size, frag.offset, video_url.c_str()); +} + +void VideoStore::writeM3U8(const std::string &m3u8_path, const std::string &video_url, bool is_complete) { + if (fragments_.empty()) return; + + const int target_duration = m3u8TargetDuration(fragments_); + + // Rewriting the whole manifest for every new fragment costs O(fragments) + // each time, so an event pays O(fragments^2) overall. That is free for the + // ten minute events a section length produces, and ruinous for anything + // longer: a monitor whose events stopped closing reached a 17MB manifest + // being rewritten every 1.2s, which saturated the disk, starved the analysis + // thread, and so prevented the very close that would have ended it. + // + // An EVENT playlist only ever gains fragments and never rewrites a line it + // has already written, so the new ones can simply go on the end. Anything + // that would change what is already there -- a different header, a different + // url repeated on every line, or the closing ENDLIST -- falls back to the + // full rewrite. + bool can_append = + !is_complete + and (m3u8_fragments_written_ > 0) + and (m3u8_fragments_written_ <= fragments_.size()) + and (target_duration == m3u8_target_duration_) + and (init_segment_end_ == m3u8_init_segment_end_) + and (video_url == m3u8_video_url_) + and (m3u8_path == m3u8_path_); + + if (can_append) { + // The offsets we remember only describe the file we left behind. If + // anything else has replaced, truncated or removed it, start again rather + // than append to something we cannot account for. + struct stat st; + const bool statted = (stat(m3u8_path.c_str(), &st) == 0); + if (!statted or (st.st_size != m3u8_bytes_written_)) { + Debug(1, "m3u8 %s is not the file we left (size %jd, expected %jd), rewriting", + m3u8_path.c_str(), + static_cast(statted ? st.st_size : -1), + static_cast(m3u8_bytes_written_)); + can_append = false; + } else if (m3u8_fragments_written_ == fragments_.size()) { + return; // nothing new to say + } + } + + FILE *fp = fopen(m3u8_path.c_str(), can_append ? "a" : "w"); if (!fp) { Error("Failed to open %s for writing: %s", m3u8_path.c_str(), strerror(errno)); + m3u8_fragments_written_ = 0; return; } - fprintf(fp, "#EXTM3U\n"); - fprintf(fp, "#EXT-X-VERSION:7\n"); - fprintf(fp, "#EXT-X-TARGETDURATION:%d\n", target_duration); - fprintf(fp, "#EXT-X-MEDIA-SEQUENCE:0\n"); - fprintf(fp, "#EXT-X-PLAYLIST-TYPE:%s\n", is_complete ? "VOD" : "EVENT"); - fprintf(fp, "#EXT-X-MAP:URI=\"%s\",BYTERANGE=\"%" PRId64 "@0\"\n", - video_url.c_str(), init_segment_end_); + size_t first = 0; + if (can_append) { + first = m3u8_fragments_written_; + } else { + std::string header = m3u8Header(target_duration, video_url, init_segment_end_, is_complete); + fwrite(header.c_str(), 1, header.size(), fp); + } - for (const auto &frag : fragments_) { - fprintf(fp, "#EXTINF:%.3f,\n", frag.duration); - fprintf(fp, "#EXT-X-BYTERANGE:%" PRId64 "@%" PRId64 "\n", frag.size, frag.offset); - fprintf(fp, "%s\n", video_url.c_str()); + for (size_t i = first; i < fragments_.size(); i++) { + std::string line = m3u8Fragment(fragments_[i], video_url); + fwrite(line.c_str(), 1, line.size(), fp); } if (is_complete) { fprintf(fp, "#EXT-X-ENDLIST\n"); } - fclose(fp); - Debug(1, "Wrote m3u8 %s with %zu fragments (complete=%d)", - m3u8_path.c_str(), fragments_.size(), is_complete); + const bool ok = (fclose(fp) == 0); + + // Measure the file rather than trusting ftell, so the next call's guard is + // comparing against what is really on disk. Anything unreadable leaves the + // state zeroed, which forces a rewrite next time. + struct stat after; + if (ok and !is_complete and (stat(m3u8_path.c_str(), &after) == 0)) { + m3u8_fragments_written_ = fragments_.size(); + m3u8_target_duration_ = target_duration; + m3u8_init_segment_end_ = init_segment_end_; + m3u8_bytes_written_ = after.st_size; + m3u8_path_ = m3u8_path; + m3u8_video_url_ = video_url; + } else { + // A completed manifest ends with ENDLIST; nothing may be appended after + // it, so any later call has to rewrite. + m3u8_fragments_written_ = 0; + if (!ok) Error("Failed to write %s: %s", m3u8_path.c_str(), strerror(errno)); + } + + Debug(1, "%s m3u8 %s: %zu of %zu fragments (complete=%d)", + can_append ? "Appended to" : "Wrote", + m3u8_path.c_str(), fragments_.size() - first, fragments_.size(), is_complete); } diff --git a/src/zm_videostore.h b/src/zm_videostore.h index 4e5befc29..bca94d39e 100644 --- a/src/zm_videostore.h +++ b/src/zm_videostore.h @@ -117,6 +117,17 @@ class VideoStore { int64_t sidx_region_size_; // how many bytes open() reserved there bool finalized_; // true once finalize() has run trailer + last-fragment recording + // What of the manifest is already on disk, and what it was written with, so + // writeM3U8 can add the new fragments to the end instead of rewriting it. + // Zeroed state means "rewrite from scratch", which is also the self-healing + // answer to anything unexpected. + size_t m3u8_fragments_written_; + int m3u8_target_duration_; + int64_t m3u8_init_segment_end_; + int64_t m3u8_bytes_written_; + std::string m3u8_path_; + std::string m3u8_video_url_; + bool setup_resampler(); int write_packet(AVPacket *pkt, AVStream *stream); @@ -142,6 +153,22 @@ class VideoStore { const std::vector &fragments() const { return fragments_; } int64_t init_segment_end() const { return init_segment_end_; } void writeM3U8(const std::string &path, const std::string &video_url, bool is_complete); + + // --- Manifest text, split out so it is testable without a VideoStore ------ + // + // An EVENT playlist is append only: the lines for a fragment never change + // once written, and only the header depends on anything global. So the + // header and a fragment's three lines are generated separately, and + // writeM3U8 appends when nothing above the new fragments would differ. + + // Rounded up, and never below 1, as EXT-X-TARGETDURATION must be a positive + // integer no smaller than any fragment. + static int m3u8TargetDuration(const std::vector &frags); + static std::string m3u8Header(int target_duration, + const std::string &video_url, + int64_t init_segment_end, + bool is_complete); + static std::string m3u8Fragment(const Fragment &frag, const std::string &video_url); // Flush queues, write trailer, close output, and record the final fragment. // Call this before writeM3U8(true) so the manifest contains every fragment. // Safe to call once; subsequent calls are no-ops. The destructor will skip diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 3a6581de2..e26f03748 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -44,6 +44,7 @@ set(TEST_SOURCES zm_stream_socket_protocol.cpp zm_time.cpp zm_utils.cpp + zm_videostore_m3u8.cpp zm_vector2.cpp zm_zone.cpp zm_zone_stride.cpp diff --git a/tests/zm_videostore_m3u8.cpp b/tests/zm_videostore_m3u8.cpp new file mode 100644 index 000000000..81408476f --- /dev/null +++ b/tests/zm_videostore_m3u8.cpp @@ -0,0 +1,150 @@ +/* + * This file is part of the ZoneMinder Project. See AUTHORS file for Copyright information + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. + * + * This program 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 General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program. If not, see . + */ + +#include "zm_catch2.h" + +#include "zm_videostore.h" + +#include +#include +#include + +namespace { + +std::vector MakeFragments(size_t n, double duration = 1.2) { + std::vector frags; + int64_t offset = 1024; + for (size_t i = 0; i < n; i++) { + VideoStore::Fragment f; + f.offset = offset; + f.size = 40000 + static_cast(i); + f.duration = duration; + frags.push_back(f); + offset += f.size; + } + return frags; +} + +// What writeM3U8 writes when it rewrites the file from scratch. +std::string WholeManifest(const std::vector &frags, + const std::string &url, int64_t init_end, bool complete) { + std::string out = VideoStore::m3u8Header( + VideoStore::m3u8TargetDuration(frags), url, init_end, complete); + for (const auto &f : frags) out += VideoStore::m3u8Fragment(f, url); + if (complete) out += "#EXT-X-ENDLIST\n"; + return out; +} + +} // namespace + +TEST_CASE("m3u8 appending produces the same manifest as rewriting") { + // This is the whole point of appending: a reader must not be able to tell + // which path wrote the file. Rewriting the manifest for every fragment is + // O(fragments) per fragment and so O(fragments^2) over an event, which is + // what let a monitor whose events stopped closing saturate its disk. + const std::string url = "index.php?view=view_video&eid=6477854&file=incomplete.h264.mp4"; + const int64_t init_end = 1024; + + SECTION("growing one fragment at a time matches a single full write") { + const auto all = MakeFragments(50); + + // Appending: header once, then each fragment's lines as it arrives. + std::string appended = VideoStore::m3u8Header( + VideoStore::m3u8TargetDuration(all), url, init_end, false); + for (const auto &f : all) appended += VideoStore::m3u8Fragment(f, url); + + REQUIRE(appended == WholeManifest(all, url, init_end, false)); + } + + SECTION("a fragment's lines never depend on what came before it") { + // The property that makes appending sound at all. + const auto few = MakeFragments(3); + const auto many = MakeFragments(500); + REQUIRE(VideoStore::m3u8Fragment(few[2], url) == VideoStore::m3u8Fragment(many[2], url)); + } +} + +TEST_CASE("m3u8 target duration") { + const std::string url = "v.mp4"; + + SECTION("is the longest fragment, rounded up") { + std::vector frags = MakeFragments(3, 1.2); + frags[1].duration = 7.3; + REQUIRE(VideoStore::m3u8TargetDuration(frags) == 8); + } + + SECTION("is never below 1, which the spec requires") { + // Sub-second fragments would otherwise round down to 0 and make the + // manifest invalid. + REQUIRE(VideoStore::m3u8TargetDuration(MakeFragments(3, 0.4)) == 1); + REQUIRE(VideoStore::m3u8TargetDuration({}) == 1); + } + + SECTION("a longer fragment changes it, which is what forces a rewrite") { + // writeM3U8 may only append while the header it already wrote still + // stands. The header carries the target duration, so this is the case it + // has to notice. + auto frags = MakeFragments(10, 1.2); + const int before = VideoStore::m3u8TargetDuration(frags); + frags.push_back({999999, 40000, 9.1}); + REQUIRE(VideoStore::m3u8TargetDuration(frags) != before); + REQUIRE(VideoStore::m3u8Header(before, url, 1024, false) + != VideoStore::m3u8Header(VideoStore::m3u8TargetDuration(frags), url, 1024, false)); + } +} + +TEST_CASE("m3u8 header") { + const auto frags = MakeFragments(2); + const std::string url = "index.php?view=view_video&eid=1&file=incomplete.h264.mp4"; + + SECTION("an in-progress event is EVENT, a finished one is VOD") { + REQUIRE(VideoStore::m3u8Header(8, url, 1024, false).find("#EXT-X-PLAYLIST-TYPE:EVENT") + != std::string::npos); + REQUIRE(VideoStore::m3u8Header(8, url, 1024, true).find("#EXT-X-PLAYLIST-TYPE:VOD") + != std::string::npos); + } + + SECTION("the init segment byte range comes from the init segment end") { + REQUIRE(VideoStore::m3u8Header(8, url, 4242, false).find("BYTERANGE=\"4242@0\"") + != std::string::npos); + } + + SECTION("the url is repeated on every fragment, so changing it needs a rewrite") { + // Which is why writeM3U8 remembers the url it wrote with: the close path + // passes a different one, pointing at the final file rather than + // incomplete.h264.mp4. + REQUIRE(VideoStore::m3u8Fragment(frags[0], "a.mp4") + != VideoStore::m3u8Fragment(frags[0], "b.mp4")); + } +} + +TEST_CASE("m3u8 fragment lines") { + VideoStore::Fragment f; + f.offset = 1024; + f.size = 40960; + f.duration = 1.234; + + SECTION("carry the duration, byte range and url a player needs") { + const std::string line = VideoStore::m3u8Fragment(f, "v.mp4"); + REQUIRE(line == "#EXTINF:1.234,\n#EXT-X-BYTERANGE:40960@1024\nv.mp4\n"); + } + + SECTION("are exactly three lines, so the append offset is predictable") { + const std::string line = VideoStore::m3u8Fragment(f, "v.mp4"); + REQUIRE(std::count(line.begin(), line.end(), '\n') == 3); + } +}