From 6b6137f8e733b159d6e0496fcbdc01249b41530a Mon Sep 17 00:00:00 2001 From: Scott Lamb Date: Tue, 18 Oct 2016 20:28:25 -0700 Subject: [PATCH] fixes to mp4 generation * typo: the subtitle should use its own mdhd, not alias the video one * use 64-bit ints for the edit lists; the 32-bit values overflow at 13.25 hours * use etags that reflect the edit list --- src/mp4-test.cc | 33 +++++++++++++++++++-------------- src/mp4.cc | 25 +++++++++++++------------ 2 files changed, 32 insertions(+), 26 deletions(-) diff --git a/src/mp4-test.cc b/src/mp4-test.cc index 0b5612a..76e4d5c 100644 --- a/src/mp4-test.cc +++ b/src/mp4-test.cc @@ -344,6 +344,16 @@ TEST_F(IntegrationTest, RoundTrip) { recording, 0, std::numeric_limits::max(), false); WriteMp4(f.get()); CompareMp4s(0); + + // This test is brittle, which is the point. Any time the digest comparison + // here fails, it can be updated, but the etag must change as well! + // Otherwise clients may combine ranges from the new format with ranges + // from the old format! + EXPECT_EQ("1e5331e8371bd97ac3158b3a86494abc87cdc70e", Digest(f.get())); + EXPECT_EQ("\"268db2cd6e4814676d38832f1f9340c7555e4e71\"", f->etag()); + + // 10 seconds later than the segment's start time. + EXPECT_EQ(1430006410, f->last_modified()); } TEST_F(IntegrationTest, RoundTripWithSubtitle) { @@ -355,6 +365,13 @@ TEST_F(IntegrationTest, RoundTripWithSubtitle) { recording, 0, std::numeric_limits::max(), true); WriteMp4(f.get()); CompareMp4s(0); + + // This test is brittle, which is the point. Any time the digest comparison + // here fails, it can be updated, but the etag must change as well! + // Otherwise clients may combine ranges from the new format with ranges + // from the old format! + EXPECT_EQ("0081a442ba73092027fc580eeac2ebf25cb1ef50", Digest(f.get())); + EXPECT_EQ("\"8a29042355e1e28c10fbba328d1ddc9d54e450cd\"", f->etag()); } TEST_F(IntegrationTest, RoundTripWithEditList) { @@ -366,25 +383,13 @@ TEST_F(IntegrationTest, RoundTripWithEditList) { recording, 1, std::numeric_limits::max(), false); WriteMp4(f.get()); CompareMp4s(-1); -} - -TEST_F(IntegrationTest, Metadata) { - Recording recording = CopyMp4ToSingleRecording(); - if (HasFailure()) { - return; - } - auto f = CreateMp4FromSingleRecording( - recording, 0, std::numeric_limits::max(), false); // This test is brittle, which is the point. Any time the digest comparison // here fails, it can be updated, but the etag must change as well! // Otherwise clients may combine ranges from the new format with ranges // from the old format! - EXPECT_EQ("1e5331e8371bd97ac3158b3a86494abc87cdc70e", Digest(f.get())); - EXPECT_EQ("\"a9ce99a83a177516e7f862fed405e2b47b7d4ae3\"", f->etag()); - - // 10 seconds later than the segment's start time. - EXPECT_EQ(1430006410, f->last_modified()); + EXPECT_EQ("685e026af44204bc9cc52115c5e17058e9fb7c70", Digest(f.get())); + EXPECT_EQ("\"1373289ddc7c05580deeeb1f1624e2d6cac7ddd3\"", f->etag()); } } // namespace diff --git a/src/mp4.cc b/src/mp4.cc index 479cea6..c95f8c1 100644 --- a/src/mp4.cc +++ b/src/mp4.cc @@ -298,10 +298,10 @@ struct EditBox { // ISO/IEC 14496-12 section 8.6.5, edts. const char type[4] = {'e', 'd', 't', 's'}; } __attribute__((packed)); -struct EditListBoxVersion0 { // ISO/IEC 14496-12 section 8.6.6, elst. +struct EditListBoxVersion1 { // ISO/IEC 14496-12 section 8.6.6, elst. uint32_t size = NET_UINT32_C(0); const char type[4] = {'e', 'l', 's', 't'}; - const uint32_t version_and_flags = NET_UINT32_C(0); + const uint32_t version_and_flags = NET_UINT32_C(1 << 24); uint32_t entry_count = NET_UINT32_C(0); }; @@ -542,8 +542,9 @@ class Mp4File : public VirtualFile { std::string segment_times; for (const auto &segment : segments_) { segment_times.clear(); - Append64(segment->pieces.sample_pos().begin, &segment_times); - Append64(segment->pieces.sample_pos().end, &segment_times); + Append64(segment->recording.start_time_90k, &segment_times); + Append32(segment->rel_start_90k, &segment_times); + Append32(segment->pieces.end_90k(), &segment_times); etag_digest->Update(segment_times); etag_digest->Update(segment->recording.sample_file_sha1); } @@ -609,11 +610,11 @@ class Mp4File : public VirtualFile { void MaybeAppendVideoEdts() { struct Entry { - Entry(int32_t segment_duration, int32_t media_time) + Entry(int64_t segment_duration, int64_t media_time) : segment_duration(segment_duration), media_time(media_time) {} - int32_t segment_duration = 0; - int32_t media_time = 0; - int32_t end() const { return segment_duration + media_time; } + int64_t segment_duration = 0; + int64_t media_time = 0; + int64_t end() const { return segment_duration + media_time; } }; std::vector entries; int64_t cur_media_time = 0; @@ -640,8 +641,8 @@ class Mp4File : public VirtualFile { for (const auto &entry : entries) { VLOG(2) << "...duration=" << entry.segment_duration << ", time=" << entry.media_time; - AppendU32(entry.segment_duration, s); - AppendU32(entry.media_time, s); + AppendU64(entry.segment_duration, s); + AppendU64(entry.media_time, s); AppendU16(1, s); // media_rate_integer AppendU16(1, s); // media_rate_fraction } @@ -761,7 +762,7 @@ class Mp4File : public VirtualFile { { CONSTRUCT_BOX(moov_subtitle_trak_mdia_); { - CONSTRUCT_BOX(moov_video_trak_mdia_mdhd_); + CONSTRUCT_BOX(moov_subtitle_trak_mdia_mdhd_); moov_subtitle_trak_mdia_mdhd_.header().creation_time = net_creation_ts; moov_subtitle_trak_mdia_mdhd_.header().modification_time = net_creation_ts; @@ -903,7 +904,7 @@ class Mp4File : public VirtualFile { Mp4Box moov_video_trak_; Mp4Box moov_video_trak_tkhd_; Mp4Box moov_video_trak_edts_; - Mp4Box moov_video_trak_edts_elst_; + Mp4Box moov_video_trak_edts_elst_; StringPieceSlice moov_video_trak_edts_elst_entries_; std::string moov_video_trak_edts_elst_entries_str_; Mp4Box moov_video_trak_mdia_;