New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 608241 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature



Sign in to add a comment

Move from content/common/gpu/media to media/gpu required git-cl-format conformity, breaking readability of some code

Project Member Reported by markdittmer@chromium.org, May 2 2016

Issue description

This bug is in response to the following code review comments on https://codereview.chromium.org/1882373004/:


https://codereview.chromium.org/1882373004/diff/120001/media/gpu/h264_decoder.cc
File media/gpu/h264_decoder.cc (right):

https://codereview.chromium.org/1882373004/diff/120001/media/gpu/h264_decoder...
media/gpu/h264_decoder.cc:1315: // else fallthrough
I believe the indent here was correct.

https://codereview.chromium.org/1882373004/diff/120001/media/gpu/ipc/service/...
File media/gpu/ipc/service/gpu_video_decode_accelerator_factory_impl.cc (right):

https://codereview.chromium.org/1882373004/diff/120001/media/gpu/ipc/service/...
media/gpu/ipc/service/gpu_video_decode_accelerator_factory_impl.cc:72: // Query
VDAs for their capabilities and construct a set of supported
I think this should also stay at the same indentation level.

https://codereview.chromium.org/1882373004/diff/120001/media/gpu/ipc/service/...
File media/gpu/ipc/service/gpu_video_encode_accelerator.cc (right):

https://codereview.chromium.org/1882373004/diff/120001/media/gpu/ipc/service/...
media/gpu/ipc/service/gpu_video_encode_accelerator.cc:80: "input_format="
<< "input_format=" << input_format
?

https://codereview.chromium.org/1882373004/diff/120001/media/gpu/ipc/service/...
media/gpu/ipc/service/gpu_video_encode_accelerator.cc:271: "frame_id="
<< "frame_id=" << params.frame_id;
?

This also happens in more places in this file.

https://codereview.chromium.org/1882373004/diff/120001/media/gpu/ipc/service/...
media/gpu/ipc/service/gpu_video_encode_accelerator.cc:311:
frame->AddDestructionObserver(media::BindToCurrentLoop(base::Bind(
To be honest, I'd prefer keeping original formatting of most changes in this
file. This is much less readable...

https://codereview.chromium.org/1882373004/diff/120001/media/gpu/v4l2_slice_v...
File media/gpu/v4l2_slice_video_decode_accelerator.cc (right):

https://codereview.chromium.org/1882373004/diff/120001/media/gpu/v4l2_slice_v...
media/gpu/v4l2_slice_video_decode_accelerator.cc:1936:
arraysize(pps->scaling_list4x4) &&
Is this correct formatting?

https://codereview.chromium.org/1882373004/diff/120001/media/gpu/vaapi_jpeg_d...
File media/gpu/vaapi_jpeg_decode_accelerator.cc (right):

https://codereview.chromium.org/1882373004/diff/120001/media/gpu/vaapi_jpeg_d...
media/gpu/vaapi_jpeg_decode_accelerator.cc:29: VAJDA_DECODER_FAILURES_MAX,
Could we please keep the comment to explain why +1?

https://codereview.chromium.org/1882373004/diff/120001/media/gpu/vaapi_video_...
File media/gpu/vaapi_video_decode_accelerator.cc (right):

https://codereview.chromium.org/1882373004/diff/120001/media/gpu/vaapi_video_...
media/gpu/vaapi_video_decode_accelerator.cc:490: // else fallthrough
Please keep the original indent.

https://codereview.chromium.org/1882373004/diff/120001/media/gpu/vaapi_video_...
File media/gpu/vaapi_video_encode_accelerator.cc (right):

https://codereview.chromium.org/1882373004/diff/120001/media/gpu/vaapi_video_...
media/gpu/vaapi_video_encode_accelerator.cc:75: VAVEA_ENCODER_FAILURES_MAX,
Could we please keep the comment at the new location? However, to be honest, it
feels to me that +1 belonged here, since we won't need it anymore if more values
are added...


The patch couldn't land with fixes to these issues because //media requires that all code conform to the output of "git cl format media". If the identified readability concerns are to be addressed, then presubmit checks on //media code may need to be modified.
 
Owner: markdittmer@chromium.org
Status: Assigned (was: Untriaged)
Owner: posciak@chromium.org
Project Member

Comment 3 by bugdroid1@chromium.org, May 18 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d6a739e430d351bf40bf175f1ca15f5f28007d6f

commit d6a739e430d351bf40bf175f1ca15f5f28007d6f
Author: posciak <posciak@chromium.org>
Date: Wed May 18 03:16:26 2016

media/gpu: restore some of the reformatting done during migration to media/gpu.

This change is purely cosmetic.

Restore some of the formatting lost after automated reformat during the
move to media/gpu.

BUG= 608241 
TEST=compile
NOPRESUBMIT=true

Review-Url: https://codereview.chromium.org/1973753003
Cr-Commit-Position: refs/heads/master@{#394318}

[modify] https://crrev.com/d6a739e430d351bf40bf175f1ca15f5f28007d6f/media/gpu/gpu_video_decode_accelerator_factory_impl.cc
[modify] https://crrev.com/d6a739e430d351bf40bf175f1ca15f5f28007d6f/media/gpu/h264_decoder.cc
[modify] https://crrev.com/d6a739e430d351bf40bf175f1ca15f5f28007d6f/media/gpu/ipc/service/gpu_video_encode_accelerator.cc
[modify] https://crrev.com/d6a739e430d351bf40bf175f1ca15f5f28007d6f/media/gpu/jpeg_decode_accelerator_unittest.cc
[modify] https://crrev.com/d6a739e430d351bf40bf175f1ca15f5f28007d6f/media/gpu/rendering_helper.cc
[modify] https://crrev.com/d6a739e430d351bf40bf175f1ca15f5f28007d6f/media/gpu/v4l2_device.cc
[modify] https://crrev.com/d6a739e430d351bf40bf175f1ca15f5f28007d6f/media/gpu/v4l2_image_processor.cc
[modify] https://crrev.com/d6a739e430d351bf40bf175f1ca15f5f28007d6f/media/gpu/v4l2_jpeg_decode_accelerator.cc
[modify] https://crrev.com/d6a739e430d351bf40bf175f1ca15f5f28007d6f/media/gpu/v4l2_slice_video_decode_accelerator.cc
[modify] https://crrev.com/d6a739e430d351bf40bf175f1ca15f5f28007d6f/media/gpu/v4l2_video_decode_accelerator.cc
[modify] https://crrev.com/d6a739e430d351bf40bf175f1ca15f5f28007d6f/media/gpu/v4l2_video_encode_accelerator.cc
[modify] https://crrev.com/d6a739e430d351bf40bf175f1ca15f5f28007d6f/media/gpu/vaapi_video_decode_accelerator.cc
[modify] https://crrev.com/d6a739e430d351bf40bf175f1ca15f5f28007d6f/media/gpu/vaapi_wrapper.cc
[modify] https://crrev.com/d6a739e430d351bf40bf175f1ca15f5f28007d6f/media/gpu/video_decode_accelerator_unittest.cc
[modify] https://crrev.com/d6a739e430d351bf40bf175f1ca15f5f28007d6f/media/gpu/video_encode_accelerator_unittest.cc

Status: Fixed (was: Assigned)

Sign in to add a comment