Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(27)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 2767003002: Revert of Add DesktopCapturerId and attach it to DesktopFrame (Closed)

Created:
3 years, 9 months ago by Henrik Grunell WebRTC
Modified:
3 years, 9 months ago
Reviewers:
Sergey Ulanov, Hzj_jie
CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com, Jamie, joedow
Target Ref:
refs/heads/master
Project:
webrtc
Visibility:
Public.

Description

Revert of Add DesktopCapturerId and attach it to DesktopFrame (patchset #4 id:100002 of https://codereview.webrtc.org/2759493002/ ) Reason for revert: I suspect that this CL breaks Chromium WebRTC FYI bots. (Thanks kjellander@ for spotting.) The added dep in BUILD.gn would be the problem. Example: https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.webrtc.fyi%2FLinux_Builder%2F15058%2F%2B%2Frecipes%2Fsteps%2Fcompile%2F0%2Fstdout FAILED: newlib_pnacl/obj/third_party/webrtc/api/libjingle_peerconnection_api/mediaconstraintsinterface.o /b/c/goma_client/gomacc ../../native_client/toolchain/linux_x86/pnacl_newlib/bin/pnacl-clang++ -MMD -MF newlib_pnacl/obj/third_party/webrtc/api/libjingle_peerconnection_api/mediaconstraintsinterface.o.d -DNACL_TC_REV=5dfe030a71ca66e72c5719ef5034c2ed24706c43 -DV8_DEPRECATION_WARNINGS -DUSE_OPENSSL_CERTS=1 -DNO_TCMALLOC -DFULL_SAFE_BROWSING -DSAFE_BROWSING_CSD -DSAFE_BROWSING_DB_LOCAL -DCHROMIUM_BUILD -DENABLE_MEDIA_ROUTER=1 -DFIELDTRIAL_TESTING_ENABLED -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D_FORTIFY_SOURCE=2 -DNDEBUG -DNVALGRIND -DWEBRTC_RESTRICT_LOGGING -DEXPAT_RELATIVE_PATH -DHAVE_SCTP -DENABLE_EXTERNAL_AUTH -DHAVE_WEBRTC_VIDEO -DHAVE_WEBRTC_VOICE -DLOGGING_INSIDE_WEBRTC -DUSE_WEBRTC_DEV_BRANCH -DFEATURE_ENABLE_VOICEMAIL -DEXPAT_RELATIVE_PATH -DGTEST_RELATIVE_PATH -DNO_MAIN_THREAD_WRAPPING -DNO_SOUND_SYSTEM -DWEBRTC_CHROMIUM_BUILD -DWEBRTC_POSIX -I../.. -Inewlib_pnacl/gen -I../../third_party/webrtc_overrides -I../../third_party -fno-strict-aliasing -Wno-builtin-macro-redefined -D__DATE__= -D__TIME__= -D__TIMESTAMP__= -fcolor-diagnostics -Wall -Werror -Wextra -Wno-missing-field-initializers -Wno-unused-parameter -Wno-c++11-narrowing -Wno-covered-switch-default -Wno-deprecated-register -Wno-unneeded-internal-declaration -Wno-inconsistent-missing-override -O2 -fno-ident -fdata-sections -ffunction-sections -g0 -fvisibility=hidden -fvisibility-inlines-hidden -std=gnu++11 -fno-rtti -fno-exceptions -c ../../third_party/webrtc/api/mediaconstraintsinterface.cc -o newlib_pnacl/obj/third_party/webrtc/api/libjingle_peerconnection_api/mediaconstraintsinterface.o In file included from ../../third_party/webrtc/api/mediaconstraintsinterface.cc:11: In file included from ../../third_party/webrtc/api/mediaconstraintsinterface.h:27: In file included from ../../third_party/webrtc/api/peerconnectioninterface.h:77: In file included from ../../third_party/webrtc/api/dtmfsenderinterface.h:16: In file included from ../../third_party/webrtc/api/mediastreaminterface.h:33: In file included from ../../third_party/webrtc/media/base/mediachannel.h:28: ../../third_party/webrtc/base/socket.h:18:10: fatal error: 'sys/socket.h' file not found #include <sys/socket.h> ^ 1 error generated. Original issue's description: > Add DesktopCapturerId and attach it to DesktopFrame > > This change adds a DesktopCapturerId namespace, and attaches an int to each > DesktopFrame. ScreenCapturerWinGdi and ScreenCapturerWinDirectx now actively set > this field to differentiate themselves. > > BUG=679523, 650926 > > Review-Url: https://codereview.webrtc.org/2759493002 > Cr-Commit-Position: refs/heads/master@{#17329} > Committed: https://chromium.googlesource.com/external/webrtc/+/41e3d9ff3bf7a7364b643f851f14062fd43fcc11 TBR=sergeyu@chromium.org,zijiehe@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=679523, 650926 Review-Url: https://codereview.webrtc.org/2767003002 Cr-Commit-Position: refs/heads/master@{#17336} Committed: https://chromium.googlesource.com/external/webrtc/+/060987699b281a1a5d2339ba0da0f2bd88b81a6e

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -25 lines) Patch
M webrtc/modules/desktop_capture/BUILD.gn View 1 chunk +0 lines, -1 line 0 comments Download
M webrtc/modules/desktop_capture/DEPS View 1 chunk +0 lines, -1 line 0 comments Download
M webrtc/modules/desktop_capture/desktop_capture_types.h View 2 chunks +0 lines, -9 lines 0 comments Download
M webrtc/modules/desktop_capture/desktop_frame.h View 3 chunks +0 lines, -10 lines 0 comments Download
M webrtc/modules/desktop_capture/desktop_frame.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M webrtc/modules/desktop_capture/win/screen_capturer_win_directx.cc View 1 chunk +0 lines, -1 line 0 comments Download
M webrtc/modules/desktop_capture/win/screen_capturer_win_gdi.cc View 1 chunk +0 lines, -1 line 0 comments Download

Messages

Total messages: 7 (3 generated)
Henrik Grunell WebRTC
Created Revert of Add DesktopCapturerId and attach it to DesktopFrame
3 years, 9 months ago (2017-03-22 08:13:23 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/2767003002/1
3 years, 9 months ago (2017-03-22 08:13:34 UTC) #3
commit-bot: I haz the power
Committed patchset #1 (id:1) as https://chromium.googlesource.com/external/webrtc/+/060987699b281a1a5d2339ba0da0f2bd88b81a6e
3 years, 9 months ago (2017-03-22 08:13:45 UTC) #6
Hzj_jie
3 years, 9 months ago (2017-03-22 17:30:08 UTC) #7
Message was sent while issue was closed. 
On 2017/03/22 08:13:45, commit-bot: I haz the power wrote:
> Committed patchset #1 (id:1) as
>
https://chromium.googlesource.com/external/webrtc/+/060987699b281a1a5d2339ba0...

The error message is triple weird, sys/socket.h is a system header file, which
should exist on any Linux distros. BUt I do find an issue,
webrtc/media:rtc_media_base depends on webrtc/base:rtc_base, which is not
allowed in Chromium I believe.

Anyway, in this change, only a FOURCC() macro is used in desktop_capture_type.h,
which is trivial, and not worthy to spend too much time on it. I will simply
copy-paste the macro to desktop_capture_types.h, and remove the dependency of
webrtc/media:rtc_media_base from desktop_capture.

Powered by Google App Engine
This is Rietveld 408576698