MediaRecorder: allow ondataavailable with muted tracks.

The WebmMuxer buffers frames guaranteeing original capture timestamps
fed out in monotonically increasing order to the WebM file. This has
the consequence that the muxer buffers all incoming frames from the
non-muted track in the presence of a simultaneously recorded muted
track.

Relax this by adding API to the WebmMuxer to tell it about when
tracks become muted, and allow it to temporarily flush out samples
while the other end is muted.

Internally, libwebm's mkvmuxer will hold on to audio frames awaiting
video frame delivery. This change also adds API to WebmMuxer to control
how often data output is forced by forcing new WebM clusters. The
cluster enforcement frequency is capped to a max frequency of 10 Hz,
and is set from MediaRecorderHandler to be the value of the
MediaRecorder |timeslice| parameter.

Tested: manually using FB testpage at crbug/1145203.
Bug: 1145203
Change-Id: I6713d14f6e478d6b05c1fe84d5471f8eae759db1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2564638
Commit-Queue: Markus Handell <handellm@google.com>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#832060}

media/muxers/webm_muxer.cc

@@ -10,6 +10,8 @@
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/sequence_checker.h"
+#include "base/time/time.h"
+#include "base/time/time_override.h"
 #include "media/base/audio_parameters.h"
 #include "media/base/limits.h"
 #include "media/base/video_frame.h"
@@ -19,6 +21,10 @@ namespace media {
 
 namespace {
 
+// Force new clusters at a maximum rate of 10 Hz.
+constexpr base::TimeDelta kMinimumForcedClusterDuration =
+    base::TimeDelta::FromMilliseconds(100);
+
 void WriteOpusHeader(const media::AudioParameters& params, uint8_t* header) {
   // See https://wiki.xiph.org/OggOpus#ID_Header.
   // Set magic signature.
@@ -192,12 +198,17 @@ WebmMuxer::~WebmMuxer() {
   Flush();
 }
 
+void WebmMuxer::SetMaximumDurationToForceDataOutput(base::TimeDelta interval) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  max_data_output_interval_ = std::max(interval, kMinimumForcedClusterDuration);
+}
+
 bool WebmMuxer::OnEncodedVideo(const VideoParameters& params,
                                std::string encoded_data,
                                std::string encoded_alpha,
                                base::TimeTicks timestamp,
                                bool is_key_frame) {
-  DVLOG(1) << __func__ << " - " << encoded_data.size() << "B";
+  DVLOG(2) << __func__ << " - " << encoded_data.size() << "B";
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(params.codec == kCodecVP8 || params.codec == kCodecVP9 ||
          params.codec == kCodecH264)
@@ -244,6 +255,7 @@ bool WebmMuxer::OnEncodedAudio(const media::AudioParameters& params,
   DVLOG(2) << __func__ << " - " << encoded_data.size() << "B";
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
+  MaybeForceNewCluster();
   if (!audio_track_index_) {
     AddAudioTrack(params);
     if (first_frame_timestamp_audio_.is_null()) {
@@ -262,11 +274,22 @@ bool WebmMuxer::OnEncodedAudio(const media::AudioParameters& params,
   return PartiallyFlushQueues();
 }
 
+void WebmMuxer::SetLiveAndEnabled(bool track_live_and_enabled, bool is_video) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  bool& written_track_live_and_enabled =
+      is_video ? video_track_live_and_enabled_ : audio_track_live_and_enabled_;
+  if (written_track_live_and_enabled != track_live_and_enabled) {
+    DVLOG(1) << __func__ << (is_video ? " video " : " audio ")
+             << "track live-and-enabled changed to " << track_live_and_enabled;
+  }
+  written_track_live_and_enabled = track_live_and_enabled;
+}
+
 void WebmMuxer::Pause() {
   DVLOG(1) << __func__;
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   if (!elapsed_time_in_pause_)
-    elapsed_time_in_pause_.reset(new base::ElapsedTimer());
+    elapsed_time_in_pause_ = std::make_unique<base::ElapsedTimer>();
 }
 
 void WebmMuxer::Resume() {
@@ -377,7 +400,9 @@ void WebmMuxer::AddAudioTrack(const media::AudioParameters& params) {
 
 mkvmuxer::int32 WebmMuxer::Write(const void* buf, mkvmuxer::uint32 len) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  DVLOG(2) << __func__ << " len " << len;
   DCHECK(buf);
+  last_data_output_timestamp_ = base::TimeTicks::Now();
   write_data_callback_.Run(
       base::StringPiece(reinterpret_cast<const char*>(buf), len));
   position_ += len;
@@ -413,9 +438,23 @@ void WebmMuxer::FlushQueues() {
 
 bool WebmMuxer::PartiallyFlushQueues() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  // Punt writing until all tracks have been created.
+  if ((has_audio_ && !audio_track_index_) ||
+      (has_video_ && !video_track_index_)) {
+    return true;
+  }
+
   bool result = true;
-  while (!(has_video_ && video_frames_.empty()) &&
-         !(has_audio_ && audio_frames_.empty()) && result) {
+  // We strictly sort by timestamp unless a track is not live-and-enabled. In
+  // that case we relax this and allow drainage of the live-and-enabled leg.
+  while ((!has_video_ || !video_frames_.empty() ||
+          !video_track_live_and_enabled_) &&
+         (!has_audio_ || !audio_frames_.empty() ||
+          !audio_track_live_and_enabled_) &&
+         result) {
+    if (video_frames_.empty() && audio_frames_.empty())
+      return true;
     result = FlushNextFrame();
   }
   return result;
@@ -437,7 +476,20 @@ bool WebmMuxer::FlushNextFrame() {
 
   EncodedFrame frame = std::move(queue->front());
   queue->pop_front();
-  auto recorded_timestamp = frame.relative_timestamp.InMicroseconds() *
+  // The logic tracking live-and-enabled that temporarily relaxes the strict
+  // timestamp sorting allows for draining a track's queue completely in the
+  // presence of the other track being muted. When the muted track becomes
+  // live-and-enabled again the sorting recommences. However, tracks get encoded
+  // data before live-and-enabled transitions to true. This can lead to us
+  // emitting non-monotonic timestamps to the muxer, which results in an error
+  // return. Fix this by enforcing monotonicity by rewriting timestamps.
+  base::TimeDelta relative_timestamp = frame.relative_timestamp;
+  DLOG_IF(WARNING, relative_timestamp < last_timestamp_written_)
+      << "Enforced a monotonically increasing timestamp. Last written "
+      << last_timestamp_written_ << " new " << relative_timestamp;
+  relative_timestamp = std::max(relative_timestamp, last_timestamp_written_);
+  last_timestamp_written_ = relative_timestamp;
+  auto recorded_timestamp = relative_timestamp.InMicroseconds() *
                             base::Time::kNanosecondsPerMicrosecond;
 
   if (force_one_libwebm_error_) {
@@ -475,4 +527,15 @@ base::TimeTicks WebmMuxer::UpdateLastTimestampMonotonically(
   return *last_timestamp;
 }
 
+void WebmMuxer::MaybeForceNewCluster() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  if (has_video_ && !max_data_output_interval_.is_zero() &&
+      !last_data_output_timestamp_.is_null()) {
+    base::TimeTicks now = base::TimeTicks::Now();
+    if (now - last_data_output_timestamp_ >= max_data_output_interval_) {
+      segment_.ForceNewClusterOnNextFrame();
+    }
+  }
+}
+
 }  // namespace media

media/muxers/webm_muxer.h

@@ -53,7 +53,7 @@ class MEDIA_EXPORT WebmMuxer : public mkvmuxer::IMkvWriter {
   // Container for the parameters that muxer uses that is extracted from
   // media::VideoFrame.
   struct MEDIA_EXPORT VideoParameters {
-    VideoParameters(scoped_refptr<media::VideoFrame> frame);
+    explicit VideoParameters(scoped_refptr<media::VideoFrame> frame);
     VideoParameters(gfx::Size visible_rect_size,
                     double frame_rate,
                     VideoCodec codec,
@@ -73,6 +73,16 @@ class MEDIA_EXPORT WebmMuxer : public mkvmuxer::IMkvWriter {
             const WriteDataCB& write_data_callback);
   ~WebmMuxer() override;
 
+  // Sets the maximum duration interval to cause data output on
+  // |write_data_callback|, provided frames are delivered. The WebM muxer can
+  // hold on to audio frames almost indefinitely in the case video is recorded
+  // and video frames are temporarily not delivered. When this method is used, a
+  // new WebM cluster is forced when the next frame arrives |duration| after the
+  // last write.
+  // The maximum duration between forced clusters is internally limited to not
+  // go below 100 ms.
+  void SetMaximumDurationToForceDataOutput(base::TimeDelta interval);
+
   // Functions to add video and audio frames with |encoded_data.data()|
   // to WebM Segment. Either one returns true on success.
   // |encoded_alpha| represents the encode output of alpha channel when
@@ -86,6 +96,12 @@ class MEDIA_EXPORT WebmMuxer : public mkvmuxer::IMkvWriter {
                       std::string encoded_data,
                       base::TimeTicks timestamp);
 
+  // WebmMuxer may hold on to data. Make sure it gets out on the next frame.
+  void ForceDataOutputOnNextFrame();
+
+  // Call to handle mute and tracks getting disabled.
+  void SetLiveAndEnabled(bool track_live_and_enabled, bool is_video);
+
   void Pause();
   void Resume();
 
@@ -140,6 +156,10 @@ class MEDIA_EXPORT WebmMuxer : public mkvmuxer::IMkvWriter {
   base::TimeTicks UpdateLastTimestampMonotonically(
       base::TimeTicks timestamp,
       base::TimeTicks* last_timestamp);
+  // Forces data output from |segment_| on the next frame if recording video,
+  // and |min_data_output_interval_| was configured and has passed since the
+  // last received video frame.
+  void MaybeForceNewCluster();
 
   // Audio codec configured on construction. Video codec is taken from first
   // received frame.
@@ -166,6 +186,19 @@ class MEDIA_EXPORT WebmMuxer : public mkvmuxer::IMkvWriter {
   const bool has_video_;
   const bool has_audio_;
 
+  // Variables to track live and enabled state of audio and video.
+  bool video_track_live_and_enabled_ = true;
+  bool audio_track_live_and_enabled_ = true;
+
+  // Maximum interval between data output callbacks (given frames arriving)
+  base::TimeDelta max_data_output_interval_;
+
+  // Last time data was output from |segment_|.
+  base::TimeTicks last_data_output_timestamp_;
+
+  // Last timestamp written into the segment.
+  base::TimeDelta last_timestamp_written_;
+
   // Callback to dump written data as being called by libwebm.
   const WriteDataCB write_data_callback_;
 

media/muxers/webm_muxer_unittest.cc

@@ -65,16 +65,14 @@ class WebmMuxerTest : public TestWithParam<TestParams> {
             GetParam().num_video_tracks,
             GetParam().num_audio_tracks,
             base::BindRepeating(&WebmMuxerTest::WriteCallback,
-                                base::Unretained(this)))),
-        last_encoded_length_(0),
-        accumulated_position_(0) {
+                                base::Unretained(this)))) {
     EXPECT_EQ(webm_muxer_->Position(), 0);
     const mkvmuxer::int64 kRandomNewPosition = 333;
     EXPECT_EQ(webm_muxer_->Position(kRandomNewPosition), -1);
     EXPECT_FALSE(webm_muxer_->Seekable());
   }
 
-  MOCK_METHOD1(WriteCallback, void(base::StringPiece));
+  MOCK_METHOD(void, WriteCallback, (base::StringPiece));
 
   void SaveEncodedDataLen(const base::StringPiece& encoded_data) {
     last_encoded_length_ = encoded_data.size();
@@ -110,8 +108,8 @@ class WebmMuxerTest : public TestWithParam<TestParams> {
 
   std::unique_ptr<WebmMuxer> webm_muxer_;
 
-  size_t last_encoded_length_;
-  int64_t accumulated_position_;
+  size_t last_encoded_length_ = 0;
+  int64_t accumulated_position_ = 0;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(WebmMuxerTest);
@@ -504,6 +502,8 @@ class WebmMuxerTestUnparametrized : public testing::Test {
             base::TimeDelta::FromMilliseconds(system_timestamp_offset_ms));
   }
 
+  MOCK_METHOD(void, OnWrite, ());
+
   base::test::TaskEnvironment environment_;
   std::unique_ptr<WebmMuxer> webm_muxer_;
   std::map<int, std::vector<int>> buffer_timestamps_ms_;
@@ -534,6 +534,7 @@ class WebmMuxerTestUnparametrized : public testing::Test {
   static constexpr int kSentinelVideoBufferTimestampMs = 1000000;
 
   void SaveChunkAndInvokeWriteCallback(base::StringPiece chunk) {
+    OnWrite();
     std::copy(chunk.begin(), chunk.end(), std::back_inserter(muxed_data_));
   }
 
@@ -601,4 +602,51 @@ TEST_F(WebmMuxerTestUnparametrized,
                                    Pair(2, ElementsAre(0, 5))));
 }
 
+TEST_F(WebmMuxerTestUnparametrized, HoldsDataUntilDurationExpiry) {
+  webm_muxer_->SetMaximumDurationToForceDataOutput(
+      base::TimeDelta::FromMilliseconds(200));
+  AddVideoAtOffset(0, /*is_key_frame=*/true);
+  AddAudioAtOffsetWithDuration(0, 10);
+  // Mute video. The muxer will hold on to audio data after this until the max
+  // data output duration is expired.
+  webm_muxer_->SetLiveAndEnabled(/*track_live_and_enabled=*/false,
+                                 /*is_video=*/true);
+  EXPECT_CALL(*this, OnWrite).Times(0);
+  AddAudioAtOffsetWithDuration(10, 10);
+  AddAudioAtOffsetWithDuration(20, 10);
+  AddAudioAtOffsetWithDuration(30, 10);
+  AddAudioAtOffsetWithDuration(40, 10);
+  Mock::VerifyAndClearExpectations(this);
+  environment_.FastForwardBy(base::TimeDelta::FromMilliseconds(200));
+  EXPECT_CALL(*this, OnWrite).Times(AtLeast(1));
+  AddAudioAtOffsetWithDuration(50, 10);
+  Mock::VerifyAndClearExpectations(this);
+  // Stop mock dispatch from happening too late in the WebmMuxer's destructor.
+  webm_muxer_ = nullptr;
+}
+
+TEST_F(WebmMuxerTestUnparametrized, DurationExpiryLimitedByMaxFrequency) {
+  webm_muxer_->SetMaximumDurationToForceDataOutput(
+      base::TimeDelta::FromMilliseconds(
+          50));  // This value is below the minimum limit of 100 ms.
+  AddVideoAtOffset(0, /*is_key_frame=*/true);
+  AddAudioAtOffsetWithDuration(0, 10);
+  // Mute video. The muxer will hold on to audio data after this until the max
+  // data output duration is expired.
+  webm_muxer_->SetLiveAndEnabled(/*track_live_and_enabled=*/false,
+                                 /*is_video=*/true);
+  EXPECT_CALL(*this, OnWrite).Times(0);
+  AddAudioAtOffsetWithDuration(10, 10);
+  AddAudioAtOffsetWithDuration(20, 10);
+  AddAudioAtOffsetWithDuration(30, 10);
+  AddAudioAtOffsetWithDuration(40, 10);
+  Mock::VerifyAndClearExpectations(this);
+  environment_.FastForwardBy(base::TimeDelta::FromMilliseconds(100));
+  EXPECT_CALL(*this, OnWrite).Times(AtLeast(1));
+  AddAudioAtOffsetWithDuration(50, 10);
+  Mock::VerifyAndClearExpectations(this);
+  // Stop mock dispatch from happening too late in the WebmMuxer's destructor.
+  webm_muxer_ = nullptr;
+}
+
 }  // namespace media

third_party/blink/renderer/modules/mediarecorder/media_recorder_handler.cc

@@ -4,6 +4,7 @@
 
 #include "third_party/blink/renderer/modules/mediarecorder/media_recorder_handler.h"
 
+#include <memory>
 #include <utility>
 
 #include "base/logging.h"
@@ -24,6 +25,7 @@
 #include "third_party/blink/renderer/platform/media_capabilities/web_media_configuration.h"
 #include "third_party/blink/renderer/platform/mediastream/media_stream_component.h"
 #include "third_party/blink/renderer/platform/mediastream/media_stream_descriptor.h"
+#include "third_party/blink/renderer/platform/mediastream/media_stream_source.h"
 #include "third_party/blink/renderer/platform/mediastream/webrtc_uma_histograms.h"
 #include "third_party/blink/renderer/platform/wtf/functional.h"
 #include "third_party/blink/renderer/platform/wtf/text/string_builder.h"
@@ -280,12 +282,14 @@ bool MediaRecorderHandler::Start(int timeslice) {
     return false;
   }
 
-  webm_muxer_.reset(
-      new media::WebmMuxer(CodecIdToMediaAudioCodec(audio_codec_id_),
-                           use_video_tracks, use_audio_tracks,
-                           WTF::BindRepeating(&MediaRecorderHandler::WriteData,
-                                              WrapWeakPersistent(this))));
-
+  webm_muxer_ = std::make_unique<media::WebmMuxer>(
+      CodecIdToMediaAudioCodec(audio_codec_id_), use_video_tracks,
+      use_audio_tracks,
+      WTF::BindRepeating(&MediaRecorderHandler::WriteData,
+                         WrapWeakPersistent(this)));
+  if (timeslice > 0) {
+    webm_muxer_->SetMaximumDurationToForceDataOutput(timeslice_);
+  }
   if (use_video_tracks) {
     // TODO(mcasas): The muxer API supports only one video track. Extend it to
     // several video tracks, see http://crbug.com/528523.
@@ -294,6 +298,7 @@ bool MediaRecorderHandler::Start(int timeslice) {
         << "Only recording first video track.";
     if (!video_tracks_[0])
       return false;
+    UpdateTrackLiveAndEnabled(*video_tracks_[0], /*is_video=*/true);
 
     MediaStreamVideoTrack* const video_track =
         static_cast<MediaStreamVideoTrack*>(
@@ -332,6 +337,7 @@ bool MediaRecorderHandler::Start(int timeslice) {
         << " tracks is not implemented.  Only recording first audio track.";
     if (!audio_tracks_[0])
       return false;
+    UpdateTrackLiveAndEnabled(*audio_tracks_[0], /*is_video=*/false);
 
     const AudioTrackRecorder::OnEncodedAudioCB on_encoded_audio_cb =
         media::BindToCurrentLoop(WTF::BindRepeating(
@@ -652,23 +658,34 @@ bool MediaRecorderHandler::UpdateTracksAndCheckIfChanged() {
   if (audio_tracks_changed)
     audio_tracks_ = audio_tracks;
 
+  if (video_tracks_.size())
+    UpdateTrackLiveAndEnabled(*video_tracks_[0], /*is_video=*/true);
+  if (audio_tracks_.size())
+    UpdateTrackLiveAndEnabled(*audio_tracks_[0], /*is_video=*/false);
+
   return video_tracks_changed || audio_tracks_changed;
 }
 
+void MediaRecorderHandler::UpdateTrackLiveAndEnabled(
+    const MediaStreamComponent& track,
+    bool is_video) {
+  const bool track_live_and_enabled =
+      track.Source()->GetReadyState() == MediaStreamSource::kReadyStateLive &&
+      track.Enabled();
+  if (webm_muxer_)
+    webm_muxer_->SetLiveAndEnabled(track_live_and_enabled, is_video);
+}
+
 void MediaRecorderHandler::OnSourceReadyStateChanged() {
   for (const auto& track : video_tracks_) {
     DCHECK(track->Source());
-    if (track->Source()->GetReadyState() !=
-        MediaStreamSource::kReadyStateEnded) {
+    if (track->Source()->GetReadyState() != MediaStreamSource::kReadyStateEnded)
       return;
-    }
   }
   for (const auto& track : audio_tracks_) {
     DCHECK(track->Source());
-    if (track->Source()->GetReadyState() !=
-        MediaStreamSource::kReadyStateEnded) {
+    if (track->Source()->GetReadyState() != MediaStreamSource::kReadyStateEnded)
       return;
-    }
   }
   // All tracks are ended, so stop the recorder in accordance with
   // https://www.w3.org/TR/mediastream-recording/#mediarecorder-methods.

third_party/blink/renderer/modules/mediarecorder/media_recorder_handler.h

@@ -119,6 +119,8 @@ class MODULES_EXPORT MediaRecorderHandler final
   void OnAudioBusForTesting(const media::AudioBus& audio_bus,
                             const base::TimeTicks& timestamp);
   void SetAudioFormatForTesting(const media::AudioParameters& params);
+  void UpdateTrackLiveAndEnabled(const MediaStreamComponent& track,
+                                 bool is_video);
 
   // Set to true if there is no MIME type configured upon Initialize()
   // and the video track's source supports encoded output, giving