928547 - "AudioStream.cpp:438:52: error: ‘GetCubebContext’ was not declared in this scope" and more, when MOZ_CUBEB is turned off

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Daniel Holbert [:dholbert]]

STR:
Try to build with this mozconfig:
ac_add_options --enable-debug --disable-optimize
ac_add_options --disable-wave
ac_add_options --disable-ogg
ac_add_options --disable-webm

ACTUAL RESULTS:
Build error with the following:
{
23:38.04 /scratch/work/builds/mozilla-inbound/mozilla/content/media/AudioStream.cpp: In static member function ‘static int mozilla::AudioStream::MaxNumberOfChannels()’:
23:38.04 /scratch/work/builds/mozilla-inbound/mozilla/content/media/AudioStream.cpp:438:52: error: ‘GetCubebContext’ was not declared in this scope
23:38.04    rv = cubeb_get_max_channel_count(GetCubebContext(), &maxNumberOfChannels);
23:38.04                                                     ^
23:38.04 /scratch/work/builds/mozilla-inbound/mozilla/content/media/AudioStream.cpp:438:75: error: ‘cubeb_get_max_channel_count’ was not declared in this scope
23:38.04    rv = cubeb_get_max_channel_count(GetCubebContext(), &maxNumberOfChannels);
23:38.04                                                                            ^
23:38.04 /scratch/work/builds/mozilla-inbound/mozilla/content/media/AudioStream.cpp:440:13: error: ‘CUBEB_OK’ was not declared in this scope
23:38.04    if (rv != CUBEB_OK) {
23:38.04              ^
23:38.04 /scratch/work/builds/mozilla-inbound/mozilla/content/media/AudioStream.cpp: In static member function ‘static int mozilla::AudioStream::PreferredSampleRate()’:
23:38.04 /scratch/work/builds/mozilla-inbound/mozilla/content/media/AudioStream.cpp:456:57: error: ‘GetCubebContext’ was not declared in this scope
23:38.04      if (cubeb_get_preferred_sample_rate(GetCubebContext(), &mPreferredSampleRate) != CUBEB_OK) {
23:38.04                                                          ^
23:38.05 /scratch/work/builds/mozilla-inbound/mozilla/content/media/AudioStream.cpp:456:81: error: ‘cubeb_get_preferred_sample_rate’ was not declared in this scope
23:38.05      if (cubeb_get_preferred_sample_rate(GetCubebContext(), &mPreferredSampleRate) != CUBEB_OK) {
23:38.05                                                                                  ^
23:38.05 /scratch/work/builds/mozilla-inbound/mozilla/content/media/AudioStream.cpp:456:86: error: ‘CUBEB_OK’ was not declared in this scope
23:38.05      if (cubeb_get_preferred_sample_rate(GetCubebContext(), &mPreferredSampleRate) != CUBEB_OK) {
23:38.05                                                                                       ^
}

From looking at AudioStream.cpp, it defines GetCubebContext() inside of a "#if defined(MOZ_CUBEB)" block, but then it invokes it at line 438 in an un-#ifdeffed block.
http://mxr.mozilla.org/mozilla-central/source/content/media/AudioStream.cpp#90
http://mxr.mozilla.org/mozilla-central/source/content/media/AudioStream.cpp#438

This usage was added in this cset for bug 865244:
http://hg.mozilla.org/mozilla-central/diff/116e84abedca/content/media/AudioStream.cpp

(NOTE: "--disable-webm" is just there to hack around bug 928530, so that you can get far enough in the build process to hit the error for this bug here.  The other two disable options are sufficient to turn MOZ_CUBEB off, which is what triggers this build error.)

Comment 1

[Daniel Holbert [:dholbert]]

FWIW, I'm now using a slightly broader set of mozconfig options to trigger this (due to new explicit build-option dependencies being introduced via bug 929398 and bug 928530).

Updated STR: Build with these options:

ac_add_options --enable-debug --disable-optimize
ac_add_options --disable-ogg
ac_add_options --disable-opus
ac_add_options --disable-webm
ac_add_options --disable-webrtc
ac_add_options --disable-wave

(If you remove --disable-wave, then MOZ_CUBEB gets defined and the problem goes away.)

Comment 2

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #0)
> This usage was added in this cset for bug 865244:
> http://hg.mozilla.org/mozilla-central/diff/116e84abedca/content/media/
> AudioStream.cpp

...and the second half of the errors are for another usage that was added in this cset for bug 918861:
http://hg.mozilla.org/mozilla-central/diff/ac461ea5f961/content/media/AudioStream.cpp

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: fix v1

This reworks the logic in each of these functions so that we handle the return-case for the cubeb_* success condition first, so that we can take the "cubeb function failed" failure code-path for builds without MOZ_CUBEB (which seems like a reasonable behavior to have in such builds).

Tangentially, this patch also removes the variable "rv" from the modified code, both for consistency[1] and because it has the wrong type[2].

[1] In other places in this file (e.g. the latter chunk of this patch) we don't bother storing the rv, except for a few places where we react to it in a more complex way.
[2] rv is currently declared as type "uint32_t", whereas we set it to the result of "int cubeb_get_max_channel_count()".

Comment 4

[Daniel Holbert [:dholbert]]

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

Comment 5

[Carsten Book [:Tomcat]]

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