Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(469)

Issue 2376013002: [Mojo Video Capture] Move conversions between mojom and media types to a separate file (Closed)

Created:
4 years, 2 months ago by chfremer
Modified:
4 years, 2 months ago
Reviewers:
emircan, mcasas
CC:
chromium-reviews, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, posciak+watch_chromium.org, yzshen+watch_chromium.org, abarth-chromium, Aaron Boodman, darin (slow to review)
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[Mojo Video Capture] Move conversions between mojom and media types to a separate file. This is a pure refactoring. No new functionality. This CL is part of the Mojo Video Capture work. For the bigger picture, see [1] CL1.6.4. Note: The step from these manual conversion to Mojo typemappings is being made in the upcoming CL1.9.3. BUG=584797 TEST=content_unittest, capture_unittest, video_capture_unittest [1] https://docs.google.com/a/chromium.org/document/d/1Qw7rw1AJy0QHXjha36jZNiEuxsxWslJ_X-zpOhijvI8/edit?usp=sharing Committed: https://crrev.com/f50710a2eb63050ef700edc3eeec3f6a59170c7d Cr-Commit-Position: refs/heads/master@{#421732}

Patch Set 1 #

Total comments: 2

Patch Set 2 : mcasas' comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+40 lines, -130 lines) Patch
M services/video_capture/BUILD.gn View 1 chunk +2 lines, -0 lines 0 comments Download
A services/video_capture/mojo_media_conversions.h View 1 1 chunk +31 lines, -0 lines 0 comments Download
A + services/video_capture/mojo_media_conversions.cc View 5 chunks +6 lines, -51 lines 0 comments Download
M services/video_capture/video_capture_device_proxy_impl.h View 1 chunk +0 lines, -14 lines 0 comments Download
M services/video_capture/video_capture_device_proxy_impl.cc View 2 chunks +1 line, -65 lines 0 comments Download

Depends on Patchset:

Dependent Patchsets:

Messages

Total messages: 17 (11 generated)
chfremer
mcasas@: PTAL emircan@: FYI
4 years, 2 months ago (2016-09-28 16:37:13 UTC) #4
mcasas
Reshuffling lgtm. https://codereview.chromium.org/2376013002/diff/1/services/video_capture/mojo_media_conversions.h File services/video_capture/mojo_media_conversions.h (right): https://codereview.chromium.org/2376013002/diff/1/services/video_capture/mojo_media_conversions.h#newcode17 services/video_capture/mojo_media_conversions.h:17: // https://crbug.com/642387 nit: reflow to previous line.
4 years, 2 months ago (2016-09-28 23:09:11 UTC) #5
chfremer
mcasas@ comments https://codereview.chromium.org/2376013002/diff/1/services/video_capture/mojo_media_conversions.h File services/video_capture/mojo_media_conversions.h (right): https://codereview.chromium.org/2376013002/diff/1/services/video_capture/mojo_media_conversions.h#newcode17 services/video_capture/mojo_media_conversions.h:17: // https://crbug.com/642387 On 2016/09/28 23:09:10, mcasas wrote: ...
4 years, 2 months ago (2016-09-28 23:19:12 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2376013002/20001
4 years, 2 months ago (2016-09-29 03:03:43 UTC) #13
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 2 months ago (2016-09-29 03:09:07 UTC) #15
commit-bot: I haz the power
4 years, 2 months ago (2016-09-29 03:10:51 UTC) #17
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/f50710a2eb63050ef700edc3eeec3f6a59170c7d
Cr-Commit-Position: refs/heads/master@{#421732}

Powered by Google App Engine
This is Rietveld 408576698