Introducing a helper wrapper for YUV-to-RGB convertion and/or scaling.

remoting/base/util.h

Index: remoting/base/util.h
diff --git a/remoting/base/util.h b/remoting/base/util.h
index c05ef2f99f9b24e262259ea7755ad063adcc9870..9d1f9c6975711ccc618b30dd26b12ed3fc6134b1 100644
--- a/remoting/base/util.h
+++ b/remoting/base/util.h
@@ -18,16 +18,32 @@ std::string GetTimestampString();
 // TODO(sergeyu): Move these methods to media.
 int GetBytesPerPixel(media::VideoFrame::Format format);
 
-// Convert YUV to RGB32 on a specific rectangle.
-void ConvertYUVToRGB32WithRect(const uint8* y_plane,
-                               const uint8* u_plane,
-                               const uint8* v_plane,
-                               uint8* rgb_plane,
-                               const SkIRect& rect,
-                               int y_stride,
-                               int uv_stride,
-                               int rgb_stride);
+// Convert and scale YUV to RGB32 on a specific rectangle. The source and
+// destination buffers are assumed to contain only |source_clip| and |dest_clip|

[Wez, 2012/02/02 01:35:29]

Why have |dest_clip| passed in, rather than letting the caller clip the
|dest_rect| and then checking that?

Why have |source_clip] at all?

[alexeypa (please no reviews), 2012/02/02 21:33:03]

|source_clip| and |dest_clip| are for the case when buffers are smaller than the
whole image. We can use such buffers as long as the copied |dest_rect| fits
both.

It seems though that these names are confusing. I'll changed them.

[Wez, 2012/02/03 22:40:35]

So |source_clip| and |dest_clip| indicate the actual source and destination
buffer dimensions, and |source_size| and |dest_size| the dimensions of the
logical input and output areas.  "clip" seems a reasonable term.

+// areas correspondingly. The scaling factor is determined as ratio between
+// |dest_size| and |source_size|. The target rectangle |dect_rect| is specified
+// in the destination coordinates.
+//
+// The routine DCHECKs that clipping boundaries are completely contained within
+// the corresponding images. It also DCHECKs that the source and target
+// rectangle fits the corresponding clipping areas.

[Wez, 2012/02/03 22:40:35]

nit: Usually you'd phrase these as pre-conditions of the API, e.g. "The source
and destination clip rectangles must fall entirely within the source or
destination dimensions, respectively" - the fact that you'll DCHECK if they
don't is not part of the API contract, just a sanity-check.

[alexeypa (please no reviews), 2012/02/03 23:29:35]

Done.

+//
+// N.B. The function requires the top left corner of |dect_rect| to have even
+// X and Y coordinates.
+void ConvertAndScaleYUVToRGB32Rect(const uint8* source_yplane,
+                                   const uint8* source_uplane,
+                                   const uint8* source_vplane,
+                                   int source_ystride,
+                                   int source_uvstride,
+                                   const SkISize& source_size,
+                                   const SkIRect& source_clip,
+                                   uint8* dest_buffer,
+                                   int dest_stride,
+                                   const SkISize& dest_size,
+                                   const SkIRect& dest_clip,
+                                   const SkIRect& dest_rect);
 
+// Convert RGB32 to YUV on a specific rectangle.
 void ConvertRGB32ToYUVWithRect(const uint8* rgb_plane,
                                uint8* y_plane,
                                uint8* u_plane,
@@ -60,6 +76,14 @@ void CopyRect(const uint8* src_plane,
               int bytes_per_pixel,
               const SkIRect& rect);
 
+void CopyRGB32Rect(const uint8* source_buffer,

[Wez, 2012/02/02 01:35:29]

This doesn't seem to be used anywhere?

[alexeypa (please no reviews), 2012/02/02 21:33:03]

This will be used by a later change. I decided to pack it to this change because
this function is RGB-to-RGB counterpart of YUV-to-RGB conversion routine above.

[Wez, 2012/02/03 22:40:35]

OK, but please mention in the CL description that you've added it.

[alexeypa (please no reviews), 2012/02/03 23:29:35]

Done.

+                   int source_stride,
+                   const SkIRect& source_clip,
+                   uint8* dest_buffer,
+                   int dest_stride,
+                   const SkIRect& dest_clip,
+                   const SkIRect& dest_rect);
+
 }  // namespace remoting
 
 #endif  // REMOTING_BASE_UTIL_H_