From fa4a172b04b6e548f90c734e1472ed80ff4ed700 Mon Sep 17 00:00:00 2001 From: Rintaro Kuroiwa Date: Thu, 10 Sep 2015 16:01:00 -0700 Subject: [PATCH] Add MpdNofitier::Flush() for explicit flushing - Without the Flush() method the implementation had to do writes in almost all methods. Usually the user of the notifier knows when to write out the MPD. Change-Id: Ic165d4594f01357a8ac7e8501eefa0f85c08d32f --- .../media/event/mpd_notify_muxer_listener.cc | 2 ++ .../mpd_notify_muxer_listener_unittest.cc | 5 +++ packager/mpd/base/dash_iop_mpd_notifier.cc | 11 ++++--- packager/mpd/base/dash_iop_mpd_notifier.h | 2 ++ .../base/dash_iop_mpd_notifier_unittest.cc | 32 +++---------------- packager/mpd/base/mpd_notifier.h | 5 +++ packager/mpd/base/simple_mpd_notifier.cc | 15 ++++----- packager/mpd/base/simple_mpd_notifier.h | 1 + .../mpd/base/simple_mpd_notifier_unittest.cc | 17 +++------- 9 files changed, 36 insertions(+), 54 deletions(-) diff --git a/packager/media/event/mpd_notify_muxer_listener.cc b/packager/media/event/mpd_notify_muxer_listener.cc index 7972deb54b..fb71f0032f 100644 --- a/packager/media/event/mpd_notify_muxer_listener.cc +++ b/packager/media/event/mpd_notify_muxer_listener.cc @@ -134,6 +134,7 @@ void MpdNotifyMuxerListener::OnMediaEnd(bool has_init_range, it->segment_file_size); } subsegments_.clear(); + mpd_notifier_->Flush(); } void MpdNotifyMuxerListener::OnNewSegment(uint64_t start_time, @@ -143,6 +144,7 @@ void MpdNotifyMuxerListener::OnNewSegment(uint64_t start_time, // TODO(kqyang): Check return result. mpd_notifier_->NotifyNewSegment( notification_id_, start_time, duration, segment_file_size); + mpd_notifier_->Flush(); } else { SubsegmentInfo subsegment = {start_time, duration, segment_file_size}; subsegments_.push_back(subsegment); diff --git a/packager/media/event/mpd_notify_muxer_listener_unittest.cc b/packager/media/event/mpd_notify_muxer_listener_unittest.cc index a67cc305b2..8f9731d861 100644 --- a/packager/media/event/mpd_notify_muxer_listener_unittest.cc +++ b/packager/media/event/mpd_notify_muxer_listener_unittest.cc @@ -55,6 +55,7 @@ class MockMpdNotifier : public MpdNotifier { AddContentProtectionElement, bool(uint32_t container_id, const ContentProtectionElement& content_protection_element)); + MOCK_METHOD0(Flush, bool()); }; } // namespace @@ -107,6 +108,7 @@ TEST_F(MpdNotifyMuxerListenerTest, VodClearContent) { EXPECT_CALL(*notifier_, NotifyNewContainer( ExpectMediaInfoEq(kExpectedDefaultMediaInfo), _)); + EXPECT_CALL(*notifier_, Flush()); FireOnMediaEndWithParams(GetDefaultOnMediaEndParams()); } @@ -174,6 +176,7 @@ TEST_F(MpdNotifyMuxerListenerTest, VodEncryptedContent) { EXPECT_CALL(*notifier_, NotifyNewContainer(ExpectMediaInfoEq(kExpectedMediaInfo), _)); + EXPECT_CALL(*notifier_, Flush()); FireOnMediaEndWithParams(GetDefaultOnMediaEndParams()); } @@ -221,6 +224,7 @@ TEST_F(MpdNotifyMuxerListenerTest, VodOnSampleDurationReady) { EXPECT_CALL(*notifier_, NotifyNewContainer(ExpectMediaInfoEq(kExpectedMediaInfo), _)); + EXPECT_CALL(*notifier_, Flush()); FireOnMediaEndWithParams(GetDefaultOnMediaEndParams()); } @@ -257,6 +261,7 @@ TEST_F(MpdNotifyMuxerListenerTest, VodOnNewSegment) { NotifyNewSegment(_, kStartTime1, kDuration1, kSegmentFileSize1)); EXPECT_CALL(*notifier_, NotifyNewSegment(_, kStartTime2, kDuration2, kSegmentFileSize2)); + EXPECT_CALL(*notifier_, Flush()); FireOnMediaEndWithParams(GetDefaultOnMediaEndParams()); } diff --git a/packager/mpd/base/dash_iop_mpd_notifier.cc b/packager/mpd/base/dash_iop_mpd_notifier.cc index 25900c06fd..57ad646552 100644 --- a/packager/mpd/base/dash_iop_mpd_notifier.cc +++ b/packager/mpd/base/dash_iop_mpd_notifier.cc @@ -94,9 +94,6 @@ bool DashIopMpdNotifier::NotifyNewContainer(const MediaInfo& media_info, *container_id = representation->id(); DCHECK(!ContainsKey(representation_map_, representation->id())); representation_map_[representation->id()] = representation; - - if (mpd_builder_->type() == MpdBuilder::kStatic) - return WriteMpdToFile(output_path_, mpd_builder_.get()); return true; } @@ -109,7 +106,7 @@ bool DashIopMpdNotifier::NotifySampleDuration(uint32_t container_id, return false; } it->second->SetSampleDuration(sample_duration); - return WriteMpdToFile(output_path_, mpd_builder_.get()); + return true; } bool DashIopMpdNotifier::NotifyNewSegment(uint32_t container_id, @@ -123,7 +120,7 @@ bool DashIopMpdNotifier::NotifyNewSegment(uint32_t container_id, return false; } it->second->AddNewSegment(start_time, duration, size); - return WriteMpdToFile(output_path_, mpd_builder_.get()); + return true; } bool DashIopMpdNotifier::AddContentProtectionElement( @@ -136,6 +133,10 @@ bool DashIopMpdNotifier::AddContentProtectionElement( return false; } +bool DashIopMpdNotifier::Flush() { + return WriteMpdToFile(output_path_, mpd_builder_.get()); +} + AdaptationSet* DashIopMpdNotifier::GetAdaptationSetForMediaInfo( const MediaInfo& media_info, ContentType content_type, diff --git a/packager/mpd/base/dash_iop_mpd_notifier.h b/packager/mpd/base/dash_iop_mpd_notifier.h index 859f21d5b9..bf2a0ade01 100644 --- a/packager/mpd/base/dash_iop_mpd_notifier.h +++ b/packager/mpd/base/dash_iop_mpd_notifier.h @@ -34,6 +34,7 @@ class DashIopMpdNotifier : public MpdNotifier { const std::string& output_path); virtual ~DashIopMpdNotifier() OVERRIDE; + /// None of the methods write out the MPD file until Flush() is called. /// @name MpdNotifier implemetation overrides. /// @{ virtual bool Init() OVERRIDE; @@ -48,6 +49,7 @@ class DashIopMpdNotifier : public MpdNotifier { virtual bool AddContentProtectionElement( uint32_t id, const ContentProtectionElement& content_protection_element) OVERRIDE; + virtual bool Flush() OVERRIDE; /// @} private: diff --git a/packager/mpd/base/dash_iop_mpd_notifier_unittest.cc b/packager/mpd/base/dash_iop_mpd_notifier_unittest.cc index 39c559f648..6ba8915fd0 100644 --- a/packager/mpd/base/dash_iop_mpd_notifier_unittest.cc +++ b/packager/mpd/base/dash_iop_mpd_notifier_unittest.cc @@ -169,13 +169,15 @@ TEST_P(DashIopMpdNotifierTest, NotifyNewContainer) { EXPECT_CALL(*default_mock_adaptation_set_, AddRepresentation(_)) .WillOnce(Return(default_mock_representation_.get())); - if (mpd_type() == MpdBuilder::kStatic) - EXPECT_CALL(*mock_mpd_builder, ToString(_)).WillOnce(Return(true)); + // This is for the Flush() below but adding expectation here because the next + // lines Pass() the pointer. + EXPECT_CALL(*mock_mpd_builder, ToString(_)).WillOnce(Return(true)); uint32_t unused_container_id; SetMpdBuilder(¬ifier, mock_mpd_builder.PassAs()); EXPECT_TRUE(notifier.NotifyNewContainer(ConvertToMediaInfo(kValidMediaInfo), &unused_container_id)); + EXPECT_TRUE(notifier.Flush()); } // Verify VOD NotifyNewContainer() operation works with different @@ -280,9 +282,6 @@ TEST_P(DashIopMpdNotifierTest, EXPECT_CALL(*sd_adaptation_set, AddRepresentation(_)) .WillOnce(Return(sd_representation.get())); - if (mpd_type() == MpdBuilder::kStatic) - EXPECT_CALL(*mock_mpd_builder, ToString(_)).WillOnce(Return(true)); - EXPECT_CALL(*mock_mpd_builder, AddAdaptationSet(_)) .WillOnce(Return(hd_adaptation_set.get())); // Called twice for the same reason as above. @@ -295,9 +294,6 @@ TEST_P(DashIopMpdNotifierTest, EXPECT_CALL(*hd_adaptation_set, AddRepresentation(_)) .WillOnce(Return(hd_representation.get())); - if (mpd_type() == MpdBuilder::kStatic) - EXPECT_CALL(*mock_mpd_builder, ToString(_)).WillOnce(Return(true)); - uint32_t unused_container_id; SetMpdBuilder(¬ifier, mock_mpd_builder.PassAs()); EXPECT_TRUE(notifier.NotifyNewContainer( @@ -391,8 +387,6 @@ TEST_P(DashIopMpdNotifierTest, NotifyNewContainersWithSameProtectedContent) { EXPECT_CALL(*default_mock_adaptation_set_, AddRole(_)).Times(0); EXPECT_CALL(*default_mock_adaptation_set_, AddRepresentation(_)) .WillOnce(Return(sd_representation.get())); - if (mpd_type() == MpdBuilder::kStatic) - EXPECT_CALL(*mock_mpd_builder, ToString(_)).WillOnce(Return(true)); // For second representation, no new AddAdaptationSet(). // And make sure that AddContentProtection() is not called. @@ -402,8 +396,6 @@ TEST_P(DashIopMpdNotifierTest, NotifyNewContainersWithSameProtectedContent) { EXPECT_CALL(*default_mock_adaptation_set_, AddRole(_)).Times(0); EXPECT_CALL(*default_mock_adaptation_set_, AddRepresentation(_)) .WillOnce(Return(hd_representation.get())); - if (mpd_type() == MpdBuilder::kStatic) - EXPECT_CALL(*mock_mpd_builder, ToString(_)).WillOnce(Return(true)); uint32_t unused_container_id; SetMpdBuilder(¬ifier, mock_mpd_builder.PassAs()); @@ -425,9 +417,6 @@ TEST_P(DashIopMpdNotifierTest, AddContentProtection) { EXPECT_CALL(*default_mock_adaptation_set_, AddRepresentation(_)) .WillOnce(Return(default_mock_representation_.get())); - if (mpd_type() == MpdBuilder::kStatic) - EXPECT_CALL(*mock_mpd_builder, ToString(_)).WillOnce(Return(true)); - uint32_t container_id; SetMpdBuilder(¬ifier, mock_mpd_builder.PassAs()); EXPECT_TRUE(notifier.NotifyNewContainer(ConvertToMediaInfo(kValidMediaInfo), @@ -518,8 +507,6 @@ TEST_P(DashIopMpdNotifierTest, SetGroup) { EXPECT_CALL(*sd_adaptation_set, AddContentProtectionElement(_)).Times(2); EXPECT_CALL(*sd_adaptation_set, AddRepresentation(_)) .WillOnce(Return(sd_representation.get())); - if (mpd_type() == MpdBuilder::kStatic) - EXPECT_CALL(*mock_mpd_builder, ToString(_)).WillOnce(Return(true)); EXPECT_CALL(*mock_mpd_builder, AddAdaptationSet(_)) .WillOnce(Return(hd_adaptation_set.get())); @@ -532,9 +519,6 @@ TEST_P(DashIopMpdNotifierTest, SetGroup) { EXPECT_CALL(*sd_adaptation_set, SetGroup(kExpectedGroupId)); EXPECT_CALL(*hd_adaptation_set, SetGroup(kExpectedGroupId)); - if (mpd_type() == MpdBuilder::kStatic) - EXPECT_CALL(*mock_mpd_builder, ToString(_)).WillOnce(Return(true)); - // This is not very nice but we need it for settings expectations later. MockMpdBuilder* mock_mpd_builder_raw = mock_mpd_builder.get(); uint32_t unused_container_id; @@ -589,9 +573,6 @@ TEST_P(DashIopMpdNotifierTest, SetGroup) { // Same group ID should be set. EXPECT_CALL(*fourk_adaptation_set, SetGroup(kExpectedGroupId)); - if (mpd_type() == MpdBuilder::kStatic) - EXPECT_CALL(*mock_mpd_builder_raw, ToString(_)).WillOnce(Return(true)); - EXPECT_TRUE(notifier.NotifyNewContainer( ConvertToMediaInfo(k4kProtectedContent), &unused_container_id)); } @@ -673,8 +654,6 @@ TEST_P(DashIopMpdNotifierTest, DoNotSetGroupIfContentTypesDifferent) { EXPECT_CALL(*video_adaptation_set, AddRole(_)).Times(0); EXPECT_CALL(*video_adaptation_set, AddRepresentation(_)) .WillOnce(Return(video_representation.get())); - if (mpd_type() == MpdBuilder::kStatic) - EXPECT_CALL(*mock_mpd_builder, ToString(_)).WillOnce(Return(true)); EXPECT_CALL(*mock_mpd_builder, AddAdaptationSet(_)) .WillOnce(Return(audio_adaptation_set.get())); @@ -683,9 +662,6 @@ TEST_P(DashIopMpdNotifierTest, DoNotSetGroupIfContentTypesDifferent) { EXPECT_CALL(*audio_adaptation_set, AddRepresentation(_)) .WillOnce(Return(audio_representation.get())); - if (mpd_type() == MpdBuilder::kStatic) - EXPECT_CALL(*mock_mpd_builder, ToString(_)).WillOnce(Return(true)); - uint32_t unused_container_id; SetMpdBuilder(¬ifier, mock_mpd_builder.PassAs()); EXPECT_TRUE(notifier.NotifyNewContainer( diff --git a/packager/mpd/base/mpd_notifier.h b/packager/mpd/base/mpd_notifier.h index 8b81631a17..24b9fb7410 100644 --- a/packager/mpd/base/mpd_notifier.h +++ b/packager/mpd/base/mpd_notifier.h @@ -83,6 +83,11 @@ class MpdNotifier { uint32_t container_id, const ContentProtectionElement& content_protection_element) = 0; + /// Call this method to force a flush. Implementations might not write out + /// the MPD to a stream (file, stdout, etc.) when the MPD is updated, this + /// forces a flush. + virtual bool Flush() = 0; + /// @return The dash profile for this object. DashProfile dash_profile() const { return dash_profile_; } diff --git a/packager/mpd/base/simple_mpd_notifier.cc b/packager/mpd/base/simple_mpd_notifier.cc index cefee2a64f..217efb7bfd 100644 --- a/packager/mpd/base/simple_mpd_notifier.cc +++ b/packager/mpd/base/simple_mpd_notifier.cc @@ -68,9 +68,6 @@ bool SimpleMpdNotifier::NotifyNewContainer(const MediaInfo& media_info, *container_id = representation->id(); DCHECK(!ContainsKey(representation_map_, representation->id())); representation_map_[representation->id()] = representation; - - if (mpd_builder_->type() == MpdBuilder::kStatic) - return WriteMpdToFile(output_path_, mpd_builder_.get()); return true; } @@ -82,10 +79,8 @@ bool SimpleMpdNotifier::NotifySampleDuration(uint32_t container_id, LOG(ERROR) << "Unexpected container_id: " << container_id; return false; } - // This sets the right frameRate for Representation or AdaptationSet, so - // write out the new MPD. it->second->SetSampleDuration(sample_duration); - return WriteMpdToFile(output_path_, mpd_builder_.get()); + return true; } bool SimpleMpdNotifier::NotifyNewSegment(uint32_t container_id, @@ -98,10 +93,8 @@ bool SimpleMpdNotifier::NotifyNewSegment(uint32_t container_id, LOG(ERROR) << "Unexpected container_id: " << container_id; return false; } - // For live, the timeline and segmentAlignment gets updated. For VOD, - // subsegmentAlignment gets updated. So write out the MPD. it->second->AddNewSegment(start_time, duration, size); - return WriteMpdToFile(output_path_, mpd_builder_.get()); + return true; } bool SimpleMpdNotifier::AddContentProtectionElement( @@ -114,6 +107,10 @@ bool SimpleMpdNotifier::AddContentProtectionElement( return false; } it->second->AddContentProtectionElement(content_protection_element); + return true; +} + +bool SimpleMpdNotifier::Flush() { return WriteMpdToFile(output_path_, mpd_builder_.get()); } diff --git a/packager/mpd/base/simple_mpd_notifier.h b/packager/mpd/base/simple_mpd_notifier.h index 7df6cd4b8d..ad33ecfbbe 100644 --- a/packager/mpd/base/simple_mpd_notifier.h +++ b/packager/mpd/base/simple_mpd_notifier.h @@ -50,6 +50,7 @@ class SimpleMpdNotifier : public MpdNotifier { virtual bool AddContentProtectionElement( uint32_t id, const ContentProtectionElement& content_protection_element) OVERRIDE; + virtual bool Flush() OVERRIDE; /// @} private: diff --git a/packager/mpd/base/simple_mpd_notifier_unittest.cc b/packager/mpd/base/simple_mpd_notifier_unittest.cc index 19a45bbc1f..cdc3092633 100644 --- a/packager/mpd/base/simple_mpd_notifier_unittest.cc +++ b/packager/mpd/base/simple_mpd_notifier_unittest.cc @@ -110,13 +110,15 @@ TEST_P(SimpleMpdNotifierTest, NotifyNewContainer) { EXPECT_CALL(*default_mock_adaptation_set_, AddRepresentation(_)) .WillOnce(Return(mock_representation.get())); - if (mpd_type == MpdBuilder::kStatic) - EXPECT_CALL(*mock_mpd_builder, ToString(_)).WillOnce(Return(true)); + // This is for the Flush() below but adding expectation here because the next + // lines Pass() the pointer. + EXPECT_CALL(*mock_mpd_builder, ToString(_)).WillOnce(Return(true)); uint32_t unused_container_id; SetMpdBuilder(¬ifier, mock_mpd_builder.PassAs()); EXPECT_TRUE(notifier.NotifyNewContainer(ConvertToMediaInfo(kValidMediaInfo), &unused_container_id)); + EXPECT_TRUE(notifier.Flush()); } // Verify NotifySampleDuration() works as expected for Live. @@ -133,7 +135,6 @@ TEST_F(SimpleMpdNotifierTest, LiveNotifySampleDuration) { .WillOnce(Return(default_mock_adaptation_set_.get())); EXPECT_CALL(*default_mock_adaptation_set_, AddRepresentation(_)) .WillOnce(Return(mock_representation.get())); - EXPECT_CALL(*mock_mpd_builder, ToString(_)).WillOnce(Return(true)); uint32_t container_id; SetMpdBuilder(¬ifier, mock_mpd_builder.PassAs()); @@ -152,7 +153,7 @@ TEST_F(SimpleMpdNotifierTest, LiveNotifySampleDuration) { // register it to its map for VOD. Must fix and enable this test. // This test can be also parmeterized just like NotifyNewContainer() test, once // it is fixed. -TEST_F(SimpleMpdNotifierTest, DISABLED_OnDemandNotifySampleDuration) { +TEST_F(SimpleMpdNotifierTest, OnDemandNotifySampleDuration) { SimpleMpdNotifier notifier(kOnDemandProfile, empty_mpd_option_, empty_base_urls_, output_path_); @@ -165,7 +166,6 @@ TEST_F(SimpleMpdNotifierTest, DISABLED_OnDemandNotifySampleDuration) { .WillOnce(Return(default_mock_adaptation_set_.get())); EXPECT_CALL(*default_mock_adaptation_set_, AddRepresentation(_)) .WillOnce(Return(mock_representation.get())); - EXPECT_CALL(*mock_mpd_builder, ToString(_)).WillOnce(Return(true)); uint32_t container_id; SetMpdBuilder(¬ifier, mock_mpd_builder.PassAs()); @@ -194,10 +194,6 @@ TEST_F(SimpleMpdNotifierTest, LiveNotifyNewSegment) { EXPECT_CALL(*default_mock_adaptation_set_, AddRepresentation(_)) .WillOnce(Return(mock_representation.get())); - // Expect call at NotifyNewSegment(). But putting expect call here because the - // next line passes the value to the notifier. - EXPECT_CALL(*mock_mpd_builder, ToString(_)).WillOnce(Return(true)); - uint32_t container_id; SetMpdBuilder(¬ifier, mock_mpd_builder.PassAs()); EXPECT_TRUE(notifier.NotifyNewContainer(ConvertToMediaInfo(kValidMediaInfo), @@ -228,9 +224,6 @@ TEST_F(SimpleMpdNotifierTest, AddContentProtectionElement) { .WillOnce(Return(default_mock_adaptation_set_.get())); EXPECT_CALL(*default_mock_adaptation_set_, AddRepresentation(_)) .WillOnce(Return(mock_representation.get())); - EXPECT_CALL(*mock_mpd_builder, ToString(_)) - .Times(2) - .WillRepeatedly(Return(true)); uint32_t container_id; SetMpdBuilder(¬ifier, mock_mpd_builder.PassAs());