[cast_streaming] End the session when both streams send no data for 15s

Previously, the Cast Streaming Session would be ended after either the
audio or video stream received no data for 15s. However, some Cast
Streaming Senders sometimes do not send video data for longer than
that when there is no video update to send, resulting in the session
being prematurely ended.

This changes the Cast Streaming Receiver behavior to only end the
session if no audio and no video data is received for 15 seconds. As
a result, the frame duration has been changed to 10 minutes to prevent
a demuxer underflow in the Chrome media pipeline, resulting in delayed
playback.

Bug: 1166371, b/172139177
Change-Id: I2f66ca009bdedb00ae79ac8e9dda8cc2ebcd16af
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2617363
Commit-Queue: Fabrice de Gans-Riberi <fdegans@chromium.org>
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843227}

fuchsia/cast_streaming/cast_streaming_session.cc

@@ -18,6 +18,9 @@
 
 namespace {
 
+// Timeout to stop the Session when no data is received.
+constexpr base::TimeDelta kNoDataTimeout = base::TimeDelta::FromSeconds(15);
+
 bool CreateDataPipeForStreamType(media::DemuxerStream::Type type,
                                  mojo::ScopedDataPipeProducerHandle* producer,
                                  mojo::ScopedDataPipeConsumerHandle* consumer) {
@@ -107,8 +110,8 @@ class CastStreamingSession::Internal
         base::BindRepeating(
             &CastStreamingSession::Client::OnAudioBufferReceived,
             base::Unretained(client_)),
-        base::BindOnce(&CastStreamingSession::Internal::OnDataTimeout,
-                       base::Unretained(this)));
+        base::BindRepeating(&base::OneShotTimer::Reset,
+                            base::Unretained(&data_timeout_timer_)));
 
     return AudioStreamInfo{
         AudioCaptureConfigToAudioDecoderConfig(audio_capture_config),
@@ -133,13 +136,15 @@ class CastStreamingSession::Internal
 
     // We can use unretained pointers here because StreamConsumer is owned by
     // this object and |client_| is guaranteed to outlive this object.
+    // |data_timeout_timer_| is also owned by this object and will outlive both
+    // StreamConsumers.
     video_consumer_ = std::make_unique<StreamConsumer>(
         video_receiver, std::move(data_pipe_producer),
         base::BindRepeating(
             &CastStreamingSession::Client::OnVideoBufferReceived,
             base::Unretained(client_)),
-        base::BindOnce(&CastStreamingSession::Internal::OnDataTimeout,
-                       base::Unretained(this)));
+        base::BindRepeating(&base::OneShotTimer::Reset,
+                            base::Unretained(&data_timeout_timer_)));
 
     return VideoStreamInfo{
         VideoCaptureConfigToVideoDecoderConfig(video_capture_config),
@@ -218,6 +223,10 @@ class CastStreamingSession::Internal
     } else {
       client_->OnSessionInitialization(std::move(audio_stream_info),
                                        std::move(video_stream_info));
+      data_timeout_timer_.Start(
+          FROM_HERE, kNoDataTimeout,
+          base::BindOnce(&CastStreamingSession::Internal::OnDataTimeout,
+                         base::Unretained(this)));
     }
   }
 
@@ -264,6 +273,9 @@ class CastStreamingSession::Internal
   std::unique_ptr<openscreen::cast::ReceiverSession> receiver_session_;
   base::OneShotTimer init_timeout_timer_;
 
+  // Timer to trigger connection closure if no data is received for 15 seconds.
+  base::OneShotTimer data_timeout_timer_;
+
   bool is_initialized_ = false;
   CastStreamingSession::Client* const client_;
   std::unique_ptr<StreamConsumer> audio_consumer_;

fuchsia/cast_streaming/stream_consumer.cc

@@ -10,30 +10,17 @@
 
 namespace cast_streaming {
 
-namespace {
-
-// Timeout to stop the Session when no data is received. This is also used for
-// the frames duration because Senders may not send a new frame for ~10s.
-// When that happens, the Chromium media pipeline may end up deciding it does
-// not have enough data, resulting in the stream being stalled. Setting the
-// frame duration to this value prevents the media pipeline from considering the
-// stream as being stalled. As a result, we end up with overlapping frames but
-// this is fine since the media pipeline mostly considers the playout time when
-// deciding which frame to present or play.
-constexpr base::TimeDelta kNoDataTimeout = base::TimeDelta::FromSeconds(15);
-
-}  // namespace
-
 StreamConsumer::StreamConsumer(openscreen::cast::Receiver* receiver,
                                mojo::ScopedDataPipeProducerHandle data_pipe,
                                FrameReceivedCB frame_received_cb,
-                               base::OnceClosure on_timeout)
+                               base::RepeatingClosure on_new_frame)
     : receiver_(receiver),
       data_pipe_(std::move(data_pipe)),
       frame_received_cb_(std::move(frame_received_cb)),
       pipe_watcher_(FROM_HERE,
                     mojo::SimpleWatcher::ArmingPolicy::MANUAL,
-                    base::SequencedTaskRunnerHandle::Get()) {
+                    base::SequencedTaskRunnerHandle::Get()),
+      on_new_frame_(std::move(on_new_frame)) {
   DCHECK(receiver_);
   receiver_->SetConsumer(this);
   MojoResult result =
@@ -44,8 +31,6 @@ StreamConsumer::StreamConsumer(openscreen::cast::Receiver* receiver,
     CloseDataPipeOnError();
     return;
   }
-
-  data_timeout_timer_.Start(FROM_HERE, kNoDataTimeout, std::move(on_timeout));
 }
 
 StreamConsumer::~StreamConsumer() {
@@ -57,7 +42,6 @@ void StreamConsumer::CloseDataPipeOnError() {
   receiver_->SetConsumer(nullptr);
   pipe_watcher_.Cancel();
   data_pipe_.reset();
-  data_timeout_timer_.Stop();
 }
 
 void StreamConsumer::OnPipeWritable(MojoResult result) {
@@ -91,7 +75,7 @@ void StreamConsumer::OnPipeWritable(MojoResult result) {
 
 void StreamConsumer::OnFramesReady(int next_frame_buffer_size) {
   DCHECK(data_pipe_);
-  data_timeout_timer_.Reset();
+  on_new_frame_.Run();
 
   if (pending_buffer_remaining_bytes_ != 0) {
     // There already is a pending frame. Ignore this one for now.
@@ -166,10 +150,16 @@ void StreamConsumer::OnFramesReady(int next_frame_buffer_size) {
            << "Received new frame. Timestamp: " << playout_time
            << ", is_key_frame: " << is_key_frame;
 
-  // See the |kNoDataTimeout| definition for why it is used for the frame
-  // duration here.
+  // Senders may not send a new video frame for a very long time if there is no
+  // update to send. When that happens, the Chromium media pipeline may end up
+  // deciding it does not have enough data, resulting in the stream being
+  // stalled. Setting the frame duration to 10 minutes prevents the media
+  // pipeline from considering the stream as being stalled. As a result, we end
+  // up with overlapping frames but this is fine since the media pipeline mostly
+  // considers the playout time when deciding which frame to present or play.
   frame_received_cb_.Run(media::mojom::DecoderBuffer::New(
-      playout_time /* timestamp */, kNoDataTimeout /* duration */,
+      playout_time /* timestamp */,
+      base::TimeDelta::FromMinutes(10) /* duration */,
       false /* is_end_of_stream */, buffer_size, is_key_frame,
       media::EmptyExtraData(), media::mojom::DecryptConfigPtr(),
       base::TimeDelta() /* front_discard */,

fuchsia/cast_streaming/stream_consumer.h

@@ -6,7 +6,6 @@
 #define FUCHSIA_CAST_STREAMING_STREAM_CONSUMER_H_
 
 #include "base/callback.h"
-#include "base/timer/timer.h"
 #include "media/mojo/mojom/media_types.mojom.h"
 #include "mojo/public/cpp/system/data_pipe.h"
 #include "mojo/public/cpp/system/simple_watcher.h"
@@ -32,11 +31,11 @@ class StreamConsumer : public openscreen::cast::Receiver::Consumer {
   // |receiver| sends frames to this object. It must outlive this object.
   // |frame_received_cb| is called on every new frame, after a new frame has
   // been written to |data_pipe|. On error, |data_pipe| will be closed.
-  // If no data is received for 10 seconds, |on_timeout| will be closed.
+  // On every new frame, |on_new_frame| will be called.
   StreamConsumer(openscreen::cast::Receiver* receiver,
                  mojo::ScopedDataPipeProducerHandle data_pipe,
                  FrameReceivedCB frame_received_cb,
-                 base::OnceClosure on_timeout);
+                 base::RepeatingClosure on_new_frame);
   ~StreamConsumer() final;
 
   StreamConsumer(const StreamConsumer&) = delete;
@@ -72,8 +71,8 @@ class StreamConsumer : public openscreen::cast::Receiver::Consumer {
   // Remaining bytes to write from |pending_buffer_| to |data_pipe_|.
   size_t pending_buffer_remaining_bytes_ = 0;
 
-  // Timer to trigger connection closure if no data is received for 10 seconds.
-  base::OneShotTimer data_timeout_timer_;
+  // Closure called on every new frame.
+  base::RepeatingClosure on_new_frame_;
 };
 
 }  // namespace cast_streaming