Rework PerformanceTracker to make it usable with WebRTC

remoting/client/plugin/pepper_video_renderer_3d.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "remoting/client/plugin/pepper_video_renderer_3d.h"
 
 #include <math.h>
 
+#include <algorithm>
 #include <utility>
 
-#include "base/callback_helpers.h"
-#include "base/stl_util.h"
 #include "ppapi/c/pp_codecs.h"
 #include "ppapi/c/ppb_opengles2.h"
 #include "ppapi/c/ppb_video_decoder.h"
 #include "ppapi/cpp/instance.h"
 #include "ppapi/lib/gl/include/GLES2/gl2.h"
 #include "ppapi/lib/gl/include/GLES2/gl2ext.h"
 #include "remoting/proto/video.pb.h"
+#include "remoting/protocol/frame_stats.h"
 #include "remoting/protocol/performance_tracker.h"
 #include "remoting/protocol/session_config.h"
 #include "third_party/webrtc/modules/desktop_capture/desktop_region.h"
 
 namespace remoting {
 
 // The implementation here requires that the decoder allocates at least 3
 // pictures. PPB_VideoDecoder didn't support this parameter prior to
 // 1.1, so we have to pass 0 for backwards compatibility with older versions of
 // the browser. Currently all API implementations allocate more than 3 buffers
 // by default.
-//
-// TODO(sergeyu): Change this to 3 once PPB_VideoDecoder v1.1 is enabled on
-// stable channel (crbug.com/520323).
-const uint32_t kMinimumPictureCount = 0;  // 3
+const uint32_t kMinimumPictureCount = 3;
 
-class PepperVideoRenderer3D::PendingPacket {
+class PepperVideoRenderer3D::FrameTracker {
  public:
-  PendingPacket(std::unique_ptr<VideoPacket> packet, const base::Closure& done)
-      : packet_(std::move(packet)), done_runner_(done) {}
+  FrameTracker(std::unique_ptr<VideoPacket> packet,
+               protocol::PerformanceTracker* perf_tracker,
+               const base::Closure& done)
+      : packet_(std::move(packet)), perf_tracker_(perf_tracker), done_(done) {
+    stats_ = protocol::FrameStats::GetForVideoPacket(*packet_);
+  }
 
-  ~PendingPacket() {}
+  ~FrameTracker() {
+    stats_.time_rendered = base::TimeTicks::Now();

[Jamie, 2016/06/24 23:53:21]

Unless you feel strongly otherwise, I think this would be better in an explicit
OnRendered method. This sort of thing is okay in a dtor if the class is defined
for use within a scope, but it seems a bit hacky here.

Failing that, at least add a comment when the list is emptied so it's clear that
something non-trivial is happening as a result.

[Sergey Ulanov, 2016/06/25 05:34:03]

Done.

+    if (perf_tracker_)
+      perf_tracker_->RecordVideoFrameStats(stats_);
+    if (!done_.is_null())
+      done_.Run();
+  }
 
-  const VideoPacket* packet() const { return packet_.get(); }
+  void OnDecoded() { stats_.time_decoded = base::TimeTicks::Now(); }
+  VideoPacket* packet() { return packet_.get(); }
 
  private:
   std::unique_ptr<VideoPacket> packet_;
-  base::ScopedClosureRunner done_runner_;
+  protocol::PerformanceTracker* perf_tracker_;
+  protocol::FrameStats stats_;
+  base::Closure done_;
 };
 
-
 class PepperVideoRenderer3D::Picture {
  public:
   Picture(pp::VideoDecoder* decoder, PP_VideoPicture picture)
       : decoder_(decoder), picture_(picture) {}
   ~Picture() { decoder_->RecyclePicture(picture_); }
 
   const PP_VideoPicture& picture() { return picture_; }
 
  private:
   pp::VideoDecoder* decoder_;
   PP_VideoPicture picture_;
 };
 
 PepperVideoRenderer3D::PepperVideoRenderer3D() : callback_factory_(this) {}
 
 PepperVideoRenderer3D::~PepperVideoRenderer3D() {
   if (shader_program_)
     gles2_if_->DeleteProgram(graphics_.pp_resource(), shader_program_);
-
-  STLDeleteElements(&pending_packets_);
 }
 
 bool PepperVideoRenderer3D::Initialize(
     pp::Instance* instance,
     const ClientContext& context,
     EventHandler* event_handler,
     protocol::PerformanceTracker* perf_tracker) {
   DCHECK(event_handler);
   DCHECK(!event_handler_);
 
@@ skipping 101 matching lines @@
 protocol::FrameConsumer* PepperVideoRenderer3D::GetFrameConsumer() {
   // GetFrameConsumer() is used only for WebRTC-based connections which are not
   // supported by the plugin.
   NOTREACHED();
   return nullptr;
 }
 
 void PepperVideoRenderer3D::ProcessVideoPacket(
     std::unique_ptr<VideoPacket> packet,
     const base::Closure& done) {
-  base::ScopedClosureRunner done_runner(done);
-
-  perf_tracker_->RecordVideoPacketStats(*packet);
+  VideoPacket* packet_ptr = packet.get();
+  std::unique_ptr<FrameTracker> frame_tracker(
+      new FrameTracker(std::move(packet), perf_tracker_, done));
 
   // Don't need to do anything if the packet is empty. Host sends empty video
   // packets when the screen is not changing.
-  if (!packet->data().size())
+  if (!packet_ptr->data().size())
     return;
 
   if (!frame_received_) {
     event_handler_->OnVideoFirstFrameReceived();
     frame_received_ = true;
   }
 
-  if (packet->format().has_screen_width() &&
-      packet->format().has_screen_height()) {
-    frame_size_.set(packet->format().screen_width(),
-                    packet->format().screen_height());
+  if (packet_ptr->format().has_screen_width() &&
+      packet_ptr->format().has_screen_height()) {
+    frame_size_.set(packet_ptr->format().screen_width(),
+                    packet_ptr->format().screen_height());
   }
 
   // Report the dirty region, for debugging, if requested.
   if (debug_dirty_region_) {
     webrtc::DesktopRegion dirty_region;
-    for (int i = 0; i < packet->dirty_rects_size(); ++i) {
-      Rect remoting_rect = packet->dirty_rects(i);
+    for (int i = 0; i < packet_ptr->dirty_rects_size(); ++i) {
+      Rect remoting_rect = packet_ptr->dirty_rects(i);
       dirty_region.AddRect(webrtc::DesktopRect::MakeXYWH(
-          remoting_rect.x(), remoting_rect.y(),
-          remoting_rect.width(), remoting_rect.height()));
+          remoting_rect.x(), remoting_rect.y(), remoting_rect.width(),
+          remoting_rect.height()));
     }
     event_handler_->OnVideoFrameDirtyRegion(dirty_region);
   }
 
-  pending_packets_.push_back(
-      new PendingPacket(std::move(packet), done_runner.Release()));
+  pending_frames_.push_back(std::move(frame_tracker));
   DecodeNextPacket();
 }
 
 void PepperVideoRenderer3D::OnInitialized(int32_t result) {
   // Assume that VP8 and VP9 codecs are always supported by the browser.
   CHECK_EQ(result, PP_OK) << "VideoDecoder::Initialize() failed: " << result;
   initialization_finished_ = true;
 
   // Start decoding in case a frame was received during decoder initialization.
   DecodeNextPacket();
 }
 
 void PepperVideoRenderer3D::DecodeNextPacket() {
-  if (!initialization_finished_ || decode_pending_ || pending_packets_.empty())
+  if (!initialization_finished_ || decode_pending_ || pending_frames_.empty())
     return;
 
-  const VideoPacket* packet = pending_packets_.front()->packet();
+  const VideoPacket* packet = pending_frames_.front()->packet();
 
   int32_t result = video_decoder_.Decode(
       packet->frame_id(), packet->data().size(), packet->data().data(),
       callback_factory_.NewCallback(&PepperVideoRenderer3D::OnDecodeDone));
   CHECK_EQ(result, PP_OK_COMPLETIONPENDING);
   decode_pending_ = true;
 }
 
 void PepperVideoRenderer3D::OnDecodeDone(int32_t result) {
   DCHECK(decode_pending_);
   decode_pending_ = false;
 
   if (result != PP_OK) {
     LOG(ERROR) << "VideoDecoder::Decode() returned " << result;
     event_handler_->OnVideoDecodeError();
     return;
   }
 
-  delete pending_packets_.front();
-  pending_packets_.pop_front();
+  pending_frames_.front()->OnDecoded();
+  // Move the frame from |pending_frames_| to |decoded_frames_|.
+  decoded_frames_.splice(decoded_frames_.end(), pending_frames_,
+                         pending_frames_.begin());
 
   DecodeNextPacket();
   GetNextPicture();
 }
 
 void PepperVideoRenderer3D::GetNextPicture() {
   if (get_picture_pending_)
     return;
 
   int32_t result =
       video_decoder_.GetPicture(callback_factory_.NewCallbackWithOutput(
           &PepperVideoRenderer3D::OnPictureReady));
   CHECK_EQ(result, PP_OK_COMPLETIONPENDING);
   get_picture_pending_ = true;
 }
 
 void PepperVideoRenderer3D::OnPictureReady(int32_t result,
                                            PP_VideoPicture picture) {
   DCHECK(get_picture_pending_);
   get_picture_pending_ = false;
 
   if (result != PP_OK) {
     LOG(ERROR) << "VideoDecoder::GetPicture() returned " << result;
     event_handler_->OnVideoDecodeError();
     return;
   }
 
-  perf_tracker_->OnFrameDecoded(picture.decode_id);
-
   // Workaround crbug.com/542945 by filling in visible_rect if it isn't set.
   if (picture.visible_rect.size.width == 0 ||
       picture.visible_rect.size.height == 0) {
     static bool warning_logged = false;
     if (!warning_logged) {
       LOG(WARNING) << "PPB_VideoDecoder doesn't set visible_rect.";
       warning_logged = true;
     }
 
     picture.visible_rect.size.width =
         std::min(frame_size_.width(), picture.texture_size.width);
     picture.visible_rect.size.height =
         std::min(frame_size_.height(), picture.texture_size.height);
   }
 
+  DCHECK_EQ(static_cast<int32_t>(picture.decode_id),
+            decoded_frames_.front()->packet()->frame_id());
+
+  // Move the frame from |decoded_frames_| to |next_picture_frames_|.
+  next_picture_frames_.splice(next_picture_frames_.end(), decoded_frames_,
+                              decoded_frames_.begin());
+
   next_picture_.reset(new Picture(&video_decoder_, picture));
 
   PaintIfNeeded();
   GetNextPicture();
 }
 
 void PepperVideoRenderer3D::PaintIfNeeded() {
   bool need_repaint = next_picture_ || (force_repaint_ && current_picture_);
   if (paint_pending_ || !need_repaint)
     return;
 
-  if (next_picture_)
+  if (next_picture_) {
     current_picture_ = std::move(next_picture_);
+    current_picture_frames_.splice(current_picture_frames_.end(),
+                                   next_picture_frames_);
+  }
 
   force_repaint_ = false;
 
   const PP_VideoPicture& picture = current_picture_->picture();
   PP_Resource graphics_3d = graphics_.pp_resource();
 
   EnsureProgramForTexture(picture.texture_target);
 
   gles2_if_->UseProgram(graphics_3d, shader_program_);
 
@@ skipping 45 matching lines @@
   int32_t result = graphics_.SwapBuffers(
       callback_factory_.NewCallback(&PepperVideoRenderer3D::OnPaintDone));
   CHECK_EQ(result, PP_OK_COMPLETIONPENDING);
   paint_pending_ = true;
 }
 
 void PepperVideoRenderer3D::OnPaintDone(int32_t result) {
   CHECK_EQ(result, PP_OK) << "Graphics3D::SwapBuffers() failed";
 
   paint_pending_ = false;
-  perf_tracker_->OnFramePainted(current_picture_->picture().decode_id);
+  current_picture_frames_.clear();
   PaintIfNeeded();
 }
 
 void PepperVideoRenderer3D::EnsureProgramForTexture(uint32_t texture_target) {
   static const char kVertexShader[] =
       "varying vec2 v_texCoord;            \n"
       "attribute vec4 a_position;          \n"
       "attribute vec2 a_texCoord;          \n"
       "uniform vec2 v_scale;               \n"
       "void main()                         \n"
@@ skipping 97 matching lines @@
   gles2_if_->AttachShader(graphics_.pp_resource(), shader_program_, shader);
   gles2_if_->DeleteShader(graphics_.pp_resource(), shader);
 }
 
 void PepperVideoRenderer3D::CheckGLError() {
   GLenum error = gles2_if_->GetError(graphics_.pp_resource());
   CHECK_EQ(error, static_cast<GLenum>(GL_NO_ERROR)) << "GL error: " << error;
 }
 
 }  // namespace remoting