VideoTrackAdapter: Add passing frames monitor, notify MSVCS -> MSVTrack(s)

content/renderer/media/video_track_adapter.cc

Index: content/renderer/media/video_track_adapter.cc
diff --git a/content/renderer/media/video_track_adapter.cc b/content/renderer/media/video_track_adapter.cc
index e51f8427bef23e69f86cae7de12ae5677e2bd70b..9b860311fe9ffb3313e1e6e7006ad95e3638be20 100644
--- a/content/renderer/media/video_track_adapter.cc
+++ b/content/renderer/media/video_track_adapter.cc
@@ -17,6 +17,9 @@ namespace content {
 
 namespace {
 
+static const float kFirstFrameTimeoutInFrameIntervals = 100.0f;

[Henrik Grunell, 2014/07/08 11:39:53]

Remove static.
Comment on these.

[mcasas, 2014/07/08 16:12:01]

Done.

+static const float kNormalFrameTimeoutInFrameIntervals = 25.0f;
+
 // Empty method used for keeping a reference to the original media::VideoFrame
 // in VideoFrameResolutionAdapter::DeliverFrame if cropping is needed.
 // The reference to |frame| is kept in the closure that calls this method.
@@ -301,7 +304,9 @@ bool VideoTrackAdapter::VideoFrameResolutionAdapter::IsEmpty() const {
 VideoTrackAdapter::VideoTrackAdapter(
     const scoped_refptr<base::MessageLoopProxy>& io_message_loop)
     : io_message_loop_(io_message_loop),
-      renderer_task_runner_(base::MessageLoopProxy::current()) {
+      renderer_task_runner_(base::MessageLoopProxy::current()),
+      frame_counter_(0),
+      frame_rate_(0.0f) {
   DCHECK(io_message_loop_);
 }
 
@@ -309,19 +314,26 @@ VideoTrackAdapter::~VideoTrackAdapter() {
   DCHECK(adapters_.empty());
 }
 
-void VideoTrackAdapter::AddTrack(const MediaStreamVideoTrack* track,
-                                 VideoCaptureDeliverFrameCB frame_callback,
-                                 int max_width,
-                                 int max_height,
-                                 double min_aspect_ratio,
-                                 double max_aspect_ratio,
-                                 double max_frame_rate) {
+void VideoTrackAdapter::AddTrack(
+    const MediaStreamVideoTrack* track,
+    VideoCaptureDeliverFrameCB frame_callback,
+    int max_width,
+    int max_height,
+    double min_aspect_ratio,
+    double max_aspect_ratio,
+    double max_frame_rate,
+    double target_frame_rate,
+    const OnMutedCallback& set_muted_state_callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
   io_message_loop_->PostTask(
       FROM_HERE,
       base::Bind(&VideoTrackAdapter::AddTrackOnIO,
                  this, track, frame_callback, gfx::Size(max_width, max_height),
                  min_aspect_ratio, max_aspect_ratio, max_frame_rate));
+  io_message_loop_->PostTask(

[Henrik Grunell, 2014/07/08 11:39:53]

Could the monitoring possibly be racy initially? Should the monitoring task be
posted before adding the track?

[mcasas, 2014/07/08 16:12:01]

The posting could be racy with respect to the frame 
action initialisation, however that would just mark the
source as "muted", which is anyway what we'd like to see.
If the source starts producing frames, then eventually
|muted==false| and everything would be fine. QED, the
race would be handled fine in this case.

+      FROM_HERE,
+      base::Bind(&VideoTrackAdapter::StartTrackMonitoringOnIO,
+                 this, set_muted_state_callback, target_frame_rate));
 }
 
 void VideoTrackAdapter::AddTrackOnIO(
@@ -360,6 +372,21 @@ void VideoTrackAdapter::RemoveTrack(const MediaStreamVideoTrack* track) {
       base::Bind(&VideoTrackAdapter::RemoveTrackOnIO, this, track));
 }
 
+void VideoTrackAdapter::StartTrackMonitoringOnIO(
+    const OnMutedCallback& set_muted_state_callback,
+    double target_frame_rate) {
+  DCHECK(io_message_loop_->BelongsToCurrentThread());
+  frame_rate_ = target_frame_rate;

[Henrik Grunell, 2014/07/08 11:39:53]

Should frame_rate_ be called target_frame_rate_?

[mcasas, 2014/07/08 16:12:01]

Done.

+  DCHECK_NE(frame_rate_, 0.0f);

[Henrik Grunell, 2014/07/08 11:39:53]

Put this dcheck before setting |frame_rate_| and check |target_frame_rate|
instead.

[mcasas, 2014/07/08 16:12:01]

Done.

+  DVLOG(1) << "Monitoring frame creation, first (large) delay: "
+      << (kFirstFrameTimeoutInFrameIntervals / frame_rate_) << "s";
+  io_message_loop_->PostDelayedTask(FROM_HERE,
+       base::Bind(&VideoTrackAdapter::CheckFramesReceivedOnIO, this,
+                  set_muted_state_callback, frame_counter_),
+       base::TimeDelta::FromSecondsD(kFirstFrameTimeoutInFrameIntervals /
+                                     frame_rate_));
+}
+
 void VideoTrackAdapter::RemoveTrackOnIO(const MediaStreamVideoTrack* track) {
   DCHECK(io_message_loop_->BelongsToCurrentThread());
   for (FrameAdapters::iterator it = adapters_.begin();
@@ -378,10 +405,26 @@ void VideoTrackAdapter::DeliverFrameOnIO(
     const base::TimeTicks& estimated_capture_time) {
   DCHECK(io_message_loop_->BelongsToCurrentThread());
   TRACE_EVENT0("video", "VideoTrackAdapter::DeliverFrameOnIO");
+  ++frame_counter_;
   for (FrameAdapters::iterator it = adapters_.begin();
        it != adapters_.end(); ++it) {
     (*it)->DeliverFrame(frame, format, estimated_capture_time);
   }
 }
 
+void VideoTrackAdapter::CheckFramesReceivedOnIO(
+    const OnMutedCallback& set_muted_state_callback,
+    uint64 frame_counter_snapshot) {

[Henrik Grunell, 2014/07/08 11:39:53]

I think the "snapshot" part of the name is somewhat unclear. A comment on this
function in the header file explaining the (trivial) algorithm would be good.
(And maybe a change of the name.)

[mcasas, 2014/07/08 16:12:01]

Done.

+  DCHECK(io_message_loop_->BelongsToCurrentThread());
+  DVLOG_IF(1, frame_counter_snapshot == frame_counter_)
+      << "No frames have passed, setting source as Muted.";
+  set_muted_state_callback.Run(frame_counter_snapshot == frame_counter_);
+
+  io_message_loop_->PostDelayedTask(FROM_HERE,
+      base::Bind(&VideoTrackAdapter::CheckFramesReceivedOnIO, this,
+          set_muted_state_callback, frame_counter_),
+      base::TimeDelta::FromSecondsD(kNormalFrameTimeoutInFrameIntervals /
+                                    frame_rate_));
+}
+
 }  // namespace content