Comment 27 for bug 732572

Revision history for this message
In , Chris Coulson (chrisccoulson) wrote :

(In reply to comment #15)
> Comment on attachment 550239 [diff] [details] [review]
> Add pulseaudio support for GTK nsISound.play impl
>
> Review of attachment 550239 [diff] [details] [review]:
> -----------------------------------------------------------------
>
> ::: widget/src/gtk2/nsSound.cpp
> @@ +129,5 @@
> > +typedef struct pa_sample_spec {
> > + pa_sample_format_t format;
> > + uint32_t rate;
> > + uint8_t channels;
> > +} pa_sample_spec;
>
> This is okay for now, but I'd prefer to include the headers. Please file a
> follow up bug to deal with this when bug 662417 is fixed.
>

Sure, will do. Will that mean that we can also make it a run-time dependency? (ie, can we eventually get rid of the dlopen/dlsym hackery?)

> @@ +417,5 @@
> > + pa_simple* s = pa_simple_new(NULL, name.get(), PA_STREAM_PLAYBACK, NULL, "mozillaSound", &ss, NULL, NULL, NULL);
> > + if (!s)
> > + return NS_ERROR_OUT_OF_MEMORY;
> > +
> > + pa_simple_write(s, audio, audio_len, NULL);
>
> You need to call pa_simple_drain here, otherwise playback may end
> prematurely when |s| is freed.

Good catch, thanks! I'll fix this now