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

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);

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());
 }
 

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,

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:

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)
+  // This is for the Flush() below but adding expectation here because the next
-    EXPECT_CALL(*mock_mpd_builder, ToString(_)).WillOnce(Return(true));
+  // lines Pass() the pointer.
+  EXPECT_CALL(*mock_mpd_builder, ToString(_)).WillOnce(Return(true));
 
   uint32_t unused_container_id;
   SetMpdBuilder(&notifier, mock_mpd_builder.PassAs<MpdBuilder>());
   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(&notifier, mock_mpd_builder.PassAs<MpdBuilder>());
   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(&notifier, mock_mpd_builder.PassAs<MpdBuilder>());
@@ -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(&notifier, mock_mpd_builder.PassAs<MpdBuilder>());
   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(&notifier, mock_mpd_builder.PassAs<MpdBuilder>());
   EXPECT_TRUE(notifier.NotifyNewContainer(

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_; }
 

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());
 }
 

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:

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)
+  // This is for the Flush() below but adding expectation here because the next
-    EXPECT_CALL(*mock_mpd_builder, ToString(_)).WillOnce(Return(true));
+  // lines Pass() the pointer.
+  EXPECT_CALL(*mock_mpd_builder, ToString(_)).WillOnce(Return(true));
 
   uint32_t unused_container_id;
   SetMpdBuilder(&notifier, mock_mpd_builder.PassAs<MpdBuilder>());
   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(&notifier, mock_mpd_builder.PassAs<MpdBuilder>());
@@ -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(&notifier, mock_mpd_builder.PassAs<MpdBuilder>());
@@ -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(&notifier, mock_mpd_builder.PassAs<MpdBuilder>());
   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(&notifier, mock_mpd_builder.PassAs<MpdBuilder>());