mirror of
https://github.com/ZoneMinder/zoneminder.git
synced 2026-10-06 17:31:58 -04:00
test: pin that manifest byte ranges cover whole fragments refs #5174
#5174 reports an event's m3u8 as invalid and proposes moving every fragment's byte range 8 bytes further into the file, from @17203 to @17211. That reading comes from ffprobe's trace, whose second column is the offset of the box *body* -- the start plus the 8 byte box header -- not the start. In the manifest quoted there the first fragment is 1434876@17203, and the trace has the moof body at 17211 and the mdat ending at 1452079, so the range spans exactly moof(264) + mdat(1434612) = 1434876 from the start of the moof. The proposed change would cut the moof header off every segment. The manifest is not contiguous, which is what draws the eye: the init range is ftyp+moov and the fragments start 16KB later, because reserve_region puts the leading sidx in between and the sidx has to END where the fragments BEGIN. Nothing fetches those bytes and HLS does not require byte ranges to abut. So assert it rather than argue it. Against sidx-moof.mp4, using the box walker the sidx tests already keep for the purpose, check that a fragment starts at its moof box rather than 8 bytes in, that fragments run back to back, that the init range is the header boxes and nothing else, and that the gap between them is exactly the reserved index. This says nothing about the browser error that prompted the report, which is not quoted in the issue; it only rules the byte ranges in or out. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
2539e6a157
commit
0bee8990be
1 file changed
+78
@@ -18,6 +18,7 @@
|
||||
#include "zm_catch2.h"
|
||||
|
||||
#include "zm_mp4_sidx.h"
|
||||
#include "zm_videostore.h"
|
||||
|
||||
#include <cmath>
|
||||
#include <cstdio>
|
||||
@@ -696,3 +697,80 @@ TEST_CASE("Mp4SidxReservesOnlyWhereFragmentsFollowTheMoov") {
|
||||
}
|
||||
avformat_close_input(&in);
|
||||
}
|
||||
|
||||
|
||||
TEST_CASE("Mp4SidxManifestRangesCoverWholeFragments") {
|
||||
// Issue #5174 reported a manifest as invalid and proposed moving each
|
||||
// fragment's byte range 8 bytes further into the file. That reads ffprobe's
|
||||
// trace column as the box offset when it is the offset plus the 8 byte box
|
||||
// header, and acting on it would cut the `moof` header off every segment.
|
||||
//
|
||||
// What the ranges actually have to be, checked against a box walker of the
|
||||
// test's own rather than against the code that writes them: the init range
|
||||
// is exactly ftyp+moov, and a fragment is a whole `moof` and the `mdat`
|
||||
// that follows it, header included.
|
||||
const std::vector<uint8_t> file = read_file(fixture("sidx-moof.mp4"));
|
||||
const std::vector<TopBox> boxes = top_level(file);
|
||||
|
||||
// The init segment is the boxes the muxer wrote for the header, which is
|
||||
// what VideoStore records as init_segment_end_ before it reserves anything.
|
||||
int64_t init_end = 0;
|
||||
for (const TopBox &box : boxes) {
|
||||
if (box.type != "ftyp" and box.type != "moov") break;
|
||||
init_end = static_cast<int64_t>(box.offset + box.size);
|
||||
}
|
||||
REQUIRE(init_end > 0);
|
||||
|
||||
std::vector<VideoStore::Fragment> frags;
|
||||
for (size_t i = 0; i + 1 < boxes.size(); i++) {
|
||||
if (boxes[i].type == "moof" and boxes[i + 1].type == "mdat") {
|
||||
frags.push_back({static_cast<int64_t>(boxes[i].offset),
|
||||
static_cast<int64_t>(boxes[i + 1].offset + boxes[i + 1].size
|
||||
- boxes[i].offset),
|
||||
1.0});
|
||||
}
|
||||
}
|
||||
REQUIRE(frags.size() >= 2);
|
||||
|
||||
const int64_t first_moof = static_cast<int64_t>(offset_of(boxes, "moof"));
|
||||
|
||||
SECTION("a fragment starts at its moof box, not past its header") {
|
||||
REQUIRE(frags[0].offset == first_moof);
|
||||
// Spelled out because it is the change #5174 asked for.
|
||||
REQUIRE(frags[0].offset != first_moof + 8);
|
||||
}
|
||||
|
||||
SECTION("fragments run back to back, so no media byte is missed") {
|
||||
for (size_t i = 1; i < frags.size(); i++) {
|
||||
REQUIRE(frags[i].offset == frags[i - 1].offset + frags[i - 1].size);
|
||||
}
|
||||
}
|
||||
|
||||
SECTION("the init range is the header boxes and nothing else") {
|
||||
const std::string header = VideoStore::m3u8Header(
|
||||
VideoStore::m3u8TargetDuration(frags), "v.mp4", init_end, true);
|
||||
REQUIRE(header.find("BYTERANGE=\"" + std::to_string(init_end) + "@0\"")
|
||||
!= std::string::npos);
|
||||
// ftyp+moov, which stops short of the reserved region.
|
||||
REQUIRE(init_end == static_cast<int64_t>(offset_of(boxes, "free")));
|
||||
}
|
||||
|
||||
SECTION("the reserved index region is the gap between them") {
|
||||
// The sidx has to END where the fragments BEGIN, so it sits in a gap that
|
||||
// neither the init range nor any fragment covers. A player skips it; this
|
||||
// is why the manifest is not contiguous and does not need to be.
|
||||
REQUIRE(init_end < frags[0].offset);
|
||||
REQUIRE(static_cast<int64_t>(offset_of(boxes, "sidx")) > init_end);
|
||||
const int64_t sidx_end = static_cast<int64_t>(
|
||||
offset_of(boxes, "sidx")
|
||||
+ be32(&file[offset_of(boxes, "sidx")]));
|
||||
REQUIRE(sidx_end == frags[0].offset);
|
||||
}
|
||||
|
||||
SECTION("the written line names that range exactly") {
|
||||
REQUIRE(VideoStore::m3u8Fragment(frags[0], "v.mp4")
|
||||
== "#EXTINF:1.000,\n#EXT-X-BYTERANGE:"
|
||||
+ std::to_string(frags[0].size) + "@"
|
||||
+ std::to_string(frags[0].offset) + "\nv.mp4\n");
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user