Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1929053003:
GN: Add video_{de,en}code_accelerator_unittest targets (Closed)
DescriptionGN: Add video_{de,en}code_accelerator_unittest targets
BUG=607362
Committed: https://crrev.com/3002ea53f5070f13db473184f0997686b4ca81cb
Cr-Commit-Position: refs/heads/master@{#390731}
Patch Set 1 #Patch Set 2 : Fixes #
Total comments: 6
Dependent Patchsets: Messages
Total messages: 22 (9 generated)
stevenjb@chromium.org changed reviewers: + dpranke@google.com, ihf@google.com, piman@chromium.org
The CQ bit was checked by stevenjb@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1929053003/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1929053003/1
dpranke@chromium.org changed reviewers: + dpranke@chromium.org
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromeos_amd64-generic_chromium_compile_only_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...) chromeos_x86-generic_chromium_compile_only_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)
The CQ bit was checked by stevenjb@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1929053003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1929053003/20001
lgtm https://codereview.chromium.org/1929053003/diff/20001/content/test/BUILD.gn File content/test/BUILD.gn (right): https://codereview.chromium.org/1929053003/diff/20001/content/test/BUILD.gn#n... content/test/BUILD.gn:929: } nit: Why is this needed? the sources don't include anything from third_party/libva. Is it a missing exported config from a dependency? https://codereview.chromium.org/1929053003/diff/20001/content/test/BUILD.gn#n... content/test/BUILD.gn:983: "//third_party/libyuv:libyuv_config", ditto (both)
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
dpranke@ - thoughts on comments by piman@? I'm happy to work on a more correct fix with a little guidance. https://codereview.chromium.org/1929053003/diff/20001/content/test/BUILD.gn File content/test/BUILD.gn (right): https://codereview.chromium.org/1929053003/diff/20001/content/test/BUILD.gn#n... content/test/BUILD.gn:929: } On 2016/04/28 23:36:48, piman OOO back 2016-5-2 wrote: > nit: Why is this needed? the sources don't include anything from > third_party/libva. Is it a missing exported config from a dependency? Otherwise we get this: In file included from ../../content/common/gpu/media/video_decode_accelerator_unittest.cc:71: In file included from ../../content/common/gpu/media/vaapi_video_decode_accelerator.h:32: In file included from ../../content/common/gpu/media/vaapi_wrapper.h:25: In file included from ../../content/common/gpu/media/va_surface.h:15: ../../third_party/libva/va/va.h:83:10: fatal error: 'va/va_version.h' file not found #include <va/va_version.h> This was copeid from the .gyp file. I'm not sure if there is a better way to do this in GN? https://codereview.chromium.org/1929053003/diff/20001/content/test/BUILD.gn#n... content/test/BUILD.gn:983: "//third_party/libyuv:libyuv_config", On 2016/04/28 23:36:48, piman OOO back 2016-5-2 wrote: > ditto (both) ditto
I'm going to move forward with this as-is, we can always clean it up later.
The CQ bit was checked by stevenjb@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dpranke@chromium.org Link to the patchset: https://codereview.chromium.org/1929053003/#ps20001 (title: "Fixes")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1929053003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1929053003/20001
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
https://codereview.chromium.org/1929053003/diff/20001/content/test/BUILD.gn File content/test/BUILD.gn (right): https://codereview.chromium.org/1929053003/diff/20001/content/test/BUILD.gn#n... content/test/BUILD.gn:929: } On 2016/04/29 17:17:56, stevenjb wrote: > On 2016/04/28 23:36:48, piman OOO back 2016-5-2 wrote: > > nit: Why is this needed? the sources don't include anything from > > third_party/libva. Is it a missing exported config from a dependency? > > Otherwise we get this: > > In file included from > ../../content/common/gpu/media/video_decode_accelerator_unittest.cc:71: > In file included from > ../../content/common/gpu/media/vaapi_video_decode_accelerator.h:32: > In file included from ../../content/common/gpu/media/vaapi_wrapper.h:25: > In file included from ../../content/common/gpu/media/va_surface.h:15: > ../../third_party/libva/va/va.h:83:10: fatal error: 'va/va_version.h' file not > found > #include <va/va_version.h> > > This was copeid from the .gyp file. I'm not sure if there is a better way to do > this in GN? //content/common should be exporting this as a public_config as well as listing it in a config. These test binaries should probably also be depending on the sub-componets of content like content_common directly, rather than content as a whole. https://codereview.chromium.org/1929053003/diff/20001/content/test/BUILD.gn#n... content/test/BUILD.gn:983: "//third_party/libyuv:libyuv_config", On 2016/04/29 17:17:57, stevenjb wrote: > On 2016/04/28 23:36:48, piman OOO back 2016-5-2 wrote: > > ditto (both) > > ditto ditto.
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/3002ea53f5070f13db473184f0997686b4ca81cb Cr-Commit-Position: refs/heads/master@{#390731}
Message was sent while issue was closed.
Description was changed from
==========
GN: Add video_{de,en}code_accelerator_unittest targets
BUG=607362
==========
to
==========
GN: Add video_{de,en}code_accelerator_unittest targets
BUG=607362
Committed: https://crrev.com/3002ea53f5070f13db473184f0997686b4ca81cb
Cr-Commit-Position: refs/heads/master@{#390731}
==========
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
