(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.
(In reply to comment #15) ------- ------- ------- ------- ------- ------- ------- ------- -- src/gtk2/ nsSound. cpp
> Comment on attachment 550239 [diff] [details] [review]
> Add pulseaudio support for GTK nsISound.play impl
>
> Review of attachment 550239 [diff] [details] [review]:
> -------
>
> ::: widget/
> @@ +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 @@ OUT_OF_ MEMORY;
> > + pa_simple* s = pa_simple_new(NULL, name.get(), PA_STREAM_PLAYBACK, NULL, "mozillaSound", &ss, NULL, NULL, NULL);
> > + if (!s)
> > + return NS_ERROR_
> > +
> > + 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