Skip to content

Commit

Permalink
Rework PerformanceTracker to make it usable with WebRTC
Browse files Browse the repository at this point in the history 
Previously PerformanceTracker was expected to be notified at the
moment when each frame is decoded and rendered. It also had
assumption that latest_event_timestamp is known for each frame
before that frame is rendered. That doesn't work with WebRTC
where frame stats will have to be sent separately from each
frame. With this change PerformanceTracker gets FrameStats
struct for each frame. That struct contains all timestamps for
the frame, which allows to log all relevant stats.

BUG=621691

Review-Url: https://codereview.chromium.org/2096643003
Cr-Commit-Position: refs/heads/master@{#402054}
  • Loading branch information
@SergeyUlanov
SergeyUlanov authored and Commit bot committed Jun 25, 2016
1 parent 5453dc6 commit 2ec2751
Show file tree
Hide file tree
Showing 11 changed files with 305 additions and 217 deletions.
96 changes: 59 additions & 37 deletions remoting/client/plugin/pepper_video_renderer_3d.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,17 @@

#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"
Expand All @@ -28,26 +28,36 @@ namespace remoting {
// 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_);
}

~FrameTracker() {
if (perf_tracker_)
perf_tracker_->RecordVideoFrameStats(stats_);
if (!done_.is_null())
done_.Run();
}

~PendingPacket() {}
void OnDecoded() { stats_.time_decoded = base::TimeTicks::Now(); }
void OnRendered() { stats_.time_rendered = base::TimeTicks::Now(); }

const VideoPacket* packet() const { return packet_.get(); }
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)
Expand All @@ -66,8 +76,6 @@ PepperVideoRenderer3D::PepperVideoRenderer3D() : callback_factory_(this) {}
PepperVideoRenderer3D::~PepperVideoRenderer3D() {
if (shader_program_)
gles2_if_->DeleteProgram(graphics_.pp_resource(), shader_program_);

STLDeleteElements(&pending_packets_);
}

bool PepperVideoRenderer3D::Initialize(
Expand Down Expand Up @@ -189,40 +197,39 @@ protocol::FrameConsumer* PepperVideoRenderer3D::GetFrameConsumer() {
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();
}

Expand All @@ -236,10 +243,10 @@ void PepperVideoRenderer3D::OnInitialized(int32_t result) {
}

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(),
Expand All @@ -258,8 +265,10 @@ void PepperVideoRenderer3D::OnDecodeDone(int32_t result) {
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();
Expand Down Expand Up @@ -287,8 +296,6 @@ void PepperVideoRenderer3D::OnPictureReady(int32_t result,
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) {
Expand All @@ -304,6 +311,13 @@ void PepperVideoRenderer3D::OnPictureReady(int32_t result,
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();
Expand All @@ -315,8 +329,11 @@ void PepperVideoRenderer3D::PaintIfNeeded() {
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;

Expand Down Expand Up @@ -382,7 +399,12 @@ 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);

for (const auto& tracker : current_picture_frames_) {
tracker->OnRendered();
}
current_picture_frames_.clear();

PaintIfNeeded();
}

Expand Down
30 changes: 21 additions & 9 deletions remoting/client/plugin/pepper_video_renderer_3d.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

#include <stdint.h>

#include <deque>
#include <list>
#include <memory>
#include <string>

Expand Down Expand Up @@ -51,13 +51,15 @@ class PepperVideoRenderer3D : public PepperVideoRenderer,
const base::Closure& done) override;

private:
class PendingPacket;
// Class responsible for tracking state of a frame until it's rendered.
class FrameTracker;

class Picture;

// Callback for pp::VideoDecoder::Initialize().
void OnInitialized(int32_t result);

// Passes one picture from |pending_packets_| to the |video_decoder_|.
// Passes one picture from |pending_frames_| to the |video_decoder_|.
void DecodeNextPacket();

// Callback for pp::VideoDecoder::Decode().
Expand Down Expand Up @@ -105,19 +107,29 @@ class PepperVideoRenderer3D : public PepperVideoRenderer,
bool get_picture_pending_ = false;
bool paint_pending_ = false;

// Queue of packets that that have been received, but haven't been passed to
// the decoder yet.
std::deque<PendingPacket*> pending_packets_;
// Frames that have been received, but haven't been passed to the decoder yet.
std::list<std::unique_ptr<FrameTracker>> pending_frames_;

// The current picture shown on the screen or being rendered. Must be deleted
// before |video_decoder_|.
std::unique_ptr<Picture> current_picture_;
// Frames that have been decoded but for which we haven't received the
// pictures yet.
std::list<std::unique_ptr<FrameTracker>> decoded_frames_;

// The next picture to be rendered. PaintIfNeeded() will copy it to
// |current_picture_| and render it after that. Must be deleted
// before |video_decoder_|.
std::unique_ptr<Picture> next_picture_;

// FrameTracker instances in |next_picture_|.
std::list<std::unique_ptr<FrameTracker>> next_picture_frames_;

// The current picture shown on the screen or being rendered. Must be deleted
// before |video_decoder_|.
std::unique_ptr<Picture> current_picture_;

// FrameTrackers for frames in |current_picture_|. The queue is emptied once
// the |current_picture_| is rendered.
std::list<std::unique_ptr<FrameTracker>> current_picture_frames_;

// Set to true if the screen has been resized and needs to be repainted.
bool force_repaint_ = false;

Expand Down
37 changes: 22 additions & 15 deletions remoting/client/software_video_renderer.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
#include "remoting/codec/video_decoder_vpx.h"
#include "remoting/proto/video.pb.h"
#include "remoting/protocol/frame_consumer.h"
#include "remoting/protocol/frame_stats.h"
#include "remoting/protocol/performance_tracker.h"
#include "remoting/protocol/session_config.h"
#include "third_party/libyuv/include/libyuv/convert_argb.h"
#include "third_party/webrtc/modules/desktop_capture/desktop_frame.h"
Expand Down Expand Up @@ -94,12 +96,15 @@ void SoftwareVideoRenderer::ProcessVideoPacket(

base::ScopedClosureRunner done_runner(done);

if (perf_tracker_)
perf_tracker_->RecordVideoPacketStats(*packet);
std::unique_ptr<protocol::FrameStats> frame_stats(new protocol::FrameStats(
protocol::FrameStats::GetForVideoPacket(*packet)));

// If the video packet is empty then drop it. Empty packets are used to
// maintain activity on the network.
// If the video packet is empty then there is nothing to decode. Empty packets
// are used to maintain activity on the network. Stats for such packets still
// need to be reported.
if (!packet->has_data() || packet->data().size() == 0) {
if (perf_tracker_)
perf_tracker_->RecordVideoFrameStats(*frame_stats);
return;
}

Expand All @@ -126,41 +131,43 @@ void SoftwareVideoRenderer::ProcessVideoPacket(
consumer_->AllocateFrame(source_size_);
frame->set_dpi(source_dpi_);

int32_t frame_id = packet->frame_id();
base::PostTaskAndReplyWithResult(
decode_task_runner_.get(), FROM_HERE,
base::Bind(&DoDecodeFrame, decoder_.get(), base::Passed(&packet),
base::Passed(&frame)),
base::Bind(&SoftwareVideoRenderer::RenderFrame,
weak_factory_.GetWeakPtr(), frame_id, done_runner.Release()));
weak_factory_.GetWeakPtr(), base::Passed(&frame_stats),
done_runner.Release()));
}

void SoftwareVideoRenderer::RenderFrame(
int32_t frame_id,
std::unique_ptr<protocol::FrameStats> stats,
const base::Closure& done,
std::unique_ptr<webrtc::DesktopFrame> frame) {
DCHECK(thread_checker_.CalledOnValidThread());

if (perf_tracker_)
perf_tracker_->OnFrameDecoded(frame_id);
stats->time_decoded = base::TimeTicks::Now();

if (!frame) {
if (!done.is_null())
done.Run();
return;
}

consumer_->DrawFrame(std::move(frame),
base::Bind(&SoftwareVideoRenderer::OnFrameRendered,
weak_factory_.GetWeakPtr(), frame_id, done));
consumer_->DrawFrame(
std::move(frame),
base::Bind(&SoftwareVideoRenderer::OnFrameRendered,
weak_factory_.GetWeakPtr(), base::Passed(&stats), done));
}

void SoftwareVideoRenderer::OnFrameRendered(int32_t frame_id,
const base::Closure& done) {
void SoftwareVideoRenderer::OnFrameRendered(
std::unique_ptr<protocol::FrameStats> stats,
const base::Closure& done) {
DCHECK(thread_checker_.CalledOnValidThread());

stats->time_rendered = base::TimeTicks::Now();
if (perf_tracker_)
perf_tracker_->OnFramePainted(frame_id);
perf_tracker_->RecordVideoFrameStats(*stats);

if (!done.is_null())
done.Run();
Expand Down
7 changes: 4 additions & 3 deletions remoting/client/software_video_renderer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
#include "base/memory/ref_counted.h"
#include "base/memory/weak_ptr.h"
#include "base/threading/thread_checker.h"
#include "remoting/protocol/performance_tracker.h"
#include "remoting/protocol/video_renderer.h"
#include "remoting/protocol/video_stub.h"
#include "third_party/webrtc/modules/desktop_capture/desktop_geometry.h"
Expand All @@ -32,6 +31,7 @@ class VideoDecoder;

namespace protocol {
class FrameConsumer;
struct FrameStats;
class PerformanceTracker;
} // namespace protocol

Expand Down Expand Up @@ -60,10 +60,11 @@ class SoftwareVideoRenderer : public protocol::VideoRenderer,
const base::Closure& done) override;

private:
void RenderFrame(int32_t frame_id,
void RenderFrame(std::unique_ptr<protocol::FrameStats> stats,
const base::Closure& done,
std::unique_ptr<webrtc::DesktopFrame> frame);
void OnFrameRendered(int32_t frame_id, const base::Closure& done);
void OnFrameRendered(std::unique_ptr<protocol::FrameStats> stats,
const base::Closure& done);

scoped_refptr<base::SingleThreadTaskRunner> decode_task_runner_;
protocol::FrameConsumer* consumer_;
Expand Down
Loading

0 comments on commit 2ec2751

Please sign in to comment.