GNOME Bugzilla – Bug 663124
improvements to pulsesink/pulsesrc usage
Last modified: 2011-11-30 03:51:01 UTC
I'm attaching two patches to fix how Empathy uses pulsesink/pulsesrc. The first adjusts latency parameters. The second fixes the sample format that we request from PA in pulsesrc.
Created attachment 200366 [details] [review] 0001-element-properties-Set-proper-latency-values-on-puls.patch
Created attachment 200367 [details] [review] 0002-audio-src-Add-a-caps-filter-to-select-appropriate-in.patch
Review of attachment 200366 [details] [review]: element-properties is not used any more in empathy-call. I think we rely on Farstream's default settings, so probably best to make this change there.
Review of attachment 200367 [details] [review]: ::: src/empathy-audio-src.c @@ +230,3 @@ gst_bin_add (GST_BIN (obj), priv->src); + caps = gst_caps_new_simple ("audio/x-raw-int", Can you please add a comment here explaining (briefly) the reason on this caps filter.
(In reply to comment #3) > Review of attachment 200366 [details] [review]: > > element-properties is not used any more in empathy-call. I think we rely on > Farstream's default settings, so probably best to make this change there. Thinking about it, this change might not make sense at all. The changes should likely go into empathy-audio-{src,sink}.c where the elements are instantiated.
Created attachment 200385 [details] [review] 0002-audio-src-Add-a-caps-filter-to-select-appropriate-in.patch Added a comment about why we add the capsfilter as requested in comment #4.
(In reply to comment #5) > (In reply to comment #3) > > Review of attachment 200366 [details] [review] [details]: > > > > element-properties is not used any more in empathy-call. I think we rely on > > Farstream's default settings, so probably best to make this change there. > > Thinking about it, this change might not make sense at all. The changes should > likely go into empathy-audio-{src,sink}.c where the elements are instantiated. Okay, took a closer look at the code and I was wrong. I've filed a bug against farstream to add this to element-properties there too -- https://bugs.freedesktop.org/show_bug.cgi?id=42460
(In reply to comment #3) > Review of attachment 200366 [details] [review]: > > element-properties is not used any more in empathy-call. I think we rely on > Farstream's default settings, so probably best to make this change there. Farstream handles payloaders/codecs. Empathy sources/sinks atm, so we should: 0: Directly set these properties on pulsesrc and pulsesink 1: remove all usage of the element-properties file in empathy and just load the farstream default & rely on latest farstream Ofcourse for this bug doing 0 is enough. Btw, is 50ms for the src and 25ms for the sink not a bit high (e.g. why not both 20ms?)
Created attachment 200861 [details] [review] audio-src,audio-sink: Use lower latency values on pulsesrc/pulsesink This sets the pulsesink and pulsesrc buffer/latency parameters to be lower and more voip-friendly. If the system cannot provide or keep up with these values, PulseAudio will automatically try to adapt and provide larger values that are achievable.
Attached an updated patch that directly sets the buffer-time and latency-time values on pulsesrc/pulsesink. Also dropped to a lower value (20ms) as suggested, since PA will automatically adjust if the system can't cope.
Comment on attachment 200861 [details] [review] audio-src,audio-sink: Use lower latency values on pulsesrc/pulsesink Looking at the buffer-time/latency-time values and how they cause PA to behave, this needs a bit more work. buffer-time = 2*latency-time is not good.
Fixed up the latency values to 40ms buffer-time and 10ms latency-time for sink and 20ms buffer-time for the source and pushed, so all done.
I've hit an issue that I bisected to this patch: audio-src,audio-sink: Use lower latency values on pulsesrc/pulsesink (1.54 KB, patch) You may wish to check out/comment this bug: https://bugzilla.gnome.org/show_bug.cgi?id=665171