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

Issue 876810 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 25
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug

Blocking:
issue 868400



Sign in to add a comment

Fold VaapiJpegDecoder into VaapiJpegDecodeAccelerator

Project Member Reported by mcasas@chromium.org, Aug 22

Issue description

VaapiJpegDecoder is a class with only one static method: Decode();
it only has one caller: VaapiJpegDecodeAccelerator (and an unused,
uncompiled and not run unittest). Consider folding the former into
the latter.

 
Also -- not to forget: 

Banned functions were used.
    media/gpu/vaapi/vaapi_jpeg_decode_accelerator.cc:159:
      arraysize is deprecated, please use base::size(array) instead 
      ( https://crbug.com/837308 ). 
    media/gpu/vaapi/vaapi_jpeg_decode_accelerator.cc:166:
      arraysize is deprecated, please use base::size(array) instead 
      ( https://crbug.com/837308 ). 
    media/gpu/vaapi/vaapi_jpeg_decode_accelerator.cc:168:
      arraysize is deprecated, please use base::size(array) instead 
      ( https://crbug.com/837308 ). 
    media/gpu/vaapi/vaapi_jpeg_decode_accelerator.cc:191:
      arraysize is deprecated, please use base::size(array) instead 
      ( https://crbug.com/837308 ). 
    media/gpu/vaapi/vaapi_jpeg_decode_accelerator_unittest.cc:42:
      Please consider using base::Bind{Once,Repeating} instead
      of base::Bind. (crbug.com/714018)
    media/gpu/vaapi/vaapi_jpeg_decode_accelerator_unittest.cc:42:
      Please consider using base::{Once,Repeating}Closure instead
      of base::Closure. (crbug.com/714018)

Project Member

Comment 2 by bugdroid1@chromium.org, Aug 23

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

commit 9fa94c6e876dbef4a89c2472a1e0fc31c7f059df
Author: Miguel Casas <mcasas@chromium.org>
Date: Thu Aug 23 19:47:47 2018

VaJpegDA: Fold vaapi_jpeg_decoder.* into vaapi_jpeg_decode_accelerator

This CL folds VaapiJpegDecoder, a class with only one static method,
into its only caller VaapiJpegDecodeAccelerator. The static method,
VaapiJpegDecoder::Decode(), is renamed to DoDecode().  This method and
the bunch of file-static ones are moved verbatim -- untouched.

It also takes the chance to clean up: removes unused includes and
member variables, and forward declares as much as possible (otherwise
va/va.h, a pretty large file was transitively included, ugh).

All in all, less lines !

Test: jpeg_decode_accelerator_unittests passing as before.
Bug:  876810 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I1f37f378121840d6d1eb83018c5122bb0ed6b01c
Reviewed-on: https://chromium-review.googlesource.com/1185218
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585582}
[modify] https://crrev.com/9fa94c6e876dbef4a89c2472a1e0fc31c7f059df/media/gpu/jpeg_decode_accelerator_unittest.cc
[modify] https://crrev.com/9fa94c6e876dbef4a89c2472a1e0fc31c7f059df/media/gpu/vaapi/BUILD.gn
[modify] https://crrev.com/9fa94c6e876dbef4a89c2472a1e0fc31c7f059df/media/gpu/vaapi/vaapi_jpeg_decode_accelerator.cc
[modify] https://crrev.com/9fa94c6e876dbef4a89c2472a1e0fc31c7f059df/media/gpu/vaapi/vaapi_jpeg_decode_accelerator.h
[rename] https://crrev.com/9fa94c6e876dbef4a89c2472a1e0fc31c7f059df/media/gpu/vaapi/vaapi_jpeg_decode_accelerator_unittest.cc
[delete] https://crrev.com/cac8d9a057dc44af44e992d0953221d30aeb3cfc/media/gpu/vaapi/vaapi_jpeg_decoder.cc
[delete] https://crrev.com/cac8d9a057dc44af44e992d0953221d30aeb3cfc/media/gpu/vaapi/vaapi_jpeg_decoder.h

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 24

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

commit d001079e29d03a3ce2a90e32b98b6611a7fe9811
Author: Miguel Casas <mcasas@chromium.org>
Date: Fri Aug 24 22:17:14 2018

cleanup style VaapiVP8Accelerator::SubmitDecode

This CL cleans up a bit said method VaapiVP8Accelerator::SubmitDecode():
- Adds {} where needed (either condition or body > 1 line).
- Removes memset() of a struct, instead using = {}, and moves the
 variable declaration closer to its use, in some cases.
- s/std::min(std::max())/base::ClampToRange()/

No new functionality intended.  This code is only used in ChromeOS.

Bug:  876810 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: If2fb903a427dea2424743217e569d09cdadefe22
Reviewed-on: https://chromium-review.googlesource.com/1188615
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586020}
[modify] https://crrev.com/d001079e29d03a3ce2a90e32b98b6611a7fe9811/media/gpu/vaapi/vaapi_vp8_accelerator.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 25

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

commit 47cbea6beb4d39baa2651a7a151bc8a42dcdce36
Author: Miguel Casas <mcasas@chromium.org>
Date: Sat Aug 25 15:23:13 2018

vaapi_jpeg_decoder{_unittest}: address some presubmit warnings

- s/arraysize/base::size/ (see  crbug.com/837308#c15  for decltype trick).
- s/base::bind/base::BindRepeating/ (and s/Closure/RepeatingClosure/).

Bug:  876810 ,  837308 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I0068d55985c5c4fdcf1ed987e278f402fd0e6ac0
Reviewed-on: https://chromium-review.googlesource.com/1189103
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586137}
[modify] https://crrev.com/47cbea6beb4d39baa2651a7a151bc8a42dcdce36/media/gpu/vaapi/vaapi_jpeg_decode_accelerator.cc
[modify] https://crrev.com/47cbea6beb4d39baa2651a7a151bc8a42dcdce36/media/gpu/vaapi/vaapi_jpeg_decode_accelerator_unittest.cc

Status: Fixed (was: Started)

Sign in to add a comment