1399833 - Uninitialized variables in CubebUtils

Status: RESOLVED FIXED

(Core :: Audio/Video: cubeb, defect, P3)

Description

[Philippe Normand [:philn]]

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.78 Safari/537.36

Steps to reproduce:

For gecko-media we don't rely on Preferences (yet?) so we need default values for at least the sCubebPlaybackLatencyInMilliseconds variable.

Comment 1

[Philippe Normand [:philn]]

Attachment: proposed.patch

Comment 2

[Maire Reavy [:mreavy]]

Does this apply to 56?

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

Maire, you don't need to worry about this. This is not a problem in Firefox. These variables are initialized when we initialize Cubeb in Firefox.

I'm setting the tracking flags to unaffected so that nobody panics about this.

For the gecko-media Rust crate, I think we can just initialize the Preferences system so that these prefs are initialized just like they are in Firefox. I'm part way through that patch.

Comment 4

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8908083 [details] [diff] [review]
proposed.patch

I think sCubebPlaybackLatencyInMillisecond should be initialized to  CUBEB_NORMAL_LATENCY_MS.

I think sCubebMSGLatencyInFrames should be initiaized to CUBEB_NORMAL_LATENCY_FRAMES.

Paul, can you take a look at this and suggest whether these are sane default values? We need these as we're exporting Gecko's media stack out into a Rust crate, and the preferences system isn't stood up yet.

Comment 5

[Paul Adenot (:padenot)]

Comment on attachment 8908083 [details] [diff] [review]
proposed.patch

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

::: dom/media/CubebUtils.cpp
@@ +106,5 @@
>    Shutdown
>  } sCubebState = CubebState::Uninitialized;
>  cubeb* sCubebContext;
> +double sVolumeScale = 1.0;
> +uint32_t sCubebPlaybackLatencyInMilliseconds = 1;

Makes this 100 milliseconds.

@@ +107,5 @@
>  } sCubebState = CubebState::Uninitialized;
>  cubeb* sCubebContext;
> +double sVolumeScale = 1.0;
> +uint32_t sCubebPlaybackLatencyInMilliseconds = 1;
> +uint32_t sCubebMSGLatencyInFrames = 128;

Make this 512 frames. Note that in practice, we get this value via cubeb, it depends widely on the platform.

Comment 6

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: set default values for variables controlled by Preferences

This is needed for Servo's gecko-media where there is no support for
Mozilla Preferences.

MozReview-Commit-ID: HbDT42SRrE3

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a0bd70eac827
set default values for variables controlled by Preferences. r=padenot

Comment 8

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

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