Video Capture Service: typemap video_capture's and media's VideoCaptureFormat

services/video_capture/public/interfaces/video_capture_format_traits.h

+// Copyright 2016 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.
+
+#ifndef SERVICES_VIDEO_CAPTURE_PUBLIC_INTERFACES_VIDEO_CAPTURE_FORMAT_TYPEMAP_H_
+#define SERVICES_VIDEO_CAPTURE_PUBLIC_INTERFACES_VIDEO_CAPTURE_FORMAT_TYPEMAP_H_
+
+#include "media/base/video_capture_types.h"
+#include "mojo/common/common_custom_types_struct_traits.h"
+#include "services/video_capture/public/interfaces/video_capture_format.mojom.h"
+#include "ui/gfx/geometry/mojo/geometry.mojom.h"
+#include "ui/gfx/geometry/mojo/geometry_struct_traits.h"
+
+namespace mojo {
+
+template <>
+struct StructTraits<video_capture::mojom::VideoCaptureFormatDataView,
+                    media::VideoCaptureFormat> {
+
+  static gfx::Size frame_size(const media::VideoCaptureFormat& r) {
+    return gfx::Size(r.frame_size.width(), r.frame_size.height());
+  }
+
+  static float frame_rate(const media::VideoCaptureFormat& r) {
+    return r.frame_rate;
+  }
+
+  static media::mojom::VideoFormat pixel_format(
+      const media::VideoCaptureFormat& r) {
+    return static_cast<media::mojom::VideoFormat>(r.pixel_format);

[dcheng, 2016/10/03 01:03:08]

Mojo has EnumTraits for mapping enums. Example:
https://cs.chromium.org/chromium/src/gpu/ipc/common/gpu_info_struct_traits.cc...

[mcasas, 2016/10/03 07:31:51]

Using it for VideoPixelStorage in l.33, but re. the mapping 
of media::VideoPixelFormat/media::mojom::VideoFormat, 
it is not  done via EnumTraits, but by hand via 
static_cast<>s  and a bunch of ASSERTs [1], what should
we do?

[1]
https://cs.chromium.org/chromium/src/media/mojo/common/media_type_converters....

[chfremer, 2016/10/03 16:26:59]

In my take on using the typmapping, I ended up removing the static_cast<> in
favor of the cleaner EnumTraits.
If we decide to go with that, please feel free to copy the corresponding code
from the not-yet-to-be-reviewed CL [1].

[1] https://codereview.chromium.org/2386183002/

[dcheng, 2016/10/03 20:45:53]

My recollection is the //media code predated the addition of EnumTraits. If we
can make EnumTraits work here, that would be nice.

+  }
+
+  static video_capture::mojom::VideoPixelStorage pixel_storage(
+      const media::VideoCaptureFormat& r) {
+    return static_cast<video_capture::mojom::VideoPixelStorage>(
+        r.pixel_storage);
+  }
+
+  static bool Read(video_capture::mojom::VideoCaptureFormatDataView data,

[dcheng, 2016/10/03 01:03:08]

Nit: Out-of-line this.

[mcasas, 2016/10/03 07:31:51]

Done.

+                   media::VideoCaptureFormat* out) {
+    gfx::Size frame_size;
+    data.ReadFrameSize(&frame_size);

[dcheng, 2016/10/03 01:03:08]

Nit: read directly into out->frame_size, and check the return value (I'm
surprised this doesn't warn, I filed
https://bugs.chromium.org/p/chromium/issues/detail?id=652129 to fix this)

[mcasas, 2016/10/03 07:31:51]

Done.

+    out->frame_size = frame_size;
+    out->frame_rate = data.frame_rate();
+    out->pixel_format =
+        static_cast<media::VideoPixelFormat>(data.pixel_format());
+    out->pixel_storage =
+        static_cast<media::VideoPixelStorage>(data.pixel_storage());
+    return true;
+  }
+};
+
+}
+
+#endif  // SERVICES_VIDEO_CAPTURE_PUBLIC_INTERFACES_VIDEO_CAPTURE_FORMAT_TYPEMAP_H_