Skip to content

Commit

Permalink
[Video Capture] Fix dangerous silent remapping of video pixel format
Browse files Browse the repository at this point in the history 
Added all video pixel formats from media::VideoPixelFormat to
media::mojom::VideoCapturePixelFormat.

Updated Mojo typemapping to have a 1:1 mapping between the two.


A recent CL https://chromium-review.googlesource.com/c/chromium/src/+/1026795
caused a regression by introducing a new mojo enum VideoCapturePixelType and
typemapping it to media::VideoPixelFormat, see crbug/839742.

A quick mitigation fix was landed and merged into M68. However, the fix is
specific to a particular single format. The current state still has the danger
of having produced similar regressions occurring for other formats, which may
simply not yet have surfaced.


Using TBR since this has been reviewed previously at
https://chromium-review.googlesource.com/c/chromium/src/+/1050489

TBR=xhwang@chromium.org,rockot@chromium.org,sandersd@chromium.org,rsesek@chromium.org,chfremer@chromium.org,lasoren@chromium.org

Bug: 852096
Change-Id: I6f6d3add996d3557f33fbd42084ad2a9844b5957
Reviewed-on: https://chromium-review.googlesource.com/1098143
Commit-Queue: Christian Fremerey <chfremer@chromium.org>
Reviewed-by: Christian Fremerey <chfremer@chromium.org>
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567347}
  • Loading branch information
Christian Fremerey authored and Commit Bot committed Jun 14, 2018
1 parent 8c022dc commit 67b738e
Show file tree
Hide file tree
Showing 2 changed files with 126 additions and 15 deletions.
27 changes: 25 additions & 2 deletions media/capture/mojom/video_capture_types.mojom
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,32 @@ import "mojo/public/mojom/base/values.mojom";
import "ui/gfx/geometry/mojo/geometry.mojom";

enum VideoCapturePixelFormat {
UNKNOWN,
I420,
Y16,
ARGB
YV12,
I422,
I420A,
I444,
NV12,
NV21,
UYVY,
YUY2,
ARGB,
XRGB,
RGB24,
RGB32,
MJPEG,
MT21,
YUV420P9,
YUV420P10,
YUV422P9,
YUV422P10,
YUV444P9,
YUV444P10,
YUV420P12,
YUV422P12,
YUV444P12,
Y16
};

enum ResolutionChangePolicy {
Expand Down
114 changes: 101 additions & 13 deletions media/capture/mojom/video_capture_types_mojom_traits.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,39 +87,58 @@ media::mojom::VideoCapturePixelFormat
EnumTraits<media::mojom::VideoCapturePixelFormat,
media::VideoPixelFormat>::ToMojom(media::VideoPixelFormat input) {
switch (input) {
case media::VideoPixelFormat::PIXEL_FORMAT_UNKNOWN:
return media::mojom::VideoCapturePixelFormat::UNKNOWN;
case media::VideoPixelFormat::PIXEL_FORMAT_I420:
return media::mojom::VideoCapturePixelFormat::I420;
case media::VideoPixelFormat::PIXEL_FORMAT_Y16:
return media::mojom::VideoCapturePixelFormat::Y16;
case media::VideoPixelFormat::PIXEL_FORMAT_ARGB:
return media::mojom::VideoCapturePixelFormat::ARGB;
case media::VideoPixelFormat::PIXEL_FORMAT_UNKNOWN:
case media::VideoPixelFormat::PIXEL_FORMAT_YV12:
return media::mojom::VideoCapturePixelFormat::YV12;
case media::VideoPixelFormat::PIXEL_FORMAT_I422:
return media::mojom::VideoCapturePixelFormat::I422;
case media::VideoPixelFormat::PIXEL_FORMAT_I420A:
return media::mojom::VideoCapturePixelFormat::I420A;
case media::VideoPixelFormat::PIXEL_FORMAT_I444:
return media::mojom::VideoCapturePixelFormat::I444;
case media::VideoPixelFormat::PIXEL_FORMAT_NV12:
return media::mojom::VideoCapturePixelFormat::NV12;
case media::VideoPixelFormat::PIXEL_FORMAT_NV21:
return media::mojom::VideoCapturePixelFormat::NV21;
case media::VideoPixelFormat::PIXEL_FORMAT_UYVY:
return media::mojom::VideoCapturePixelFormat::UYVY;
case media::VideoPixelFormat::PIXEL_FORMAT_YUY2:
return media::mojom::VideoCapturePixelFormat::YUY2;
case media::VideoPixelFormat::PIXEL_FORMAT_ARGB:
return media::mojom::VideoCapturePixelFormat::ARGB;
case media::VideoPixelFormat::PIXEL_FORMAT_XRGB:
return media::mojom::VideoCapturePixelFormat::XRGB;
case media::VideoPixelFormat::PIXEL_FORMAT_RGB24:
return media::mojom::VideoCapturePixelFormat::RGB24;
case media::VideoPixelFormat::PIXEL_FORMAT_RGB32:
return media::mojom::VideoCapturePixelFormat::RGB32;
case media::VideoPixelFormat::PIXEL_FORMAT_MJPEG:
return media::mojom::VideoCapturePixelFormat::MJPEG;
case media::VideoPixelFormat::PIXEL_FORMAT_MT21:
return media::mojom::VideoCapturePixelFormat::MT21;
case media::VideoPixelFormat::PIXEL_FORMAT_YUV420P9:
return media::mojom::VideoCapturePixelFormat::YUV420P9;
case media::VideoPixelFormat::PIXEL_FORMAT_YUV420P10:
return media::mojom::VideoCapturePixelFormat::YUV420P10;
case media::VideoPixelFormat::PIXEL_FORMAT_YUV422P9:
return media::mojom::VideoCapturePixelFormat::YUV422P9;
case media::VideoPixelFormat::PIXEL_FORMAT_YUV422P10:
return media::mojom::VideoCapturePixelFormat::YUV422P10;
case media::VideoPixelFormat::PIXEL_FORMAT_YUV444P9:
return media::mojom::VideoCapturePixelFormat::YUV444P9;
case media::VideoPixelFormat::PIXEL_FORMAT_YUV444P10:
return media::mojom::VideoCapturePixelFormat::YUV444P10;
case media::VideoPixelFormat::PIXEL_FORMAT_YUV420P12:
return media::mojom::VideoCapturePixelFormat::YUV420P12;
case media::VideoPixelFormat::PIXEL_FORMAT_YUV422P12:
return media::mojom::VideoCapturePixelFormat::YUV422P12;
case media::VideoPixelFormat::PIXEL_FORMAT_YUV444P12:
// Any pixel format requested via a media::VideoPixelFormat that is not
// supported by the video capture service is interpreted as requesting
// I420.
return media::mojom::VideoCapturePixelFormat::I420;
return media::mojom::VideoCapturePixelFormat::YUV444P12;
case media::VideoPixelFormat::PIXEL_FORMAT_Y16:
return media::mojom::VideoCapturePixelFormat::Y16;
}
NOTREACHED();
return media::mojom::VideoCapturePixelFormat::I420;
Expand All @@ -131,14 +150,83 @@ bool EnumTraits<media::mojom::VideoCapturePixelFormat,
FromMojom(media::mojom::VideoCapturePixelFormat input,
media::VideoPixelFormat* output) {
switch (input) {
case media::mojom::VideoCapturePixelFormat::UNKNOWN:
*output = media::PIXEL_FORMAT_UNKNOWN;
return true;
case media::mojom::VideoCapturePixelFormat::I420:
*output = media::VideoPixelFormat::PIXEL_FORMAT_I420;
*output = media::PIXEL_FORMAT_I420;
return true;
case media::mojom::VideoCapturePixelFormat::Y16:
*output = media::VideoPixelFormat::PIXEL_FORMAT_Y16;
case media::mojom::VideoCapturePixelFormat::YV12:
*output = media::PIXEL_FORMAT_YV12;
return true;
case media::mojom::VideoCapturePixelFormat::I422:
*output = media::PIXEL_FORMAT_I422;
return true;
case media::mojom::VideoCapturePixelFormat::I420A:
*output = media::PIXEL_FORMAT_I420A;
return true;
case media::mojom::VideoCapturePixelFormat::I444:
*output = media::PIXEL_FORMAT_I444;
return true;
case media::mojom::VideoCapturePixelFormat::NV12:
*output = media::PIXEL_FORMAT_NV12;
return true;
case media::mojom::VideoCapturePixelFormat::NV21:
*output = media::PIXEL_FORMAT_NV21;
return true;
case media::mojom::VideoCapturePixelFormat::UYVY:
*output = media::PIXEL_FORMAT_UYVY;
return true;
case media::mojom::VideoCapturePixelFormat::YUY2:
*output = media::PIXEL_FORMAT_YUY2;
return true;
case media::mojom::VideoCapturePixelFormat::ARGB:
*output = media::VideoPixelFormat::PIXEL_FORMAT_ARGB;
*output = media::PIXEL_FORMAT_ARGB;
return true;
case media::mojom::VideoCapturePixelFormat::XRGB:
*output = media::PIXEL_FORMAT_XRGB;
return true;
case media::mojom::VideoCapturePixelFormat::RGB24:
*output = media::PIXEL_FORMAT_RGB24;
return true;
case media::mojom::VideoCapturePixelFormat::RGB32:
*output = media::PIXEL_FORMAT_RGB32;
return true;
case media::mojom::VideoCapturePixelFormat::MJPEG:
*output = media::PIXEL_FORMAT_MJPEG;
return true;
case media::mojom::VideoCapturePixelFormat::MT21:
*output = media::PIXEL_FORMAT_MT21;
return true;
case media::mojom::VideoCapturePixelFormat::YUV420P9:
*output = media::PIXEL_FORMAT_YUV420P9;
return true;
case media::mojom::VideoCapturePixelFormat::YUV420P10:
*output = media::PIXEL_FORMAT_YUV420P10;
return true;
case media::mojom::VideoCapturePixelFormat::YUV422P9:
*output = media::PIXEL_FORMAT_YUV422P9;
return true;
case media::mojom::VideoCapturePixelFormat::YUV422P10:
*output = media::PIXEL_FORMAT_YUV422P10;
return true;
case media::mojom::VideoCapturePixelFormat::YUV444P9:
*output = media::PIXEL_FORMAT_YUV444P9;
return true;
case media::mojom::VideoCapturePixelFormat::YUV444P10:
*output = media::PIXEL_FORMAT_YUV444P10;
return true;
case media::mojom::VideoCapturePixelFormat::YUV420P12:
*output = media::PIXEL_FORMAT_YUV420P12;
return true;
case media::mojom::VideoCapturePixelFormat::YUV422P12:
*output = media::PIXEL_FORMAT_YUV422P12;
return true;
case media::mojom::VideoCapturePixelFormat::YUV444P12:
*output = media::PIXEL_FORMAT_YUV444P12;
return true;
case media::mojom::VideoCapturePixelFormat::Y16:
*output = media::PIXEL_FORMAT_Y16;
return true;
}
NOTREACHED();
Expand Down

0 comments on commit 67b738e

Please sign in to comment.