Remove FORMAT_RGB from gfx::PngCodec

media/gpu/rendering_helper.cc

 // Copyright 2013 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 "media/gpu/rendering_helper.h"
 
 #include <string.h>
 
 #include <algorithm>
 #include <memory>
@@ skipping 692 matching lines @@
   task_runner_ = nullptr;
   gl_context_ = NULL;
   gl_surface_ = NULL;
 
   render_as_thumbnails_ = false;
   frame_count_ = 0;
   thumbnails_fbo_id_ = 0;
   thumbnails_texture_id_ = 0;
 }
 
-void RenderingHelper::GetThumbnailsAsRGB(std::vector<unsigned char>* rgb,
-                                         bool* alpha_solid,
-                                         base::WaitableEvent* done) {
+void RenderingHelper::GetThumbnailsAsRGBA(std::vector<unsigned char>* rgba,
+                                          bool* alpha_solid,
+                                          base::WaitableEvent* done) {
   CHECK(render_as_thumbnails_);
 
   const size_t num_pixels = thumbnails_fbo_size_.GetArea();
-  std::vector<unsigned char> rgba;
-  rgba.resize(num_pixels * 4);
+  rgba->resize(num_pixels * 4);
   glBindFramebufferEXT(GL_FRAMEBUFFER, thumbnails_fbo_id_);
   glPixelStorei(GL_PACK_ALIGNMENT, 1);
   // We can only count on GL_RGBA/GL_UNSIGNED_BYTE support.
   glReadPixels(0, 0,
                thumbnails_fbo_size_.width(), thumbnails_fbo_size_.height(),
                GL_RGBA,
                GL_UNSIGNED_BYTE,
                &rgba[0]);
   glBindFramebufferEXT(GL_FRAMEBUFFER,
                        gl_surface_->GetBackingFramebufferObject());
-  rgb->resize(num_pixels * 3);
-  // Drop the alpha channel, but check as we go that it is all 0xff.
+
+  // Check if the alpha channel is all 0xff.
   bool solid = true;
-  unsigned char* rgb_ptr = &((*rgb)[0]);
-  unsigned char* rgba_ptr = &rgba[0];
   for (size_t i = 0; i < num_pixels; ++i) {
-    *rgb_ptr++ = *rgba_ptr++;
-    *rgb_ptr++ = *rgba_ptr++;
-    *rgb_ptr++ = *rgba_ptr++;
-    solid = solid && (*rgba_ptr == 0xff);
-    rgba_ptr++;
+    solid = solid && ((*rgba)[4 * i + 3] == 0xff);

[scroggo_chromium, 2017/06/13 14:54:00]

Once solid is false, there's no need to continue this loop, right?

Taking a step back, this method is only called in one place, this method isn't
doing anything the client couldn't do themselves to detect alpha, and it's not
doing it along the way (e.g. it used to detect alpha while it iterated through
the pixels and copied the other components). Why not move this check into the
client code?

[msarett1, 2017/06/13 16:32:03]

sgtm, done.

   }
   *alpha_solid = solid;
 
   done->Signal();
 }
 
 void RenderingHelper::Flush(size_t window_id) {
   videos_[window_id].is_flushing = true;
 }
 
@@ skipping 160 matching lines @@
   // When the rendering falls behind, drops frames.
   while (scheduled_render_time_ < target) {
     scheduled_render_time_ += frame_duration_;
     DropOneFrameForAllVideos();
   }
 
   task_runner_->PostDelayedTask(FROM_HERE, render_task_.callback(),
                                 target - now);
 }
 }  // namespace media