|
|
Chromium Code Reviews|
Created:
4 years, 1 month ago by Yusuke Sato Modified:
4 years, 1 month ago CC:
chromium-reviews, elijahtaylor+arcwatch_chromium.org, yusukes+watch_chromium.org, posciak+watch_chromium.org, hidehiko+watch_chromium.org, lhchavez+watch_chromium.org, piman+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Descriptionarc: enable use_new_wrapper_types for video_accelerator.mojom
BUG=662510
BUG=624136
TEST=try
Committed: https://crrev.com/fd879031d441f0cf3927dac7762b20630d314c6b
Cr-Commit-Position: refs/heads/master@{#432961}
Patch Set 1 #
Total comments: 2
Patch Set 2 : address comment #Patch Set 3 : rebased BUILD.gn, no code change #
Total comments: 2
Patch Set 4 : address comment from Pawel #Patch Set 5 : final rebase #
Messages
Total messages: 34 (23 generated)
The CQ bit was checked by yusukes@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
yusukes@chromium.org changed reviewers: + lhchavez@chromium.org, posciak@chromium.org
Owners, please take a look.
lgtm https://codereview.chromium.org/2505733003/diff/1/chrome/gpu/gpu_arc_video_se... File chrome/gpu/gpu_arc_video_service.cc (right): https://codereview.chromium.org/2505733003/diff/1/chrome/gpu/gpu_arc_video_se... chrome/gpu/gpu_arc_video_service.cc:267: std::vector<ArcVideoAccelerator::DmabufPlane> converted_planes; It's maybe better to create a typemapping for the DmabufPlane to avoid the conversion altogether. Up to you to do it/delegate it to someone else or leave a TODO+file a bug.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by yusukes@chromium.org to run a CQ dry run
Pawel: please take a look https://codereview.chromium.org/2505733003/diff/1/chrome/gpu/gpu_arc_video_se... File chrome/gpu/gpu_arc_video_service.cc (right): https://codereview.chromium.org/2505733003/diff/1/chrome/gpu/gpu_arc_video_se... chrome/gpu/gpu_arc_video_service.cc:267: std::vector<ArcVideoAccelerator::DmabufPlane> converted_planes; On 2016/11/16 03:04:57, Luis Héctor Chávez wrote: > It's maybe better to create a typemapping for the DmabufPlane to avoid the > conversion altogether. Up to you to do it/delegate it to someone else or leave a > TODO+file a bug. Let me do it in a different CL. Added TODO and filed a bug.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: cast_shell_android on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
The CQ bit was checked by yusukes@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
lgtm % nit https://codereview.chromium.org/2505733003/diff/40001/chrome/gpu/gpu_arc_vide... File chrome/gpu/gpu_arc_video_service.cc (right): https://codereview.chromium.org/2505733003/diff/40001/chrome/gpu/gpu_arc_vide... chrome/gpu/gpu_arc_video_service.cc:273: converted_planes.emplace_back(0, 0); Nit: it might actually be better to just report an error to client with INVALID_ARGUMENT via OnError...
The CQ bit was checked by yusukes@chromium.org to run a CQ dry run
https://codereview.chromium.org/2505733003/diff/40001/chrome/gpu/gpu_arc_vide... File chrome/gpu/gpu_arc_video_service.cc (right): https://codereview.chromium.org/2505733003/diff/40001/chrome/gpu/gpu_arc_vide... chrome/gpu/gpu_arc_video_service.cc:273: converted_planes.emplace_back(0, 0); On 2016/11/17 01:24:18, Pawel Osciak wrote: > Nit: it might actually be better to just report an error to client with > INVALID_ARGUMENT via OnError... Done.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
thanks, still lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by yusukes@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from lhchavez@chromium.org Link to the patchset: https://codereview.chromium.org/2505733003/#ps60001 (title: "address comment from Pawel")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Failed to apply patch for components/arc/BUILD.gn:
While running git apply --index -p1;
error: patch failed: components/arc/BUILD.gn:131
error: components/arc/BUILD.gn: patch does not apply
Patch: components/arc/BUILD.gn
Index: components/arc/BUILD.gn
diff --git a/components/arc/BUILD.gn b/components/arc/BUILD.gn
index
2a28210490d2a3beb170ab079bc36b10bbbdfa4b..337ac57705342c6ae360ce3b54e9eb77b2fa4366
100644
--- a/components/arc/BUILD.gn
+++ b/components/arc/BUILD.gn
@@ -131,7 +131,6 @@ mojom("arc_bindings_old_types") {
"common/bluetooth.mojom",
"common/enterprise_reporting.mojom",
"common/net.mojom",
- "common/video_accelerator.mojom",
]
public_deps = [
@@ -167,6 +166,7 @@ mojom("arc_bindings") {
"common/storage_manager.mojom",
"common/tts.mojom",
"common/video.mojom",
+ "common/video_accelerator.mojom",
"common/wallpaper.mojom",
]
The CQ bit was checked by yusukes@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from lhchavez@chromium.org, posciak@chromium.org Link to the patchset: https://codereview.chromium.org/2505733003/#ps80001 (title: "final rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== arc: enable use_new_wrapper_types for video_accelerator.mojom BUG=662510 BUG=624136 TEST=try ========== to ========== arc: enable use_new_wrapper_types for video_accelerator.mojom BUG=662510 BUG=624136 TEST=try Committed: https://crrev.com/fd879031d441f0cf3927dac7762b20630d314c6b Cr-Commit-Position: refs/heads/master@{#432961} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/fd879031d441f0cf3927dac7762b20630d314c6b Cr-Commit-Position: refs/heads/master@{#432961} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
