Correct some rounding errors introduced recently.

media/base/video_frame.cc

 // Copyright (c) 2011 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/base/video_frame.h"
 
 #include "base/logging.h"
 
 namespace media {
 
@@ skipping 100 matching lines @@
   DCHECK(format_ == VideoFrame::YV12 || format_ == VideoFrame::YV16);
   // Align Y rows at 32-bit (4 byte) boundaries.  The stride for both YV12 and
   // YV16 is 1/2 of the stride of Y.  For YV12, every row of bytes for U and V
   // applies to two rows of Y (one byte of UV for 4 bytes of Y), so in the
   // case of YV12 the strides are identical for the same width surface, but the
   // number of bytes allocated for YV12 is 1/2 the amount for U & V as YV16.
   // We also round the height of the surface allocated to be an even number
   // to avoid any potential of faulting by code that attempts to access the Y
   // values of the final row, but assumes that the last row of U & V applies to
   // a full two rows of Y.
-  size_t y_height = RoundUp(rows(VideoFrame::kYPlane), 2);
+  size_t y_height = rows(VideoFrame::kYPlane);

[scherkus (not reviewing), 2011/11/10 04:09:41]

this change looks OK and avoids over-allocating one extra row

   size_t y_stride = RoundUp(row_bytes(VideoFrame::kYPlane), 4);
   size_t uv_stride = RoundUp(row_bytes(VideoFrame::kUPlane), 4);
-  size_t uv_height = RoundUp(rows(VideoFrame::kUPlane), 2);
+  size_t uv_height = rows(VideoFrame::kUPlane);
   size_t y_bytes = y_height * y_stride;
   size_t uv_bytes = uv_height * uv_stride;
 
   uint8* data = new uint8[y_bytes + (uv_bytes * 2) + kFramePadBytes];
   planes_ = VideoFrame::kNumYUVPlanes;
   COMPILE_ASSERT(0 == VideoFrame::kYPlane, y_plane_data_must_be_index_0);
   data_[VideoFrame::kYPlane] = data;
   data_[VideoFrame::kUPlane] = data + y_bytes;
   data_[VideoFrame::kVPlane] = data + y_bytes + uv_bytes;
   strides_[VideoFrame::kYPlane] = y_stride;
@@ skipping 53 matching lines @@
     case RGB565:
     case RGB24:
     case RGB32:
     case RGBA:
       return width_;
 
     case YV12:
     case YV16:
       if (plane == kYPlane)
         return width_;
-      return width_ / 2;
+      return RoundUp(width_, 2) / 2;

[scherkus (not reviewing), 2011/11/10 04:09:41]

so we're always rounding up post-divide?

i.e., this should be equivalent to (width_ / 2) | 0x1

[Chris Evans, 2011/11/10 06:32:38]

The intent is to round up pre-divide -- the divide is executed on the return
value of RoundUp. (The old way was to round up post-divide and that led to the
under-allocation issues).
More like (width_ / 2) + (width_ & 1)

The way I conceptualized these changes is to imagine a 1x1 video frame. We need
to allocate at least 1x1 of working space, even for planes that are scaled by
half on both the horiz and vert dimensions.

 
     default:
       break;
   }
 
   // Intentionally leave out non-production formats.
   NOTREACHED() << "Unsupported video frame format: " << format_;
   return 0;
 }
 
 int VideoFrame::rows(size_t plane) const {
   DCHECK(IsValidPlane(plane));
   switch (format_) {
     case RGB555:
     case RGB565:
     case RGB24:
     case RGB32:
     case RGBA:
     case YV16:
       return height_;
 
     case YV12:
       if (plane == kYPlane)
         return height_;
-      return height_ / 2;
+      return RoundUp(height_, 2) / 2;
 
     default:
       break;
   }
 
   // Intentionally leave out non-production formats.
   NOTREACHED() << "Unsupported video frame format: " << format_;
   return 0;
 }
 
 uint8* VideoFrame::data(size_t plane) const {
   DCHECK(IsValidPlane(plane));
   return data_[plane];
 }
 
 bool VideoFrame::IsEndOfStream() const {
   return format_ == VideoFrame::EMPTY;
 }
 
 }  // namespace media