980211 - Suppress clang warnings in third-party media code: speex, theora, and vpx

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Chris Peterson [:cpeterson]]

Attachment: suppress-media-warnings.patch

This patch suppresses the following clang warnings from these third-party codecs. They now compile with zero warnings on OS X (with Firefox's current warning levels). :D

media/libtheora/lib/state.c:717:21 [-Wtautological-compare] comparison of unsigned enum expression < 0 is always false
media/libtheora/lib/state.c:718:20 [-Wtautological-compare] comparison of unsigned enum expression < 0 is always false

media/libspeex_resampler/src/resample.c:631:20 [-Wsign-compare] comparison of integers of different signs: 'int' and 'unsigned int'
media/libspeex_resampler/src/resample.c:712:23 [-Wsign-compare] comparison of integers of different signs: 'int' and 'unsigned int'
media/libspeex_resampler/src/resample.c:720:23 [-Wsign-compare] comparison of integers of different signs: 'int' and 'unsigned int'
media/libspeex_resampler/src/resample.c:723:20 [-Wsign-compare] comparison of integers of different signs: 'int' and 'unsigned int'
media/libspeex_resampler/src/resample.c:730:23 [-Wsign-compare] comparison of integers of different signs: 'int' and 'unsigned int'
media/libspeex_resampler/src/resample.c:896:21 [-Wsign-compare] comparison of integers of different signs: 'int' and 'unsigned int'
media/libspeex_resampler/src/resample.c:899:20 [-Wsign-compare] comparison of integers of different signs: 'int' and 'unsigned int'
media/libspeex_resampler/src/resample.c:952:19 [-Wsign-compare] comparison of integers of different signs: 'int' and 'unsigned int'
media/libspeex_resampler/src/resample.c:959:19 [-Wsign-compare] comparison of integers of different signs: 'int' and 'unsigned int'
media/libspeex_resampler/src/resample.c:969:16 [-Wsign-compare] comparison of integers of different signs: 'int' and 'unsigned int'
media/libspeex_resampler/src/resample_sse.h:47:17 [-Wsign-compare] comparison of integers of different signs: 'int' and 'unsigned int'
media/libspeex_resampler/src/resample_sse.h:72:14 [-Wsign-compare] comparison of integers of different signs: 'int' and 'unsigned int'
media/libspeex_resampler/src/resample_sse.h:108:14 [-Wsign-compare] comparison of integers of different signs: 'int' and 'unsigned int'
media/libspeex_resampler/src/resample_sse.h:134:12 [-Wsign-compare] comparison of integers of different signs: 'int' and 'unsigned int'

media/libvpx/vpx/src/svc_encodeframe.c:231:13 [-Wsign-compare] comparison of integers of different signs: 'int' and 'SVC_LOG_LEVEL' (aka 'enum SVC_LOG_LEVEL')
media/libvpx/vp8/encoder/onyx_if.c:1666:36 [-Wsign-compare] comparison of integers of different signs: 'unsigned int' and 'int'
media/libvpx/vp8/encoder/onyx_if.c:1705:41 [-Wsign-compare] comparison of integers of different signs: 'unsigned int' and 'int'
media/libvpx/vp8/encoder/onyx_if.c:1707:42 [-Wsign-compare] comparison of integers of different signs: 'unsigned int' and 'int'
media/libvpx/vp8/encoder/onyx_if.c:5475:29 [-Wsign-compare] comparison of integers of different signs: 'int' and 'unsigned int'
media/libvpx/vp8/encoder/onyx_if.c:5475:60 [-Wsign-compare] comparison of integers of different signs: 'int' and 'unsigned int'
media/libvpx/vp8/encoder/onyx_if.c:5531:14 [-Wsign-compare] comparison of integers of different signs: 'unsigned int' and 'int'
media/libvpx/vp8/encoder/onyx_if.c:5531:45 [-Wsign-compare] comparison of integers of different signs: 'unsigned int' and 'int'
media/libvpx/vp8/encoder/ratectrl.c:718:46 [-Wsign-compare] comparison of integers of different signs: 'unsigned int' and 'int'
media/libvpx/vp8/vp8_cx_iface.c:615:61 [-Wsign-compare] comparison of integers of different signs: 'const int' and 'unsigned int'
media/libvpx/vp8/vp8_cx_iface.c:734:24 [-Wsign-compare] comparison of integers of different signs: 'int' and 'unsigned int'
media/libvpx/vp9/encoder/vp9_onyx_if.c:1627:17 [-Wsign-compare] comparison of integers of different signs: 'int' and 'unsigned long'
media/libvpx/vp9/encoder/vp9_onyx_if.c:2063:17 [-Wsign-compare] comparison of integers of different signs: 'int' and 'unsigned long'
media/libvpx/vp9/encoder/vp9_onyx_if.c:4158:27 [-Wsign-compare] comparison of integers of different signs: 'int' and 'unsigned int'
media/libvpx/vp9/encoder/vp9_onyx_if.c:4158:58 [-Wsign-compare] comparison of integers of different signs: 'int' and 'unsigned int'
media/libvpx/vp9/encoder/vp9_onyx_if.c:4203:12 [-Wsign-compare] comparison of integers of different signs: 'unsigned int' and 'int'
media/libvpx/vp9/encoder/vp9_onyx_if.c:4203:43 [-Wsign-compare] comparison of integers of different signs: 'unsigned int' and 'int'
media/libvpx/vp9/encoder/vp9_rdopt.c:749:22 [-Wsign-compare] comparison of integers of different signs: 'int' and 'const TX_SIZE'
media/libvpx/vp9/encoder/vp9_rdopt.c:765:22 [-Wsign-compare] comparison of integers of different signs: 'int' and 'const TX_SIZE'
media/libvpx/vp9/encoder/vp9_rdopt.c:857:22 [-Wsign-compare] comparison of integers of different signs: 'int' and 'const TX_SIZE'
media/libvpx/vp9/encoder/vp9_rdopt.c:871:22 [-Wsign-compare] comparison of integers of different signs: 'int' and 'const TX_SIZE'
media/libvpx/vp9/encoder/vp9_rdopt.c:879:22 [-Wsign-compare] comparison of integers of different signs: 'int' and 'const TX_SIZE'
media/libvpx/vp9/encoder/vp9_rdopt.c:1911:54 [-Wsign-compare] comparison of integers of different signs: 'int' and 'const BLOCK_SIZE' (aka 'const enum BLOCK_SIZE')
media/libvpx/vp9/encoder/vp9_rdopt.c:2156:14 [-Wsign-compare] comparison of integers of different signs: 'unsigned int' and 'int'
media/libvpx/vp9/encoder/vp9_rdopt.c:2689:50 [-Wsign-compare] comparison of integers of different signs: 'int' and 'BLOCK_SIZE' (aka 'enum BLOCK_SIZE')
media/libvpx/vp9/vp9_cx_iface.c:507:57 [-Wsign-compare] comparison of integers of different signs: 'const int' and 'unsigned int'
media/libvpx/vp9/vp9_cx_iface.c:587:22 [-Wsign-compare] comparison of integers of different signs: 'int' and 'unsigned int'

Comment 1

[Chris Peterson [:cpeterson]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f81c8b82ac70

Comment 2

[Tom S. (please needinfo tschuster)]

Comment on attachment 8386589 [details] [diff] [review]
suppress-media-warnings.patch

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

::: media/libspeex_resampler/src/moz.build
@@ +46,1 @@
>      if CONFIG['GNU_CC']:

This looks wrong, you changed this code to only run under mac, instead of every x86 platform. I think shouldn't use Darwin to check for clang, instead check for it directly. (I use clang on Linux) This applies to everything else.

Comment 3

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/f81c8b82ac70

Comment 4

[Chris Peterson [:cpeterson]]

(In reply to Tom Schuster [:evilpie] from comment #2)
> Comment on attachment 8386589 [details] [diff] [review]
> suppress-media-warnings.patch
> 
> Review of attachment 8386589 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: media/libspeex_resampler/src/moz.build
> @@ +46,1 @@
> >      if CONFIG['GNU_CC']:
> 
> This looks wrong, you changed this code to only run under mac, instead of
> every x86 platform. I think shouldn't use Darwin to check for clang, instead
> check for it directly. (I use clang on Linux) This applies to everything
> else.

oops! That was a bad merge. I will back it out.

Yes, these changes really should have checked whether the compiler was clang instead of the OS, but CONFIG['CLANG_CC'] was not defined in my OS X builds and CONFIG['GCC_CC'] was. I will investigate whether there is a bug in our autoconf scripts and file a new bug.

Comment 5

[Chris Peterson [:cpeterson]]

Backout part of cset f81c8b82ac70 for bad merge of libspeex moz.build:
https://hg.mozilla.org/integration/mozilla-inbound/rev/19839a359db1

Comment 6

[Chris Peterson [:cpeterson]]

I filed bug 981507. When building with clang on OS X, moz.build defines CONFIG['CLANG_CXX'], CONFIG['GNU_CXX'], CONFIG['GNU_CC'], but not CONFIG['CLANG_CC']. I should have tested CLANG_CXX but the moz.build example code I had followed used GNU_CC instead of GNU_CXX. :|

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/19839a359db1

Comment 8

[Chris Peterson [:cpeterson]]

Attachment: suppress-media-warnings-v2.patch

This patch fixes a bad merge from my previous patch and changes the moz.build checks from OS_TARGET==Darwin to GNU_CC/CLANG_CXX so the warning suppressions benefit both gcc and clang on both OS X and Linux.

Comment 9

[Chris Peterson [:cpeterson]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5eea530e5b76

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/5eea530e5b76