Refactor webrtc/modules/video_processing for GN check

webrtc/modules/video_processing/BUILD.gn

Index: webrtc/modules/video_processing/BUILD.gn
diff --git a/webrtc/modules/video_processing/BUILD.gn b/webrtc/modules/video_processing/BUILD.gn
index b1ab8ecc5e38f638af207f74bb4b2aee33478c01..d6dcc3432fa9c38494dd429d87713abdc3bf31fd 100644
--- a/webrtc/modules/video_processing/BUILD.gn
+++ b/webrtc/modules/video_processing/BUILD.gn
@@ -26,6 +26,7 @@ rtc_static_library("video_processing") {
   ]
 
   deps = [
+    "../../base:rtc_base_approved",
     "../../common_audio",
     "../../common_video",
     "../../modules/utility",
@@ -47,10 +48,19 @@ rtc_static_library("video_processing") {
 if (build_video_processing_sse2) {
   rtc_static_library("video_processing_sse2") {
     sources = [
+      "util/denoiser_filter.cc",

[kjellander_webrtc, 2016/12/20 13:25:19]

This looks incorrect. These files are already listed under the
"video_processing" target above. 
This target, IIUC, is only meant to contain the SEE2 implementation.

[mbonadei, 2016/12/20 13:39:43]

Yes, that's correct. But `util/denoiser_filter_sse2.h` includes
`util/denoiser_filter.h` and then `util/denoiser_filter.cc` includes
`util/denoiser_filter_c.h`.

The tools suggestion is an explixit `dep` to `video_processing` but this causes
a circular dependency because `video_processing` depends on
`video_processing_sse2` (line 36).

This is basically a workaround to break the circular dependency but yes... It
doesn't look good to me too. Am I missing something?

[kjellander_webrtc, 2016/12/20 14:00:21]

I see. It might be better to let the code owners of this part deal with the
refactoring to remove the circular dependency. 
But if you want to try, you could move the util/denoiser_filter* files into a
new target 'video_processing_base' or similar, and make both 'video_processing'
and 'video_processing_sse2' depend on that instead. That might solve the
circular dependency. 
If not, I think it's out of the scope for our work to resolve such existing
circular dependencies. Then just disable the include checks for this target with
a comment like this (previous CLs by me and Edward did this for some of the
targets):

# TODO(mbonadei): Remove (bugs.webrtc.org/6828)
# Errors on cyclic dependency with :video_processing if enabled.
check_includes = false

[mbonadei, 2016/12/20 15:54:28]

Yes, unfortunately it is not possible to remove it without a refactoring of the
code. `denoiser_filter.cc` depends on `denoiser_filter_sse2` in the
`DenoiserFilter::Create` method.

Thanks for the wise advice.

+      "util/denoiser_filter.h",
+      "util/denoiser_filter_c.cc",
+      "util/denoiser_filter_c.h",
       "util/denoiser_filter_sse2.cc",
       "util/denoiser_filter_sse2.h",
     ]
 
+    deps = [
+      "../../base:rtc_base_approved",
+      "../../system_wrappers",
+    ]
+
     if (!build_with_chromium && is_clang) {
       # Suppress warnings from the Chromium Clang plugin (bugs.webrtc.org/163).
       suppressed_configs += [ "//build/config/clang:find_bad_constructs" ]