986793 - Suppress gcc warnings in third-party code: libcubeb and libsoundtouch

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Chris Peterson [:cpeterson]]

Attachment: suppress-libcubeb-libsoundtouch-warnings.patch

Android's gcc reports the following warnings in third-party code we probably don't want to patch just to fix minor compiler warnings:

media/libcubeb/src/cubeb_audiotrack.c:301:3 [-Wpointer-sign] pointer targets in passing argument 3 of 'audiotrack_get_min_frame_count' differ in signedness
media/libcubeb/src/cubeb_audiotrack.c:317:3 [-Wpointer-sign] pointer targets in passing argument 1 of 'ctx->klass.get_output_samplingrate' differ in signedness
media/libcubeb/src/cubeb_opensl.c:331:5 [-Wpointer-sign] pointer targets in passing argument 1 of 'get_output_frame_count' differ in signedness

media/libcubeb/src/cubeb_opensl.c:74:3 [-Wdeclaration-after-statement] ISO C90 forbids mixed declarations and code
media/libcubeb/src/cubeb_opensl.c:143:3 [-Wdeclaration-after-statement] ISO C90 forbids mixed declarations and code
media/libcubeb/src/cubeb_opensl.c:170:3 [-Wdeclaration-after-statement] ISO C90 forbids mixed declarations and code
media/libcubeb/src/cubeb_opensl.c:191:3 [-Wdeclaration-after-statement] ISO C90 forbids mixed declarations and code
media/libcubeb/src/cubeb_opensl.c:377:3 [-Wdeclaration-after-statement] ISO C90 forbids mixed declarations and code
media/libcubeb/src/cubeb_opensl.c:416:3 [-Wdeclaration-after-statement] ISO C90 forbids mixed declarations and code
media/libcubeb/src/cubeb_opensl.c:422:3 [-Wdeclaration-after-statement] ISO C90 forbids mixed declarations and code
media/libcubeb/src/cubeb_opensl.c:425:3 [-Wdeclaration-after-statement] ISO C90 forbids mixed declarations and code
media/libcubeb/src/cubeb_opensl.c:429:3 [-Wdeclaration-after-statement] ISO C90 forbids mixed declarations and code
media/libcubeb/src/cubeb_opensl.c:432:3 [-Wdeclaration-after-statement] ISO C90 forbids mixed declarations and code
media/libcubeb/src/cubeb_opensl.c:437:3 [-Wdeclaration-after-statement] ISO C90 forbids mixed declarations and code
media/libcubeb/src/cubeb_opensl.c:444:3 [-Wdeclaration-after-statement] ISO C90 forbids mixed declarations and code
media/libcubeb/src/cubeb_opensl.c:452:3 [-Wdeclaration-after-statement] ISO C90 forbids mixed declarations and code
media/libcubeb/src/cubeb_opensl.c:501:3 [-Wdeclaration-after-statement] ISO C90 forbids mixed declarations and code
media/libcubeb/src/cubeb_opensl.c:514:3 [-Wdeclaration-after-statement] ISO C90 forbids mixed declarations and code

media/libsoundtouch/src/FIRFilter.cpp:231:10 [-Wunused-but-set-variable] variable 'uExtensions' set but not used
media/libsoundtouch/src/TDStretch.cpp:612:10 [-Wunused-but-set-variable] variable 'uExtensions' set but not used

Comment 1

[Paul Adenot (:padenot)]

Comment on attachment 8395178 [details] [diff] [review]
suppress-libcubeb-libsoundtouch-warnings.patch

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

cubeb is not really third party, I'd rather fix those.

We already carry a patch on top of SoundTouch, we can fold those changes in.

Comment 2

[Paul Adenot (:padenot)]

Attachment: Fix warnings in cubeb_audiotrack.c, and allow mixed declaration and code in cubeb_opensl.c. r=

I fixed cubeb_audiotrack.c, as it was a real error. I found moving the
declaration to the top of the bloc in cubeb_opensl.c less clear, so I just
enabled the warning there.

Comment 3

[Paul Adenot (:padenot)]

Attachment: Fix set but unused variable warning in SoundTouch. r=

Fixed by properly #ifdefing the declaration out and adding the changes to our
in-tree patch.

Comment 4

[Chris Peterson [:cpeterson]]

Comment on attachment 8395569 [details] [diff] [review]
Fix warnings in cubeb_audiotrack.c, and allow mixed declaration and code in cubeb_opensl.c. r=

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

::: media/libcubeb/src/cubeb_audiotrack.c
@@ +313,5 @@
>  audiotrack_get_preferred_sample_rate(cubeb * ctx, uint32_t * rate)
>  {
>    status_t rv;
>  
> +  rv = ctx->klass.get_output_samplingrate((int32_t)rate, 3 /* MUSIC */);

You are casting the rate pointer to an int32_t. Did you intend `(int32_t *)rate`? :)

Comment 5

[Chris Peterson [:cpeterson]]

Comment on attachment 8395570 [details] [diff] [review]
Fix set but unused variable warning  in SoundTouch. r=

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

LGTM!

Comment 6

[Paul Adenot (:padenot)]

Attachment: Fix warnings in cubeb_audiotrack.c, and allow mixed declaration and code in cubeb_opensl.c. r=

Oops.

Comment 7

[Chris Peterson [:cpeterson]]

Comment on attachment 8395700 [details] [diff] [review]
Fix warnings in cubeb_audiotrack.c, and allow mixed declaration and code in cubeb_opensl.c. r=

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

LGTM

Comment 8

[Paul Adenot (:padenot)]

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

Comment 9

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

https://hg.mozilla.org/mozilla-central/rev/5fb566733c46
https://hg.mozilla.org/mozilla-central/rev/d076aa4a9fa2

Comment 10

[Chris Peterson [:cpeterson]]

Paul, I still see the same -Wpointer-sign warnings in cubeb reported in comment 0 plus a (new?) -Wsign-compare warning. Do you see these warnings? I'm building Firefox for Android using gcc 4.7.

media/libcubeb/src/cubeb_audiotrack.c:301:3 [-Wpointer-sign] pointer targets in passing argument 3 of 'audiotrack_get_min_frame_count' differ in signedness
media/libcubeb/src/cubeb_audiotrack.c:350:3 [-Wpointer-sign] pointer targets in passing argument 3 of 'audiotrack_get_min_frame_count' differ in signedness
media/libcubeb/src/cubeb_opensl.c:570:3 [-Wpointer-sign] pointer targets in passing argument 1 of 'stm->context->get_output_latency' differ in signedness
media/libcubeb/src/cubeb_opensl.c:575:12 [-Wsign-compare] comparison between signed and unsigned integer expressions

Comment 11

[Paul Adenot (:padenot)]

I filed bug 988827 for this.