Created attachment 453804 [details, diff] webrtc/modules/desktop_capture patch ../../third_party/webrtc/modules/desktop_capture/screen_capturer_x11.cc:415:10: error: cannot bind ‘std::unique_ptr<webrtc::{anonymous}::ScreenCapturerLinux>’ lvalue to ‘std::unique_ptr<webrtc::{anonymous}::ScreenCapturerLinux>&&’ return capturer; ^ In file included from /usr/lib/gcc/x86_64-pc-linux-gnu/4.9.3/include/g++-v4/memory:81:0, from ../../third_party/webrtc/modules/desktop_capture/screen_capturer_x11.cc:13: /usr/lib/gcc/x86_64-pc-linux-gnu/4.9.3/include/g++-v4/bits/unique_ptr.h:220:2: note: initializing argument 1 of ‘std::unique_ptr<_Tp, _Dp>::unique_ptr(std::unique_ptr<_Up, _Ep>&&) [with _Up = webrtc::{anonymous}::ScreenCapturerLinux; _Ep = std::default_delete<webrtc::{anonymous}::ScreenCapturerLinux>; <template-parameter-2-3> = void; _Tp = webrtc::DesktopCapturer; _Dp = std::default_delete<webrtc::DesktopCapturer>]’ unique_ptr(unique_ptr<_Up, _Ep>&& __u) noexcept ^ ../../third_party/webrtc/modules/desktop_capture/screen_capturer_x11.cc:416:1: warning: control reaches end of non-void function [-Wreturn-type] } ^ for the lazy readers: I found above error, and I suggest my patch to fix an error occured by type mismatch between function return type and return object. NOTE: This package is currently masked, because it's testing by developers and/or it's unstable.
What specific version(s) of chromium have you tested? Also, please provide emerge --info and attach a full build log.
Created attachment 453946 [details] emerge --info
Created attachment 453952 [details] build log I can reproduce in an amd64 stable container.
(In reply to Mike Gilbert from comment #3) > Created attachment 453952 [details] > build log > > I can reproduce in an amd64 stable container. Thank you, Mike! I have the same problem.
and I tried two version, 56.0.2914.3 and here version someone said onto the summary(title?) field.
Unfortunately, I'm really not familiar enough with the WebRTC code or C++ in general to be able to properly review this patch. Could you please report this issue upstream? Bug tracker: http://bugs.webrtc.org/ Code submission guide: https://webrtc.org/contributing/
A datapoint: I too had to add the capture patch to successfully emerge chromium-56.0.2924.21 with gcc-4.9.3. The upstream issue https://bugs.chromium.org/p/webrtc/issues/detail?id=6760 is currently at WontFix. www-client/chromium-56.0.2924.21::local-repo was built with the following: USE="cups hangouts (pic) proprietary-codecs pulseaudio system-ffmpeg tcmalloc widevine -custom-cflags -gnome -gnome-keyring (-gtk3) -kerberos (-neon) (-selinux) -suid -test" ABI_X86="64" L10N="fi -am -ar -bg -bn -ca -cs -da -de -el -en-GB -es -es-419 -et -fa -fil -fr -gu -he -hi -hr -hu -id -it -ja -kn -ko -lt -lv -ml -mr -ms -nb -nl -pl -pt-BR -pt-PT -ro -ru -sk -sl -sr -sv -sw -ta -te -th -tr -uk -vi -zh-CN -zh-TW" CXXFLAGS="-march=amdfam10 -O2 -pipe -fno-delete-null-pointer-checks" $ diff /usr{,/local}/portage/www-client/chromium/chromium-56.0.2924.21.ebuild 165a166 > "${FILESDIR}/${PN}-56-webrtc_capture.patch"
Same problem here.
*** Bug 602554 has been marked as a duplicate of this bug. ***
(In reply to Jouni Kosonen from comment #7) > A datapoint: I too had to add the capture patch to successfully emerge > chromium-56.0.2924.21 with gcc-4.9.3. Thanks for the patch! Version 56.0.2924.21 (64-bit) > The upstream issue https://bugs.chromium.org/p/webrtc/issues/detail?id=6760 > is currently at WontFix. Ugh!
Can someone try patching: return capturer; to return std::move(capturer); and let me know if it fixes the issue (I am away from my desktop, but believe that this is the 'correct' solution).
Upstream seems like they will accept patches, but only if you follow their code submission guide. https://webrtc.org/contributing/ As I indicated before, I am not qualified to submit a patch myself.
(In reply to Freddie Witherden from comment #11) > Can someone try patching: > > return capturer; > > to > > return std::move(capturer); > > and let me know if it fixes the issue (I am away from my desktop, but > believe that this is the 'correct' solution). I can confirm this compiles fine with your solution.
(In reply to Mike Gilbert from comment #12) > Upstream seems like they will accept patches, but only if you follow their > code submission guide. > > https://webrtc.org/contributing/ > > As I indicated before, I am not qualified to submit a patch myself. The issue itself is a GCC bug (4.9.x at least, maybe 4.8.x, too). Newer versions of GCC, and all of the versions of Clang I have to hand, accept the code fragment without issue. (Although, for those that are interested, ICC also chokes unless one adds move semantics.) Given the position of the maintainers for the average user it might be better to upgrade to GCC 5.4.0.
(In reply to Norman Shulman from comment #10) > (In reply to Jouni Kosonen from comment #7) > > A datapoint: I too had to add the capture patch to successfully emerge > > chromium-56.0.2924.21 with gcc-4.9.3. > > Thanks for the patch! It's not mine, it's from Seong-ho Cho – thanks belong there. The Freddie Witherden version from comment #11 worked here too: --- a/third_party/webrtc/modules/desktop_capture/screen_capturer_x11.cc +++ b/third_party/webrtc/modules/desktop_capture/screen_capturer_x11.cc @@ -434,2 +434,2 @@ - return capturer; + return std::move(capturer); }
- return capturer; + return std::unique_ptr<DesktopCapturer>(static_cast<DesktopCapturer*>(capturer.release())); Fixed the issue for me (at least, chromium compiles again. If that break webrtc? I don't know).
chromium-57.0.2946.0 has the same webrtc problem, so the fix/patch needs to be used for that version too (Same file, same line).
commit 4e7604710d87d31d8836086bf2e1c2ffa4271235 Author: Mike Gilbert <floppym@gentoo.org> Date: Wed Dec 14 16:00:26 2016 -0500 www-client/chromium: require >=gcc-5 Package-Manager: Portage-2.3.3_p7, Repoman-2.3.1 www-client/chromium/chromium-56.0.2924.21.ebuild | 15 +++++++++++---- www-client/chromium/chromium-57.0.2946.0.ebuild | 15 +++++++++++---- 2 files changed, 22 insertions(+), 8 deletions(-)
Thanks for testing my patch, but huuummmmmmmmm ... I didn't "feature" test, I'm not sure this patch made webrtc working well. I just checked successful compilation. I need someone who claims for my patch. so I did not yet submit this patch onto the upstream. The main objective of my patch is just resolving "static cast" problem as we know. I don't know whether capturer object should be remained on the memory as is or not, so I'm not sure that applying Freddie's solution(applying std::move()) is ok. but, anyway, I have consider to test Freddie's suggestion. and... yes. This is the bug of gcc-4.9(version-specific bug), we need modify my suggestion to make be able to build on every version of gcc(without modifing there source line as we can). anyway, my suggestion is just for gcc-4.9, so it should be applied only for gcc-4.9.
(In reply to Seong-ho Cho from comment #20) > Thanks for testing my patch, but huuummmmmmmmm ... Thanks anyway, and the report did end up getting a resolution in the tree. Of course, the resolution in comment #19 ended up causing rebuilds of 655 packages to me personally as per[1] but this bug did serve to isolate the problem and to point to solutions. Adding move semantics as in comment #11 would have been lighter but given the lack of interest in the upstream probably not sustainable in the long run. [1]https://wiki.gentoo.org/wiki/Upgrading_from_gcc-4.x_to_gcc-5.x
(In reply to Jouni Kosonen from comment #21) > (In reply to Seong-ho Cho from comment #20) > > Thanks for testing my patch, but huuummmmmmmmm ... > > Thanks anyway, and the report did end up getting a resolution in the tree. > Thank you. Than's so clear advise for me. umm then ... how would we better to deal with this issue? It seems actually bug-fixed by comment #19. yeah. I think it is better someone to change this bug status as "fixed". ;)
I was hoping someone would actually get a patch accepted upstream to make it work with stable gcc 4.9. However it seems that will not actually happen. So it is "FIXED" indeed.
Soap verified that using std::move() to work around this is "correct". I sent a patch upstream. https://codereview.webrtc.org/2642053003/
https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=6159ac3f9f5d2eb5be6cbd6737c5ad901e3011c5 warning: only found copies from modified paths due to too many files. commit 6159ac3f9f5d2eb5be6cbd6737c5ad901e3011c5 Author: Mike Gilbert <floppym@gentoo.org> Date: Sun Jan 22 00:22:50 2017 -0500 www-client/chromium: bump to 56.0.2924.67 Fix build with gcc 4.9. Bug: https://bugs.gentoo.org/600288 Package-Manager: Portage-2.3.3_p32, Repoman-2.3.1_p25 www-client/chromium/Manifest | 2 +- ...2924.21.ebuild => chromium-56.0.2924.67.ebuild} | 17 ++++---- www-client/chromium/files/chromium-56-gcc4.patch | 48 ++++++++++++++++++++++ 3 files changed, 57 insertions(+), 10 deletions(-)
