From 61c58724ec7b4e00d1b1b5bf7e332b42064d778b Mon Sep 17 00:00:00 2001 From: KongQun Yang Date: Tue, 18 Jul 2017 12:23:42 -0700 Subject: [PATCH] Remove size parameter in OnMediaEnd Derive file size from media ranges instead. Also fix subsegment range reporting for WebM (internal variable, no output affected). Change-Id: I5f8152dff4c2cd5fbae5550992b86a669e278f7b --- .../media/event/hls_notify_muxer_listener.cc | 3 +-- .../media/event/hls_notify_muxer_listener.h | 3 +-- .../hls_notify_muxer_listener_unittest.cc | 6 ++--- packager/media/event/mock_muxer_listener.cc | 5 ++-- packager/media/event/mock_muxer_listener.h | 24 +++++++++---------- .../media/event/mpd_notify_muxer_listener.cc | 5 ++-- .../media/event/mpd_notify_muxer_listener.h | 3 +-- .../mpd_notify_muxer_listener_unittest.cc | 3 +-- packager/media/event/muxer_listener.h | 7 +++--- .../media/event/muxer_listener_internal.cc | 16 +++++++++---- .../media/event/muxer_listener_internal.h | 1 - .../media/event/muxer_listener_test_helper.cc | 10 ++++++-- .../media/event/muxer_listener_test_helper.h | 1 - .../vod_media_info_dump_muxer_listener.cc | 5 ++-- .../vod_media_info_dump_muxer_listener.h | 3 +-- ...media_info_dump_muxer_listener_unittest.cc | 4 +--- packager/media/formats/mp2t/ts_muxer.cc | 2 +- packager/media/formats/mp4/mp4_muxer.cc | 10 +------- .../formats/webm/single_segment_segmenter.cc | 8 ++++--- packager/media/formats/webm/webm_muxer.cc | 10 +------- 20 files changed, 56 insertions(+), 73 deletions(-) diff --git a/packager/media/event/hls_notify_muxer_listener.cc b/packager/media/event/hls_notify_muxer_listener.cc index 49c1a237a4..b26b52db55 100644 --- a/packager/media/event/hls_notify_muxer_listener.cc +++ b/packager/media/event/hls_notify_muxer_listener.cc @@ -116,8 +116,7 @@ void HlsNotifyMuxerListener::OnMediaStart(const MuxerOptions& muxer_options, void HlsNotifyMuxerListener::OnSampleDurationReady(uint32_t sample_duration) {} void HlsNotifyMuxerListener::OnMediaEnd(const MediaRanges& media_ranges, - float duration_seconds, - uint64_t file_size) { + float duration_seconds) { // TODO(kqyang): Should we just Flush here to avoid calling Flush explicitly? // Don't flush the notifier here. Flushing here would write all the playlists // before all Media Playlists are read. Which could cause problems diff --git a/packager/media/event/hls_notify_muxer_listener.h b/packager/media/event/hls_notify_muxer_listener.h index edd7b780e4..e39e5e5d4a 100644 --- a/packager/media/event/hls_notify_muxer_listener.h +++ b/packager/media/event/hls_notify_muxer_listener.h @@ -54,8 +54,7 @@ class HlsNotifyMuxerListener : public MuxerListener { ContainerType container_type) override; void OnSampleDurationReady(uint32_t sample_duration) override; void OnMediaEnd(const MediaRanges& media_ranges, - float duration_seconds, - uint64_t file_size) override; + float duration_seconds) override; void OnNewSegment(const std::string& file_name, uint64_t start_time, uint64_t duration, diff --git a/packager/media/event/hls_notify_muxer_listener_unittest.cc b/packager/media/event/hls_notify_muxer_listener_unittest.cc index 5b025f877d..9fe6842af5 100644 --- a/packager/media/event/hls_notify_muxer_listener_unittest.cc +++ b/packager/media/event/hls_notify_muxer_listener_unittest.cc @@ -319,7 +319,7 @@ TEST_F(HlsNotifyMuxerListenerTest, OnSampleDurationReady) { // Make sure it doesn't crash. TEST_F(HlsNotifyMuxerListenerTest, OnMediaEnd) { // None of these values matter, they are not used. - listener_.OnMediaEnd(MuxerListener::MediaRanges(), 0, 0); + listener_.OnMediaEnd(MuxerListener::MediaRanges(), 0); } TEST_F(HlsNotifyMuxerListenerTest, OnNewSegment) { @@ -383,7 +383,7 @@ TEST_F(HlsNotifyMuxerListenerTest, NoSegmentTemplateOnMediaEnd) { EXPECT_CALL(mock_notifier_, NotifyNewSegment(_, StrEq("filename.mp4"), kStartTime, kDuration, kSegmentStartOffset, kFileSize)); - listener_.OnMediaEnd(ranges, 200000, 98234328); + listener_.OnMediaEnd(ranges, 200000); } // Verify that when there is a mismatch in the number of calls to @@ -435,7 +435,7 @@ TEST_F(HlsNotifyMuxerListenerTest, EXPECT_CALL(mock_notifier_, NotifyNewSegment(_, StrEq("filename.mp4"), kStartTime, kDuration, kSegmentStartOffset, kFileSize)); - listener_.OnMediaEnd(ranges, 200000, 98234328); + listener_.OnMediaEnd(ranges, 200000); } } // namespace media diff --git a/packager/media/event/mock_muxer_listener.cc b/packager/media/event/mock_muxer_listener.cc index 2e358e1e5e..26ec3b7ff2 100644 --- a/packager/media/event/mock_muxer_listener.cc +++ b/packager/media/event/mock_muxer_listener.cc @@ -13,8 +13,7 @@ MockMuxerListener::MockMuxerListener() {} MockMuxerListener::~MockMuxerListener() {} void MockMuxerListener::OnMediaEnd(const MediaRanges& range, - float duration_seconds, - uint64_t file_size) { + float duration_seconds) { const bool has_init_range = static_cast(range.init_range); Range init_range = {}; if (has_init_range) { @@ -29,7 +28,7 @@ void MockMuxerListener::OnMediaEnd(const MediaRanges& range, OnMediaEndMock(has_init_range, init_range.start, init_range.end, has_index_range, index_range.start, index_range.end, !range.subsegment_ranges.empty(), range.subsegment_ranges, - duration_seconds, file_size); + duration_seconds); } } // namespace media diff --git a/packager/media/event/mock_muxer_listener.h b/packager/media/event/mock_muxer_listener.h index 648726e603..622ffc466a 100644 --- a/packager/media/event/mock_muxer_listener.h +++ b/packager/media/event/mock_muxer_listener.h @@ -40,23 +40,21 @@ class MockMuxerListener : public MuxerListener { MOCK_METHOD1(OnSampleDurationReady, void(uint32_t sample_duration)); - MOCK_METHOD10(OnMediaEndMock, - void(bool has_init_range, - uint64_t init_range_start, - uint64_t init_range_end, - bool has_index_range, - uint64_t index_range_start, - uint64_t index_range_end, - bool has_subsegment_ranges, - const std::vector subsegment_ranges, - float duration_seconds, - uint64_t file_size)); + MOCK_METHOD9(OnMediaEndMock, + void(bool has_init_range, + uint64_t init_range_start, + uint64_t init_range_end, + bool has_index_range, + uint64_t index_range_start, + uint64_t index_range_end, + bool has_subsegment_ranges, + const std::vector subsegment_ranges, + float duration_seconds)); // Windows 32 bit cannot mock MediaRanges because it has Optionals that use // memory alignment of 8 bytes. The compiler fails if it is mocked. void OnMediaEnd(const MediaRanges& range, - float duration_seconds, - uint64_t file_size) override; + float duration_seconds) override; MOCK_METHOD4(OnNewSegment, void(const std::string& segment_name, diff --git a/packager/media/event/mpd_notify_muxer_listener.cc b/packager/media/event/mpd_notify_muxer_listener.cc index 4fa5c61f87..78cf1a2560 100644 --- a/packager/media/event/mpd_notify_muxer_listener.cc +++ b/packager/media/event/mpd_notify_muxer_listener.cc @@ -110,8 +110,7 @@ void MpdNotifyMuxerListener::OnSampleDurationReady( } void MpdNotifyMuxerListener::OnMediaEnd(const MediaRanges& media_ranges, - float duration_seconds, - uint64_t file_size) { + float duration_seconds) { if (mpd_notifier_->dash_profile() == DashProfile::kLive) { DCHECK(subsegments_.empty()); // TODO(kqyang): Set mpd duration to |duration_seconds|, which is more @@ -122,7 +121,7 @@ void MpdNotifyMuxerListener::OnMediaEnd(const MediaRanges& media_ranges, } DCHECK(media_info_); - if (!internal::SetVodInformation(media_ranges, duration_seconds, file_size, + if (!internal::SetVodInformation(media_ranges, duration_seconds, media_info_.get())) { LOG(ERROR) << "Failed to generate VOD information from input."; return; diff --git a/packager/media/event/mpd_notify_muxer_listener.h b/packager/media/event/mpd_notify_muxer_listener.h index 630f1b2a80..d6ab9cc699 100644 --- a/packager/media/event/mpd_notify_muxer_listener.h +++ b/packager/media/event/mpd_notify_muxer_listener.h @@ -46,8 +46,7 @@ class MpdNotifyMuxerListener : public MuxerListener { ContainerType container_type) override; void OnSampleDurationReady(uint32_t sample_duration) override; void OnMediaEnd(const MediaRanges& media_ranges, - float duration_seconds, - uint64_t file_size) override; + float duration_seconds) override; void OnNewSegment(const std::string& file_name, uint64_t start_time, uint64_t duration, diff --git a/packager/media/event/mpd_notify_muxer_listener_unittest.cc b/packager/media/event/mpd_notify_muxer_listener_unittest.cc index b425ef9478..8fce1eea8d 100644 --- a/packager/media/event/mpd_notify_muxer_listener_unittest.cc +++ b/packager/media/event/mpd_notify_muxer_listener_unittest.cc @@ -83,8 +83,7 @@ class MpdNotifyMuxerListenerTest : public ::testing::TestWithParam { void FireOnMediaEndWithParams(const OnMediaEndParameters& params) { // On success, this writes the result to |temp_file_path_|. - listener_->OnMediaEnd(params.media_ranges, params.duration_seconds, - params.file_size); + listener_->OnMediaEnd(params.media_ranges, params.duration_seconds); } std::unique_ptr listener_; diff --git a/packager/media/event/muxer_listener.h b/packager/media/event/muxer_listener.h index 4991034994..1c44d3b1bb 100644 --- a/packager/media/event/muxer_listener.h +++ b/packager/media/event/muxer_listener.h @@ -105,12 +105,11 @@ class MuxerListener { /// Called when all files are written out and the muxer object does not output /// any more files. /// Note: This event might not be very interesting to MPEG DASH Live profile. - /// @param media_ranges is the ranges of the media file. + /// @param media_ranges is the ranges of the media file. It should have ranges + /// for the entire file, using which the file size can be calculated. /// @param duration_seconds is the length of the media in seconds. - /// @param file_size is the size of the file in bytes. virtual void OnMediaEnd(const MediaRanges& media_ranges, - float duration_seconds, - uint64_t file_size) = 0; + float duration_seconds) = 0; /// Called when a segment has been muxed and the file has been written. /// Note: For some implementations, this is used to signal new subsegments. diff --git a/packager/media/event/muxer_listener_internal.cc b/packager/media/event/muxer_listener_internal.cc index 4b98b26bad..6c087d6e88 100644 --- a/packager/media/event/muxer_listener_internal.cc +++ b/packager/media/event/muxer_listener_internal.cc @@ -192,13 +192,8 @@ bool GenerateMediaInfo(const MuxerOptions& muxer_options, bool SetVodInformation(const MuxerListener::MediaRanges& media_ranges, float duration_seconds, - uint64_t file_size, MediaInfo* media_info) { DCHECK(media_info); - if (file_size == 0) { - LOG(ERROR) << "File size not specified."; - return false; - } if (duration_seconds <= 0.0f) { // Non positive second media must be invalid media. @@ -220,6 +215,17 @@ bool SetVodInformation(const MuxerListener::MediaRanges& media_ranges, media_info->set_media_duration_seconds(duration_seconds); if (!media_info->has_bandwidth()) { + // Calculate file size from media_ranges. + uint64_t file_size = 0; + if (media_ranges.init_range) + file_size = std::max(file_size, media_ranges.init_range->end + 1); + if (media_ranges.index_range) + file_size = std::max(file_size, media_ranges.index_range->end + 1); + if (!media_ranges.subsegment_ranges.empty()) { + file_size = + std::max(file_size, media_ranges.subsegment_ranges.back().end + 1); + } + media_info->set_bandwidth( EstimateRequiredBandwidth(file_size, duration_seconds)); } diff --git a/packager/media/event/muxer_listener_internal.h b/packager/media/event/muxer_listener_internal.h index b58341dbc0..ef2ee4b37d 100644 --- a/packager/media/event/muxer_listener_internal.h +++ b/packager/media/event/muxer_listener_internal.h @@ -37,7 +37,6 @@ bool GenerateMediaInfo(const MuxerOptions& muxer_options, /// @return true on success, false otherwise. bool SetVodInformation(const MuxerListener::MediaRanges& media_ranges, float duration_seconds, - uint64_t file_size, MediaInfo* media_info); /// @param protection_scheme specifies the protection scheme: 'cenc', 'cens', diff --git a/packager/media/event/muxer_listener_test_helper.cc b/packager/media/event/muxer_listener_test_helper.cc index 1c26a393c2..d40db1d26a 100644 --- a/packager/media/event/muxer_listener_test_helper.cc +++ b/packager/media/event/muxer_listener_test_helper.cc @@ -64,8 +64,9 @@ OnMediaEndParameters GetDefaultOnMediaEndParams() { const uint64_t kInitRangeEnd = kInitRangeStart + 120; const uint64_t kIndexRangeStart = kInitRangeEnd + 1; const uint64_t kIndexRangeEnd = kIndexRangeStart + 100; + const uint64_t kMediaSegmentRangeStart = kIndexRangeEnd + 1; + const uint64_t kMediaSegmentRangeEnd = 9999; const float kMediaDuration = 10.5f; - const uint64_t kFileSize = 10000; MuxerListener::MediaRanges media_ranges; Range init_range; init_range.start = kInitRangeStart; @@ -76,7 +77,12 @@ OnMediaEndParameters GetDefaultOnMediaEndParams() { index_range.end = kIndexRangeEnd; media_ranges.index_range =index_range; - OnMediaEndParameters param = {media_ranges, kMediaDuration, kFileSize}; + Range media_segment_range; + media_segment_range.start = kMediaSegmentRangeStart; + media_segment_range.end = kMediaSegmentRangeEnd; + media_ranges.subsegment_ranges.push_back(media_segment_range); + + OnMediaEndParameters param = {media_ranges, kMediaDuration}; return param; } diff --git a/packager/media/event/muxer_listener_test_helper.h b/packager/media/event/muxer_listener_test_helper.h index 48b16d65c7..b2ec566d15 100644 --- a/packager/media/event/muxer_listener_test_helper.h +++ b/packager/media/event/muxer_listener_test_helper.h @@ -76,7 +76,6 @@ struct VideoStreamInfoParameters { struct OnMediaEndParameters { MuxerListener::MediaRanges media_ranges; float duration_seconds; - uint64_t file_size; }; // Creates StreamInfo instance from VideoStreamInfoParameters. diff --git a/packager/media/event/vod_media_info_dump_muxer_listener.cc b/packager/media/event/vod_media_info_dump_muxer_listener.cc index ceb5e25244..b09b373c05 100644 --- a/packager/media/event/vod_media_info_dump_muxer_listener.cc +++ b/packager/media/event/vod_media_info_dump_muxer_listener.cc @@ -73,10 +73,9 @@ void VodMediaInfoDumpMuxerListener::OnSampleDurationReady( } void VodMediaInfoDumpMuxerListener::OnMediaEnd(const MediaRanges& media_ranges, - float duration_seconds, - uint64_t file_size) { + float duration_seconds) { DCHECK(media_info_); - if (!internal::SetVodInformation(media_ranges, duration_seconds, file_size, + if (!internal::SetVodInformation(media_ranges, duration_seconds, media_info_.get())) { LOG(ERROR) << "Failed to generate VOD information from input."; return; diff --git a/packager/media/event/vod_media_info_dump_muxer_listener.h b/packager/media/event/vod_media_info_dump_muxer_listener.h index 431ceb5cb0..2e180a1b0d 100644 --- a/packager/media/event/vod_media_info_dump_muxer_listener.h +++ b/packager/media/event/vod_media_info_dump_muxer_listener.h @@ -45,8 +45,7 @@ class VodMediaInfoDumpMuxerListener : public MuxerListener { ContainerType container_type) override; void OnSampleDurationReady(uint32_t sample_duration) override; void OnMediaEnd(const MediaRanges& media_ranges, - float duration_seconds, - uint64_t file_size) override; + float duration_seconds) override; void OnNewSegment(const std::string& file_name, uint64_t start_time, uint64_t duration, diff --git a/packager/media/event/vod_media_info_dump_muxer_listener_unittest.cc b/packager/media/event/vod_media_info_dump_muxer_listener_unittest.cc index df78bc00a1..3c9413d6a9 100644 --- a/packager/media/event/vod_media_info_dump_muxer_listener_unittest.cc +++ b/packager/media/event/vod_media_info_dump_muxer_listener_unittest.cc @@ -95,9 +95,7 @@ class VodMediaInfoDumpMuxerListenerTest : public ::testing::Test { void FireOnMediaEndWithParams(const OnMediaEndParameters& params) { // On success, this writes the result to |temp_file_path_|. - listener_->OnMediaEnd(params.media_ranges, - params.duration_seconds, - params.file_size); + listener_->OnMediaEnd(params.media_ranges, params.duration_seconds); } void ExpectTempFileToEqual(const std::string& expected_protobuf) { diff --git a/packager/media/formats/mp2t/ts_muxer.cc b/packager/media/formats/mp2t/ts_muxer.cc index e53968510f..92e3bf99c6 100644 --- a/packager/media/formats/mp2t/ts_muxer.cc +++ b/packager/media/formats/mp2t/ts_muxer.cc @@ -61,7 +61,7 @@ void TsMuxer::FireOnMediaEndEvent() { // For now, there is no single file TS segmenter. So all the values passed // here are left empty. MuxerListener::MediaRanges range; - muxer_listener()->OnMediaEnd(range, 0, 0); + muxer_listener()->OnMediaEnd(range, 0); } } // namespace mp2t diff --git a/packager/media/formats/mp4/mp4_muxer.cc b/packager/media/formats/mp4/mp4_muxer.cc index 8fe9d2188c..ee5035a62c 100644 --- a/packager/media/formats/mp4/mp4_muxer.cc +++ b/packager/media/formats/mp4/mp4_muxer.cc @@ -469,15 +469,7 @@ void MP4Muxer::FireOnMediaEndEvent() { media_range.subsegment_ranges = segmenter_->GetSegmentRanges(); const float duration_seconds = static_cast(segmenter_->GetDuration()); - - const int64_t file_size = - File::GetFileSize(options().output_file_name.c_str()); - if (file_size <= 0) { - LOG(ERROR) << "Invalid file size: " << file_size; - return; - } - - muxer_listener()->OnMediaEnd(media_range, duration_seconds, file_size); + muxer_listener()->OnMediaEnd(media_range, duration_seconds); } uint64_t MP4Muxer::IsoTimeNow() { diff --git a/packager/media/formats/webm/single_segment_segmenter.cc b/packager/media/formats/webm/single_segment_segmenter.cc index 7335007a61..ead19d3f99 100644 --- a/packager/media/formats/webm/single_segment_segmenter.cc +++ b/packager/media/formats/webm/single_segment_segmenter.cc @@ -56,15 +56,17 @@ std::vector SingleSegmentSegmenter::GetSegmentRanges() { for (int32_t i = 0; i < cues()->cue_entries_size() - 1; ++i) { const mkvmuxer::CuePoint* cue_point = cues()->GetCueByIndex(i); Range r; - r.start = cue_point->cluster_pos(); - r.end = cues()->GetCueByIndex(i + 1)->cluster_pos() - 1; + // Cue point cluster position is relative to segment payload pos. + r.start = segment_payload_pos() + cue_point->cluster_pos(); + r.end = + segment_payload_pos() + cues()->GetCueByIndex(i + 1)->cluster_pos() - 1; ranges.push_back(r); } Range last_range; const mkvmuxer::CuePoint* last_cue_point = cues()->GetCueByIndex(cues()->cue_entries_size() - 1); - last_range.start = last_cue_point->cluster_pos(); + last_range.start = segment_payload_pos() + last_cue_point->cluster_pos(); last_range.end = last_range.start + cluster()->Size() - 1; ranges.push_back(last_range); return ranges; diff --git a/packager/media/formats/webm/webm_muxer.cc b/packager/media/formats/webm/webm_muxer.cc index 6d46b85704..93dda11a5b 100644 --- a/packager/media/formats/webm/webm_muxer.cc +++ b/packager/media/formats/webm/webm_muxer.cc @@ -122,15 +122,7 @@ void WebMMuxer::FireOnMediaEndEvent() { media_range.subsegment_ranges = segmenter_->GetSegmentRanges(); const float duration_seconds = segmenter_->GetDurationInSeconds(); - - const int64_t file_size = - File::GetFileSize(options().output_file_name.c_str()); - if (file_size <= 0) { - LOG(ERROR) << "Invalid file size: " << file_size; - return; - } - - muxer_listener()->OnMediaEnd(media_range, duration_seconds, file_size); + muxer_listener()->OnMediaEnd(media_range, duration_seconds); } } // namespace webm