GNOME Bugzilla – Bug 567794
[PATCH] Rework pulse plugin
Last modified: 2009-01-28 22:34:04 UTC
Heya! Please merge this patch for the pulse plugin for Gst: http://0pointer.de/public/gst-pulse-update.patch It changes quite a few things: * Hook pulsesink's volume property up with the stream volume -- not the sink volume in PA. * Read the device description directly from the sink instead of going via the mixer. * Properly implement _reset() methods for both sink and source to avoid deadlocks when shutting down a pipeline. * Replace all simple pa_threaded_mainloop_wait() by proper loops to guarantee that we wait for the right event in case multiple events are fired. While this is not strictly necessary in many cases it certainly is more correct and makes me sleep better at night. * Replace CHECK_DEAD_GOTO macros with proper functions * Extend the number of supported channels to 32 since that is the actual limit in PA. * Get rid of _dispose() methods since we don't need them. * Increase the volume property upper limit of the sink to 1000. * Reset function pointers after we disconnect a stream/context. Better fix for bug 556986. * Reset the state of the element properly if open/prepare fails * Cork the PA stream when the pipeline is paused. This allows the PA daemon to close audio device on pause and thus save a bit of power. * Set PA stream properties based on GST tags such as GST_TAG_TITLE, GST_TAG_ARTIST, and so on. The patch is against 0.10.11. The four patches that have been commited to CVS after that release should either be reversed anyway (see bug 567748) or are incorporated in this patch.
Maybe I should mention that I did some testing here locally but stuff like dynamic changing of the volume hasn't really been tested since i know no client which uses that.
This patch is using API and constants that didn't exist in the pulseaudio I have - 0.9.10. Applying it would make the pulse plugin unusable on Intrepid (and what other distros?), which seems like a pretty bad idea. here's some compile log: pulsesink.c: In function ‘gst_pulsesink_init’: pulsesink.c:266: error: implicit declaration of function ‘pa_sample_spec_init’ pulsesink.c: In function ‘gst_pulsesink_sink_input_info_cb’: pulsesink.c:390: error: implicit declaration of function ‘pa_cvolume_max’ pulsesink.c: In function ‘gst_pulsesink_is_dead’: pulsesink.c:395: error: implicit declaration of function ‘PA_CONTEXT_IS_GOOD’ pulsesink.c:396: error: implicit declaration of function ‘PA_STREAM_IS_GOOD’ pulsesink.c: In function ‘gst_pulsesink_prepare’: pulsesink.c:787: error: ‘PA_STREAM_ADJUST_LATENCY’ undeclared (first use in this function) pulsesink.c:787: error: (Each undeclared identifier is reported only once pulsesink.c:787: error: for each function it appears in.) pulsesink.c: In function ‘gst_pulsesink_change_props’: pulsesink.c:1035: error: ‘PA_PROP_MEDIA_TITLE’ undeclared (first use in this function) pulsesink.c:1036: error: ‘PA_PROP_MEDIA_ARTIST’ undeclared (first use in this function) pulsesink.c:1037: error: ‘PA_PROP_MEDIA_LANGUAGE’ undeclared (first use in this function) pulsesink.c:1038: error: ‘PA_PROP_MEDIA_FILENAME’ undeclared (first use in this function) pulsesink.c:1043: error: ‘pa_proplist’ undeclared (first use in this function) pulsesink.c:1043: error: ‘pl’ undeclared (first use in this function) pulsesink.c:1043: error: invalid operands to binary * (have ‘const gchar * const*’ and ‘const gchar * const*’) pulsesink.c:1043: error: statement with no effect pulsesink.c:1044: error: ISO C90 forbids mixed declarations and code pulsesink.c:1048: error: implicit declaration of function ‘pa_proplist_new’ pulsesink.c:1048: error: statement with no effect pulsesink.c:1056: error: implicit declaration of function ‘pa_proplist_sets’ pulsesink.c:1072: error: implicit declaration of function ‘pa_stream_proplist_update’ pulsesink.c:1072: error: ‘PA_UPDATE_REPLACE’ undeclared (first use in this function) pulsesink.c:1072: error: assignment makes pointer from integer without a cast pulsesink.c:1091: error: implicit declaration of function ‘pa_proplist_free’ If you know what version the various API was introduced in, perhaps we can add some defines to only use things when they're available?
Brrh. That's all 0.9.11 stuff. Almost everything that is warned about is pretty trivial to replace resp. disable for old pa versions. I'll update the patch.
Did you get a chance to update the patch? It doesn't look like the file linked above has changed. Since we now have git, you might like to rework it against git head? We probably need a configure bit to detect the available PA version. I can do that bit though.
I made an initial attempt at merging this patch with the changes to ext/pulse since gst-plugins-good 0.10.11, and adding conditional defines for compilation compatibility with older pulseaudio installs. I pushed WIP branch pulse-bg567794 up to fdo, or see http://cgit.freedesktop.org/gstreamer/gst-plugins-good/commit/?id=ebe6eacfef8d09278e5508c9f13d0fff92e4f967 I'm not sure my patch is correct with regard to how I disabled things on older PA, please check it over.
Actually, try the pulse-bg567794 branch git://anongit.freedesktop.org/~thaytan/gst-plugins-good http://cgit.freedesktop.org/~thaytan/gst-plugins-good
Created attachment 127376 [details] [review] fixed patch
That fixed patch simple readds a pa_operation_unref you apparently removed. Otherwise I couldn't spot any issues, so please merge.
Committed, thanks!