Add support for creating HW codecs in VideoProcessor tests.

Add support for creating HW codecs in the VideoProcessor tests.

This CL adds the ability to _create_ HW codecs (Android and iOS) in the
VideoProcessor integration tests. Since the VideoProcessor class is not thread
safe yet, this CL does not add the ability to _use_ HW codecs in the tests. A
follow-up CL is planned that will add this ability.

This CL further adds a separate build target which is used to separate the
"plot" versions of the integration tests from the "correctness" versions. The
former will be run manually on devices, whereas the latter are used on the
trybots/buildbots to find regressions in the SW codecs. The underlying test
is the same, however.

BUG=webrtc:6634
Review-Url: https://codereview.webrtc.org/2695653002
Cr-Commit-Position: refs/heads/master@{#16716}
Committed: https://chromium.googlesource.com/external/webrtc/+/6bb8e0efd3187e11ac512d84d665b4cffacad43e

[brandtr, 2017-02-13 10:52:59]

Patchset #1 (id:1) has been deleted

[brandtr, 2017-02-13 10:55:46]

Patchset #1 (id:20001) has been deleted

[brandtr, 2017-02-13 11:43:59]

Description was changed from

==========
Add support for creating HW codecs in VideoProcessor tests.

This CL adds the ability to _create_ HW codecs (Android and iOS) in the
VideoProcessor integration tests. Since the VideoProcessor class is not thread
safe yet, this CL does not add the ability to _use_ HW codecs in the tests. A
follow-up CL is planned that will add this ability.

This CL further adds a separate build target which is used to separate the
"plot" versions of the integrations tests to the "correctness" versions. The
former will be run manually on devices, whereas the latter are used on the
trybots/buildbots to find regressions. The underlying test is the same, however.

BUG=webrtc:6634
==========

to

==========
Add support for creating HW codecs in the VideoProcessor tests.

This CL adds the ability to _create_ HW codecs (Android and iOS) in the
VideoProcessor integration tests. Since the VideoProcessor class is not thread
safe yet, this CL does not add the ability to _use_ HW codecs in the tests. A
follow-up CL is planned that will add this ability.

This CL further adds a separate build target which is used to separate the
"plot" versions of the integration tests from the "correctness" versions. The
former will be run manually on devices, whereas the latter are used on the
trybots/buildbots to find regressions. The underlying test is the same, however.

BUG=webrtc:6634
==========

[brandtr, 2017-02-13 12:40:20]

Description was changed from

==========
Add support for creating HW codecs in the VideoProcessor tests.

This CL adds the ability to _create_ HW codecs (Android and iOS) in the
VideoProcessor integration tests. Since the VideoProcessor class is not thread
safe yet, this CL does not add the ability to _use_ HW codecs in the tests. A
follow-up CL is planned that will add this ability.

This CL further adds a separate build target which is used to separate the
"plot" versions of the integration tests from the "correctness" versions. The
former will be run manually on devices, whereas the latter are used on the
trybots/buildbots to find regressions. The underlying test is the same, however.

BUG=webrtc:6634
==========

to

==========
Add support for creating HW codecs in the VideoProcessor tests.

This CL adds the ability to _create_ HW codecs (Android and iOS) in the
VideoProcessor integration tests. Since the VideoProcessor class is not thread
safe yet, this CL does not add the ability to _use_ HW codecs in the tests. A
follow-up CL is planned that will add this ability.

This CL further adds a separate build target which is used to separate the
"plot" versions of the integration tests from the "correctness" versions. The
former will be run manually on devices, whereas the latter are used on the
trybots/buildbots to find regressions in the SW codecs. The underlying test
is the same, however.

BUG=webrtc:6634
==========

[brandtr, 2017-02-13 12:40:26]

brandtr@webrtc.org changed reviewers:
+ asapersson@webrtc.org, kjellander@webrtc.org, sakal@webrtc.org,
sprang@webrtc.org

[brandtr, 2017-02-13 12:40:27]

Please take a look.

asapersson: General review, incl. VideoProcessor changes.
sakal: android_test_initializer
kjellander: BUILD and DEPS files.
sprang: video_coding OWNER :)

https://codereview.webrtc.org/2695653002/diff/40001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/BUILD.gn (right):

https://codereview.webrtc.org/2695653002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/BUILD.gn:414: "//resources/foreman_128x96.yuv",
These need to be uploaded to Gstorage.

https://codereview.webrtc.org/2695653002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/BUILD.gn:456:
"../../sdk/android:libjingle_peerconnection_java",
When I put this dependency in :video_coding_videoprocessor_integration_test, the
corresponding .java files are not put in the apk, leading to runtime errors.

https://codereview.webrtc.org/2695653002/diff/40001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/codecs/test/android_test_initializer.cc
(right):

https://codereview.webrtc.org/2695653002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/codecs/test/android_test_initializer.cc:11: #include
<pthread.h>
This file is inspired by
https://cs.chromium.org/chromium/src/third_party/webrtc/pc/test/androidtestin...
and
https://cs.chromium.org/chromium/src/third_party/webrtc/modules/audio_device/....

The point of this code is to dynamically load the Java classes from
webrtc/sdk/android/src/jni, so that we can instantiate Android HW codecs in the
C++ code. Since we don't control the JNI_OnLoad method (that one is defined in
the NativeTest harness of //testing/android), we explicitly load our Java
classes using appropriate calls to the functions in this file.

https://codereview.webrtc.org/2695653002/diff/40001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/codecs/test/videoprocessor.h (left):

https://codereview.webrtc.org/2695653002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/codecs/test/videoprocessor.h:250: RTC_NOTREACHED();
Android HW decoders use this callback.

[sakal, 2017-02-13 12:52:39]

android_test_initializer lgtm.

It is really nice that we are finally getting some test coverage for the
hardware encoders.

[kjellander_webrtc, 2017-02-13 14:31:08]

https://codereview.webrtc.org/2695653002/diff/40001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/BUILD.gn (right):

https://codereview.webrtc.org/2695653002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/BUILD.gn:416: "//resources/foreman_176x144.yuv",
We already have //resources/paris_qcif.yuv or do we have to have the same video
for the 176x144 size?

https://codereview.webrtc.org/2695653002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/BUILD.gn:418: "//resources/foreman_352x288.yuv",
352×288 is the size of cif, so this file should be the same as
//resources/foreman_cif.yuv, which seems redundant I guess we could just add
another copy of that .sha1 file with another filename if you rely on the
specific naming of the files for the test.

https://codereview.webrtc.org/2695653002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/BUILD.gn:432: # This executable is meant for local
codec perf testing and should not be run
Interesting. Is the perf testing that is performed covered by our existing perf
tests in other places then?

https://codereview.webrtc.org/2695653002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/BUILD.gn:456:
"../../sdk/android:libjingle_peerconnection_java",
On 2017/02/13 12:40:26, brandtr wrote:
> When I put this dependency in :video_coding_videoprocessor_integration_test,
the
> corresponding .java files are not put in the apk, leading to runtime errors.

Mirko: You can probably explain this better than me.  Is there a supported way
to do that brandtr@ wishes here?

https://codereview.webrtc.org/2695653002/diff/40001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/DEPS (right):

https://codereview.webrtc.org/2695653002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/DEPS:2: "+base",
Please don't add this so widely. Put it in something stricter instead, like
this:

specific_include_rules = {
  "codecs/test/android_test_initializer\.cc": [
    "+base/android",
  ],
}

https://codereview.webrtc.org/2695653002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/DEPS:9: "+webrtc/media/engine",
I'm not sure what the plan is moving forward here (if video coding production
code is going to depend on these in the future), but let's keep this strict as
well for now:

specific_include_rules = {
  ".*test\.cc": [
     "+webrtc/media/engine",
     "+webrtc/sdk",
  ],
}

[brandtr, 2017-02-13 16:23:02]

Thanks for quick feedback.

+mbonadei for question from kjellander.

https://codereview.webrtc.org/2695653002/diff/40001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/BUILD.gn (right):

https://codereview.webrtc.org/2695653002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/BUILD.gn:416: "//resources/foreman_176x144.yuv",
On 2017/02/13 14:31:07, kjellander_webrtc wrote:
> We already have //resources/paris_qcif.yuv or do we have to have the same
video
> for the 176x144 size?

Will reuse existing one.

https://codereview.webrtc.org/2695653002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/BUILD.gn:418: "//resources/foreman_352x288.yuv",
On 2017/02/13 14:31:08, kjellander_webrtc wrote:
> 352×288 is the size of cif, so this file should be the same as
> //resources/foreman_cif.yuv, which seems redundant I guess we could just add
> another copy of that .sha1 file with another filename if you rely on the
> specific naming of the files for the test.

Thanks for catching this. The same file should be fine, but I'll check with
asapersson@.

https://codereview.webrtc.org/2695653002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/BUILD.gn:432: # This executable is meant for local
codec perf testing and should not be run
On 2017/02/13 14:31:07, kjellander_webrtc wrote:
> Interesting. Is the perf testing that is performed covered by our existing
perf
> tests in other places then?

Yes. The idea is that the existing perf tests in
videoprocessor_integrationtest.cc are run in "correctness" mode (EXPECTs turned
on) on the regular bots, whereas the new perf tests in
plot_videoprocessor_integrationtest.cc are run manually in "perf mode" (EXPECTs
turned off). The former are for catching regressions, whereas the latter are for
active experimentation on a device. The former already run on the bots, whereas
the latter shouldn't.

I created this build target because that was the simplest way that I could think
of for building the tests in plot_videoprocessor_integration.test, without
adding them to the bots. It's my understanding that all gtests that are added to
the modules_tests target will run on the bots?

https://codereview.webrtc.org/2695653002/diff/40001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/DEPS (right):

https://codereview.webrtc.org/2695653002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/DEPS:2: "+base",
On 2017/02/13 14:31:08, kjellander_webrtc wrote:
> Please don't add this so widely. Put it in something stricter instead, like
> this:
> 
> specific_include_rules = {
>   "codecs/test/android_test_initializer\.cc": [
>     "+base/android",
>   ],
> }

Sounds good. I wasn't aware of "specific_include_rules".

https://codereview.webrtc.org/2695653002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/DEPS:9: "+webrtc/media/engine",
On 2017/02/13 14:31:08, kjellander_webrtc wrote:
> I'm not sure what the plan is moving forward here (if video coding production
> code is going to depend on these in the future), but let's keep this strict as
> well for now:
> 
> specific_include_rules = {
>   ".*test\.cc": [
>      "+webrtc/media/engine",
>      "+webrtc/sdk",
>   ],
> }

Done.

[mbonadei, 2017-02-13 21:19:09]

mbonadei@webrtc.org changed reviewers:
+ mbonadei@webrtc.org

[mbonadei, 2017-02-13 21:19:10]

https://codereview.webrtc.org/2695653002/diff/40001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/BUILD.gn (right):

https://codereview.webrtc.org/2695653002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/BUILD.gn:456:
"../../sdk/android:libjingle_peerconnection_java",
On 2017/02/13 14:31:08, kjellander_webrtc wrote:
> On 2017/02/13 12:40:26, brandtr wrote:
> > When I put this dependency in :video_coding_videoprocessor_integration_test,
> the
> > corresponding .java files are not put in the apk, leading to runtime errors.
> 
> Mirko: You can probably explain this better than me.  Is there a supported way
> to do that brandtr@ wishes here?

Unfortunately I came across a similar problem some weeks ago [1] and it seems
that "deps" are not transitive in some Gn templates (but in this case there
should also be another problem). 

I have also tried to take a look at 'rtc_test' and 'test' templates but they are
not easy to grasp.

I try to explain my theory... I am not 100% sure about everything but hopefully
it will explain some parts of the problem. The target type 'rtc_source_set' is
basically a wrapper around 'source_set' and it works as expected (but in that
case we are dealing with C++ code). When we have to deal with Java code we have
to use templates and their behavior is slightly different (I don't know how they
integrate with native Gn targets behind the curtain). In this case we are adding
a dependency with Java code to an 'rtc_source_set' and I think that this is not
supported, while adding a Java dependency to 'rtc_test' works because 'rtc_test'
is based on 'test' [2] which is a template that deals with Java code [3] by
design.

Henrik: Maybe we can create a bug and try to shed some light on this. I can do
that, not in the next couple of weeks but I can try to understand and write
something. The integration between native targets and templates seems to be
tricky.

[1] -
https://codereview.webrtc.org/2646443002/diff/1/webrtc/examples/BUILD.gn#newc...
[2] -
https://chromium.googlesource.com/external/webrtc/+/master/webrtc/webrtc.gni#232
[3] -
https://cs.chromium.org/chromium/src/testing/test.gni?q=testing/test+package:...

[brandtr, 2017-02-14 08:35:26]

https://codereview.webrtc.org/2695653002/diff/40001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/BUILD.gn (right):

https://codereview.webrtc.org/2695653002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/BUILD.gn:456:
"../../sdk/android:libjingle_peerconnection_java",
On 2017/02/13 21:19:10, mbonadei wrote:
> On 2017/02/13 14:31:08, kjellander_webrtc wrote:
> > On 2017/02/13 12:40:26, brandtr wrote:
> > > When I put this dependency in
:video_coding_videoprocessor_integration_test,
> > the
> > > corresponding .java files are not put in the apk, leading to runtime
errors.
> > 
> > Mirko: You can probably explain this better than me.  Is there a supported
way
> > to do that brandtr@ wishes here?
> 
> Unfortunately I came across a similar problem some weeks ago [1] and it seems
> that "deps" are not transitive in some Gn templates (but in this case there
> should also be another problem). 
> 
> I have also tried to take a look at 'rtc_test' and 'test' templates but they
are
> not easy to grasp.
> 
> I try to explain my theory... I am not 100% sure about everything but
hopefully
> it will explain some parts of the problem. The target type 'rtc_source_set' is
> basically a wrapper around 'source_set' and it works as expected (but in that
> case we are dealing with C++ code). When we have to deal with Java code we
have
> to use templates and their behavior is slightly different (I don't know how
they
> integrate with native Gn targets behind the curtain). In this case we are
adding
> a dependency with Java code to an 'rtc_source_set' and I think that this is
not
> supported, while adding a Java dependency to 'rtc_test' works because
'rtc_test'
> is based on 'test' [2] which is a template that deals with Java code [3] by
> design.
> 
> Henrik: Maybe we can create a bug and try to shed some light on this. I can do
> that, not in the next couple of weeks but I can try to understand and write
> something. The integration between native targets and templates seems to be
> tricky.
> 
> [1] -
>
https://codereview.webrtc.org/2646443002/diff/1/webrtc/examples/BUILD.gn#newc...
> [2] -
>
https://chromium.googlesource.com/external/webrtc/+/master/webrtc/webrtc.gni#232
> [3] -
>
https://cs.chromium.org/chromium/src/testing/test.gni?q=testing/test+package:...

Your theory is compatible with my experience: adding the Java dep to the
'rtc_test' target works, but adding it to a 'rtc_source_set' that is being
depended on does not work.

[kjellander_webrtc, 2017-02-14 11:00:15]

lgtm but please have a look at my comments.

https://codereview.webrtc.org/2695653002/diff/40001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/BUILD.gn (right):

https://codereview.webrtc.org/2695653002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/BUILD.gn:416: "//resources/foreman_176x144.yuv",
On 2017/02/13 16:23:02, brandtr wrote:
> On 2017/02/13 14:31:07, kjellander_webrtc wrote:
> > We already have //resources/paris_qcif.yuv or do we have to have the same
> video
> > for the 176x144 size?
> 
> Will reuse existing one.

Were you going to reuse paris_qcif.yuv? It seems you now added a
foreman_qcif.yuv instead.

https://codereview.webrtc.org/2695653002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/BUILD.gn:432: # This executable is meant for local
codec perf testing and should not be run
On 2017/02/13 16:23:02, brandtr wrote:
> On 2017/02/13 14:31:07, kjellander_webrtc wrote:
> > Interesting. Is the perf testing that is performed covered by our existing
> perf
> > tests in other places then?
> 
> Yes. The idea is that the existing perf tests in
> videoprocessor_integrationtest.cc are run in "correctness" mode (EXPECTs
turned
> on) on the regular bots, whereas the new perf tests in
> plot_videoprocessor_integrationtest.cc are run manually in "perf mode"
(EXPECTs
> turned off). The former are for catching regressions, whereas the latter are
for
> active experimentation on a device. The former already run on the bots,
whereas
> the latter shouldn't.
> 
> I created this build target because that was the simplest way that I could
think
> of for building the tests in plot_videoprocessor_integration.test, without
> adding them to the bots. It's my understanding that all gtests that are added
to
> the modules_tests target will run on the bots?

Acknowledged. Thanks for explaining.

https://codereview.webrtc.org/2695653002/diff/60001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/DEPS (right):

https://codereview.webrtc.org/2695653002/diff/60001/webrtc/modules/video_codi...
webrtc/modules/video_coding/DEPS:13: "android_test_initializer\.cc": [
Don't you need the codecs/test/ in the path? Maybe it's not needed...

https://codereview.webrtc.org/2695653002/diff/60001/webrtc/modules/video_codi...
webrtc/modules/video_coding/DEPS:17: "videoprocessor_integrationtest\.h": [
I'd recommend ".*test\.cc" instead. It would avoid the extra hassle when adding
more tests in the future.

[brandtr, 2017-02-14 14:22:37]

https://codereview.webrtc.org/2695653002/diff/40001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/BUILD.gn (right):

https://codereview.webrtc.org/2695653002/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/BUILD.gn:416: "//resources/foreman_176x144.yuv",
On 2017/02/14 11:00:14, kjellander_webrtc wrote:
> On 2017/02/13 16:23:02, brandtr wrote:
> > On 2017/02/13 14:31:07, kjellander_webrtc wrote:
> > > We already have //resources/paris_qcif.yuv or do we have to have the same
> > video
> > > for the 176x144 size?
> > 
> > Will reuse existing one.
> 
> Were you going to reuse paris_qcif.yuv? It seems you now added a
> foreman_qcif.yuv instead.

Oops, I must have confused this comment with the next.

We do not want to reuse paris_qcif.yuv here, since when comparing metrics we
would like to have the same source material for all resolutions.

https://codereview.webrtc.org/2695653002/diff/60001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/DEPS (right):

https://codereview.webrtc.org/2695653002/diff/60001/webrtc/modules/video_codi...
webrtc/modules/video_coding/DEPS:13: "android_test_initializer\.cc": [
On 2017/02/14 11:00:15, kjellander_webrtc wrote:
> Don't you need the codecs/test/ in the path? Maybe it's not needed...

Without that prefix, 'git cl presubmit' passes, but with the prefix I get
errors:

** Presubmit ERRORS **
You added one or more #includes that violate checkdeps rules.
  webrtc/modules/video_coding/codecs/test/android_test_initializer.cc
      Illegal include: "webrtc/sdk/android/src/jni/classreferenceholder.h"
    Because of "-webrtc" from webrtc's include_rules. \
  webrtc/modules/video_coding/codecs/test/android_test_initializer.cc
      Illegal include: "webrtc/sdk/android/src/jni/jni_helpers.h"
    Because of "-webrtc" from webrtc's include_rules. \
  webrtc/modules/video_coding/codecs/test/android_test_initializer.cc
      Illegal include: "base/android/jni_android.h"
    Because of "-base" from webrtc's include_rules. \
  webrtc/modules/video_coding/codecs/test/videoprocessor_integrationtest.h
      Illegal include: "webrtc/sdk/android/src/jni/androidmediadecoder_jni.h"
    Because of "-webrtc" from webrtc's include_rules. \
  webrtc/modules/video_coding/codecs/test/videoprocessor_integrationtest.h
      Illegal include: "webrtc/sdk/android/src/jni/androidmediaencoder_jni.h"
    Because of "-webrtc" from webrtc's include_rules. \
  webrtc/modules/video_coding/codecs/test/videoprocessor_integrationtest.h
      Illegal include:
"webrtc/sdk/objc/Framework/Classes/h264_video_toolbox_decoder.h"
    Because of "-webrtc" from webrtc's include_rules. \
  webrtc/modules/video_coding/codecs/test/videoprocessor_integrationtest.h
      Illegal include:
"webrtc/sdk/objc/Framework/Classes/h264_video_toolbox_encoder.h"
    Because of "-webrtc" from webrtc's include_rules. \
  webrtc/modules/video_coding/codecs/test/videoprocessor_integrationtest.h
      Illegal include: "webrtc/media/engine/webrtcvideodecoderfactory.h"
    Because of "-webrtc" from webrtc's include_rules. \
  webrtc/modules/video_coding/codecs/test/videoprocessor_integrationtest.h
      Illegal include: "webrtc/media/engine/webrtcvideoencoderfactory.h"
    Because of "-webrtc" from webrtc's include_rules.

https://codereview.webrtc.org/2695653002/diff/60001/webrtc/modules/video_codi...
webrtc/modules/video_coding/DEPS:17: "videoprocessor_integrationtest\.h": [
On 2017/02/14 11:00:15, kjellander_webrtc wrote:
> I'd recommend ".*test\.cc" instead. It would avoid the extra hassle when
adding
> more tests in the future.

Done. In order to get it to pass, I had to do ".*test\.h" however. Do you think
I should add ".*test\.cc" as well, even though it's not strictly needed here?

(I tried to add ".*test\.{cc,h}", to no avail.)

[kjellander_webrtc, 2017-02-14 14:30:13]

still lgtm

https://codereview.webrtc.org/2695653002/diff/60001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/DEPS (right):

https://codereview.webrtc.org/2695653002/diff/60001/webrtc/modules/video_codi...
webrtc/modules/video_coding/DEPS:13: "android_test_initializer\.cc": [
On 2017/02/14 14:22:37, brandtr wrote:
> On 2017/02/14 11:00:15, kjellander_webrtc wrote:
> > Don't you need the codecs/test/ in the path? Maybe it's not needed...
> 
> Without that prefix, 'git cl presubmit' passes, but with the prefix I get
> errors:
> 
> ** Presubmit ERRORS **
> You added one or more #includes that violate checkdeps rules.
>   webrtc/modules/video_coding/codecs/test/android_test_initializer.cc
>       Illegal include: "webrtc/sdk/android/src/jni/classreferenceholder.h"
>     Because of "-webrtc" from webrtc's include_rules. \
>   webrtc/modules/video_coding/codecs/test/android_test_initializer.cc
>       Illegal include: "webrtc/sdk/android/src/jni/jni_helpers.h"
>     Because of "-webrtc" from webrtc's include_rules. \
>   webrtc/modules/video_coding/codecs/test/android_test_initializer.cc
>       Illegal include: "base/android/jni_android.h"
>     Because of "-base" from webrtc's include_rules. \
>   webrtc/modules/video_coding/codecs/test/videoprocessor_integrationtest.h
>       Illegal include: "webrtc/sdk/android/src/jni/androidmediadecoder_jni.h"
>     Because of "-webrtc" from webrtc's include_rules. \
>   webrtc/modules/video_coding/codecs/test/videoprocessor_integrationtest.h
>       Illegal include: "webrtc/sdk/android/src/jni/androidmediaencoder_jni.h"
>     Because of "-webrtc" from webrtc's include_rules. \
>   webrtc/modules/video_coding/codecs/test/videoprocessor_integrationtest.h
>       Illegal include:
> "webrtc/sdk/objc/Framework/Classes/h264_video_toolbox_decoder.h"
>     Because of "-webrtc" from webrtc's include_rules. \
>   webrtc/modules/video_coding/codecs/test/videoprocessor_integrationtest.h
>       Illegal include:
> "webrtc/sdk/objc/Framework/Classes/h264_video_toolbox_encoder.h"
>     Because of "-webrtc" from webrtc's include_rules. \
>   webrtc/modules/video_coding/codecs/test/videoprocessor_integrationtest.h
>       Illegal include: "webrtc/media/engine/webrtcvideodecoderfactory.h"
>     Because of "-webrtc" from webrtc's include_rules. \
>   webrtc/modules/video_coding/codecs/test/videoprocessor_integrationtest.h
>       Illegal include: "webrtc/media/engine/webrtcvideoencoderfactory.h"
>     Because of "-webrtc" from webrtc's include_rules.

I see. It doesn't take the path into account then (I didn't look into the
details on how the check is implemented).

https://codereview.webrtc.org/2695653002/diff/60001/webrtc/modules/video_codi...
webrtc/modules/video_coding/DEPS:17: "videoprocessor_integrationtest\.h": [
On 2017/02/14 14:22:37, brandtr wrote:
> On 2017/02/14 11:00:15, kjellander_webrtc wrote:
> > I'd recommend ".*test\.cc" instead. It would avoid the extra hassle when
> adding
> > more tests in the future.
> 
> Done. In order to get it to pass, I had to do ".*test\.h" however. Do you
think
> I should add ".*test\.cc" as well, even though it's not strictly needed here?
> 
> (I tried to add ".*test\.{cc,h}", to no avail.)

Hmm, 
By looking at other examples
(https://cs.chromium.org/chromium/src/apps/DEPS?rcl=ea69b8739b1bf1a220ea541cca...)
it seems this should work at least:
"(.*test\.cc|.*test\.h)"

[brandtr, 2017-02-14 14:34:20]

https://codereview.webrtc.org/2695653002/diff/60001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/DEPS (right):

https://codereview.webrtc.org/2695653002/diff/60001/webrtc/modules/video_codi...
webrtc/modules/video_coding/DEPS:17: "videoprocessor_integrationtest\.h": [
On 2017/02/14 14:30:13, kjellander_webrtc wrote:
> On 2017/02/14 14:22:37, brandtr wrote:
> > On 2017/02/14 11:00:15, kjellander_webrtc wrote:
> > > I'd recommend ".*test\.cc" instead. It would avoid the extra hassle when
> > adding
> > > more tests in the future.
> > 
> > Done. In order to get it to pass, I had to do ".*test\.h" however. Do you
> think
> > I should add ".*test\.cc" as well, even though it's not strictly needed
here?
> > 
> > (I tried to add ".*test\.{cc,h}", to no avail.)
> 
> Hmm, 
> By looking at other examples
>
(https://cs.chromium.org/chromium/src/apps/DEPS?rcl=ea69b8739b1bf1a220ea541cca...)
> it seems this should work at least:
> "(.*test\.cc|.*test\.h)"

It did. Thanks!

[brandtr, 2017-02-15 14:49:46]

Rebase.

[brandtr, 2017-02-15 15:48:03]

Patchset #6 (id:140001) has been deleted

[brandtr, 2017-02-16 14:00:34]

Patchset #10 (id:240001) has been deleted

[brandtr, 2017-02-17 08:13:39]

brandtr@webrtc.org changed reviewers:
- mbonadei@webrtc.org

[brandtr, 2017-02-17 08:13:40]

asapersson, sprang: ping
kjellander, sakal: I made some changes to BUILD.gn and
android_test_initializer.{cc,h}. Feel free to take a second look.

https://codereview.webrtc.org/2695653002/diff/160001/webrtc/modules/video_cod...
File webrtc/modules/video_coding/codecs/test/videoprocessor_integrationtest.h
(right):

https://codereview.webrtc.org/2695653002/diff/160001/webrtc/modules/video_cod...
webrtc/modules/video_coding/codecs/test/videoprocessor_integrationtest.h:143:
#if defined(WEBRTC_ANDROID)
This didn't work out when running multiple tests: the first test would then
initialize and free the Java object registrations, and the second test could
then not initialize the object registrations again.

Inspired by
https://cs.chromium.org/chromium/src/third_party/webrtc/pc/peerconnectionfact...
and
https://cs.chromium.org/chromium/src/third_party/webrtc/modules/audio_device/...,
I simply dropped the freeing of the Java object registrations. This should be
fine, since AFAICT the dynamic lib loaded by the Chromium NativeTest apk never
is unloaded.

[åsapersson, 2017-02-17 09:32:36]

video_coding/codecs/test: lgtm

[sprang_webrtc, 2017-02-17 11:37:50]

lgtm

[kjellander_webrtc, 2017-02-19 09:04:46]

lgtm

[brandtr, 2017-02-20 09:46:07]

The CQ bit was checked by brandtr@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-20 09:46:11]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-02-20 10:22:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-20 10:22:10]

Dry run: This issue passed the CQ dry run.

[brandtr, 2017-02-20 12:32:58]

The CQ bit was checked by brandtr@webrtc.org

[brandtr, 2017-02-20 12:32:59]

The patchset sent to the CQ was uploaded after l-g-t-m from sakal@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2695653002/#ps280001 (title:
"Don't forget the mac!")

[commit-bot: I haz the power, 2017-02-20 12:33:12]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-02-20 12:35:53]

CQ is committing da patch.

Bot data: {"patchset_id": 280001, "attempt_start_ts": 1487593978740590,
"parent_rev": "8dd4ec3324215aee19a58c4d050175bb93af1948", "commit_rev":
"6bb8e0efd3187e11ac512d84d665b4cffacad43e"}

[commit-bot: I haz the power, 2017-02-20 12:35:58]

Description was changed from

==========
Add support for creating HW codecs in the VideoProcessor tests.

This CL adds the ability to _create_ HW codecs (Android and iOS) in the
VideoProcessor integration tests. Since the VideoProcessor class is not thread
safe yet, this CL does not add the ability to _use_ HW codecs in the tests. A
follow-up CL is planned that will add this ability.

This CL further adds a separate build target which is used to separate the
"plot" versions of the integration tests from the "correctness" versions. The
former will be run manually on devices, whereas the latter are used on the
trybots/buildbots to find regressions in the SW codecs. The underlying test
is the same, however.

BUG=webrtc:6634
==========

to

==========
Add support for creating HW codecs in the VideoProcessor tests.

This CL adds the ability to _create_ HW codecs (Android and iOS) in the
VideoProcessor integration tests. Since the VideoProcessor class is not thread
safe yet, this CL does not add the ability to _use_ HW codecs in the tests. A
follow-up CL is planned that will add this ability.

This CL further adds a separate build target which is used to separate the
"plot" versions of the integration tests from the "correctness" versions. The
former will be run manually on devices, whereas the latter are used on the
trybots/buildbots to find regressions in the SW codecs. The underlying test
is the same, however.

BUG=webrtc:6634

Review-Url: https://codereview.webrtc.org/2695653002
Cr-Commit-Position: refs/heads/master@{#16716}
Committed:
https://chromium.googlesource.com/external/webrtc/+/6bb8e0efd3187e11ac512d84d...
==========

[commit-bot: I haz the power, 2017-02-20 12:36:00]

Committed patchset #11 (id:280001) as
https://chromium.googlesource.com/external/webrtc/+/6bb8e0efd3187e11ac512d84d...