1122547 - media/webrtc/trunk/webrtc/modules/video_coding/main/source/receiver.cc:162:14: error: call of overloaded 'abs(int64_t&)' is ambiguous

Status: RESOLVED FIXED

(Core :: WebRTC, defect)

Description

[Jan Beich]

next_render_time_ms and now_ms are of |int64_t| type. On FreeBSD i386 (and other 32bit archs) it would be |long long|. While FreeBSD has llabs() it doesn't define _GLIBCXX_USE_C99 which means no std::llabs(). For old libstdc++ (from GCC 4.2.1) this also means no |long long std::abs(long long)|.

https://svnweb.freebsd.org/changeset/base/255294
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54686

Not sure why GCC-4.8 with _GLIBCXX_USE_LONG_LONG disabled doesn't fail like Clang-3.4 + libstdc++-4.2.

In file included from media/webrtc/trunk/webrtc/modules/modules_webrtc_video_coding/Unified_cpp__webrtc_video_coding0.cpp:137:
media/webrtc/trunk/webrtc/modules/video_coding/main/source/receiver.cc:162:14: error:
      call to 'abs' is ambiguous
  } else if (std::abs(next_render_time_ms - now_ms) > max_video_delay_ms_) {
             ^~~~~~~~
/usr/include/stdlib.h:83:6: note: candidate function
int      abs(int) __pure2;
         ^
/usr/include/c++/4.2/cstdlib:143:3: note: candidate function
  abs(long __i) { return labs(__i); }
  ^
In file included from media/webrtc/trunk/webrtc/modules/modules_webrtc_video_coding/Unified_cpp__webrtc_video_coding0.cpp:137:
media/webrtc/trunk/webrtc/modules/video_coding/main/source/receiver.cc:167:35: error:
      call to 'abs' is ambiguous
                 static_cast<int>(std::abs(next_render_time_ms - now_ms)),
                                  ^~~~~~~~
/usr/include/stdlib.h:83:6: note: candidate function
int      abs(int) __pure2;
         ^
/usr/include/c++/4.2/cstdlib:143:3: note: candidate function
  abs(long __i) { return labs(__i); }
  ^
2 errors generated.

Comment 1

[Jan Beich]

Attachment: use mozilla::Abs

mozilla::Abs() adds |unsigned| so casting is required to avoid

  In file included from media/webrtc/trunk/webrtc/modules/modules_webrtc_video_coding/Unified_cpp__webrtc_video_coding0.cpp:137:
  media/webrtc/trunk/webrtc/modules/video_coding/main/source/receiver.cc:164:57: warning:
        comparison of integers of different signs: 'typename detail::AbsReturnType<long long>::Type'
        (aka 'unsigned long long') and 'int' [-Wsign-compare]
    } else if (mozilla::Abs(next_render_time_ms - now_ms) > max_video_delay_ms_) {
               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~
  1 warning generated.

Comment 2

[Jan Beich]

Attachment: use mozilla::Abs, v2

Oops, use static_cast like the rest of the file.

Comment 3

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8550322 [details] [diff] [review]
use mozilla::Abs, v2

Review of attachment 8550322 [details] [diff] [review]:
-----------------------------------------------------------------

::: media/webrtc/trunk/webrtc/modules/video_coding/main/source/receiver.cc
@@ +161,5 @@
>    // Assume that render timing errors are due to changes in the video stream.
>    if (next_render_time_ms < 0) {
>      timing_error = true;
> +  } else if (static_cast<int>(mozilla::Abs(next_render_time_ms - now_ms)) >
> +             max_video_delay_ms_) {

We really, really try to avoid mozilla-isms in webrtc.org import files, unless #ifdefed (and mostly we try to avoid those other than platform ifdefs like GONK and BSD OS ifdefs).

a) can we redo this to be non-mozilla-specific but still have the std::abs() functionality needed here?  Perhaps by casting other bits - such that it's upstreamable.

b) if not, let's make it #ifdef'd to the BSDs in question, and try to avoid mozillaisms in that as well.

Comment 4

[Jan Beich]

Attachment: overload to int

Let's cast before dropping sign. Return value of overloaded std::abs() is still signed, anyway. And WEBRTC_TRACE already assumes time delta cannot be larger than |int|.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=97bc729b1d24

Comment 5

[(Out Sep 17-23) Ryan VanderMeulen]

https://hg.mozilla.org/integration/mozilla-inbound/rev/741ba2a22877

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/741ba2a22877