[Chromoting] Fallback to use software renderer if desktop image size is larger than GPU limitaiton

remoting/client/plugin/pepper_video_renderer_3d.cc

Index: remoting/client/plugin/pepper_video_renderer_3d.cc
diff --git a/remoting/client/plugin/pepper_video_renderer_3d.cc b/remoting/client/plugin/pepper_video_renderer_3d.cc
index b60f9bd72bb9e99d3745093ef73a7b674226b289..eb9b2e33d15a81d153521041a393b574d5641fe4 100644
--- a/remoting/client/plugin/pepper_video_renderer_3d.cc
+++ b/remoting/client/plugin/pepper_video_renderer_3d.cc
@@ -91,13 +91,24 @@ void PepperVideoRenderer3D::SetPepperContext(
   DCHECK(event_handler);
   DCHECK(!event_handler_);
 
+  fallback_renderer_.SetPepperContext(instance, event_handler);
+
   event_handler_ = event_handler;
   pp_instance_ = instance;
 }
 
 void PepperVideoRenderer3D::OnViewChanged(const pp::View& view) {
+  fallback_renderer_.OnViewChanged(view);
+
   pp::Size size = view.GetRect().size();
   float scale = view.GetDeviceScale();
+  if (scale == 0) {

[Sergey Ulanov, 2016/10/12 21:53:31]

scale is never expected to be 0. maybe replace it with DCHECK()?

[Hzj_jie, 2016/10/12 23:42:46]

Done.

+    LOG(WARNING) << "Unexpected device scale factor received.";
+    scale = 1;
+  }
+  gl_scaled_limits_.texture_size = gl_limits_.texture_size / scale;

[Sergey Ulanov, 2016/10/12 21:53:31]

I'm not sure this is correct. textures should not be scaled until they are
rendered, so I don't see how the scale factor can affect max texture size. If
you believe this is correct, maybe add a comment to explain it.

[Hzj_jie, 2016/10/12 23:42:46]

Yes, the GetDeviceScale returns a scale factor to represent pixels per logical
pixel. But hardware knows only hardware pixel. So if an hardware supports only
8192 x 8192, and we want to render a 5000 x 5000 in logic pixel, with scale
factor as 2, the hardware will still need to render 10000 x 10000 hardware
pixels, which exceeds the limitation.
I will add comment to explain this behavior.

But these lines have no relationship with the following view_size_.set()
statement. I put them here simply because this is the only place we can get a
pp::View object.

+  gl_scaled_limits_.viewport_size[0] = gl_limits_.viewport_size[0] / scale;
+  gl_scaled_limits_.viewport_size[1] = gl_limits_.viewport_size[1] / scale;
   view_size_.set(ceilf(size.width() * scale), ceilf(size.height() * scale));
   graphics_.ResizeBuffers(view_size_.width(), view_size_.height());
 
@@ -106,12 +117,15 @@ void PepperVideoRenderer3D::OnViewChanged(const pp::View& view) {
 }
 
 void PepperVideoRenderer3D::EnableDebugDirtyRegion(bool enable) {
+  fallback_renderer_.EnableDebugDirtyRegion(enable);
   debug_dirty_region_ = enable;
 }
 
 bool PepperVideoRenderer3D::Initialize(
     const ClientContext& context,
     protocol::FrameStatsConsumer* stats_consumer) {
+  CHECK(fallback_renderer_.Initialize(context, stats_consumer));

[Sergey Ulanov, 2016/10/12 21:53:31]

Please change this to 
  if (!fallback_renderer_.Initialize(context, stats_consumer))
    LOG(FATAL) << "Failed to initialize fallback_renderer_";

CHECKS() and DCHECKS() should not have side-effects. This code will break if one
decides to replace this CHECK with a DCHECK.

[Hzj_jie, 2016/10/12 23:42:46]

Done.

+
   stats_consumer_ = stats_consumer;
 
   const int32_t context_attributes[] = {
@@ -167,6 +181,11 @@ bool PepperVideoRenderer3D::Initialize(
   gles2_if_->BufferData(graphics_3d, GL_ARRAY_BUFFER, sizeof(kVertices),
                         kVertices, GL_STATIC_DRAW);
 
+  gles2_if_->GetIntegerv(
+      graphics_3d, GL_MAX_TEXTURE_SIZE, &gl_limits_.texture_size);
+  gles2_if_->GetIntegerv(
+      graphics_3d, GL_MAX_VIEWPORT_DIMS, gl_limits_.viewport_size);
+
   CheckGLError();
 
   return true;
@@ -174,6 +193,8 @@ bool PepperVideoRenderer3D::Initialize(
 
 void PepperVideoRenderer3D::OnSessionConfig(
     const protocol::SessionConfig& config) {
+  fallback_renderer_.OnSessionConfig(config);
+
   PP_VideoProfile video_profile = PP_VIDEOPROFILE_VP8_ANY;
   switch (config.video_config().codec) {
     case protocol::ChannelConfig::CODEC_VP8:
@@ -212,6 +233,33 @@ protocol::FrameStatsConsumer* PepperVideoRenderer3D::GetFrameStatsConsumer() {
 void PepperVideoRenderer3D::ProcessVideoPacket(
     std::unique_ptr<VideoPacket> packet,
     const base::Closure& done) {
+  if (!use_fallback_renderer_) {
+    use_fallback_renderer_ =
+        packet->format().has_screen_width() &&
+        packet->format().has_screen_height() &&
+        (packet->format().screen_width() > gl_scaled_limits_.texture_size ||

[Sergey Ulanov, 2016/10/12 21:53:31]

We only want to initialize fallback when resolution changes - that guarantees
that the fallback software decoder will get first key frame after resolution
change.

[Hzj_jie, 2016/10/12 23:42:46]

Sorry, I cannot quite get the point of your comment. If there is not screen
resolution change, the result of this condition won't be impacted.

+         packet->format().screen_height() > gl_scaled_limits_.texture_size ||
+         packet->format().screen_width() > gl_scaled_limits_.viewport_size[0] ||

[Sergey Ulanov, 2016/10/12 21:53:31]

It seems strange to compare host screen size with client viewport size. It looks
to me that the right thing to do is to compare view_size_ and
gl_limits_.viewport_size in OnViewChanged() when the view is resized.
Here when the screen gets resized we only need to make sure that the screen fits
into a texture.

[Hzj_jie, 2016/10/12 23:42:46]

Have I made a misunderstanding here? I believe the hardware limitation impacts
the maximum size of image we are rendering, i.e. packet->format().screen_width()
& packet->format().screen_height(), instead of view_size_.

And viewport size here is just a name in OpenGl, means the maximum image it can
render.

+         packet->format().screen_height() > gl_scaled_limits_.viewport_size[1]);
+    if (use_fallback_renderer_) {

[Sergey Ulanov, 2016/10/12 21:53:31]

It would be easier to understand this code if it was written with a single if
statement:
if (!use_fallback_renderer_ &&
    packet->format().has_screen_width() && 
    .....) {
  use_fallback_renderer_ = true;
  ... state cleanup ...
}

[Hzj_jie, 2016/10/12 23:42:46]

Done.

+      // Clear current instance and use fallback_renderer_.
+      current_picture_frames_.clear();
+      current_picture_.reset();
+      next_picture_frames_.clear();
+      next_picture_.reset();
+      decoded_frames_.clear();
+      pending_frames_.clear();
+      graphics_ = pp::Graphics3D();
+      video_decoder_ = pp::VideoDecoder();
+    }
+  }
+
+  if (use_fallback_renderer_) {
+    fallback_renderer_.GetVideoStub()->ProcessVideoPacket(
+        std::move(packet), done);
+    return;
+  }
+
   VideoPacket* packet_ptr = packet.get();
   std::unique_ptr<FrameTracker> frame_tracker(
       new FrameTracker(std::move(packet), stats_consumer_, done));