Audit/fix use of media::VideoFrame::coded_size()

content/common/gpu/media/android_video_encode_accelerator.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 "content/common/gpu/media/android_video_encode_accelerator.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
@@ skipping 184 matching lines @@
     const scoped_refptr<VideoFrame>& frame,
     bool force_keyframe) {
   DVLOG(3) << __PRETTY_FUNCTION__ << ": " << force_keyframe;
   DCHECK(thread_checker_.CalledOnValidThread());
   RETURN_ON_FAILURE(frame->format() == VideoFrame::I420,
                     "Unexpected format",
                     kInvalidArgumentError);
 
   // MediaCodec doesn't have a way to specify stride for non-Packed formats, so
   // we insist on being called with packed frames and no cropping :(
-  RETURN_ON_FAILURE(frame->row_bytes(VideoFrame::kYPlane) ==
-                            frame->stride(VideoFrame::kYPlane) &&
-                        frame->row_bytes(VideoFrame::kUPlane) ==
-                            frame->stride(VideoFrame::kUPlane) &&
-                        frame->row_bytes(VideoFrame::kVPlane) ==
-                            frame->stride(VideoFrame::kVPlane) &&
-                        gfx::Rect(frame->coded_size()) == frame->visible_rect(),
-                    "Non-packed frame, or visible rect != coded size",
-                    kInvalidArgumentError);
+  RETURN_ON_FAILURE(
+      frame->row_bytes(VideoFrame::kYPlane) ==
+              frame->stride(VideoFrame::kYPlane) &&
+          frame->row_bytes(VideoFrame::kUPlane) ==
+              frame->stride(VideoFrame::kUPlane) &&
+          frame->row_bytes(VideoFrame::kVPlane) ==
+              frame->stride(VideoFrame::kVPlane) &&
+          frame->coded_size().width() == frame->visible_rect().right(),

[Ami GONE FROM CHROMIUM, 2014/02/27 01:20:12]

this is a weaker check than before (e.g. a right-aligned visible subrect will
pass the new test but not the old).  why?

[sheu, 2014/02/28 00:40:19]

My thought was that as long as the stride matched the right side of the visible
rect, we should be safe with the assumption that stride == coded_size.

I'll strengthen this one.

+      "Non-packed frame, or visible_rect/coded_size mismatch",
+      kInvalidArgumentError);
 
   pending_frames_.push(MakeTuple(frame, force_keyframe, base::Time::Now()));
   DoIOTask();
 }
 
 void AndroidVideoEncodeAccelerator::UseOutputBitstreamBuffer(
     const media::BitstreamBuffer& buffer) {
   DVLOG(3) << __PRETTY_FUNCTION__ << ": bitstream_buffer_id=" << buffer.id();
   DCHECK(thread_checker_.CalledOnValidThread());
   RETURN_ON_FAILURE(buffer.size() >= media_codec_->GetOutputBuffersCapacity(),
@@ skipping 181 matching lines @@
   base::MessageLoop::current()->PostTask(
       FROM_HERE,
       base::Bind(&VideoEncodeAccelerator::Client::BitstreamBufferReady,
                  client_ptr_factory_.GetWeakPtr(),
                  bitstream_buffer.id(),
                  size,
                  key_frame));
 }
 
 }  // namespace content