EVEA cleanup: use video utility functions in media::*

media/base/video_frame.cc

Index: media/base/video_frame.cc
diff --git a/media/base/video_frame.cc b/media/base/video_frame.cc
index 08e7e1ad7a236f3ab29d5e7ee6fc9da2098fb630..5c20861d149596110d2ebdd534f10a59d8413e21 100644
--- a/media/base/video_frame.cc
+++ b/media/base/video_frame.cc
@@ -124,9 +124,13 @@ scoped_refptr<VideoFrame> VideoFrame::WrapExternalSharedMemory(
     const gfx::Rect& visible_rect,
     const gfx::Size& natural_size,
     uint8* data,
+    size_t data_size,
     base::SharedMemoryHandle handle,
     base::TimeDelta timestamp,
     const base::Closure& no_longer_needed_cb) {
+  if (data_size < AllocationSize(format, coded_size))

[Cris Neckar, 2013/08/23 22:25:44]

Allocation size will overflow for large values which could allow this check to
pass

[sheu, 2013/08/26 21:07:07]

I did the check at a higher level now.  Do we need to check explicitly here?

[Ami GONE FROM CHROMIUM, 2013/08/26 22:00:23]

Are you referring to IsValidConfig()?  Because that's only DCHECK'd, no?

[sheu, 2013/08/27 22:05:56]

I'm doing the DCHECKS when values for this are being passed in over IPC.  I
think that should be sufficient.

[Ami GONE FROM CHROMIUM, 2013/08/27 23:57:08]

I think the point is that the danger from malicious renderers will ignore
DCHECKs in the release build.

[sheu, 2013/08/28 00:03:59]

Whoops, I meant to say that it's being checked at the IPC layer explicitly and
error-ed out at that API layer if it's out of bounds.

So it's being checked in release too.

+    return NULL;
+
   switch (format) {
     case I420: {
       scoped_refptr<VideoFrame> frame(new VideoFrame(
@@ -230,8 +234,8 @@ size_t VideoFrame::NumPlanes(Format format) {
     case VideoFrame::RGB32:
       return 1;
     case VideoFrame::YV12:
-    case VideoFrame::YV16:
     case VideoFrame::I420:

[Ami GONE FROM CHROMIUM, 2013/08/26 22:00:23]

Why this reorder, which puts separates two similar values (YV12/YV16), but more
importantly puts it out of order of enum declaration?

[sheu, 2013/08/27 22:05:56]

I did this to match the order in AllocationSize() below.  I'll revert.

+    case VideoFrame::YV16:
       return 3;
     case VideoFrame::YV12A:
       return 4;
@@ -243,6 +247,30 @@ size_t VideoFrame::NumPlanes(Format format) {
   return 0;
 }
 
+// static
+size_t VideoFrame::AllocationSize(Format format, const gfx::Size& coded_size) {
+  switch (format) {
+    case VideoFrame::RGB32:
+      return coded_size.GetArea() * 4;
+    case VideoFrame::YV12:
+    case VideoFrame::I420:
+      return coded_size.GetArea() * 3 / 2;
+    case VideoFrame::YV12A:
+      return coded_size.GetArea() * 5 / 2;

[Ami GONE FROM CHROMIUM, 2013/08/26 22:00:23]

For this and l.257 above, is it necessary to check that height & width are even?
 Otherwise shouldn't these be ceil's?

[sheu, 2013/08/27 22:05:56]

Done.

+    case VideoFrame::YV16:
+      return coded_size.GetArea() * 2;
+    case VideoFrame::INVALID:
+    case VideoFrame::EMPTY:
+    case VideoFrame::NATIVE_TEXTURE:
+#if defined(GOOGLE_TV)
+    case VideoFrame::HOLE:
+#endif
+      break;
+  }
+  NOTREACHED() << "Unsupported video frame format: " << format;
+  return 0;
+}
+
 static inline size_t RoundUp(size_t value, size_t alignment) {
   // Check that |alignment| is a power of 2.
   DCHECK((alignment + (alignment - 1)) == (alignment | (alignment - 1)));