[pulseaudio-discuss] [PATCH v3 0/2] .d directories for configuration
Earlier discussions: - v1: http://thread.gmane.org/gmane.comp.audio.pulseaudio.general/23592 - v2: http://thread.gmane.org/gmane.comp.audio.pulseaudio.general/24578 Changes in v3: - Load the .d files after, not before, the main file. Changes v2: - Split the original patch into two parts. - Added documentation to the man pages. - Changed a debug message about non-existent .d directory from "scandir(\"%s\") failed: %s", dir_name, pa_cstrerror(errno) to "%s does not exist, ignoring.", dir_name Tanu Kaskinen (2): conf-parser: add support for .d directories client-conf, daemon-conf: enable .d directories man/pulse-client.conf.5.xml.in | 19 +++ man/pulse-daemon.conf.5.xml.in | 25 ++-- src/daemon/daemon-conf.c| 2 +- src/modules/alsa/alsa-mixer.c | 4 ++-- src/modules/module-augment-properties.c | 2 +- src/pulse/client-conf.c | 2 +- src/pulsecore/conf-parser.c | 42 +++-- src/pulsecore/conf-parser.h | 8 ++- 8 files changed, 85 insertions(+), 19 deletions(-) -- 2.6.2 ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] Run a user specified program on suspend/resume.
On Mon, 2015-12-07 at 18:19 +0200, Tanu Kaskinen wrote: > On Fri, 2015-08-21 at 23:29 +0300, Joonas Govenius wrote: > > This allows calling a user-specified external program whenever a > > source or sink is suspended or resumed (by specifying > > external_suspend_handler="" as an option to > > module-suspend-on-idle). The external executable is called > > with "--suspended" or "--resumed" followed by "--source > > " or "--sink ". > > > > My use case for this feature is to cut the power to my active > > speakers in order to eliminate hissing. I've been using the patch > > (applied to pulseaudio 4.0) on Ubuntu 14.04 since February. I > > have not tested it with later versions, except to check that it > > compiles. I could do some further testing if the patch is > > otherwise acceptable/useful enough. > > > > Some things I'm not sure about: > > > > * What happens on Windows? Does fork() work and if not, what does > > it return? Maybe some of the code should be wrapped > > with "#ifndef OS_IS_WIN32". > > > > * Security considerations? This might provide a sneaky way to run > > malicious code repeatedly, but only if you have write access to > > the config file. In that case you are probably screwed in a > > multitude of ways already... > > Write access to the file system is not needed. The only thing that is > needed is access to PulseAudio, because you can load modules also at > runtime. Using the module arguments to pass the executable doesn't seem > safe enough. Maybe we could add suspend-on-idle.conf, and have that the > only way to specify the external_suspend_handler option? By the way, why does this functionality need to be in the daemon? Clients can monitor the device state, so why not trigger the script from client side? -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 09/13] loopback: Track the amount of jitter
On Wed, 2015-02-25 at 19:43 +0100, Georg Chini wrote:
> ---
> src/modules/module-loopback.c | 18 +-
> 1 file changed, 17 insertions(+), 1 deletion(-)
The commit message should say something about why the jitter is tracked.
> diff --git a/src/modules/module-loopback.c b/src/modules/module-loopback.c
> index cbd0ac9..b733663 100644
> --- a/src/modules/module-loopback.c
> +++ b/src/modules/module-loopback.c
> @@ -95,6 +95,8 @@ struct userdata {
>
> pa_usec_t source_latency_sum;
> pa_usec_t sink_latency_sum;
> +pa_usec_t next_latency;
> +double latency_error;
>
> bool in_pop;
> bool pop_called;
> @@ -263,15 +265,22 @@ static void adjust_rates(struct userdata *u) {
> (double) current_latency / PA_USEC_PER_MSEC,
> (double) corrected_latency / PA_USEC_PER_MSEC,
> ((double) u->latency_snapshot.sink_latency +
> current_buffer_latency + u->latency_snapshot.source_latency) /
> PA_USEC_PER_MSEC);
> -pa_log_debug("Latency difference: %0.2f ms, rate difference: %i Hz",
> +pa_log_debug("Latency difference: %0.2f ± %0.2f ms, rate difference: %i
> Hz",
What does "± %0.2f ms" mean? Is the real latency difference between
those bounds with 100% confidence, or less than 100% confidence?
> (double) latency_difference / PA_USEC_PER_MSEC,
> +(double) 2.5 * u->latency_error * final_latency /
> PA_USEC_PER_MSEC,
Why is that 2.5 there?
--
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 10/13] loopback: Added a deadband to reduce rate hunting
On Wed, 2015-02-25 at 19:43 +0100, Georg Chini wrote:
> With USB or Bluetooth sources, the controller exhibited random
> deviations of new_rate around the correct value, due to latency jitter.
> Use the already-known latency error due to jitter, and assume that there
> is no latency difference if the difference is below that error margin.
> ---
> src/modules/module-loopback.c | 10 +++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/src/modules/module-loopback.c b/src/modules/module-loopback.c
> index b733663..6b48fc6 100644
> --- a/src/modules/module-loopback.c
> +++ b/src/modules/module-loopback.c
> @@ -183,12 +183,12 @@ static void teardown(struct userdata *u) {
> /* rate controller
> * - maximum deviation from base rate is less than 1%
> * - can create audible artifacts by changing the rate too quickly
> - * - exhibits hunting with USB or Bluetooth sources
> + * - deadband to handle error of latency measurement
> */
> static uint32_t rate_controller(
> uint32_t base_rate,
> pa_usec_t adjust_time,
> -int32_t latency_difference_usec) {
> +int32_t latency_difference_usec, pa_usec_t
> latency_error_usec) {
>
> uint32_t new_rate;
> double min_cycles;
> @@ -198,6 +198,10 @@ static uint32_t rate_controller(
> min_cycles = (double)abs(latency_difference_usec) / adjust_time / 0.0095
> + 1;
> new_rate = base_rate * (1.0 + (double)latency_difference_usec /
> min_cycles / adjust_time);
>
> +/* Adjust as good as physics allows (with some safety margin) */
> +if (abs(latency_difference_usec) <= 2.5 * latency_error_usec +
> adjust_time / 2 / base_rate + 100)
> + new_rate = base_rate;
There's that magic number 2.5 again...
What's that "adjust_time / 2 / base_rate"? The unit of that is seconds
squared, doesn't make sense to me...
Are you claiming that this results in adjustment that is as close to
optimal as physics allows? If so, what's the basis for that claim?
--
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 09/13] loopback: Track the amount of jitter
On Sat, 2015-12-12 at 11:52 +0100, Georg Chini wrote:
> On 08.12.2015 21:47, Tanu Kaskinen wrote:
> > On Wed, 2015-02-25 at 19:43 +0100, Georg Chini wrote:
> > > ---
> > > src/modules/module-loopback.c | 18 +-
> > > 1 file changed, 17 insertions(+), 1 deletion(-)
> > The commit message should say something about why the jitter is tracked.
>
> OK
>
> > > diff --git a/src/modules/module-loopback.c b/src/modules/module-loopback.c
> > > index cbd0ac9..b733663 100644
> > > --- a/src/modules/module-loopback.c
> > > +++ b/src/modules/module-loopback.c
> > > @@ -95,6 +95,8 @@ struct userdata {
> > >
> > > pa_usec_t source_latency_sum;
> > > pa_usec_t sink_latency_sum;
> > > +pa_usec_t next_latency;
> > > +double latency_error;
> > >
> > > bool in_pop;
> > > bool pop_called;
> > > @@ -263,15 +265,22 @@ static void adjust_rates(struct userdata *u) {
> > > (double) current_latency / PA_USEC_PER_MSEC,
> > > (double) corrected_latency / PA_USEC_PER_MSEC,
> > > ((double) u->latency_snapshot.sink_latency +
> > > current_buffer_latency + u->latency_snapshot.source_latency) /
> > > PA_USEC_PER_MSEC);
> > > -pa_log_debug("Latency difference: %0.2f ms, rate difference: %i Hz",
> > > +pa_log_debug("Latency difference: %0.2f ± %0.2f ms, rate difference:
> > > %i Hz",
> > What does "± %0.2f ms" mean? Is the real latency difference between
> > those bounds with 100% confidence, or less than 100% confidence?
>
> This is just an indication of the current error (with respect to the model),
> so not 100% confidence.
Can you assign any particular confidence to the numbers? If not, then
the numbers have very little meaning... They can be useful for seeing
whether the jitter is higher or lower compared to some other
measurements, but they say very little about whether the latency
difference really is between those bounds.
At the very least there should be a comment explaining how the numbers
should be interpreted.
> >
> > > (double) latency_difference / PA_USEC_PER_MSEC,
> > > +(double) 2.5 * u->latency_error * final_latency /
> > > PA_USEC_PER_MSEC,
> > Why is that 2.5 there?
> >
>
> Good question. I think it was copied over from the definition of the
> deadband in the next
> patch. The observed errors are however in good agreement with that factor.
What does that mean? What would be different if the factor wasn't
there? How would it be worse than with the magic factor?
--
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] typedefs.h: Move some typedefs to a separate file
On Thu, 2015-11-26 at 18:29 +0100, David Henningsson wrote: > The relationship between sinks, sources, cards, profiles, and ports > is becoming ever more intertwined, to the point that if you try to > include one file from the other, you're likely to end up with some > weird error somewhere else. > > Work around this by creating a new typedefs.h, which does not depend > on anything else, and just creates a few typedefs. > > (Can be expanded with more typedefs in the future if the need arises.) > > Signed-off-by: David Henningsson > --- > src/pulsecore/card.h | 7 +++ > src/pulsecore/client.h| 3 +-- > src/pulsecore/core.h | 3 +-- > src/pulsecore/device-port.h | 3 +-- > src/pulsecore/sink-input.h| 3 +-- > src/pulsecore/sink.h | 4 +--- > src/pulsecore/source-output.h | 3 +-- > src/pulsecore/source.h| 3 +-- > src/pulsecore/typedefs.h | 37 + > 9 files changed, 47 insertions(+), 19 deletions(-) > create mode 100644 src/pulsecore/typedefs.h I pushed this now. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] is there simple remote control?
On Sat, 2015-12-12 at 20:42 +, haman...@t-online.de wrote: > Hi, > > imagine this situation: box A is sort of an entertainment station, and it is > using PA for sound. > Now box B is an asterisk telephony server. Now, when the phone rings, it > would be great > to reduce the valume of sound or mute it, and restore when the phone is hung > up. > Is there anything ready to use? > When googling for this subject, I saw sort of a complete gui remote control > (reverb). > However, the telephony box will likely have neither graphics nor sound > software installed If I understood correctly, you want to control the volume of box A from box B. You can do that by enabling network access on box A, configuring PulseAudio clients on box B to connect to box A and using pactl on box B to set the volume. How to enable network access on box A: Load module-native-protocol-tcp. If you're running pulseaudio in the system mode, load it in /etc/pulse/system.pa. If you're running pulseaudio in the user mode, copy /etc/pulse/default.pa to ~/.config/pulse/default.pa of the appropriate user and put the load command there (don't edit /etc/pulse/default.pa directly, because you don't want all users to try to accept network connections on the same TCP port). See https://wiki.freedesktop.org/www/Software/PulseAudio/Documentation/ User/Modules/ for information about various options that module-native- protocol-tcp accepts. For example, you may want to configure how authorization is done (or not done). How to configure PulseAudio clients on box B: Add "default-server = boxA" to ~/.config/pulse/client.conf of the appropriate user (or /etc/pulse/client.conf, if you want all users to share the same configuration). "boxA" can be either an IP address or a DNS name. If you enable cookie-based authorization (which is the default), you will have to copy the cookie from box A to ~/.config/pulse/cookie on box B for all users on box B that should be able to have access on box A. The cookie location on box A is ~/.config/pulse/cookie if pulseaudio runs in the user mode, and /var/lib/pulse/cookie if pulseaudio runs in the system mode. How to use pactl: See "man pactl". -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] sink-input: Don't access resampler to get silence memchunk
On Tue, 2015-12-15 at 21:49 +0530, a...@accosted.net wrote: > From: Arun Raghavan > > There doesn't appear to be a good reason to restrict the memchunk length > to the resample max block size -- we're going to have the memory around > anyway. I think the reason is to make sure that we don't feed the resampler bigger chunks than what it can handle. The resampler has to allocate other memblocks during its operation, and those memblocks may be bigger than the input block, so if the input block is too large, the requirements for the other blocks will grow beyond the mempool max block size. However, pa_sink_input_peek() seems to protect against this anyway when doing resampling (it processes the input in smaller pieces if it's larger than the resampler max block size), so maybe this change is safe anyway. > Moreover, callers of pa_sink_input_get_silence() don't seem to > actually care about the chunk itself, just the memblock for creating > their own pa_memblockq. I don't understand this comment. pa_memblockq cares about the chunk itself, not just the memblock. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2 01/25] echo-cancel: Update webrtc-audio-processing usage to new API
(I'm just glancing through, this is not a proper review.) On Wed, 2015-12-16 at 09:09 +0530, a...@accosted.net wrote: > From: Arun Raghavan > > --- > configure.ac | 2 +- > src/Makefile.am | 2 +- > src/modules/echo-cancel/webrtc.cc | 54 > +-- > 3 files changed, 31 insertions(+), 27 deletions(-) > > diff --git a/configure.ac b/configure.ac > index b9cd3d1..26c3e29 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -1371,7 +1371,7 @@ AC_ARG_ENABLE([webrtc-aec], > AS_HELP_STRING([--enable-webrtc-aec], [Enable the optional WebRTC-based > echo canceller])) > > AS_IF([test "x$enable_webrtc_aec" != "xno"], > -[PKG_CHECK_MODULES(WEBRTC, [ webrtc-audio-processing ], [HAVE_WEBRTC=1], > [HAVE_WEBRTC=0])], > +[PKG_CHECK_MODULES(WEBRTC, [ webrtc-audio-processing > 0.1 ], > [HAVE_WEBRTC=1], [HAVE_WEBRTC=0])], I think it would be better to use >= 0.2 (or whatever is the minimum required version). > [HAVE_WEBRTC=0]) > > AS_IF([test "x$enable_webrtc_aec" = "xyes" && test "x$HAVE_WEBRTC" = "x0"], > diff --git a/src/Makefile.am b/src/Makefile.am > index f1bd38d..533b646 100644 > --- a/src/Makefile.am > +++ b/src/Makefile.am > @@ -50,7 +50,7 @@ AM_CPPFLAGS = \ > -DPULSE_LOCALEDIR=\"$(localedir)\" > AM_CFLAGS = \ > $(PTHREAD_CFLAGS) > -AM_CXXFLAGS = $(AM_CFLAGS) > +AM_CXXFLAGS = $(AM_CFLAGS) -std=c++11 This seems like material for a separate patch. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2 01/25] echo-cancel: Update webrtc-audio-processing usage to new API
On Wed, 2015-12-16 at 09:48 +0530, Arun Raghavan wrote: > On 16 December 2015 at 09:38, Tanu Kaskinen wrote: > > (I'm just glancing through, this is not a proper review.) > > > > On Wed, 2015-12-16 at 09:09 +0530, a...@accosted.net wrote: > > > From: Arun Raghavan > > > > > > --- > > > configure.ac | 2 +- > > > src/Makefile.am | 2 +- > > > src/modules/echo-cancel/webrtc.cc | 54 +-- > > > > > > 3 files changed, 31 insertions(+), 27 deletions(-) > > > > > > diff --git a/configure.ac b/configure.ac > > > index b9cd3d1..26c3e29 100644 > > > --- a/configure.ac > > > +++ b/configure.ac > > > @@ -1371,7 +1371,7 @@ AC_ARG_ENABLE([webrtc-aec], > > > AS_HELP_STRING([--enable-webrtc-aec], [Enable the optional > > > WebRTC-based echo canceller])) > > > > > > AS_IF([test "x$enable_webrtc_aec" != "xno"], > > > -[PKG_CHECK_MODULES(WEBRTC, [ webrtc-audio-processing ], > > > [HAVE_WEBRTC=1], [HAVE_WEBRTC=0])], > > > +[PKG_CHECK_MODULES(WEBRTC, [ webrtc-audio-processing > 0.1 > > > ], [HAVE_WEBRTC=1], [HAVE_WEBRTC=0])], > > > > I think it would be better to use >= 0.2 (or whatever is the > > minimum > > required version). > > Sure, I can do that. Was easier to test this way before doing a 0.2 > release of webrtc-audio-processing. > > > > [HAVE_WEBRTC=0]) > > > > > > AS_IF([test "x$enable_webrtc_aec" = "xyes" && test > > > "x$HAVE_WEBRTC" = "x0"], > > > diff --git a/src/Makefile.am b/src/Makefile.am > > > index f1bd38d..533b646 100644 > > > --- a/src/Makefile.am > > > +++ b/src/Makefile.am > > > @@ -50,7 +50,7 @@ AM_CPPFLAGS = \ > > > -DPULSE_LOCALEDIR=\"$(localedir)\" > > > AM_CFLAGS = \ > > > $(PTHREAD_CFLAGS) > > > -AM_CXXFLAGS = $(AM_CFLAGS) > > > +AM_CXXFLAGS = $(AM_CFLAGS) -std=c++11 > > > > This seems like material for a separate patch. > > This is needed for the code to compile at all, is why I included it > here. Ok, makes sense. It would be good to note the reason in the commit message, though. Does the requirement for c++11 come from the library or from the code changes in this patch? -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2 08/25] echo-cancel: Mark private function as static
On Wed, 2015-12-16 at 09:09 +0530, a...@accosted.net wrote:
> From: Arun Raghavan
>
> ---
> src/modules/echo-cancel/webrtc.cc | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/modules/echo-cancel/webrtc.cc
> b/src/modules/echo-cancel/webrtc.cc
> index 3be7fe5..a8b69b5 100644
> --- a/src/modules/echo-cancel/webrtc.cc
> +++ b/src/modules/echo-cancel/webrtc.cc
> @@ -78,9 +78,9 @@ static int routing_mode_from_string(const char *rmode) {
> return -1;
> }
>
> -void pa_webrtc_ec_fixate_spec(pa_sample_spec *rec_ss, pa_channel_map
> *rec_map,
> - pa_sample_spec *play_ss, pa_channel_map
> *play_map,
> - pa_sample_spec *out_ss, pa_channel_map
> *out_map)
> +static void pa_webrtc_ec_fixate_spec(pa_sample_spec *rec_ss, pa_channel_map
> *rec_map,
> + pa_sample_spec *play_ss, pa_channel_map
> *play_map,
> + pa_sample_spec *out_ss, pa_channel_map
> *out_map)
> {
> rec_ss->format = PA_SAMPLE_S16NE;
> play_ss->format = PA_SAMPLE_S16NE;
The pa_ prefix should be dropped for static functions, and "webrtc"
(maybe also "ec") seems redundant too.
--
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2 15/25] build-sys: Move to compiling with C11 support
On Wed, 2015-12-16 at 09:10 +0530, a...@accosted.net wrote: > From: Arun Raghavan > > This is needed for building with anonymous unions. A bunch of calls to > fail() that used to mysteriously work need fixing -- fail() is a macro > that takes a printf-style message as an argument. Not passing this > somehow worked with the previous compiler flags, but breaks with > -std=c11. > --- > configure.ac | 3 ++- > src/tests/connect-stress.c | 6 +++--- > src/tests/cpu-mix-test.c | 2 +- > src/tests/cpu-remap-test.c | 4 ++-- > src/tests/cpu-sconv-test.c | 4 ++-- > src/tests/cpu-volume-test.c | 2 +- > src/tests/cpulimit-test.c| 4 ++-- > src/tests/extended-test.c| 4 ++-- > src/tests/get-binary-name-test.c | 2 +- > src/tests/interpol-test.c| 2 +- > src/tests/mult-s16-test.c| 2 +- > src/tests/sync-playback.c| 4 ++-- > 12 files changed, 20 insertions(+), 19 deletions(-) > > diff --git a/configure.ac b/configure.ac > index 26c3e29..6fc77c5 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -80,7 +80,6 @@ AC_PROG_LN_S > # CC > > AC_PROG_CC > -AC_PROG_CC_C99 > AM_PROG_CC_C_O > # Only required if you want the WebRTC canceller -- no runtime dep on > # libstdc++ otherwise > @@ -176,6 +175,8 @@ esac > > Compiler flags > > +AX_APPEND_COMPILE_FLAGS([-std=c11], [], [-pedantic -Werror]) We discussed this a while back in irc, and my recollection is that we agreed that some macro with "CHECK" in its name seemed the best fit. AX_APPEND_COMPILE_FLAGS is not good, because it silently drops the flag if -std=c11 isn't supported. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 12/13] loopback: Validate the rate parameter
On Wed, 2015-02-25 at 19:43 +0100, Georg Chini wrote:
> ---
> src/modules/module-loopback.c | 5 +
> src/pulse/sample.c| 5 -
> 2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/src/modules/module-loopback.c b/src/modules/module-loopback.c
> index d22afb5..7474ef2 100644
> --- a/src/modules/module-loopback.c
> +++ b/src/modules/module-loopback.c
> @@ -1039,6 +1039,11 @@ int pa__init(pa_module *m) {
> goto fail;
> }
>
> +if (ss.rate < 4000 || ss.rate > PA_RATE_MAX) {
> +pa_log("Invalid rate specification, valid range is 4000 Hz to %i
> Hz", PA_RATE_MAX);
> +goto fail;
> +}
> +
I pushed this change...
> if (pa_modargs_get_value(ma, "format", NULL))
> format_set = true;
>
> diff --git a/src/pulse/sample.c b/src/pulse/sample.c
> index 1331c72..69ac83f 100644
> --- a/src/pulse/sample.c
> +++ b/src/pulse/sample.c
> @@ -106,7 +106,10 @@ int pa_sample_format_valid(unsigned format) {
> }
>
> int pa_sample_rate_valid(uint32_t rate) {
> -return rate > 0 && rate <= PA_RATE_MAX;
> +/* The extra 1% is due to module-loopback: it temporarily sets
> + * a higher-than-nominal rate to get rid of excessive buffer
> + * latency */
> +return rate > 0 && rate <= PA_RATE_MAX * 101 / 100;
...but I left this one out. The two changes fix two separate issues, so
they should be in separate patches.
Making pa_sample_rate_valid() accept values above PA_RATE_MAX isn't
very nice, but I can see how it's better than the current behaviour,
which probably can cause the daemon to crash. Alexander mentioned in
IRC that he'd prefer doing the adaptive resampling inside module-
loopback rather than using pa_sink_input_set_rate(), and this change
demonstrates one good reason for that. I'm not asking you to implement
that change, but it certainly would be nice.
--
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 12/13] loopback: Validate the rate parameter
On Fri, 2015-12-18 at 07:47 +0100, Georg Chini wrote:
> On 18.12.2015 06:49, Tanu Kaskinen wrote:
> > On Wed, 2015-02-25 at 19:43 +0100, Georg Chini wrote:
> >> ---
> >> src/modules/module-loopback.c | 5 +
> >> src/pulse/sample.c | 5 -
> >> 2 files changed, 9 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/src/modules/module-loopback.c b/src/modules/module-loopback.c
> >> index d22afb5..7474ef2 100644
> >> --- a/src/modules/module-loopback.c
> >> +++ b/src/modules/module-loopback.c
> >> @@ -1039,6 +1039,11 @@ int pa__init(pa_module *m) {
> >> goto fail;
> >> }
> >>
> >> + if (ss.rate < 4000 || ss.rate > PA_RATE_MAX) {
> >> + pa_log("Invalid rate specification, valid range is 4000 Hz to %i
> >> Hz", PA_RATE_MAX);
> >> + goto fail;
> >> + }
> >> +
> > I pushed this change...
> >
> >> if (pa_modargs_get_value(ma, "format", NULL))
> >> format_set = true;
> >>
> >> diff --git a/src/pulse/sample.c b/src/pulse/sample.c
> >> index 1331c72..69ac83f 100644
> >> --- a/src/pulse/sample.c
> >> +++ b/src/pulse/sample.c
> >> @@ -106,7 +106,10 @@ int pa_sample_format_valid(unsigned format) {
> >> }
> >>
> >> int pa_sample_rate_valid(uint32_t rate) {
> >> - return rate > 0 && rate <= PA_RATE_MAX;
> >> + /* The extra 1% is due to module-loopback: it temporarily sets
> >> + * a higher-than-nominal rate to get rid of excessive buffer
> >> + * latency */
> >> + return rate > 0 && rate <= PA_RATE_MAX * 101 / 100;
> > ...but I left this one out. The two changes fix two separate issues, so
> > they should be in separate patches.
> >
> > Making pa_sample_rate_valid() accept values above PA_RATE_MAX isn't
> > very nice, but I can see how it's better than the current behaviour,
> > which probably can cause the daemon to crash. Alexander mentioned in
> > IRC that he'd prefer doing the adaptive resampling inside module-
> > loopback rather than using pa_sink_input_set_rate(), and this change
> > demonstrates one good reason for that. I'm not asking you to implement
> > that change, but it certainly would be nice.
>
> Yes, doing it inside the module would really be a good idea. The
> main issue my controller is fighting against are the systematic
> errors introduced by changing the rate of the sink input. When
> you change the rate, the next latency report has an extremely
> high error.
> Can you point me to some code that implements resampling
> so that I have an example of what to do?
Good that you asked, because it got me thinking about this a bit more,
and I'm starting to think that it's not worth it after all. Adding
another resampler would in many situations mean that the audio would
get resampled twice (or three times, if we count the source ouput
resampler, but that's beside the point). That doesn't seem like a good
idea due to the extra cpu load.
Something that could be considered is to have separate "nominal rate"
and "real rate" stored in pa_sink_input, so that the small rate tweaks
that module-loopback applies wouldn't be visible where they don't need
to be visible, and pa_sample_rate_valid() wouldn't have to accept rates
higher than PA_RATE_MAX.
(If you anyway want to see code that does resampling, grep for
"pa_resampler_run".)
--
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 12/13] loopback: Validate the rate parameter
On Fri, 2015-12-18 at 09:10 +0100, Georg Chini wrote: > On 18.12.2015 09:07, Tanu Kaskinen wrote: > > Good that you asked, because it got me thinking about this a bit more, > > and I'm starting to think that it's not worth it after all. Adding > > another resampler would in many situations mean that the audio would > > get resampled twice (or three times, if we count the source ouput > > resampler, but that's beside the point). That doesn't seem like a good > > idea due to the extra cpu load. > > > > Something that could be considered is to have separate "nominal rate" > > and "real rate" stored in pa_sink_input, so that the small rate tweaks > > that module-loopback applies wouldn't be visible where they don't need > > to be visible, and pa_sample_rate_valid() wouldn't have to accept rates > > higher than PA_RATE_MAX. > > > > (If you anyway want to see code that does resampling, grep for > > "pa_resampler_run".) > > Wouldn't it avoid additional resampling when you set the sink_input rate > to the sink rate? Yes, but I fear that would require replicating the pa_sink_input.render_memblockq buffer and some of its rewinding logic in the loopback module to deal with stream moves. I don't think it's worth it. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH] always-sink: simplify hook management with pa_module_hook_connect()
---
src/modules/module-always-sink.c | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/src/modules/module-always-sink.c b/src/modules/module-always-sink.c
index b5721bf..12f63f7 100644
--- a/src/modules/module-always-sink.c
+++ b/src/modules/module-always-sink.c
@@ -47,7 +47,6 @@ static const char* const valid_modargs[] = {
};
struct userdata {
-pa_hook_slot *put_slot, *unlink_slot;
uint32_t null_module;
bool ignore;
char *sink_name;
@@ -162,8 +161,8 @@ int pa__init(pa_module*m) {
m->userdata = u = pa_xnew(struct userdata, 1);
u->sink_name = pa_xstrdup(pa_modargs_get_value(ma, "sink_name",
DEFAULT_SINK_NAME));
-u->put_slot = pa_hook_connect(&m->core->hooks[PA_CORE_HOOK_SINK_PUT],
PA_HOOK_LATE, (pa_hook_cb_t) put_hook_callback, u);
-u->unlink_slot =
pa_hook_connect(&m->core->hooks[PA_CORE_HOOK_SINK_UNLINK], PA_HOOK_EARLY,
(pa_hook_cb_t) unlink_hook_callback, u);
+pa_module_hook_connect(m, &m->core->hooks[PA_CORE_HOOK_SINK_PUT],
PA_HOOK_LATE, (pa_hook_cb_t) put_hook_callback, u);
+pa_module_hook_connect(m, &m->core->hooks[PA_CORE_HOOK_SINK_UNLINK],
PA_HOOK_EARLY, (pa_hook_cb_t) unlink_hook_callback, u);
u->null_module = PA_INVALID_INDEX;
u->ignore = false;
@@ -182,10 +181,6 @@ void pa__done(pa_module*m) {
if (!(u = m->userdata))
return;
-if (u->put_slot)
-pa_hook_slot_free(u->put_slot);
-if (u->unlink_slot)
-pa_hook_slot_free(u->unlink_slot);
if (u->null_module != PA_INVALID_INDEX && m->core->state !=
PA_CORE_SHUTDOWN)
pa_module_unload_request_by_index(m->core, u->null_module, true);
--
2.6.4
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH] sink-input, source-output: don't allow moving of streams connected to moving filter devices
This fixes a crash that was observed in the following scenario:
A phone applications creates playback and recording streams with
filter.want=echo-cancel. An echo-cancel sink gets created. The alsa
card profile has enabled a 4.0 sink, and the user changes the profile
to have a stereo sink instead. A complex sequence of events happens:
the echo-cancel sink input gets detached from the alsa sink, the 4.0
sink gets removed, which triggers loading of a null sink by
module-always-sink, and that in turn triggers rerouting in
module-device-manager, which decides to move the phone stream to the
null sink. Moving a sink input involves sending a message to the IO
thread of the old sink, but in this case the old sink is the
echo-cancel sink, which was detached from the now-dead alsa sink.
Therefore, the echo-cancel sink doesn't have an IO thread associated
with it, and as can be expected, sending a message to a non-existing
thread results in a crash.
The crash can be avoided by disallowing moving streams that are
connected to filter devices that themselves are being moved. The
profile switch still doesn't work smoothly, though, because the
echo-cancel streams don't support moving, so when the old alsa sink
goes away, module-echo-cancel gets unloaded, and since the
echo-cancel sink input got detached before that, the phone sink input
isn't movable either and gets killed. But that's a separate issue.
BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=93443
---
src/pulsecore/sink-input.c| 26 ++
src/pulsecore/source-output.c | 27 +++
2 files changed, 53 insertions(+)
diff --git a/src/pulsecore/sink-input.c b/src/pulsecore/sink-input.c
index 539ae17..0e01893 100644
--- a/src/pulsecore/sink-input.c
+++ b/src/pulsecore/sink-input.c
@@ -1528,6 +1528,22 @@ static bool find_filter_sink_input(pa_sink_input
*target, pa_sink *s) {
return false;
}
+static bool is_filter_sink_moving(pa_sink_input *i) {
+pa_sink *sink = i->sink;
+
+if (!sink)
+return false;
+
+while (sink->input_to_master) {
+sink = sink->input_to_master->sink;
+
+if (!sink)
+return true;
+}
+
+return false;
+}
+
/* Called from main context */
bool pa_sink_input_may_move_to(pa_sink_input *i, pa_sink *dest) {
pa_sink_input_assert_ref(i);
@@ -1547,6 +1563,16 @@ bool pa_sink_input_may_move_to(pa_sink_input *i, pa_sink
*dest) {
return false;
}
+/* If this sink input is connected to a filter sink that itself is moving,
+ * then don't allow the move. Moving requires sending a message to the IO
+ * thread of the old sink, and if the old sink is a filter sink that is
+ * moving, there's no IO thread associated to the old sink. */
+if (is_filter_sink_moving(i)) {
+pa_log_debug("Can't move input from filter sink %s, because the filter
sink itself is currently moving.",
+ i->sink->name);
+return false;
+}
+
if (pa_idxset_size(dest->inputs) >= PA_MAX_INPUTS_PER_SINK) {
pa_log_warn("Failed to move sink input: too many inputs per sink.");
return false;
diff --git a/src/pulsecore/source-output.c b/src/pulsecore/source-output.c
index 9000972..24e9df4 100644
--- a/src/pulsecore/source-output.c
+++ b/src/pulsecore/source-output.c
@@ -1184,6 +1184,22 @@ static bool find_filter_source_output(pa_source_output
*target, pa_source *s) {
return false;
}
+static bool is_filter_source_moving(pa_source_output *o) {
+pa_source *source = o->source;
+
+if (!source)
+return false;
+
+while (source->output_from_master) {
+source = source->output_from_master->source;
+
+if (!source)
+return true;
+}
+
+return false;
+}
+
/* Called from main context */
bool pa_source_output_may_move_to(pa_source_output *o, pa_source *dest) {
pa_source_output_assert_ref(o);
@@ -1202,6 +1218,17 @@ bool pa_source_output_may_move_to(pa_source_output *o,
pa_source *dest) {
return false;
}
+/* If this source output is connected to a filter source that itself is
+ * moving, then don't allow the move. Moving requires sending a message to
+ * the IO thread of the old source, and if the old source is a filter
+ * source that is moving, there's no IO thread associated to the old
+ * source. */
+if (is_filter_source_moving(o)) {
+pa_log_debug("Can't move output from filter source %s, because the
filter source itself is currently moving.",
+ o->source->name);
+return false;
+}
+
if (pa_idxset_size(dest->outputs) >= PA_MAX_OUTPUTS_PER_SOURCE) {
pa_log_warn("Failed to move source output: too many outputs per
source.");
return false;
--
2.6.4
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] sink-input, source-output: don't allow moving of streams connected to moving filter devices
On Mon, 2015-12-21 at 06:55 +0530, Arun Raghavan wrote:
> On 20 December 2015 at 15:48, Tanu Kaskinen wrote:
> > This fixes a crash that was observed in the following scenario:
> >
> > A phone applications creates playback and recording streams with
> > filter.want=echo-cancel. An echo-cancel sink gets created. The alsa
> > card profile has enabled a 4.0 sink, and the user changes the profile
> > to have a stereo sink instead. A complex sequence of events happens:
> > the echo-cancel sink input gets detached from the alsa sink, the 4.0
> > sink gets removed, which triggers loading of a null sink by
> > module-always-sink, and that in turn triggers rerouting in
> > module-device-manager, which decides to move the phone stream to the
> > null sink. Moving a sink input involves sending a message to the IO
> > thread of the old sink, but in this case the old sink is the
> > echo-cancel sink, which was detached from the now-dead alsa sink.
> > Therefore, the echo-cancel sink doesn't have an IO thread associated
> > with it, and as can be expected, sending a message to a non-existing
> > thread results in a crash.
> >
> > The crash can be avoided by disallowing moving streams that are
> > connected to filter devices that themselves are being moved. The
> > profile switch still doesn't work smoothly, though, because the
> > echo-cancel streams don't support moving, so when the old alsa sink
> > goes away, module-echo-cancel gets unloaded, and since the
> > echo-cancel sink input got detached before that, the phone sink input
> > isn't movable either and gets killed. But that's a separate issue.
> >
> > BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=93443
> > ---
> > src/pulsecore/sink-input.c| 26 ++
> > src/pulsecore/source-output.c | 27 +++
> > 2 files changed, 53 insertions(+)
> >
> > diff --git a/src/pulsecore/sink-input.c b/src/pulsecore/sink-input.c
> > index 539ae17..0e01893 100644
> > --- a/src/pulsecore/sink-input.c
> > +++ b/src/pulsecore/sink-input.c
> > @@ -1528,6 +1528,22 @@ static bool find_filter_sink_input(pa_sink_input
> > *target, pa_sink *s) {
> > return false;
> > }
> >
> > +static bool is_filter_sink_moving(pa_sink_input *i) {
> > +pa_sink *sink = i->sink;
> > +
> > +if (!sink)
> > +return false;
> > +
> > +while (sink->input_to_master) {
> > +sink = sink->input_to_master->sink;
> > +
> > +if (!sink)
> > +return true;
> > +}
> > +
> > +return false;
> > +}
> > +
> > /* Called from main context */
> > bool pa_sink_input_may_move_to(pa_sink_input *i, pa_sink *dest) {
> > pa_sink_input_assert_ref(i);
> > @@ -1547,6 +1563,16 @@ bool pa_sink_input_may_move_to(pa_sink_input *i,
> > pa_sink *dest) {
> > return false;
> > }
> >
> > +/* If this sink input is connected to a filter sink that itself is
> > moving,
> > + * then don't allow the move. Moving requires sending a message to the
> > IO
> > + * thread of the old sink, and if the old sink is a filter sink that is
> > + * moving, there's no IO thread associated to the old sink. */
> > +if (is_filter_sink_moving(i)) {
> > +pa_log_debug("Can't move input from filter sink %s, because the
> > filter sink itself is currently moving.",
> > + i->sink->name);
> > +return false;
> > +}
> > +
> > if (pa_idxset_size(dest->inputs) >= PA_MAX_INPUTS_PER_SINK) {
> > pa_log_warn("Failed to move sink input: too many inputs per
> > sink.");
> > return false;
> > diff --git a/src/pulsecore/source-output.c b/src/pulsecore/source-output.c
> > index 9000972..24e9df4 100644
> > --- a/src/pulsecore/source-output.c
> > +++ b/src/pulsecore/source-output.c
> > @@ -1184,6 +1184,22 @@ static bool
> > find_filter_source_output(pa_source_output *target, pa_source *s) {
> > return false;
> > }
> >
> > +static bool is_filter_source_moving(pa_source_output *o) {
> > +pa_source *source = o->source;
> > +
> > +if (!source)
> > +return false;
> > +
> > +while (source->output_from_master) {
> > +source = source->output_from_master->source;
> > +
> > +if (!source)
> > +return true;
> > +
Re: [pulseaudio-discuss] [PATCH] daemon: Sanitise message about unsupported high res timers
On Mon, 2015-12-21 at 10:59 +0530, a...@accosted.net wrote:
> From: Arun Raghavan
>
> ---
> src/daemon/main.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/daemon/main.c b/src/daemon/main.c
> index c7b15e7..c2f47b6 100644
> --- a/src/daemon/main.c
> +++ b/src/daemon/main.c
> @@ -998,9 +998,9 @@ int main(int argc, char *argv[]) {
> pa_disable_sigpipe();
>
> if (pa_rtclock_hrtimer())
> -pa_log_info("Fresh high-resolution timers available! Bon appetit.");
> +pa_log_info("System supports high resolution timers");
> else
> -pa_log_info("Dude, your kernel stinks! The chef's recommendation
> today is Linux with high-resolution timers enabled.");
> +pa_log_info("System appears to not support high resolution timers");
>
> if (conf->lock_memory) {
> #if defined(HAVE_SYS_MMAN_H) && !defined(__ANDROID__)
Ack.
--
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Change sound via acpid
On Tue, 2015-12-22 at 21:39 -0800, John W wrote: > Hello, > > I'm trying to make the mute/unmute key on my laptop work. > It generates acpi events, but does not generate XF86AudioMute X11 > message - In fact, I prefer it to be that way, since I don't want to > rely upon X11 running in order to do this. > > I'm trying to figure out the "right" way to do this. > N.B.: It is my understanding that running in system mode, while it may > work, is not the "right" way to do this. I am looking for an > alternative to that, if possible. > > My acpi script runs (as root), and I want it to toggle the mute state. > > I tried "amixer set Master toggle", but that doesn't work as root. > I can reproduce it with "sudo" - see below: > > $ sudo amixer set Master toggle > ALSA lib pulse.c:243:(pulse_connect) PulseAudio: Unable to > connect: Connection refused > > amixer: Mixer attach default error: Connection refused > > From reading around, I came to learn that this can be caused by a > missing "cookie" file. > However, even copying the /home/myuser/.config/pulse/cookie file to > /root/.config/pulse/cookie did not solve this issue. > > Question 0: Is there any way to get logging information about these > denials, from pulseaudio? The usual advice is to run pulseaudio as a > non-daemon, with "-" option. I can do this, but even so, none of > these "Connection refused" incidents appear. > Is there somewhere else I should look? > > Question 1: > What other reason could there be for this "Connection refused" from > pulse? Any ideas? > Is there any good way for me to confirm that my copied cookie file is > correct, to try to narrow down the problem? "Connection refused" doesn't mean that the daemon refused the client connection (that would be "Access denied" or something like that). "Connection refused" means that the client didn't find the daemon. If you meant to connect to the pulseaudio daemon of "myuser" under "root", that will fail, because root will look for root's pulseaudio daemon socket, not myuser's daemon socket. > So, I abandoned that approach. Even in the best case, this would > require re-copying the cookie every time I rebooted, or having my > script figure out who spawned pulseaudio, then look in their user > directory, etc. Seems hacky to me. > > I learned about "system mode", but given that the developers > themselves say it is unsupported and bad, I am not touching that. Nor > do I trust all that pulseaudio code running as root (: It's not true that the system mode is unsupported, and pulseaudio will run under the user "pulse" in the system mode, not under root. That said, using the system mode doesn't seem like the right solution in this case. > Next, I found out about the unix socket authentication options in > /etc/pulse/default.pa > I created /etc/pulse/client.conf, and added: > > default-server = unix:/tmp/pulse-access-socket > > And made a corresponding change in /etc/pulse/default.pa: > > load-module module-native-protocol-unix auth-anonymous=1 > socket=/tmp/pulse-access-socket > > But this does not seem to work. > Looking at the manpages, there are some hints that maybe these options > only apply in system mode? I am not sure... The options aren't specific to the system mode. Without error messages, I don't know why it didn't work. If there are multiple users running pulseaudio, one problem would be all of them try to listen on the same socket, which will inevitably fail. > Question 2: > Can anyone confirm or deny that that method is legitimate? > > And really, any other suggestions of how to get this to work are welcome. What do you want to mute? All sinks, including bluetooth etc? Or only the internal sound card? If it's sufficient to just mute one alsa sound card, you could use amixer to mute the hardware directly. PulseAudio will notice that and update its internal state accordingly. This is the command: amixer -c0 set Master toggle "-c0" may refer to the wrong sound card, though (and it may refer to a different sound card after rebooting). "-cNAME" is better, where NAME is the sound card name shown in brackets in /proc/asound/cards. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 02/11] pulsecore: srbchannel: Introduce per-client SHM files
On Sat, 2015-12-26 at 21:51 +0200, Ahmed S. Darwish wrote:
> Hi Tanu,
>
> Sorry for the very late replies :-)
>
> On Tue, Sep 22, 2015 at 01:01:24PM +0300, Tanu Kaskinen wrote:
> > On Sun, 2015-09-20 at 23:28 +0200, Ahmed S. Darwish wrote:
> > > The PA daemon currently uses a server-wide SHM file for all clients
> > > sending and receiving commands using the srbchannel.
> > >
> > > To protect the clients from each other, and to provide the necessary
> > > groundwork later for policy and sandboxing, create the srbchannel SHM
> > > files on a per-client basis.
> > >
> > > Signed-off-by: Ahmed S. Darwish
> > > ---
> > > src/pulsecore/client.c | 5 +
> > > src/pulsecore/client.h | 7 +++
> > > src/pulsecore/core.c| 15 +--
> > > src/pulsecore/core.h| 6 ++
> > > src/pulsecore/protocol-native.c | 16
> > > 5 files changed, 31 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/src/pulsecore/client.c b/src/pulsecore/client.c
> > > index 003bcf8..5d60ad6 100644
> > > --- a/src/pulsecore/client.c
> > > +++ b/src/pulsecore/client.c
> > > @@ -69,6 +69,8 @@ pa_client *pa_client_new(pa_core *core,
> > > pa_client_new_data *data) {
> > > c->sink_inputs = pa_idxset_new(NULL, NULL);
> > > c->source_outputs = pa_idxset_new(NULL, NULL);
> > >
> > > +c->rw_mempool = NULL;
> >
> > You could a patch that changes
> >
> > c = pa_xnew(pa_client, 1);
> >
> > to
> >
> > c = pa_xnew0(pa_client, 1);
> >
> > so that the struct fields don't need to be initialized to zero
> > manually.
> >
>
> Now done by David's 0284363aa1da05 commit :-)
>
> > ...hmmm... Is there actually any good reason to have rw_mempool in
> > pa_client? Could it be moved to the native protocol? It looks like the
> > only user outside the native protocol is pa_core_maybe_vacuum().
> >
>
> ACK.
>
> Now changed in v2: the per-client mempool (rw_mempool) is created on
> the server side (inside pa_native_connection) when the server receives
> a new client connection request at pa_native_protocol_connect().
>
> This indeed goes better with the global pattern of all mempools being
> created on the server side.
>
> > If I understand vacuuming correctly, it means telling the kernel that
> > the memory in unused memblocks probably won't be used any time soon,
> > and that we also don't care about the current data in them, so the
> > kernel can optimize memory usage accordingly. pa_core_maybe_vacuum()
> > only vacuums when there's no audio moving in the system.
>
> That's what I understood from the code, yes.
>
> > When you
> > change rw_mempools to be created separately for each client, I think it
> > would make sense to vacuum the per-client mempool every time the client
> > stops streaming. That logic could live inside the native protocol, so
> > the core wouldn't have to know about rw_mempools.
> >
>
> I see, makes sense.
>
> I've implemented this by vacuuming upon receiving CORK_PLAYBACK_STREAM
> or CORK_RECORD_STREAM. Hopefully this is the right approach.
Do you take into account the case where there are two streams and only
one of them stops? I think vacuuming should be done only when the state
changes from "some streams active" to "no streams active".
In addition to corking, the active -> not active transition can also
happen when the client tears down a stream.
--
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] sound routing
On Sat, 2015-12-26 at 23:41 +0200, Cristian Serban wrote: > Hi guys, > I am new to pulse, and would like to ask you if what I want can be achieved > with pulse. > I have a PI running raspbian with attached a USB sound adaptor and a > bluetooth adaptor. I want to capture sound over the line in/mic jack of the > sound adaptor and stream it over bluetooth to my headset. > I have managed to setup bluetooth working fine, I can play mp3 with mplayer. > Could pulse help me do this routing from sound line in to bluetooth ? If > yes how? :) You can load module-loopback with the source argument pointing to the usb sound card source and the sink argument pointing to the bluetooth sink, like this: pactl load-module module-loopback source=SOURCE sink=SINK You can find the appropriate values for SOURCE and SINK in "pactl list" output. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 02/11] pulsecore: srbchannel: Introduce per-client SHM files
On Sun, 2015-12-27 at 20:34 +0200, Ahmed S. Darwish wrote:
> On Sun, Dec 27, 2015 at 02:10:47PM +0200, Tanu Kaskinen wrote:
> > On Sat, 2015-12-26 at 21:51 +0200, Ahmed S. Darwish wrote:
> > > I've implemented this by vacuuming upon receiving CORK_PLAYBACK_STREAM
> > > or CORK_RECORD_STREAM. Hopefully this is the right approach.
> >
> > Do you take into account the case where there are two streams and only
> > one of them stops? I think vacuuming should be done only when the state
> > changes from "some streams active" to "no streams active".
>
> Hmm, my knowledge is still limited in this area. I thought
> there's a one-to-one relation between a per-client connection
> and a stream.
>
> After some thinking, I guess this can happen when one client
> creates both a playback stream and a recording one?
A client can have any number of streams over the same connection. For
example, pavucontrol creates many capture streams, since it has many
volume meters.
> > In addition to corking, the active -> not active transition can also
> > happen when the client tears down a stream.
> >
>
> PA_COMMAND_FLUSH_{PLAYBACK,RECORDING}`_STREAM?
PA_COMMAND_DELETE_{PLAYBACK,RECORD}_STREAM and
PA_COMMAND_FINISH_UPLOAD_STREAM.
In addition to those, streams may also die when the server kills them.
--
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] sound routing
On Sun, 2015-12-27 at 15:48 +0200, Cristian Serban wrote: > sounds great! thanks > it seems i have some problems with installation: > sudo apt-get install pulseaudio > Unpacking pulseaudio (from .../pulseaudio_2.0-6.1_armhf.deb) ... > Selecting previously unselected package pulseaudio-module-x11. > Unpacking pulseaudio-module-x11 (from > .../pulseaudio-module-x11_2.0-6.1_armhf.deb) ... > Setting up pulseaudio (2.0-6.1) ... > *Illegal instruction* > *Illegal instruction* > *Illegal instruction* > pulseaudio start/running, process 13712 > Setting up pulseaudio-module-x11 (2.0-6.1) ... > > > > > > then in logs I have: > grep -i pulse /var/log/syslog > Dec 27 15:35:02 raspbmc init: pulseaudio main process (505) terminated with > status 1 > Dec 27 15:35:42 raspbmc pulseaudio[12751]: [pulseaudio] main.c: Running in > system mode, but --disallow-module-loading not set! > Dec 27 15:35:42 raspbmc pulseaudio[12751]: [pulseaudio] main.c: OK, so you > are running PA in system mode. Please note that you most likely shouldn't > be doing that. > Dec 27 15:35:42 raspbmc pulseaudio[12751]: [pulseaudio] main.c: If you do > it nonetheless then it's your own fault if things don't work as expected. > Dec 27 15:35:42 raspbmc pulseaudio[12751]: [pulseaudio] main.c: Please read > http://pulseaudio.org/wiki/WhatIsWrongWithSystemMode for an explanation why > system mode is usually a bad idea. > Dec 27 15:35:48 raspbmc pulseaudio[12751]: [pulseaudio] > module-default-device-restore.c: Failed to save default sink: Permission > denied > Dec 27 15:35:48 raspbmc pulseaudio[12751]: [pulseaudio] > module-default-device-restore.c: Failed to save default source: Permission > denied > > > Any hints? I'm not sure what you want to solve. The log doesn't show any fatal errors. The "Permission denied" errors seem like a packaging issue, so questions about those should be directed to the distribution developers. I have a question, though: if you didn't have pulseaudio installed, how did you set up bluetooth audio? I thought you had it working with pulseaudio. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] alsa: Don't disable timer-based scheduling on USB devices
On Tue, 2015-12-29 at 06:01 +0530, a...@accosted.net wrote: > From: Arun Raghavan > > This isn't a great fix, but we need ALSA API to do this right. In the > mean time, USB devices work fine with timer-based scheduling, so there's > no reason to force a large minimum latency by disabling tsched on them. > --- > src/modules/alsa/alsa-util.c | 21 +++-- > 1 file changed, 19 insertions(+), 2 deletions(-) Looks good to me. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Since pulseaudio 5 speaker output is on headphones channel, and plugging/unplugging headphones resets settings
On Sun, 2015-12-27 at 22:25 +0100, Mark Caglienzi wrote: > Hello to all on the list, > I submitted this bug report on the Debian BTS: > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=808170 > regarding pulseaudio >= 5.x and I was instructed to ask > the same question here. > > The bug report contains all the logs with pulseaudio 4.x > (the last version that works correctly in my system) and > pulseaudio 7.x (the version currently in Debian Testing, > that does not work). > > I tried months ago with pulseaudio 5.x and then I pinned > apt to pulseaudio 4.x. After a dist-upgrade I tried pulseaudio > 7.x hoping for something better, but I experienced the same > problem. > > For the description of the problem and the verbose log files > you can click on the DBTS link for the #808170 bug. Not all symptoms point to it, but one known source of volume resets is that both your regular user and gdm run their own pulseaudio daemons, and plugging in and unplugging headphones causes interference between the two. If you kill the pulseaudio process that gdm created, do things get better? Note that once you kill the process, gdm will respawn a new daemon instance, but that's ok, because it won't be able to access the mixer as long as you keep your login session active. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Question about writable data
On Wed, 2015-12-30 at 16:46 +0200, Alexandros Chronopoulos wrote: > Hi, > > I am using PulseAudio api to implement a scenario which synchronize an > input stream with an output stream. Synchronize means that whenever input > data is available the same number of frames should be set for writing in > the output. > > In order to achieve that I use two separate streams one for record and the > other for playback. In the record call back I check the writable size and > if it is big enough I write the data otherwise I reject the packet. This > causes some distortion in the output especially when latency is small. > > Trying to overcome this problem I stopped checking for writable data. I use > pa_stream_begin_write instead before the pa_stream_write to allocated the > desired data size. This approach seems to work very well even for very > small latency, even when the reported writable size is zero. > > My question is, if the approach above is safe and what is the actual > representation of the number returned from pa_stream_writable_size method? Hmm, I see the documentation for pa_stream_writable_size() is wrong: "Return the number of bytes that may be written using pa_stream_write()." The documentation implies that you can't write more than that, but actually the returned value tells how much the server has requested from you. If you write more than the server has requested, the extra amount will be buffered in a per-stream buffer in the server. The size of the stream buffer is the maxlength parameter of pa_buffer_attr. The default maxlength allows you to write a lot to the stream buffer before writes start to fail. Writing at the same rate as you read from the capture stream, like you do, is safe if the sink and the source share the same clock source. If they use different clocks, however, there will be some clock drift, which means that either the playback buffer runs empty at some point, causing a drop-out, or the playback buffer accumulates slowly more and more data, causing the latency to grow. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 2/2] pulse: Bump PA_RATE_MAX to 384 kHz
On Thu, 2015-12-31 at 09:42 +0530, a...@accosted.net wrote: > From: Arun Raghavan > > This will likely be needed in the future when we start supporting high > bitrate passthrough, and there actually seem to be people 352/384 kHz > out there (potentially as an intermediate production step). What's your source for claiming that there seems to be people with such PCM files (I'm assuming you implied PCM files)? If it's only used as an intermediate production step, is there any evidence that people would want to play that intermediate stuff via pulseaudio? The mpv bug was about a dsf file (which means a dsd stream from sacd?). Supposedly the stream contents were non-PCM data containing more than 2 channels, meant to be played in passthrough mode. And I guess alsa has specified that such content should be wrapped in a 2-channel 384 kHz float fake-PCM stream? I don't like extending the rate range for PCM streams just because alsa's passthrough implementation happens to specify such parameters for fake-PCM streams, but maybe there aren't any better options... -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 1/2] format: Make pa_format_info_valid() stricter for PCM
On Thu, 2015-12-31 at 09:42 +0530, a...@accosted.net wrote:
> From: Arun Raghavan
>
> We should do stricter validation when we can.
> ---
> src/pulse/format.c | 8 +++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/src/pulse/format.c b/src/pulse/format.c
> index c2a1552..b07940a 100644
> --- a/src/pulse/format.c
> +++ b/src/pulse/format.c
> @@ -101,7 +101,13 @@ void pa_format_info_free(pa_format_info *f) {
> }
>
> int pa_format_info_valid(const pa_format_info *f) {
> -return (f->encoding >= 0 && f->encoding < PA_ENCODING_MAX && f->plist !=
> NULL);
> +pa_sample_spec ss;
> +
> +if (pa_format_info_is_pcm(f)) {
> +pa_format_info_to_sample_spec(f, &ss, NULL);
> +return pa_sample_spec_valid(&ss);
> +} else
> +return (f->encoding >= 0 && f->encoding < PA_ENCODING_MAX &&
> f->plist != NULL);
> }
>
> int pa_format_info_is_pcm(const pa_format_info *f) {
Looks good to me, and this seems appropriate for 8.0 too.
--
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 2/2] pulse: Bump PA_RATE_MAX to 384 kHz
On Thu, 2015-12-31 at 11:32 +0530, Arun Raghavan wrote: > On 31 December 2015 at 11:15, Tanu Kaskinen wrote: > > On Thu, 2015-12-31 at 09:42 +0530, a...@accosted.net wrote: > > > From: Arun Raghavan > > > > > > This will likely be needed in the future when we start supporting high > > > bitrate passthrough, and there actually seem to be people 352/384 kHz > > > out there (potentially as an intermediate production step). > > > > What's your source for claiming that there seems to be people with such > > PCM files (I'm assuming you implied PCM files)? If it's only used as an > > intermediate production step, is there any evidence that people would > > want to play that intermediate stuff via pulseaudio? > > > > The mpv bug was about a dsf file (which means a dsd stream from sacd?). > > My source is the Internet. :) > > https://en.wikipedia.org/wiki/Digital_eXtreme_Definition Thanks! It seems that DXD is used at least in one store for downloadable music[1], and it's delivered as FLAC files (so DXD is not a file format, it's just a specific set of PCM parameters). FLAC supports sample rates up to 655 kHz... [1] https://www.promates.com/music-store/hd-audio > > Supposedly the stream contents were non-PCM data containing more than 2 > > channels, meant to be played in passthrough mode. And I guess alsa has > > specified that such content should be wrapped in a 2-channel 384 kHz > > float fake-PCM stream? I don't like extending the rate range for PCM > > streams just because alsa's passthrough implementation happens to > > specify such parameters for fake-PCM streams, but maybe there aren't > > any better options... > > I certainly don't know of better options. FWIW, some HDA codecs do > report support for this rate as well. Presumably either the DAC > supports it (I'd be curious to find out why), or it's supported for > HBR passthrough. Either way, we'll probably need to end up supporting > this. Yes, there seem to be good reasons to bump the max rate. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Change sound via acpid
On Wed, 2015-12-30 at 12:11 -0800, John W wrote: > On 12/22/15, Tanu Kaskinen wrote: > > > > It's not true that the system mode is unsupported > > I've seen this link referenced a few places. Maybe it's out of date or > wrong, but definitely seems pretty clear on the point: > http://www.freedesktop.org/wiki/Software/PulseAudio/Documentation/User/WhatIsWrongWithSystemWide/ > > It mentions "You are using PA against the explicit recommendations of > the maintainers...". The wiki page is unnecessarily negative. > It is good to know that PA will run as the 'pulse' user, though. > > > > > > Next, I found out about the unix socket authentication options in > > > /etc/pulse/default.pa > > > ... > > > > The options aren't specific to the system mode. Without error messages, > > I don't know why it didn't work. If there are multiple users running > > pulseaudio, one problem would be all of them try to listen on the same > > socket, which will inevitably fail. > > I only want to run the daemon once, but I want multiple users (in my > case, the "main" user as well as root, via the acpid script) to be > able to communicate with it to affect the audio. > > You mention error messages - I would love to provide better ones. Do > you know any way to? > Perhaps a way to see what PA is getting from its configuration files, > what the final "effective config" is at the end of the process, etc? Well, the error message from "pactl info" would be the first step. If it's "connection refused", either the socket doesn't exist, or the option you set in /etc/pulse/client.conf didn't have effect. But anyway, I don't think loading an extra instance of module-native- protocol-unix is the best alternative here. > > > > If it's sufficient to just mute one alsa sound > > card, you could use amixer to mute the hardware directly. PulseAudio > > will notice that and update its internal state accordingly. This is the > > command: > > > > amixer -c0 set Master toggle > > > > "-c0" may refer to the wrong sound card, though (and it may refer to a > > different sound card after rebooting). "-cNAME" is better, where NAME > > is the sound card name shown in brackets in /proc/asound/cards. > > > > I have tried the "-c" option, and it has ... "sort of" success (: > > In particular, mute works, but unmute does not. > It seems that mute will affect the "mono" as well as stereo L/R > channels, but unmute only affects mono. So the effect is that it stays > muted. I don't think the channels are relevant here. The problem is that when you mute Master, PulseAudio notices that the current audio path got muted, updates its own sink mute status, and that in turn causes PulseAudio to mute also other mixer controls in the same path (likely Speaker or Headphone). When you then unmute Master, the mute state in PulseAudio doesn't change, because Speaker/Headphone are still muted in alsa (this behaviour can probably be regarded as a bug in PulseAudio, fixes welcome). You could do multiple amixer calls to unmute all necessary controls, but if you switch between headphones and speakers, the script gets a bit more complicated, since you have to first figure out which path is currently active. If you only need to care about root getting access to your main user (that is, you're the only human user of the laptop), I think the easiest solution would be to use "sudo --user=john --set-home pactl set-sink-mute SINK toggle". Replace SINK with the alsa sink name, you can find the name with "pactl list sinks". -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Question about writable data
On Thu, 2015-12-31 at 19:39 +0200, Alexandros Chronopoulos wrote: > Source and sinks that share the same clock source are those found in the > same sound card, right? I don't know how sound cards are usually designed, but I would guess that yes, the audio pipelines on the same sound card probably share the clock. > Which are the options if this is not the case, if > for example I use a USB mic with my system speakers? Is there any way to > prevent the process from clock driff? You can't just make the problem go away. Either you accept that there will be occasional glitches, or you manipulate the audio data so that it plays back a bit faster or slower than it would normally do. PulseAudio has module-loopback, which can be used to connect a source to a sink, and module-loopback adapts to the clock drift by converting the audio data to slightly different sample rate. I don't know what your application does, but if it doesn't do anything else than move audio from a source to a sink, you can replace your application with module-loopback. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Since pulseaudio 5 speaker output is on headphones channel, and plugging/unplugging headphones resets settings
On Sun, 2016-01-03 at 14:37 +0100, Mark Caglienzi wrote: > On Tue, 2015-12-29 at 21:50 +0200, Tanu Kaskinen wrote: > > Not all symptoms point to it, but one known source of volume resets is > > that both your regular user and gdm run their own pulseaudio daemons, > > and plugging in and unplugging headphones causes interference between > > the two. If you kill the pulseaudio process that gdm created, do things > > get better? Note that once you kill the process, gdm will respawn a new > > daemon instance, but that's ok, because it won't be able to access the > > mixer as long as you keep your login session active. > > Thanks for the reply, but I don't use gdm on my system. > My machine is a Debian testing (stretch, at the moment) installation, > with fluxbox as window manager and lightdm as graphical login manager. When you are logged in, does "ps aux" show multiple pulseaudio instances running? Maybe lightdm works similarly to gdm. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH 0/2] Two volume factor fixes
I noticed a strange error message about pa_cvolume_remap() failing in a server log, which turned out to be a bug in volume_factor_source handling. I found another bug when reviewing the volume factor code. Here are fixes for both bugs. Tanu Kaskinen (2): source-output: do volume_factor_source application before resampling source-output: remap volume_factor_source when starting move src/pulsecore/source-output.c | 23 ++- 1 file changed, 10 insertions(+), 13 deletions(-) -- 2.6.4 ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH 2/2] source-output: remap volume_factor_source when starting move
This gets rid of an error message from the debug log. If
volume_factor_source would actually be used somewhere, this bug would
have caused more severe problems.
volume_factor_source should have the source's channel map. When moving
the stream, the volume needs to be remapped from the old source's
channel map to the new source's map. However, when the stream is being
moved, there is a period where the old source has already been
forgotten and the new source isn't yet known, so the remapping can't
be done directly between the two channel maps. Instead, the volume is
remapped from the old source's map to the stream's own map when the
move starts, and again remapped from the stream's map to the new
source's map when the move finishes.
The first remapping was missing, causing the second remapping fail and
print an error to the log.
(I checked the sink input code as well. It didn't have this bug.)
---
src/pulsecore/source-output.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/pulsecore/source-output.c b/src/pulsecore/source-output.c
index 26a7123..5cf551f 100644
--- a/src/pulsecore/source-output.c
+++ b/src/pulsecore/source-output.c
@@ -1242,6 +1242,9 @@ int pa_source_output_start_move(pa_source_output *o) {
pa_assert_se(pa_asyncmsgq_send(o->source->asyncmsgq,
PA_MSGOBJECT(o->source), PA_SOURCE_MESSAGE_REMOVE_OUTPUT, o, 0, NULL) == 0);
pa_source_update_status(o->source);
+
+pa_cvolume_remap(&o->volume_factor_source, &o->source->channel_map,
&o->channel_map);
+
o->source = NULL;
pa_source_output_unref(o);
--
2.6.4
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH 1/2] source-output: do volume_factor_source application before resampling
Applying the volume after resampling means mismatch between the volume
channel map and the data channel map.
volume_factor_source is not currently used anywhere, so this bug
hasn't been causing any problems. I noticed it while reading the code.
---
src/pulsecore/source-output.c | 20 +++-
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/src/pulsecore/source-output.c b/src/pulsecore/source-output.c
index 9000972..26a7123 100644
--- a/src/pulsecore/source-output.c
+++ b/src/pulsecore/source-output.c
@@ -792,14 +792,14 @@ void pa_source_output_push(pa_source_output *o, const
pa_memchunk *chunk) {
pa_volume_memchunk(&qchunk, &o->source->sample_spec,
&o->thread_info.soft_volume);
}
-if (!o->thread_info.resampler) {
-if (nvfs) {
-pa_memchunk_make_writable(&qchunk, 0);
-pa_volume_memchunk(&qchunk, &o->thread_info.sample_spec,
&o->volume_factor_source);
-}
+if (nvfs) {
+pa_memchunk_make_writable(&qchunk, 0);
+pa_volume_memchunk(&qchunk, &o->source->sample_spec,
&o->volume_factor_source);
+}
+if (!o->thread_info.resampler)
o->push(o, &qchunk);
-} else {
+else {
pa_memchunk rchunk;
if (mbs == 0)
@@ -810,14 +810,8 @@ void pa_source_output_push(pa_source_output *o, const
pa_memchunk *chunk) {
pa_resampler_run(o->thread_info.resampler, &qchunk, &rchunk);
-if (rchunk.length > 0) {
-if (nvfs) {
-pa_memchunk_make_writable(&rchunk, 0);
-pa_volume_memchunk(&rchunk, &o->thread_info.sample_spec,
&o->volume_factor_source);
-}
-
+if (rchunk.length > 0)
o->push(o, &rchunk);
-}
if (rchunk.memblock)
pa_memblock_unref(rchunk.memblock);
--
2.6.4
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] sink-input, source-output: Add some debug output on start_move()
On Mon, 2016-01-11 at 12:52 +0530, a...@accosted.net wrote:
> From: Arun Raghavan
>
> ---
> src/pulsecore/sink-input.c| 2 ++
> src/pulsecore/source-output.c | 2 ++
> 2 files changed, 4 insertions(+)
>
> diff --git a/src/pulsecore/sink-input.c b/src/pulsecore/sink-input.c
> index 539ae17..8ec63b5 100644
> --- a/src/pulsecore/sink-input.c
> +++ b/src/pulsecore/sink-input.c
> @@ -1580,6 +1580,8 @@ int pa_sink_input_start_move(pa_sink_input *i) {
> if ((r =
> pa_hook_fire(&i->core->hooks[PA_CORE_HOOK_SINK_INPUT_MOVE_START], i)) < 0)
> return r;
>
> +pa_log_debug("Starting to move sink input %u from '%s'", (unsigned)
> i->index, i->sink->name);
> +
> /* Kill directly connected outputs */
> while ((o = pa_idxset_first(i->direct_outputs, NULL))) {
> pa_assert(o != p);
> diff --git a/src/pulsecore/source-output.c b/src/pulsecore/source-output.c
> index 9000972..c73c548 100644
> --- a/src/pulsecore/source-output.c
> +++ b/src/pulsecore/source-output.c
> @@ -1230,6 +1230,8 @@ int pa_source_output_start_move(pa_source_output *o) {
> if ((r =
> pa_hook_fire(&o->core->hooks[PA_CORE_HOOK_SOURCE_OUTPUT_MOVE_START], o)) < 0)
> return r;
>
> +pa_log_debug("Starting to move source output %u from '%s'", (unsigned)
> o->index, o->source->name);
> +
> origin = o->source;
>
> pa_idxset_remove_by_data(o->source->outputs, o, NULL);
Ack.
--
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Question about writable data
On Mon, 2016-01-11 at 11:27 +0200, Alexandros Chronopoulos wrote: > On Fri, Jan 1, 2016 at 10:24 AM, Tanu Kaskinen wrote: > > > > > I don't know how sound cards are usually designed, but I would guess > > that yes, the audio pipelines on the same sound card probably share the > > clock. > > > > Is there an API available to discover/compare the clock of the source/sink > for each platform? Not to my knowledge. Certainly not in PulseAudio. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH] stream: fix incorrect doc for pa_stream_writable_size()
The old documentation implied that it wouldn't be possible to write more than the returned amount, which was incorrect. --- src/pulse/stream.h | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/pulse/stream.h b/src/pulse/stream.h index 70fa415..802660d 100644 --- a/src/pulse/stream.h +++ b/src/pulse/stream.h @@ -588,7 +588,14 @@ int pa_stream_peek( * calling pa_stream_peek(). */ int pa_stream_drop(pa_stream *p); -/** Return the number of bytes that may be written using pa_stream_write(). */ +/** Return the number of bytes that the server has requested to be written. + * + * Contrary to what might be expected from the function name, it's usually + * possible to write more than the returned amount, but typically it doesn't + * make sense to do that, because that will likely make the stream latency + * exceed the target latency (which is configured with the tlength parameter in + * pa_buffer_attr). + */ size_t pa_stream_writable_size(pa_stream *p); /** Return the number of bytes that may be read using pa_stream_peek(). */ -- 2.6.4 ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] Added set-(sink|source)-latency-offset commands to pactl and pacmd.
On Tue, 2016-01-12 at 17:05 +1100, Chris Billington wrote: > Hi all, > > Just to bump this thread, what are the odds of getting this patch included > in pulseaudio? It would be useful to many of the network audio streaming > services that use pulseaudio (specifically my one :p). The odds are good, since this is a welcome addition. I don't know when the patch will be merged, however (in any case it's too late for 8.0). I don't promise a quick review myself due to lack of time. Peter already commented, though, and he is able to approve patches. Peter, do you plan to review this newest version of the patch? I do have some comments from just glancing at the patch, though: It would have been nice to separate the patch to the part that modifies the protocol and the part(s) that add the new feature to pactl and pacmd. The commit message should contain the justification for the change (which you provided in a separate mail). The references to 8.0 need to be changed to 9.0. It would be good to have better separation between the sink/source latency offset and the port latency offset. The patch doesn't seem to handle it particularly well if a sink has a latency offset set and the same sink also has a port that has a latency offset set. It would make sense to add those two together when querying the sink latency. It would be good to save the offset on disk. The place to do that would be module-device-restore. If you try to implement this, however, it might turn out to be a bit tricky, because module-device-restore deals with both per-sink and per-port entries depending on whether the sink has ports. The sink latency offset should not be saved separately for each port, but it's not obvious how to achieve that when module-device- restore is in the "per-port mode". -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] Added set-(sink|source)-latency-offset commands to pactl and pacmd.
On Tue, 2016-01-12 at 09:45 +0200, Tanu Kaskinen wrote: > On Tue, 2016-01-12 at 17:05 +1100, Chris Billington wrote: > > Hi all, > > > > Just to bump this thread, what are the odds of getting this patch included > > in pulseaudio? It would be useful to many of the network audio streaming > > services that use pulseaudio (specifically my one :p). > > The odds are good, since this is a welcome addition. I don't know when > the patch will be merged, however (in any case it's too late for 8.0). > I don't promise a quick review myself due to lack of time. Peter > already commented, though, and he is able to approve patches. Peter, do > you plan to review this newest version of the patch? > > I do have some comments from just glancing at the patch, though: > > It would have been nice to separate the patch to the part that modifies > the protocol and the part(s) that add the new feature to pactl and > pacmd. > > The commit message should contain the justification for the change > (which you provided in a separate mail). > > The references to 8.0 need to be changed to 9.0. > > It would be good to have better separation between the sink/source > latency offset and the port latency offset. The patch doesn't seem to > handle it particularly well if a sink has a latency offset set and the > same sink also has a port that has a latency offset set. It would make > sense to add those two together when querying the sink latency. > > It would be good to save the offset on disk. The place to do that would > be module-device-restore. If you try to implement this, however, it > might turn out to be a bit tricky, because module-device-restore deals > with both per-sink and per-port entries depending on whether the sink > has ports. The sink latency offset should not be saved separately for > each port, but it's not obvious how to achieve that when module-device- > restore is in the "per-port mode". I forgot one more thing that would be good to have: updates to the bash and zsh completion files. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 11/11] pulsecore: srbchannel: Enable memfd support; pump protocol version
On Wed, 2016-01-13 at 00:19 +0200, Ahmed S. Darwish wrote:
> Hi Tanu,
>
> On Thu, Oct 01, 2015 at 11:02:55AM +0300, Tanu Kaskinen wrote:
> > On Sun, 2015-09-20 at 23:39 +0200, Ahmed S. Darwish wrote:
> > > diff --git a/src/pulse/context.c b/src/pulse/context.c
> > > index ede01fa..f272cd6 100644
> > > --- a/src/pulse/context.c
> > > +++ b/src/pulse/context.c
> > > @@ -591,6 +591,8 @@ static void setup_context(pa_context *c, pa_iochannel
> > > *io) {
> > > pa_tagstruct_putu32(t, PA_PROTOCOL_VERSION | (c->do_shm ?
> > > 0x8000U : 0));
> > > pa_tagstruct_put_arbitrary(t, cookie, sizeof(cookie));
> > >
> > > +pa_tagstruct_put_boolean(t, pa_memfd_is_supported());
> >
> > This breaks compatibility with old servers that expect the tagstruct to
> > end after the cookie.
> >
> > The "memfd supported" bit could be added to the version field like the
> > "shm supported" bit.
> >
>
> Unfortunately this will also break compatibility with old servers.
>
> Right now, PA uses the client's version number most-significant bit as
> a marker for SHM support.
>
> Afterwards, the server clears _only_ that bit:
>
> if (c->version >= 13) {
> shm_on_remote = !!(c->version & 0x8000U);
> c->version &= 0x7FFFU;
> }
>
> So if I overload any of the bits above, e.g. the second MSB bit
> 0x4000, "old" servers will not clear it and will interpret the
> client version as 0x401F instead of just v31 0x1F :-(
That should be ok, since 0x401F will be larger than the old
server's version, so the negotiated version will be the old server's
version. The same thing happens when you have version 12 server and
version 13 client with shm support. The server doesn't clear the MSB,
so it thinks the client has very large version.
> > Another approach would be to do add a new command
> > to do memfd negotiation before enabling srbchannel. Abusing the version
> > field is ugly, adding another negotiation adds more connection setup
> > overhead. I don't know which approach we should choose.
> >
>
> Can we agree that _from this point forward_, the server clears the
> client's version most-significant _16_ bits? This will give us 15
> free new flags to play with.
I can agree to that. 16 bits should still be plenty for the version
number.
--
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Fwd: [LOW LATENCY] PA_simple
On Wed, 2016-01-13 at 10:19 +0530, Swapnali Patil wrote:
> Hello all,
> we are using Pulse Audio API to playback audio received from network. So
> far we are using pa_simple_api, with this we get latency of 2+ seconds.
> From internet documents we got to about PA_STREAM_ADJUST_LATENCY flags. But
> it seem this flag can be set for PA_STREAM_API.
The PA_STREAM_ADJUST_LATENCY flag is automatically set for all streams
created with the simple API.
> We tried to set /use
> PA_STREAM_API as mentioned below
> . but it fails in
>
> pa_stream_new
> () it self.
>
> pa_ml = pa_mainloop_new();
> pa_mlapi = pa_mainloop_get_api(pa_ml);
> pa_ctx = pa_context_new(pa_mlapi, "ivcloudapp");
> pa_context_connect(pa_ctx, NULL,PA_CONTEXT_NOFLAGS, NULL);
>
> if(!s)
> {
> s =
>
> pa_stream_new(pa_ctx, "Playback", &ss, NULL);
> if (!s) {
> printf("pa_stream_new failed\n");
This will fail, because pa_context_connect() doesn't immediately
establish the connection. You need to monitor the connection state by
setting up a callback with pa_context_set_state_callback().
> Please give direction how we can set low latency with pa_simple_api.
The latency is controlled by the tlength parameter in pa_buffer_attr,
which you pass to pa_simple_new().
--
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] Release notes draft completed
Hi all, I finished the initial draft of the 8.0 release notes: https://wiki.freedesktop.org/www/Software/PulseAudio/Notes/8.0/ Is there anything missing or needing changes? Please reply this message or just go ahead and edit the wiki page. Perhaps the NetBSD and/or OS X changes should be mentioned? Kamil and Mihai, how would you summarize the changes you have made? It seems that module-coreaudio-detect wasn't loaded by default in previous versions, and that seems like an important change affecting end users. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] Toggling hardware volume for a card or port in pavucontrol
Hi, Related to previous discussions[1][2][3] about dealing with volume on certain type of usb sound cards, I'd like to make it possible to enable and disable hardware volume for individual cards and/or ports in pavucontrol. Here's the specific use case: the Terratec Aureon Dual USB sound card has a single output jack that can used in both analog and digital modes (there are other similar usb sound cards too). PulseAudio has no way to know which mode is in use. Hardware volume works only in the analog mode. Currently that means that PulseAudio's volume control has no effect in the digital mode, because PulseAudio controls a mixer element that doesn't do anything. I want to disable hardware volume for that card by default, because that's the only safe default. That reduces the audio quality a bit, however, so it would be good to allow concerned users to enable hardware volume for the card without too much hassle. I'd like some feedback to guide the design: 0) Is this feature worth the maintenance burden the new code causes, or should I just forget about this? (My answer: it's worth doing.) 1) Should the client API be specific to alsa, or should it be added to the core interface? A third option exists too: don't make it specific to alsa, but implement the client API in a module with a protocol extension, and hook to the new module from alsa code. (My answer: core.) 2) Should the hardware volume toggle be associated with a card or a port or both? Associating it with the card would make the UI a bit simpler. Logically, though, the toggle should be associated with a port, because in the specific use case that I'm concentrating on the volume problem concerns only the output path, not the input path. There's no reason to disable hardware volume for the input path. Whatever we choose, it's always possible to extend the code to support both per-card and per-port toggles. (My answer: start with per-port.) [1] http://thread.gmane.org/gmane.comp.audio.pulseaudio.general/22253 [2] http://thread.gmane.org/gmane.comp.audio.pulseaudio.general/23351 [3] http://thread.gmane.org/gmane.comp.audio.pulseaudio.general/23434 -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Toggling hardware volume for a card or port in pavucontrol
On Thu, 2016-01-14 at 00:36 +0800, Tom Yan wrote: > Hmm, but a single jack doesn't mean that the driver should only > provide a single "device" for the card? The interface/signal is still > different, no? The driver provides only one playback device. I think the driver doesn't have any way to know the mode either, although I could be wrong (if it does have the information, it would be great if the driver would expose the information to the userspace too). -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Release notes draft completed
On Thu, 2016-01-14 at 08:18 +0530, Arun Raghavan wrote: > On 14 January 2016 at 00:48, Kamil Rytarowski wrote: > > -BEGIN PGP SIGNED MESSAGE- > > Hash: SHA256 > > > > On 13.01.2016 10:10, Tanu Kaskinen wrote: > > > Hi all, > > > > > > I finished the initial draft of the 8.0 release notes: > > > https://wiki.freedesktop.org/www/Software/PulseAudio/Notes/8.0/ > > > > > > Is there anything missing or needing changes? Please reply this > > > message or just go ahead and edit the wiki page. > > > > > > Perhaps the NetBSD and/or OS X changes should be mentioned? Kamil > > > and Mihai, how would you summarize the changes you have made? It > > > seems that module-coreaudio-detect wasn't loaded by default in > > > previous versions, and that seems like an important change > > > affecting end users. > > Yes, the main change is that our configuration should work > out-of-the-box on OS X. > > > I would summarize them as reduced delta between pkgsrc and upstream > > version, pkgsrc is actively used by POSIX-compat systems, notably by > > NetBSD and SunOS. I now added some notes about the OS X and NetBSD changes. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Release notes draft completed
On Thu, 2016-01-14 at 10:05 +0100, David Henningsson wrote: > > On 2016-01-13 10:10, Tanu Kaskinen wrote: > > Hi all, > > > > I finished the initial draft of the 8.0 release notes: > > https://wiki.freedesktop.org/www/Software/PulseAudio/Notes/8.0/ > > > > Is there anything missing or needing changes? Please reply this message > > or just go ahead and edit the wiki page. > > I've added a section about the routing changes, as well as a few words > about PULSE_LOG_JOURNAL and module-dbus-protocol. > > I was unsure of where to best add those words though, so feel free to > review and/or reorder as you see fit. I moved everything from the "changes in more detail" section under the "notes for end users" section. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Release notes draft completed
On Thu, 2016-01-14 at 11:28 +0200, Ahmed S. Darwish wrote: > On Wed, Jan 13, 2016 at 11:10:31AM +0200, Tanu Kaskinen wrote: > > Hi all, > > > > I finished the initial draft of the 8.0 release notes: > > https://wiki.freedesktop.org/www/Software/PulseAudio/Notes/8.0/ > > > > Is there anything missing or needing changes? Please reply this message > > or just go ahead and edit the wiki page. > > > > Can we mention the newly-added memory consumption benchmark scripts? Ok, I added that. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] PulseAudio translation
On Fri, 2016-01-15 at 13:05 +0200, Mr. M. wrote: > Hello guys, > > I would like to translate PulseAudio, PulseAudio Volume Control and > PulseAudio preferences into Lithuanian language. > But I can not find appropriate POT files because every file I got had > different number of strings (some 453, some 583..) so I'm not sure > which > one is right. > > Where could I get correct POT files that I could later attach to > freedesktop bugzilla? I don't know (I'm not familiar with the translator workflow), and I'm not sure if any of the translators follow this list closely. I added Yuri Chornoivan to Cc (just a random choice based on who happened to send the latest translation update), maybe he can help. I would be interested to know some basics of the translation workflow too, so thatI could add a readme file in the po/ directory like pavucontrol has (unfortunately the pavucontrol instructions don't seem to be directly applicable to pulseaudio). -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Toggling hardware volume for a card or port in pavucontrol
On Wed, 2016-01-13 at 16:19 +0200, Tanu Kaskinen wrote: > Hi, > > Related to previous discussions[1][2][3] about dealing with volume on > certain type of usb sound cards, I'd like to make it possible to enable > and disable hardware volume for individual cards and/or ports in > pavucontrol. > > Here's the specific use case: the Terratec Aureon Dual USB sound card > has a single output jack that can used in both analog and digital modes > (there are other similar usb sound cards too). PulseAudio has no way to > know which mode is in use. Hardware volume works only in the analog > mode. Currently that means that PulseAudio's volume control has no > effect in the digital mode, because PulseAudio controls a mixer element > that doesn't do anything. > > I want to disable hardware volume for that card by default, because > that's the only safe default. That reduces the audio quality a bit, > however, so it would be good to allow concerned users to enable > hardware volume for the card without too much hassle. > > I'd like some feedback to guide the design: > > 0) Is this feature worth the maintenance burden the new code causes, or > should I just forget about this? (My answer: it's worth doing.) > > 1) Should the client API be specific to alsa, or should it be added to > the core interface? A third option exists too: don't make it specific > to alsa, but implement the client API in a module with a protocol > extension, and hook to the new module from alsa code. (My answer: > core.) Arun commented in IRC that another alternative would be to use the "configuration system"[1] that Arun would like to implement. Although I had some reservations at first, I now think that the proposed configuration system fits this use case well, so I have nothing against doing the hardware volume toggle implementation that way. The recent discussion about the configuration system has been only between Arun and me in IRC, however, so it may be that others disagree about the desirability of the new system. Feedback about that, too, would be very welcome. [1] https://wiki.freedesktop.org/www/Software/PulseAudio/Documentation/Developer/ConfigurationSystem/ -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] sink-input: Don't access resampler to get silence memchunk
On Wed, 2015-12-16 at 09:52 +0530, Arun Raghavan wrote: > On 16 December 2015 at 00:36, Tanu Kaskinen wrote: > > On Tue, 2015-12-15 at 21:49 +0530, a...@accosted.net wrote: > > > From: Arun Raghavan > > > > > > There doesn't appear to be a good reason to restrict the memchunk length > > > to the resample max block size -- we're going to have the memory around > > > anyway. > > > > I think the reason is to make sure that we don't feed the resampler > > bigger chunks than what it can handle. The resampler has to allocate > > other memblocks during its operation, and those memblocks may be bigger > > than the input block, so if the input block is too large, the > > requirements for the other blocks will grow beyond the mempool max > > block size. > > > > However, pa_sink_input_peek() seems to protect against this anyway when > > doing resampling (it processes the input in smaller pieces if it's > > larger than the resampler max block size), so maybe this change is safe > > anyway. > > > > > Moreover, callers of pa_sink_input_get_silence() don't seem to > > > actually care about the chunk itself, just the memblock for creating > > > their own pa_memblockq. > > > > I don't understand this comment. pa_memblockq cares about the chunk > > itself, not just the memblock. > > I misread that code. It does work with the chunk, not the memblock of course. Do you plan to send v2 of this patch some time soon? -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2 01/25] echo-cancel: Update webrtc-audio-processing usage to new API
On Wed, 2015-12-16 at 09:09 +0530, a...@accosted.net wrote:
> From: Arun Raghavan
>
> ---
> configure.ac | 2 +-
> src/Makefile.am | 2 +-
> src/modules/echo-cancel/webrtc.cc | 54
> +--
> 3 files changed, 31 insertions(+), 27 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index b9cd3d1..26c3e29 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1371,7 +1371,7 @@ AC_ARG_ENABLE([webrtc-aec],
> AS_HELP_STRING([--enable-webrtc-aec], [Enable the optional WebRTC-based
> echo canceller]))
>
> AS_IF([test "x$enable_webrtc_aec" != "xno"],
> -[PKG_CHECK_MODULES(WEBRTC, [ webrtc-audio-processing ], [HAVE_WEBRTC=1],
> [HAVE_WEBRTC=0])],
> +[PKG_CHECK_MODULES(WEBRTC, [ webrtc-audio-processing > 0.1 ],
> [HAVE_WEBRTC=1], [HAVE_WEBRTC=0])],
> [HAVE_WEBRTC=0])
>
> AS_IF([test "x$enable_webrtc_aec" = "xyes" && test "x$HAVE_WEBRTC" = "x0"],
> diff --git a/src/Makefile.am b/src/Makefile.am
> index f1bd38d..533b646 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -50,7 +50,7 @@ AM_CPPFLAGS = \
> -DPULSE_LOCALEDIR=\"$(localedir)\"
> AM_CFLAGS = \
> $(PTHREAD_CFLAGS)
> -AM_CXXFLAGS = $(AM_CFLAGS)
> +AM_CXXFLAGS = $(AM_CFLAGS) -std=c++11
> SERVER_CFLAGS = -D__INCLUDED_FROM_PULSE_AUDIO
>
> AM_LIBADD = $(PTHREAD_LIBS) $(INTLLIBS)
> diff --git a/src/modules/echo-cancel/webrtc.cc
> b/src/modules/echo-cancel/webrtc.cc
> index 511c7ee..4a23377 100644
> --- a/src/modules/echo-cancel/webrtc.cc
> +++ b/src/modules/echo-cancel/webrtc.cc
> @@ -33,8 +33,8 @@ PA_C_DECL_BEGIN
> #include "echo-cancel.h"
> PA_C_DECL_END
>
> -#include
> -#include
> +#include
> +#include
>
> #define BLOCK_SIZE_US 1
>
> @@ -80,6 +80,7 @@ bool pa_webrtc_ec_init(pa_core *c, pa_echo_canceller *ec,
> pa_sample_spec *out_ss, pa_channel_map *out_map,
> uint32_t *nframes, const char *args) {
> webrtc::AudioProcessing *apm = NULL;
> +webrtc::ProcessingConfig pconfig;
> bool hpf, ns, agc, dgc, mobile, cn;
> int rm = -1;
> pa_modargs *ma;
> @@ -153,7 +154,7 @@ bool pa_webrtc_ec_init(pa_core *c, pa_echo_canceller *ec,
> }
> }
>
> -apm = webrtc::AudioProcessing::Create(0);
> +apm = webrtc::AudioProcessing::Create();
>
> out_ss->format = PA_SAMPLE_S16NE;
> *play_ss = *out_ss;
> @@ -163,22 +164,19 @@ bool pa_webrtc_ec_init(pa_core *c, pa_echo_canceller
> *ec,
> *rec_ss = *out_ss;
> *rec_map = *out_map;
>
> -apm->set_sample_rate_hz(out_ss->rate);
> -
> -apm->set_num_channels(out_ss->channels, out_ss->channels);
> -apm->set_num_reverse_channels(play_ss->channels);
> +pconfig = {
> +webrtc::StreamConfig(out_ss->rate, out_ss->channels, false), /*
> input stream */
> +webrtc::StreamConfig(out_ss->rate, out_ss->channels, false), /*
> output stream */
> +webrtc::StreamConfig(out_ss->rate, out_ss->channels, false), /*
> reverse input stream */
> +webrtc::StreamConfig(out_ss->rate, out_ss->channels, false), /*
> reverse output stream */
> +};
> +apm->Initialize(pconfig);
>
> if (hpf)
> apm->high_pass_filter()->Enable(true);
>
> if (!mobile) {
> -if (ec->params.drift_compensation) {
> -
> apm->echo_cancellation()->set_device_sample_rate_hz(out_ss->rate);
> -apm->echo_cancellation()->enable_drift_compensation(true);
> -} else {
> -apm->echo_cancellation()->enable_drift_compensation(false);
> -}
> -
> +
> apm->echo_cancellation()->enable_drift_compensation(ec->params.drift_compensation);
> apm->echo_cancellation()->Enable(true);
> } else {
> apm->echo_control_mobile()->set_routing_mode(static_cast(rm));
> @@ -225,7 +223,7 @@ fail:
> if (ma)
> pa_modargs_free(ma);
> if (apm)
> -webrtc::AudioProcessing::Destroy(apm);
> +delete apm;
>
> return false;
> }
> @@ -235,10 +233,13 @@ void pa_webrtc_ec_play(pa_echo_canceller *ec, const
> uint8_t *play) {
> webrtc::AudioFrame play_frame;
> const pa_sample_spec *ss = &ec->params.priv.webrtc.sample_spec;
>
> -play_frame._audioChannel = ss->channels;
> -play_frame._frequencyInHz = ss->rate;
> -play_frame._payloadDataLengthInSamples =
> ec->params.priv.webrtc.blocksize / pa_frame_size(ss);
> -memcpy(play_frame._payloadData, play, ec->params.priv.webrtc.blocksize);
> +play_frame.num_channels_ = ss->channels;
> +play_frame.sample_rate_hz_ = ss->rate;
> +play_frame.interleaved_ = false;
Using non-interleaved data looked strange, but I thought that you
probably know what you are doing. Then I accidentally noticed that
patch 19 is a fixup for the interleaved setting. Why is patch 19 not
squashed into this initial patch? Are there other similar fixups? Could
you resend with fixup patches squashed into the patches tha
Re: [pulseaudio-discuss] Question regarding latencies reported by alsa-source and alsa-sink
On Sat, 2016-01-16 at 23:22 +0100, Georg Chini wrote: > Hello, > > I have been working on a new version of module-loopback for quite a > while now. > I am also replacing the smoother in alsa-source and alsa-sink to achieve > more > precise latency reports. > Now I hit a problem, were I am not sure how to solve it. The get_latency > calls > of alsa-sink and alsa-source truncate the latency to 0 when it becomes > negative. > Would it be possible to let the calls return negative values? > Due to the error in the initial conditions those values still make sense > and clipping > them causes massive problems for the rate controller of module-loopback. > > If it is not possible to let the calls return negative values, do you > have any other > idea how to work around the problem? Reporting negative latency doesn't make sense, since it implies time travel. Where do those negative latency values come from? Filtering out obviously incorrect values seems appropriate for most use cases. If you really need access to "unfiltered" values, maybe pa_sink_get_latency() could have an "allow_negative" parameter or something similar. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] sink-input: Don't access resampler to get silence memchunk
On Mon, 2016-01-18 at 12:38 +0530, Arun Raghavan wrote: > On 16 January 2016 at 13:08, Tanu Kaskinen wrote: > > On Wed, 2015-12-16 at 09:52 +0530, Arun Raghavan wrote: > > > On 16 December 2015 at 00:36, Tanu Kaskinen wrote: > > > > On Tue, 2015-12-15 at 21:49 +0530, a...@accosted.net wrote: > > > > > From: Arun Raghavan > > > > > > > > > > There doesn't appear to be a good reason to restrict the memchunk > > > > > length > > > > > to the resample max block size -- we're going to have the memory > > > > > around > > > > > anyway. > > > > > > > > I think the reason is to make sure that we don't feed the resampler > > > > bigger chunks than what it can handle. The resampler has to allocate > > > > other memblocks during its operation, and those memblocks may be bigger > > > > than the input block, so if the input block is too large, the > > > > requirements for the other blocks will grow beyond the mempool max > > > > block size. > > > > > > > > However, pa_sink_input_peek() seems to protect against this anyway when > > > > doing resampling (it processes the input in smaller pieces if it's > > > > larger than the resampler max block size), so maybe this change is safe > > > > anyway. > > > > > > > > > Moreover, callers of pa_sink_input_get_silence() don't seem to > > > > > actually care about the chunk itself, just the memblock for creating > > > > > their own pa_memblockq. > > > > > > > > I don't understand this comment. pa_memblockq cares about the chunk > > > > itself, not just the memblock. > > > > > > I misread that code. It does work with the chunk, not the memblock of > > > course. > > > > Do you plan to send v2 of this patch some time soon? > > Do you see a need to change anything other than the commit message? I > can just drop the last sentence. No, just the commit message needs editing. In addition to dropping the last sentence, the first sentence could use some editing too, because it doesn't address what I'm guessing is the real reason why the length restriction was originally added. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Question regarding latencies reported by alsa-source and alsa-sink
On Sun, 2016-01-17 at 19:07 +0100, Georg Chini wrote: > On 17.01.2016 17:17, Georg Chini wrote: > > On 17.01.2016 14:40, Tanu Kaskinen wrote: > > > On Sat, 2016-01-16 at 23:22 +0100, Georg Chini wrote: > > > > Hello, > > > > > > > > I have been working on a new version of module-loopback for quite a > > > > while now. > > > > I am also replacing the smoother in alsa-source and alsa-sink to > > > > achieve > > > > more > > > > precise latency reports. > > > > Now I hit a problem, were I am not sure how to solve it. The > > > > get_latency > > > > calls > > > > of alsa-sink and alsa-source truncate the latency to 0 when it becomes > > > > negative. > > > > Would it be possible to let the calls return negative values? > > > > Due to the error in the initial conditions those values still make > > > > sense > > > > and clipping > > > > them causes massive problems for the rate controller of > > > > module-loopback. > > > > > > > > If it is not possible to let the calls return negative values, do you > > > > have any other > > > > idea how to work around the problem? > > > Reporting negative latency doesn't make sense, since it implies time > > > travel. Where do those negative latency values come from? > > > > I know it requires time travel ... > > Negative values are due to the fact that you have to assume > > some point in time as the starting point. In my new code this > > is done directly while the smoother does the same more indirectly > > (and also delivers negative values). > > At that starting point, a certain number of samples have already been > > played or recorded. If that number has an error, you can get negative > > latency values (which are in the order of a few hundred usec). Ok. It's not necessary to assume a starting time, though, unless you use the system clock in the latency reports. The current implementation uses the system clock, and apparenty you use it too in your new code. > > > Filtering out obviously incorrect values seems appropriate for most use > > > cases. If you really need access to "unfiltered" values, maybe > > > pa_sink_get_latency() could have an "allow_negative" parameter or > > > something similar. > > > > Wouldn't that require changes also on the client side? Or do clients > > never > > call this function directly? Clients can't call pa_sink_get_latency() directly. The pa_sink_* functions aren't included in the client library. Even if a client would be misguided enough to link to the server's internal library, it wouldn't work, because the client runs in a different process. > > Would it perhaps make sense to add a > > pa_sink_get_unfiltered_latency() call? > > > > Meanwhile I implemented those calls (or rather their _within_thread_ > variants) > for alsa sink and source. > If a sink or source does not have it (like bluetooth), the "normal" latency > is returned. > Would you be OK with that approach? I suggested a boolean flag, because I expect that to result in less code duplication. If my expectation is wrong, or there's some problem with using the flag, then adding separate functions is fine. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Using module-combine-sink with intermittent devices
On Tue, 2016-01-19 at 18:50 -0600, Matt Feifarek wrote: > I keep getting crashes in the log like this (which seem to be related to > latency): > E: [alsa-sink-ICE1724 IEC958] sink.c: Assertion > '!s->thread_info.rewind_requested' failed at pulsecore/sink.c:1323, > function pa_sink_render_into_full(). Aborting. > Aborted (core dumped) This is probably already fixed in 7.0 by this commit: http://cgit.freedesktop.org/pulseaudio/pulseaudio/commit/?id=4604af71981117854188dbb9a19f55f6b972fde6 -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Setting gain when plugging headphone on (non-simple) ALSA controls
On Wed, 2016-01-20 at 12:07 +, Sylvain Bougerel wrote: > Hi all, > > I have a problem of hearing white-noise on my headphones when I plug them > in my laptop. I know the work around: > > $ amixer -c 0 cset 'numid=10' 1 > numid=10,iface=MIXER,name='Headphone Mic Boost Volume' > ; type=INTEGER,access=rw---R--,values=2,min=0,max=3,step=0 > : values=1,1 > | dBscale-min=0.00dB,step=10.00dB,mute=0 > > OR > > $ amixer -c0 sset 'Headphone Mic Boost' 1 > Simple mixer control 'Headphone Mic Boost',0 > Capabilities: volume > Playback channels: Front Left - Front Right > Capture channels: Front Left - Front Right > Limits: 0 - 3 > Front Left: 1 [33%] [10.00dB] > Front Right: 1 [33%] [10.00dB] > > Done, no more white noise. It's all well, but the settings are not > remembered; every time pulseaudio restarts (when I login to my Desktop, > basically): > > $ amixer -c 0 cget 'numid=10' > numid=10,iface=MIXER,name='Headphone Mic Boost Volume' > ; type=INTEGER,access=rw---R--,values=2,min=0,max=3,step=0 > : values=0,0 > | dBscale-min=0.00dB,step=10.00dB,mute=0 > > Notice the 0,0? And the white noise is back. > > So my end goal is to find my way into hacking pulseaudio so that this is > taken care of without having to do it myself via command line everytime I > use my headphones. > > That's where my understanding of things starts to break down. Basically, I > think it's not possible to control this property in pulseaudio at the > moment, so I may need to write/update a module (frankly I'm not actually > sure of that - but I'm not afraid of coding C). I started reading the > coding page on the pulseaudio modules of freedesktop.org, but that did not > really inspire me to find a solution to my issue. So I would like to know > if one of you could guide me a bit. > > To help you guide me, I dump the output of a few commands, which I hope > you'll find useful. > > Looking forward to hearing from you soon, > Sylvain It's weird that a mic boost would affect headphone output. One explanation would be that the mic signal is fed directly to the headphones, but in that case increasing the boost should increase the noise, not decrease it. I guess it's a hardware or driver bug. To work around this, you can remove the [Element Headphone Mic Boost] sections from these two files: /usr/share/pulseaudio/alsa-mixer/paths/analog-input-internal-mic.conf /usr/share/pulseuadio/alsa-mixer/paths/analog-input-headphone-mic.conf After that PulseAudio won't touch the boost volume any more. Whenever pulseaudio is updated, the package manager will remove your modifications, so you have to redo the modifications. That's still better than having to reset the mixer after every reboot... -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Configuration system proposal
On Mon, 2016-01-18 at 09:59 +0530, Arun Raghavan wrote: > Hi folks, > Tanu already brought this up, posting some context: > > On Fri, 2016-01-15 at 19:52 +0200, Tanu Kaskinen wrote: > > Arun commented in IRC that another alternative would be to use the > > "configuration system"[1] that Arun would like to implement. Although > > I > > had some reservations at first, I now think that the proposed > > configuration system fits this use case well, so I have nothing > > against > > doing the hardware volume toggle implementation that way. The recent > > discussion about the configuration system has been only between Arun > > and me in IRC, however, so it may be that others disagree about the > > desirability of the new system. Feedback about that, too, would be > > very > > welcome. > > > > [1] > > https://wiki.freedesktop.org/www/Software/PulseAudio/Documentation/Developer/ConfigurationSystem/ > > Comments would indeed be welcome, and I'm happy to expand on any of the > bits that aren't clear as well. I have a question regarding the on-disk format. Let's say that we have an alsa card that has options hardware_volume_enabled and ignore_dB. hardware_volume_enabled is a core option, i.e. not specific to alsa, and ignore_dB is specific to alsa. The current proposal would probably be to write the options on disk as follows: core.port..port..hardware_volume_enabled = true alsa.port..port..ignore_dB = true What do you think of the following alternative format? [core.port .port.] hardware_volume_enabled = true [alsa.port .port.] ignore_dB = true I would prefer this approach, because it seems cleaner to me, and it's more consistent with how the existing alsa mixer configuration looks like. > I'm not sure exactly when I'll be able to get to this, but having the > basic ideas hammered out should make it easier to get started whenever > time allows (or for anyone else to jump in as well). I plan to jump in... -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v3 01/24] echo-cancel: Update webrtc-audio-processing usage to new API
On Mon, 2016-01-18 at 13:06 +0530, a...@accosted.net wrote: > From: Arun Raghavan > > The code now needs C++11 support to compile with the updated > webrtc-audio-processing library. > --- > configure.ac | 2 +- > src/Makefile.am | 2 +- > src/modules/echo-cancel/webrtc.cc | 54 > +-- > 3 files changed, 31 insertions(+), 27 deletions(-) Looks good to me! -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v3 02/24] echo-cancel: Allow enabling the extended filter in webrtc AEC
On Mon, 2016-01-18 at 13:06 +0530, a...@accosted.net wrote:
> From: Arun Raghavan
>
> This creates a longer filter that is more complex and less sensitive to
> incorrect delay reporting from the hardware. There is also a
> delay-agnostic mode that can eventually be enabled if required.
>
> In some very quick testing, not enabling this seems to provide better
> results during double-talk.
> ---
> src/modules/echo-cancel/webrtc.cc | 16 ++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/src/modules/echo-cancel/webrtc.cc
> b/src/modules/echo-cancel/webrtc.cc
> index 3e5a144..f4f1395 100644
> --- a/src/modules/echo-cancel/webrtc.cc
> +++ b/src/modules/echo-cancel/webrtc.cc
> @@ -46,6 +46,7 @@ PA_C_DECL_END
> #define DEFAULT_ROUTING_MODE "speakerphone"
> #define DEFAULT_COMFORT_NOISE true
> #define DEFAULT_DRIFT_COMPENSATION false
> +#define DEFAULT_EXTENDED_FILTER false
>
> static const char* const valid_modargs[] = {
> "high_pass_filter",
> @@ -56,6 +57,7 @@ static const char* const valid_modargs[] = {
> "routing_mode",
> "comfort_noise",
> "drift_compensation",
> +"extended_filter",
> NULL
> };
>
> @@ -81,7 +83,8 @@ bool pa_webrtc_ec_init(pa_core *c, pa_echo_canceller *ec,
> uint32_t *nframes, const char *args) {
> webrtc::AudioProcessing *apm = NULL;
> webrtc::ProcessingConfig pconfig;
> -bool hpf, ns, agc, dgc, mobile, cn;
> +webrtc::Config config;
> +bool hpf, ns, agc, dgc, mobile, cn, ext_filter;
> int rm = -1;
> pa_modargs *ma;
>
> @@ -154,7 +157,16 @@ bool pa_webrtc_ec_init(pa_core *c, pa_echo_canceller *ec,
> }
> }
>
> -apm = webrtc::AudioProcessing::Create();
> +ext_filter = DEFAULT_EXTENDED_FILTER;
> +if (pa_modargs_get_value_boolean(ma, "extended_filter", &ext_filter) <
> 0) {
> +pa_log("Failed to parse extended_filter value");
> +goto fail;
> +}
> +
> +if (ext_filter)
> +config.Set(new webrtc::ExtendedFilter(true));
Could these last two lines be replaced with
config.Set(new webrtc::ExtendedFilter(ext_filter));
?
Otherwise the patch looks good.
--
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v3 03/24] echo-cancel: Add support for the webrtc intelligibility enhancer
On Mon, 2016-01-18 at 13:06 +0530, a...@accosted.net wrote:
> From: Arun Raghavan
>
> Just exposing this, disabled by default. It's not used by Chromium at
> the moment.
> ---
> src/modules/echo-cancel/webrtc.cc | 14 --
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/src/modules/echo-cancel/webrtc.cc
> b/src/modules/echo-cancel/webrtc.cc
> index f4f1395..bbfa43f 100644
> --- a/src/modules/echo-cancel/webrtc.cc
> +++ b/src/modules/echo-cancel/webrtc.cc
> @@ -47,6 +47,7 @@ PA_C_DECL_END
> #define DEFAULT_COMFORT_NOISE true
> #define DEFAULT_DRIFT_COMPENSATION false
> #define DEFAULT_EXTENDED_FILTER false
> +#define DEFAULT_INTELLIGIBILITY_ENHANCER false
>
> static const char* const valid_modargs[] = {
> "high_pass_filter",
> @@ -58,6 +59,7 @@ static const char* const valid_modargs[] = {
> "comfort_noise",
> "drift_compensation",
> "extended_filter",
> +"intelligibility_enhancer",
> NULL
> };
>
> @@ -84,7 +86,7 @@ bool pa_webrtc_ec_init(pa_core *c, pa_echo_canceller *ec,
> webrtc::AudioProcessing *apm = NULL;
> webrtc::ProcessingConfig pconfig;
> webrtc::Config config;
> -bool hpf, ns, agc, dgc, mobile, cn, ext_filter;
> +bool hpf, ns, agc, dgc, mobile, cn, ext_filter, intelligibility;
> int rm = -1;
> pa_modargs *ma;
>
> @@ -163,8 +165,16 @@ bool pa_webrtc_ec_init(pa_core *c, pa_echo_canceller *ec,
> goto fail;
> }
>
> +intelligibility = DEFAULT_INTELLIGIBILITY_ENHANCER;
> +if (pa_modargs_get_value_boolean(ma, "intelligibility_enhancer",
> &intelligibility) < 0) {
> +pa_log("Failed to parse intelligibility_enhancer value");
> +goto fail;
> +}
> +
> if (ext_filter)
> config.Set(new webrtc::ExtendedFilter(true));
> +if (intelligibility)
> +config.Set(new
> webrtc::Intelligibility(true));
The same comment as in the previous patch: could the if statement be
eliminated?
>
> apm = webrtc::AudioProcessing::Create(config);
>
> @@ -253,7 +263,7 @@ void pa_webrtc_ec_play(pa_echo_canceller *ec, const
> uint8_t *play) {
> pa_assert(play_frame.samples_per_channel_ <=
> webrtc::AudioFrame::kMaxDataSizeSamples);
> memcpy(play_frame.data_, play, ec->params.priv.webrtc.blocksize);
>
> -apm->AnalyzeReverseStream(&play_frame);
> +apm->ProcessReverseStream(&play_frame);
This looks like a potentially unrelated change. Why is this change
done?
--
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v3 03/24] echo-cancel: Add support for the webrtc intelligibility enhancer
On Mon, 2016-01-25 at 09:36 +0530, Arun Raghavan wrote:
> On 24 January 2016 at 22:00, Tanu Kaskinen wrote:
> > On Mon, 2016-01-18 at 13:06 +0530, a...@accosted.net wrote:
> > > @@ -253,7 +263,7 @@ void pa_webrtc_ec_play(pa_echo_canceller *ec, const
> > > uint8_t *play) {
> > > pa_assert(play_frame.samples_per_channel_ <=
> > > webrtc::AudioFrame::kMaxDataSizeSamples);
> > > memcpy(play_frame.data_, play, ec->params.priv.webrtc.blocksize);
> > >
> > > -apm->AnalyzeReverseStream(&play_frame);
> > > +apm->ProcessReverseStream(&play_frame);
> >
> > This looks like a potentially unrelated change. Why is this change
> > done?
>
> It is needed for the intelligibility enhancer to work, but I later
> realised we don't actually support this -- unlike
> AnalyzeReverseStream(), ProcessReverseStream() modifies the playback
> samples. This is something that needs to be done in the future
> (there's a later commit that disables this that I could not squash due
> to some intermediate changes).
Does that mean that the intelligibility enhancer feature doesn't work
at all? In that case this patch should be dropped.
--
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v3 04/24] echo-cancel: Use correct sample spec parameters for webrtc AEC init
On Mon, 2016-01-18 at 13:06 +0530, a...@accosted.net wrote:
> From: Arun Raghavan
>
> ---
> src/modules/echo-cancel/webrtc.cc | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/modules/echo-cancel/webrtc.cc
> b/src/modules/echo-cancel/webrtc.cc
> index bbfa43f..ec63e26 100644
> --- a/src/modules/echo-cancel/webrtc.cc
> +++ b/src/modules/echo-cancel/webrtc.cc
> @@ -187,10 +187,10 @@ bool pa_webrtc_ec_init(pa_core *c, pa_echo_canceller
> *ec,
> *rec_map = *out_map;
>
> pconfig = {
> -webrtc::StreamConfig(out_ss->rate, out_ss->channels, false), /*
> input stream */
> +webrtc::StreamConfig(rec_ss->rate, rec_ss->channels, false), /*
> input stream */
> webrtc::StreamConfig(out_ss->rate, out_ss->channels, false), /*
> output stream */
> -webrtc::StreamConfig(out_ss->rate, out_ss->channels, false), /*
> reverse input stream */
> -webrtc::StreamConfig(out_ss->rate, out_ss->channels, false), /*
> reverse output stream */
> +webrtc::StreamConfig(play_ss->rate, play_ss->channels, false), /*
> reverse input stream */
> +webrtc::StreamConfig(play_ss->rate, play_ss->channels, false), /*
> reverse output stream */
> };
> apm->Initialize(pconfig);
Shouldn't this be fixed in patch 1?
--
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v3 03/24] echo-cancel: Add support for the webrtc intelligibility enhancer
On Mon, 2016-01-25 at 15:03 +0530, Arun Raghavan wrote:
> On 25 January 2016 at 13:30, Tanu Kaskinen wrote:
> > On Mon, 2016-01-25 at 09:36 +0530, Arun Raghavan wrote:
> > > On 24 January 2016 at 22:00, Tanu Kaskinen wrote:
> > > > On Mon, 2016-01-18 at 13:06 +0530, a...@accosted.net wrote:
> > > > > @@ -253,7 +263,7 @@ void pa_webrtc_ec_play(pa_echo_canceller *ec,
> > > > > const uint8_t *play) {
> > > > > pa_assert(play_frame.samples_per_channel_ <=
> > > > > webrtc::AudioFrame::kMaxDataSizeSamples);
> > > > > memcpy(play_frame.data_, play, ec->params.priv.webrtc.blocksize);
> > > > >
> > > > > -apm->AnalyzeReverseStream(&play_frame);
> > > > > +apm->ProcessReverseStream(&play_frame);
> > > >
> > > > This looks like a potentially unrelated change. Why is this change
> > > > done?
> > >
> > > It is needed for the intelligibility enhancer to work, but I later
> > > realised we don't actually support this -- unlike
> > > AnalyzeReverseStream(), ProcessReverseStream() modifies the playback
> > > samples. This is something that needs to be done in the future
> > > (there's a later commit that disables this that I could not squash due
> > > to some intermediate changes).
> >
> > Does that mean that the intelligibility enhancer feature doesn't work
> > at all? In that case this patch should be dropped.
>
> I was thinking of leaving this and the corresponding disable-patch in
> as documentation of what doesn't work, but Ic an just drop it if
> that's the preference.
I'd prefer not merging broken patches. If the echo-cancel module has
options for all webrtc features, then adding the warning here would be
ok (instead of a separate patch), but if the feature coverage is
partial anyway, then just drop the broken options.
--
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v3 05/24] echo-cancel: Canceller may use different spec for playback and capture
On Mon, 2016-01-18 at 13:06 +0530, a...@accosted.net wrote: > From: Arun Raghavan > > This sets up the default sink sample spec to match what we expect, > rather than assuming that the canceller will set this up (our assumption > is that we'll use 32 kHz mono unless someone explicitly overrides this). I have trouble understanding this description. Maybe rewording is needed? What does "what we expect" mean? You set the rate and channels to 32 kHz mono, why is that our "expectation"? Does something break if this expectation is violated via user configuration? The sink and source rates seem to be configurable (matching rates between source and sink is enforced, though). The sink channel setup seems to be forced to be mono, while the source channel map can be configured. What are the reasons behind this particular mix of configurability and hardcoding? Some comments about this in the sample spec initialization code might be a good idea. I'm not sure why you describe the choice of using 32 kHz mono as an "assumption". Defaulting to 32 kHz mono is what the code does, in what way is that an assumption? Overall, it remains unclear why this change is done. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH] device-manager: improve logging about non-existing data
Previously a missing key would cause this kind of log output:
D: [pulseaudio] module-device-manager.c: Database contains invalid data for
key: sink:auto_null (probably pre-v1.0 data)
D: [pulseaudio] module-device-manager.c: Attempting to load legacy (pre-v1.0)
data for key: sink:auto_null
D: [pulseaudio] module-device-manager.c: Size does not match.
D: [pulseaudio] module-device-manager.c: Unable to load legacy (pre-v1.0) data
for key: sink:auto_null. Ignoring.
That is now replaced with
D: [pulseaudio] module-device-manager.c: Database contains no data for key:
sink:auto_null
---
src/modules/module-device-manager.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/modules/module-device-manager.c
b/src/modules/module-device-manager.c
index f125bdd..1a0a53e 100644
--- a/src/modules/module-device-manager.c
+++ b/src/modules/module-device-manager.c
@@ -292,8 +292,10 @@ static struct entry* entry_read(struct userdata *u, const
char *name) {
pa_zero(data);
-if (!pa_database_get(u->database, &key, &data))
-goto fail;
+if (!pa_database_get(u->database, &key, &data)) {
+pa_log_debug("Database contains no data for key: %s", name);
+return NULL;
+}
t = pa_tagstruct_new_fixed(data.data, data.size);
e = entry_new();
--
2.7.0.rc3
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v3 06/24] echo-cancel: Relax restrictions on webrtc AEC stream config
On Mon, 2016-01-18 at 13:06 +0530, a...@accosted.net wrote: > From: Arun Raghavan > > This exposes the range of configurations the library actually supports. Looks good to me. The commit message could be more specific about how exactly the configuration restrictions are relaxed. It's a bit tedious to try to figure that out by staring at the code. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v3 07/24] echo-cancel: Add a modarg to use sink/source master format and spec
On Mon, 2016-01-18 at 13:06 +0530, a...@accosted.net wrote: > From: Arun Raghavan > > This allows us to inherit the sample spec parameters from the sink and > source master (rather than forcing 32 kHz / mono). It is still possible > to override some of the parameters for the source side with modargs. What reasons do you see for using or not using parameters from the master devices? Should use_master_format perhaps be enabled by default? -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 6/6] pactl, pacmd, cli-command: Added set-(sink|source)-latency-offset commands.
On Fri, 2016-01-29 at 11:11 +1100, Chris Billington wrote: > Hi all, > > I was wondering if anyone has had the time to look at my patch? > > No rush if there are more important things to be doing! I haven't had time yet. I will get to it eventually. > > The other four patches are implementing the setting of individual latency > > offsets of sinks and sources. This now plays nicely with port latency > > offsets. The way I've done this is to rename everything pertaining to the > > latency offsets inherited from ports, so that they are all called > > sink->port_latency_offset etc. Then I've introduced a new latency offset > > for sinks and sources, called just sink->latency_offset etc, which is for > > the latency offset set on that sink or source alone. The total latency > > reported by the device then includes the sum of both of them. Sounds very good. > > The only thing I didn't do was restoring from disk. Tanu, you said this > > might be tricky, but I wasn't able to understand module-device-restore > > enough to even see the trickiness you were talking about, I wasn't able to > > get that far! I'm not very familiar with the codebase - I'm just here > > selfishly to try and get my feature in so my application works better :p. > > So apologies for that, I hope the changes are worthy without it. I think it's ok to leave the persistence feature out. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v3 08/24] echo-cancel: Mark private function as static
On Mon, 2016-01-18 at 13:06 +0530, a...@accosted.net wrote:
> From: Arun Raghavan
>
> ---
> src/modules/echo-cancel/webrtc.cc | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/modules/echo-cancel/webrtc.cc
> b/src/modules/echo-cancel/webrtc.cc
> index 8dd1805..4dbd2fc 100644
> --- a/src/modules/echo-cancel/webrtc.cc
> +++ b/src/modules/echo-cancel/webrtc.cc
> @@ -78,9 +78,9 @@ static int routing_mode_from_string(const char *rmode) {
> return -1;
> }
>
> -void pa_webrtc_ec_fixate_spec(pa_sample_spec *rec_ss, pa_channel_map
> *rec_map,
> - pa_sample_spec *play_ss, pa_channel_map
> *play_map,
> - pa_sample_spec *out_ss, pa_channel_map
> *out_map)
> +static void pa_webrtc_ec_fixate_spec(pa_sample_spec *rec_ss, pa_channel_map
> *rec_map,
> + pa_sample_spec *play_ss, pa_channel_map
> *play_map,
> + pa_sample_spec *out_ss, pa_channel_map
> *out_map)
> {
> rec_ss->format = PA_SAMPLE_S16NE;
> play_ss->format = PA_SAMPLE_S16NE;
Ack.
--
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Bluetooth : A2DP and AVRCP is working fine , but HFP is showing working but No sound -Mic/Speaker ?
On Fri, 2016-01-29 at 11:01 +, Samiran Sarkar wrote: > Hi Friends, > > I 'm using an Intel IVI development board, > > Configuration is like this, using Bluez -5.37, Pulseaudio 6.0 , ofono 1.17, > Bluetooth A2DP and AVRCP is working fine , HFP is showing working but no > sound - > > I can make outgoing calls/ can accept incoming calls > expected target hardware should have audio output) but > – there is no sound on the Speaker either Mic can't input voice over BT ( > We are using USB (Jabra) Mic and headphone ) > > Any pointer so that quickly we can fix this issue ? Maybe you are lacking module-loopback between the bluetooth and alsa devices? The log messages that you attached contained something about module-loopback failing to load. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Bluetooth : A2DP and AVRCP is working fine , but HFP is showing working but No sound -Mic/Speaker ?
On Fri, 2016-01-29 at 13:32 +, Samiran Sarkar wrote:
> Thanks Tanu , really appreciated for your support
>
> 1. pactl set-card-profile bluez_card.F8_32_E4_FB_FD_28 headset_audio_gateway
> then
> Loop back is like this
> 2. pacmd load-module module-loopback source=bluez_source.F8_32_E4_FB_FD_28
> sink=alsa_output.usb-Jabra_Jabra_UC_VOICE_150_duo_-00-duo.analog-stereo
>
> But I can listen in the headphone speaker whatever I 'm speaking over MIC (
> it seems loop back within the device)
Maybe the module-loopback source output got moved from the bluez source
to the alsa source for some reason? You can verify what streams are
connected to what devices with "pactl list", and prevent unwanted
source output moves from happening by giving parameter
"source_dont_move=true" to module-loopback ("sink_dont_move" exists
too, which will prevent the sink input from moving from the initial
sink to somewhere else).
--
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v3 09/24] echo-cancel: Allow enabling tracing output from the webrtc canceller
On Mon, 2016-01-18 at 13:06 +0530, a...@accosted.net wrote:
> From: Arun Raghavan
>
> ---
> src/modules/echo-cancel/echo-cancel.h | 1 +
> src/modules/echo-cancel/webrtc.cc | 34 ++
> 2 files changed, 35 insertions(+)
>
> diff --git a/src/modules/echo-cancel/echo-cancel.h
> b/src/modules/echo-cancel/echo-cancel.h
> index 29d1574..142f8ac 100644
> --- a/src/modules/echo-cancel/echo-cancel.h
> +++ b/src/modules/echo-cancel/echo-cancel.h
> @@ -67,6 +67,7 @@ struct pa_echo_canceller_params {
> uint32_t blocksize;
> pa_sample_spec sample_spec;
> bool agc;
> +bool trace;
> } webrtc;
> #endif
> /* each canceller-specific structure goes here */
> diff --git a/src/modules/echo-cancel/webrtc.cc
> b/src/modules/echo-cancel/webrtc.cc
> index 4dbd2fc..53ab7e1 100644
> --- a/src/modules/echo-cancel/webrtc.cc
> +++ b/src/modules/echo-cancel/webrtc.cc
> @@ -35,6 +35,7 @@ PA_C_DECL_END
>
> #include
> #include
> +#include
>
> #define BLOCK_SIZE_US 1
>
> @@ -48,6 +49,7 @@ PA_C_DECL_END
> #define DEFAULT_DRIFT_COMPENSATION false
> #define DEFAULT_EXTENDED_FILTER false
> #define DEFAULT_INTELLIGIBILITY_ENHANCER false
> +#define DEFAULT_TRACE false
>
> static const char* const valid_modargs[] = {
> "high_pass_filter",
> @@ -60,6 +62,7 @@ static const char* const valid_modargs[] = {
> "drift_compensation",
> "extended_filter",
> "intelligibility_enhancer",
> +"trace",
> NULL
> };
>
> @@ -78,6 +81,20 @@ static int routing_mode_from_string(const char *rmode) {
> return -1;
> }
>
> +class PaWebrtcTraceCallback : public webrtc::TraceCallback {
> +void Print(webrtc::TraceLevel level, const char *message, int length)
I hope the presence of a length parameter doesn't mean that message
isn't null-terminated.
> +{
> +if (level & webrtc::kTraceError || level & webrtc::kTraceCritical)
> +pa_log(message);
> +else if (level & webrtc::kTraceWarning)
> +pa_log_warn(message);
> +else if (level & webrtc::kTraceInfo)
> +pa_log_info(message);
> +else
> +pa_log_debug(message);
> +}
> +};
> +
> static void pa_webrtc_ec_fixate_spec(pa_sample_spec *rec_ss, pa_channel_map
> *rec_map,
> pa_sample_spec *play_ss, pa_channel_map
> *play_map,
> pa_sample_spec *out_ss, pa_channel_map
> *out_map)
> @@ -201,6 +218,18 @@ bool pa_webrtc_ec_init(pa_core *c, pa_echo_canceller *ec,
> if (intelligibility)
> config.Set(new webrtc::Intelligibility(true));
>
> +ec->params.priv.webrtc.trace = DEFAULT_TRACE;
> +if (pa_modargs_get_value_boolean(ma, "trace",
> &ec->params.priv.webrtc.trace) < 0) {
> +pa_log("Failed to parse trace value");
> +goto fail;
> +}
> +
> +if (ec->params.priv.webrtc.trace) {
> +webrtc::Trace::CreateTrace();
> +webrtc::Trace::set_level_filter(webrtc::kTraceAll);
> +webrtc::Trace::SetTraceCallback(new PaWebrtcTraceCallback());
Does webrtc::Trace delete the object that you allocate here?
> +}
> +
> pa_webrtc_ec_fixate_spec(rec_ss, rec_map, play_ss, play_map, out_ss,
> out_map);
>
> apm = webrtc::AudioProcessing::Create(config);
> @@ -263,6 +292,8 @@ bool pa_webrtc_ec_init(pa_core *c, pa_echo_canceller *ec,
> fail:
> if (ma)
> pa_modargs_free(ma);
> +if (ec->params.priv.webrtc.trace)
> +webrtc::Trace::ReturnTrace();
What does this do? Is it safe to call this if you haven't yet set up
the tracing?
--
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] switch-on-port-available: Switch from HDMI to analog; but not the other way around
On Fri, 2016-01-29 at 20:33 +0100, David Henningsson wrote: > If you have headphones plugged in and plug in HDMI; you want sound > to stay on headphones. > If you have HDMI plugged in and you plug in headphones; you want sound > to switch to headphones. > > Hence we need to take priority into account as well when determining > whether to switch to a new profile or not. I think the same logic should apply to input ports too. Otherwise looks good to me. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] switch-on-port-available: Switch from HDMI to analog; but not the other way around
On Sat, 2016-01-30 at 12:07 +0500, Alexander E. Patrakov wrote: > 30.01.2016 00:33, David Henningsson wrote: > > If you have headphones plugged in and plug in HDMI; you want sound > > to stay on headphones. > > If you have HDMI plugged in and you plug in headphones; you want sound > > to switch to headphones. > > > > Hence we need to take priority into account as well when determining > > whether to switch to a new profile or not. > > > > -if (sink->active_port->available != PA_AVAILABLE_NO) > > +if ((sink->active_port->available != PA_AVAILABLE_NO) && > > (sink->active_port->priority >= prio)) > > You are comparing profile priority and port priority here. You seem to be misreading something. prio is the priority of the port being activated. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v3 10/24] echo-cancel: Deal with volume limit breakage in webrtc AGC
On Mon, 2016-01-18 at 13:06 +0530, a...@accosted.net wrote: > From: Arun Raghavan > > The AGC code no longer seems to honour the analog volume limits we set, > and internally uses 0-255 as the volume range. So we switch to use that > (keeping the old API usage as is in case this gets fixed upstream). > --- > src/modules/echo-cancel/webrtc.cc | 25 + > 1 file changed, 21 insertions(+), 4 deletions(-) Looks good to me. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Bluetooth : A2DP and AVRCP is working fine , but HFP is showing working but No sound -Mic/Speaker ?
On Mon, 2016-02-01 at 09:06 +, Samiran Sarkar wrote: > root@baytrail32:~# journalctl -u pulseaudio > -- Logs begin at Sun 2012-01-01 00:00:34 UTC, end at Thu 2015-12-10 10:16:46 > UTC. -- > Dec 10 10:09:45 baytrail32 pulseaudio[312]: E: [pulseaudio] main.c: Sink > alsa_output.usb-Jabra_Jabra_UC_VOICE_150_duo_- > Dec 10 10:09:45 baytrail32 pulseaudio[312]: E: [pulseaudio] main.c: Source > alsa_input.usb-Jabra_Jabra_UC_VOICE_150_duo_ > Dec 10 10:10:18 baytrail32 pulseaudio[312]: E: [pulseaudio] backend-ofono.c: > Deferred setup failed on fd -1: Transport endpoint is > Dec 10 10:15:52 baytrail32 pulseaudio[312]: E: [pulseaudio] backend-ofono.c: > Deferred setup failed on fd -1: Transport endpoint is > Dec 10 10:16:30 baytrail32 systemd[1]: pulseaudio.service: main process > exited, code=killed, status=11/SEGV Segfaulting is never good... > Dec 10 10:16:30 baytrail32 systemd[1]: Unit pulseaudio.service entered failed > state. > Dec 10 10:16:36 baytrail32 pulseaudio[362]: E: [pulseaudio] main.c: Sink > alsa_output.usb-Jabra_Jabra_UC_VOICE_150_duo_- > Dec 10 10:16:36 baytrail32 pulseaudio[362]: E: [pulseaudio] main.c: Source > alsa_input.usb-Jabra_Jabra_UC_VOICE_150_duo_ > Dec 10 10:16:36 baytrail32 pulseaudio[362]: E: [pulseaudio] backend-ofono.c: > Deferred setup failed on fd -1: Transport endpoint is > > > #pactl list modules > > Module #23 > Name: module-bluez5-device > Argument: path=/org/bluez/hci0/dev_F8_32_E4_FB_FD_28 > Usage counter: 2 > Properties: > module.author = "João Paulo Rechi Vita" > module.description = "BlueZ 5 Bluetooth audio sink and source" > module.version = "6.0-dirty" > > Module #24 > Name: module-loopback > Argument: sink="bluez_sink.F8_32_E4_FB_FD_28" sink_dont_move="true" > source_output_properties="media.role=phone" > Usage counter: n/a > Properties: > module.author = "Pierre-Louis Bossart" > module.description = "Loopback from source to sink" > module.version = "6.0-dirty" > > Module #25 > Name: module-loopback > Argument: source="bluez_source.F8_32_E4_FB_FD_28" > source_dont_move="true" sink_input_properties="media.role=phone" > Usage counter: n/a > Properties: > module.author = "Pierre-Louis Bossart" > module.description = "Loopback from source to sink" > module.version = "6.0-dirty" > > - > --- > --- > --- > This( Above) is the normal condition -- if I make a call I can't > transfer my Smart Phone voice to Bluetooth Device/ By the "Bluetooth Device", do you mean the device that is running PulseAudio? I'm assuming that the setup is a device running PulseAudio with alsa input and output, connected to a phone via Bluetooth. "pactl list modules" doesn't show how audio is routed. "pactl list sink-inputs" and "pactl list source-outputs" will show what streams exist in the system. For each stream the listing includes the module that created the stream and the device that the stream is connected to. Based on that information you can figure out the real connections that the loopback modules are making between devices. > then if I do loopback it allows to transfer Voice from Phone > to Bluetooth device ( no sound at Phone , making sure that > Conversation has transferred to BT Device) Since you already have the two loopbacks loaded by module-bluetooth- policy, you shouldn't need to load any additional loopbacks. > #pacmd load-module module-loopback source=bluez_source.F8_32_E4_FB_FD_28 > sink=alsa_output.usb-Jabra_Jabra_UC_VOICE_150_duo_-00-duo.analog-stereo > > again whatever I 'm speaking over MIC I 'm able to listen in > headphone Speaker. But Voice conversation is missing which supposed > to get via Bluetooth Is "MIC" the alsa source and "headphone Speaker" the alsa sink? If so, I don't know why the loopback module that you loaded manually would cause such connection. The only reason I can think of is that something moves the source output of the loopback to the alsa source. Hence I suggested using the "source_dont_move" parameter. But as I said, loading module-loopback manually should not be done if you're trying to use module-bluetooth-policy to manage the loopbacks. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v3 11/24] echo-cancel: Start capture at a sane value if we're doing webrtc AGC
On Mon, 2016-01-18 at 13:06 +0530, a...@accosted.net wrote:
> From: Arun Raghavan
>
> This is required to make sure the capture output has sufficient energy
> for the AGC to do its job.
I think the word "value" in the first line of the commit message should
be replaced with "volume". After reading just the commit message, I
didn't know what the patch was about.
> ---
> src/modules/echo-cancel/echo-cancel.h | 1 +
> src/modules/echo-cancel/webrtc.cc | 14 +-
> 2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/src/modules/echo-cancel/echo-cancel.h
> b/src/modules/echo-cancel/echo-cancel.h
> index 142f8ac..b570095 100644
> --- a/src/modules/echo-cancel/echo-cancel.h
> +++ b/src/modules/echo-cancel/echo-cancel.h
> @@ -68,6 +68,7 @@ struct pa_echo_canceller_params {
> pa_sample_spec sample_spec;
> bool agc;
> bool trace;
> +bool first;
> } webrtc;
> #endif
> /* each canceller-specific structure goes here */
> diff --git a/src/modules/echo-cancel/webrtc.cc
> b/src/modules/echo-cancel/webrtc.cc
> index c2585a8..6431651 100644
> --- a/src/modules/echo-cancel/webrtc.cc
> +++ b/src/modules/echo-cancel/webrtc.cc
> @@ -52,6 +52,7 @@ PA_C_DECL_END
> #define DEFAULT_TRACE false
>
> #define WEBRTC_AGC_MAX_VOLUME 255
> +#define WEBRTC_AGC_START_VOLUME 85
>
> static const char* const valid_modargs[] = {
> "high_pass_filter",
> @@ -297,6 +298,7 @@ bool pa_webrtc_ec_init(pa_core *c, pa_echo_canceller *ec,
> ec->params.priv.webrtc.sample_spec = *out_ss;
> ec->params.priv.webrtc.blocksize = (uint64_t)pa_bytes_per_second(out_ss)
> * BLOCK_SIZE_US / PA_USEC_PER_SEC;
> *nframes = ec->params.priv.webrtc.blocksize / pa_frame_size(out_ss);
> +ec->params.priv.webrtc.first = true;
>
> pa_modargs_free(ma);
> return true;
> @@ -354,7 +356,17 @@ void pa_webrtc_ec_record(pa_echo_canceller *ec, const
> uint8_t *rec, uint8_t *out
> apm->ProcessStream(&out_frame);
>
> if (ec->params.priv.webrtc.agc) {
> -new_volume = apm->gain_control()->stream_analog_level();
> +if (PA_UNLIKELY(ec->params.priv.webrtc.first)) {
> +/* We start at a sane default volume (taken from the Chromium
> + * condition on the experimental AGC in audio_processing.h).
> This is
> + * needed to make sure that there's enough energy in the capture
> + * signal for the AGC to work */
> +ec->params.priv.webrtc.first = false;
> +new_volume = WEBRTC_AGC_START_VOLUME;
This is done only on initial loading of the module. Does AGC work ok
after resuming a suspended source?
--
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v3 05/24] echo-cancel: Canceller may use different spec for playback and capture
On Tue, 2016-02-02 at 09:53 +0530, Arun Raghavan wrote: > On Tue, 2016-01-26 at 13:42 +0200, Tanu Kaskinen wrote: > > On Mon, 2016-01-18 at 13:06 +0530, a...@accosted.net wrote: > > > From: Arun Raghavan > > > > > > This sets up the default sink sample spec to match what we expect, > > > rather than assuming that the canceller will set this up (our > > > assumption > > > is that we'll use 32 kHz mono unless someone explicitly overrides > > > this). > > > > I have trouble understanding this description. Maybe rewording is > > needed? > > > > What does "what we expect" mean? You set the rate and channels to 32 > > kHz mono, why is that our "expectation"? Does something break if this > > expectation is violated via user configuration? The sink and source > > rates seem to be configurable (matching rates between source and sink > > is enforced, though). The sink channel setup seems to be forced to be > > mono, while the source channel map can be configured. What are the > > reasons behind this particular mix of configurability and hardcoding? > > Some comments about this in the sample spec initialization code might > > be a good idea. > > > > I'm not sure why you describe the choice of using 32 kHz mono as an > > "assumption". Defaulting to 32 kHz mono is what the code does, in > > what > > way is that an assumption? > > > > Overall, it remains unclear why this change is done. > > In the original code, I picked 32 kHz mono as the format to keep > computational complexity at something that seemed would work well on > low-end hardware (basically netbooks) without losing too much in the > way of quality. > > Prior to this change, the code assumes that the canceller will pick > something similar for us on the sink side, based on what we're > requesting on the source side (which is that fixed format). > > Does that make more sense now? Maybe. Is this correct: The intention has always been to configure low enough parameters to keep CPU consumption down. Prior to this change, we assumed that the EC backend would override the sink parameters based on the source parameters to achieve this goal, and with this change we remove that assumption by forcing the default parameters for the sink to be low enough. I hope you rewrite the commit message. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v3 07/24] echo-cancel: Add a modarg to use sink/source master format and spec
On Tue, 2016-02-02 at 09:56 +0530, Arun Raghavan wrote: > On Thu, 2016-01-28 at 09:40 +0200, Tanu Kaskinen wrote: > > On Mon, 2016-01-18 at 13:06 +0530, a...@accosted.net wrote: > > > From: Arun Raghavan > > > > > > This allows us to inherit the sample spec parameters from the sink > > > and > > > source master (rather than forcing 32 kHz / mono). It is still > > > possible > > > to override some of the parameters for the source side with > > > modargs. > > > > What reasons do you see for using or not using parameters from the > > master devices? Should use_master_format perhaps be enabled by > > default? > > My original testing showed that these parameters provided a decent > perf/quality trade-off on lower end hardware (which I no longer have > access to). I figure it makes sense to continue with that for now, and > in the future this can be relaxed (use_master_format=yes could be the > default, and resource-constrained systems can disable it). Ok. Worth mentioning in the commit message. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v3 11/24] echo-cancel: Start capture at a sane value if we're doing webrtc AGC
On Tue, 2016-02-02 at 09:59 +0530, Arun Raghavan wrote:
> On Mon, 2016-02-01 at 13:18 +0200, Tanu Kaskinen wrote:
> > On Mon, 2016-01-18 at 13:06 +0530, a...@accosted.net wrote:
> > > From: Arun Raghavan
> > >
> > > This is required to make sure the capture output has sufficient
> > > energy
> > > for the AGC to do its job.
> >
> > I think the word "value" in the first line of the commit message
> > should
> > be replaced with "volume". After reading just the commit message, I
> > didn't know what the patch was about.
> >
> > > ---
> > > src/modules/echo-cancel/echo-cancel.h | 1 +
> > > src/modules/echo-cancel/webrtc.cc | 14 +-
> > > 2 files changed, 14 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/src/modules/echo-cancel/echo-cancel.h
> > > b/src/modules/echo-cancel/echo-cancel.h
> > > index 142f8ac..b570095 100644
> > > --- a/src/modules/echo-cancel/echo-cancel.h
> > > +++ b/src/modules/echo-cancel/echo-cancel.h
> > > @@ -68,6 +68,7 @@ struct pa_echo_canceller_params {
> > > pa_sample_spec sample_spec;
> > > bool agc;
> > > bool trace;
> > > +bool first;
> > > } webrtc;
> > > #endif
> > > /* each canceller-specific structure goes here */
> > > diff --git a/src/modules/echo-cancel/webrtc.cc b/src/modules/echo-
> > > cancel/webrtc.cc
> > > index c2585a8..6431651 100644
> > > --- a/src/modules/echo-cancel/webrtc.cc
> > > +++ b/src/modules/echo-cancel/webrtc.cc
> > > @@ -52,6 +52,7 @@ PA_C_DECL_END
> > > #define DEFAULT_TRACE false
> > >
> > > #define WEBRTC_AGC_MAX_VOLUME 255
> > > +#define WEBRTC_AGC_START_VOLUME 85
> > >
> > > static const char* const valid_modargs[] = {
> > > "high_pass_filter",
> > > @@ -297,6 +298,7 @@ bool pa_webrtc_ec_init(pa_core *c,
> > > pa_echo_canceller *ec,
> > > ec->params.priv.webrtc.sample_spec = *out_ss;
> > > ec->params.priv.webrtc.blocksize =
> > > (uint64_t)pa_bytes_per_second(out_ss) * BLOCK_SIZE_US /
> > > PA_USEC_PER_SEC;
> > > *nframes = ec->params.priv.webrtc.blocksize /
> > > pa_frame_size(out_ss);
> > > +ec->params.priv.webrtc.first = true;
> > >
> > > pa_modargs_free(ma);
> > > return true;
> > > @@ -354,7 +356,17 @@ void pa_webrtc_ec_record(pa_echo_canceller
> > > *ec, const uint8_t *rec, uint8_t *out
> > > apm->ProcessStream(&out_frame);
> > >
> > > if (ec->params.priv.webrtc.agc) {
> > > -new_volume = apm->gain_control()->stream_analog_level();
> > > +if (PA_UNLIKELY(ec->params.priv.webrtc.first)) {
> > > +/* We start at a sane default volume (taken from the
> > > Chromium
> > > + * condition on the experimental AGC in
> > > audio_processing.h). This is
> > > + * needed to make sure that there's enough energy in
> > > the capture
> > > + * signal for the AGC to work */
> > > +ec->params.priv.webrtc.first = false;
> > > +new_volume = WEBRTC_AGC_START_VOLUME;
> >
> > This is done only on initial loading of the module. Does AGC work ok
> > after resuming a suspended source?
>
> After suspend/resume is not a problem since the source volume won't get
> reset from whatever it was set to before. This is mostly to make sure
> that when the AGC first starts, it is able to actually get going.
Ok. Is the problem that if the initial volume is too low, AGC won't
work, but too high initial volume is fine? What if the user sets the
source volume manually? Does AGC stop working? If not, why is that
different from the initialization phase? (I'm just trying to understand
the problem. If manual volume changes mess up AGC, I don't think that's
something that this patch needs to care about.)
--
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v3 12/24] echo-cancel: Allow enabling of the webrtc experimental AGC mechanism
On Mon, 2016-01-18 at 13:06 +0530, a...@accosted.net wrote: > From: Arun Raghavan > > --- > src/modules/echo-cancel/webrtc.cc | 12 +++- > 1 file changed, 11 insertions(+), 1 deletion(-) How does this interact with "regular" AGC? Can both be enabled at the same time? -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v3 13/24] echo-cancel: Add a modarg toggle for VAD in the webrtc canceller
On Mon, 2016-01-18 at 13:06 +0530, a...@accosted.net wrote: > From: Arun Raghavan > > --- > src/modules/echo-cancel/webrtc.cc | 13 +++-- > 1 file changed, 11 insertions(+), 2 deletions(-) Looks good to me. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Way to treat (almost) equal sound-devices different?
On Wed, 2016-02-03 at 12:42 +0100, Stefan Neufeind wrote: > Hi, > > I'm using an external dock which contains sound-output. One such > dock-box at work, one in the home-office. Basically it's usb-connected > with an additional external sound-device in the dock. > > Pulseaudio knows when the dock is not connected and falls back to > internal sound-output. > > But is there a way to differentiate which of the two docks is connected, > maybe by taking the serial-number of the dock into account? > > I didn't find any status of "speakers (not) connected" in pulseaudio for > that device unfortunately. Of course that would work for me as well. > > My current idea was that at work where there are no external speakers > connected currently it might want to use the internal sound while at > home it should use the additional output from the dock. Compare the output from "pactl list sinks" between the two docks. Are there any differences? If not, then I don't think it's possible to distinguish the docks from each other. If the names of the sinks are different, then it might be possible to achieve what you want with just some configuration: put "load-module module-device-manager do_routing=true" to /etc/pulse/default.pa and then configure the device priorities as appropriate. Unfortunately, the only way to configure the priorities is KDE's KMix, as far as I know, so if you're not using KDE and using KMix isn't practical, then this approach won't work. If the names of the sinks are same, but there are some differences in the sink properties, then you could write a script that monitors what sound cards are present and changes the default sink based on that. This alternative works also if the lack of KMix prevents you from using the previous approach. I hope we'll improve in this area in the future. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Problem with switch-on-connect
On Thu, 2016-02-04 at 00:03 +0300, Nikolay Amiantov wrote: > Hi all, > > I have a laptop with an internal audio output device and wireless > headphones with a USB dongle which registers itself as an another > device. I want fallback device and all current and future audio streams > to automatically switch to headphones when I plug them in. From reading > manuals, it seems I need combination of two modules: > > * module-switch-on-connect to switch all current streams and fallback > device to the new one on connect; > * module-stream-restore restore_device=false to forget all device > settings, so all new streams are connected to the fallback device, which > should be headphones. > > Alas, it doesn't work for me -- on connection the fallback device is > switched, but not current streams neither future ones. If I kill > pulseaudio and clear ~/.config/pulse things seem to work correctly for > some time, but stop after that! I haven't yet been able to understand > exactly what is needed to happen for this to stop working. > > Any ideas what should I do? Thanks in advance! > > I use pulseaudio 7.1 on NixOS (a Linux distribution). > My configuration file: > https://github.com/abbradar/dotfiles/blob/master/default.pa Do you use KDE? If you load module-stream-restore with restore_device=false, and the fallback sink is set correctly, the only reason I can think of why streams would not go to the fallback sink is that module-device-manager is loaded. The module is not in default.pa, but start-pulseaudio-x11 loads it when run as part of KDE session initialization, and I think KMix might load it too if not already loaded. When things don't work, does "pactl list modules" show module- device-manager? -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v3 14/24] build-sys: Move to compiling with C11 support
On Mon, 2016-01-18 at 13:06 +0530, a...@accosted.net wrote: > @@ -176,6 +175,8 @@ esac > > Compiler flags > > +AX_CHECK_COMPILE_FLAG([-std=c11], [CFLAGS="$CFLAGS -std=c11"], [], > [-pedantic -Werror]) This does nothing if the compile flag isn't supported. Shouldn't we fail in configure if -std=c11 doesn't work? Also, I don't think we should set CFLAGS. Adding -std=c11 to AM_CFLAGS in src/Makefile.am seems like the right thing to do. See https://www.gnu.org/software/automake/manual/html_node/User-Variables.html#User-Variables -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v3 14/24] build-sys: Move to compiling with C11 support
On Thu, 2016-02-04 at 10:30 +0530, Arun Raghavan wrote: > On 4 February 2016 at 10:22, Arun Raghavan wrote: > > On Thu, 2016-02-04 at 06:04 +0200, Tanu Kaskinen wrote: > > > On Mon, 2016-01-18 at 13:06 +0530, a...@accosted.net wrote: > > > > @@ -176,6 +175,8 @@ esac > > > > > > > > Compiler flags > > > > > > > > +AX_CHECK_COMPILE_FLAG([-std=c11], [CFLAGS="$CFLAGS -std=c11"], [], > > > > [-pedantic -Werror]) > > > > > > This does nothing if the compile flag isn't supported. Shouldn't we > > > fail in configure if -std=c11 doesn't work? > > > > Right, I'm rewriting this line as: > > > > AX_CHECK_COMPILE_FLAG([-std=c11], > > [CFLAGS="$CFLAGS -std=c11"], > > [AC_MSG_ERROR([*** Compiler does not support -std=c11])], > > [-pedantic -Werror]) > > > > > Also, I don't think we should set CFLAGS. Adding -std=c11 to > > > AM_CFLAGS > > > in src/Makefile.am seems like the right thing to do. See > > > https://www.gnu.org/software/automake/manual/html_node/User-Variables > > > .html#User-Variables > > > > This would make it inconsistent with the rest of configure.ac, though. > > It'd be nice to change everything to use AM_CFLAGS but that should be a > > separate change. I don't buy this argument. There's no consistency in the first place. We set some flags via CFLAGS and some via AM_CFLAGS. > > One more thing that's missing in this change is an addition > > of ax_check_compile_flag.m4. I'll squash that in too. > > The AX_* macros also seem to work with CFLAGS, btw. What do you mean by that? -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v3 14/24] build-sys: Move to compiling with C11 support
On Thu, 2016-02-04 at 10:57 +0530, Arun Raghavan wrote: > On Thu, 2016-02-04 at 07:13 +0200, Tanu Kaskinen wrote: > > I don't buy this argument. There's no consistency in the first place. > > We set some flags via CFLAGS and some via AM_CFLAGS. > > Looking at this again, I don't see any place in the configure checks > where AM_CFLAGS is set (and it looks to me like that this would not be > appropriate anyway -- AM_CFLAGS seems meant to be used withing > Makefile.am, not configure.ac). Yes, configure doesn't set AM_CFLAGS. I didn't suggest setting AM_CFLAGS from the configure script. For example, gcov flags are set in AM_CFLAGS in Makefile.am based on configure results. So configure can be used to check things, and the flags can be set according to the checks to AM_CFLAGS, all without touching CFLAGS. > > > > One more thing that's missing in this change is an addition > > > > of ax_check_compile_flag.m4. I'll squash that in too. > > > > > > The AX_* macros also seem to work with CFLAGS, btw. > > > > What do you mean by that? > > If you look at what AX_APPEND_COMPILE_FLAGS does, for example, it sets > CFLAGS directly. AX_APPEND_COMPILE_FLAGS takes as a parameter the variable to which the flags are appended. Only when the parameter is omitted the macro will modify CFLAGS. We could save the flags to a separate variable that is added to AM_CFLAGS in Makefile.am. We only use AX_APPEND_COMPILE_FLAGS to set non-mandatory flags, though. Nothing breaks if the user overrides those flags. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] RFC: Switch to HDMI or not?
On Thu, 2016-02-04 at 11:35 +0100, David Henningsson wrote: > > On 2016-02-04 08:47, Alexander E. Patrakov wrote: > > 04.02.2016 10:45, David Henningsson пишет: > > > 6b) It seems non-trivial, and I have a gut feeling it will break some > > > other use case, that neither of us is thinking about right now. > > > > Based on our past experience here, I agree. This same point can equally well be made for David's proposal, though. > > > 6c) I realize neither of 6a) or 6b) are particularly strong arguments > > > against actually fixing a problem... > > > > I think here you meant "trying to fix". > > > > I also think the real problem here is to convince others that you have > > indeed fixed the original problem :) so for me 6b is strong enough. > > > > Internal speakers toggle between "no" and "unknown" today; i e, when you > unplug your headphones, internal speakers go to "unknown" rather than > "yes". This somewhat indicates that speakers are now available, but you > did not make an active choice to use them. Here's a plausible use case that will break in a very annoying way with your proposal and doesn't break with my proposal: let's say that the user prefers to use headphones when they are plugged in, and the internal speakers when the headphones are not plugged in, and never the HDMI output. Now whenever headphones are disconnected, the routing logic that you propose will choose the HDMI output, because it has higher priority than the internal speakers. With my proposal the system remembers that the internal speakers are the preferred output port of the sound card (which is actually a misrepresentation of the user's intent, because the user prefers the headphones over the internal speakers, but luckily that issue is masked by the speakers becoming unavailable when the headphones are plugged in). -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] RFC: Switch to HDMI or not?
On Fri, 2016-02-05 at 08:51 +0100, David Henningsson wrote: > > On 2016-02-04 13:54, David Henningsson wrote: > > > > > > On 2016-02-04 12:50, Tanu Kaskinen wrote: > > > On Thu, 2016-02-04 at 11:35 +0100, David Henningsson wrote: > > > > > > > > On 2016-02-04 08:47, Alexander E. Patrakov wrote: > > > > > 04.02.2016 10:45, David Henningsson пишет: > > > > > > 6b) It seems non-trivial, and I have a gut feeling it will break > > > > > > some > > > > > > other use case, that neither of us is thinking about right now. > > > > > > > > > > Based on our past experience here, I agree. > > > > > > This same point can equally well be made for David's proposal, though. > > Indeed. I was thinking my risk was lower because it only dealt with > HDMI. But maybe that was wrong... > > Anyhow; your proposal involves trying to save the last "user selected" > port; how do you determine if a port is user selected or not? (If the > port the user wants to select is already set just by selecting profile, > then there is no point for the GUI application to call pa_sink_set_port.) The logic goes as follows (transcribed from code I wrote yesterday) (this description is just for output ports, but it works the same way for input ports): If the profile change was not made by the user, do nothing. If the profile change didn't affect output, do nothing. If the number of sinks on the new profile is not 1, unset the preferred output port. Rationale: if the number is higher than 1, we don't know which sink the user prefers the most. If the number is 0, the user doesn't seem to care about output at all. If the new profile contains sources and they are not the same as with the old profile, we don't know if the user wanted to activate the new sink or the new source, so unset the preferred output port. If we've got this far, we know the user wanted to activate the sink on the new profile. The user may or may not have wanted to activate the port that got selected by default for the new sink, but in any case we set the preferred port to the currently active port. The reason why we can do this is that the user will surely manually change the port if she isn't happy with the default, and when the port change occurs, we update the preferred port at that time. > > > > > > 6c) I realize neither of 6a) or 6b) are particularly strong > > > > > > arguments > > > > > > against actually fixing a problem... > > > > > > > > > > I think here you meant "trying to fix". > > > > > > > > > > I also think the real problem here is to convince others that you have > > > > > indeed fixed the original problem :) so for me 6b is strong enough. > > > > > > > > > > > > > Internal speakers toggle between "no" and "unknown" today; i e, when you > > > > unplug your headphones, internal speakers go to "unknown" rather than > > > > "yes". This somewhat indicates that speakers are now available, but you > > > > did not make an active choice to use them. > > > > > > Here's a plausible use case that will break in a very annoying way with > > > your proposal and doesn't break with my proposal: let's say that the > > > user prefers to use headphones when they are plugged in, and the > > > internal speakers when the headphones are not plugged in, and never the > > > HDMI output. Now whenever headphones are disconnected, the routing > > > logic that you propose will choose the HDMI output, because it has > > > higher priority than the internal speakers. With my proposal the system > > > remembers that the internal speakers are the preferred output port of > > > the sound card (which is actually a misrepresentation of the user's > > > intent, because the user prefers the headphones over the internal > > > speakers, but luckily that issue is masked by the speakers becoming > > > unavailable when the headphones are plugged in). > > > > Okay, that makes sense. Scratch my proposal. > > On second thought; what if the module were to, in addition to setting > "unknown" instead of "yes", also would set its priority to 1? I guess you'd also reset the priority to the original value when the user activates the HDMI profile? I think that would work. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v3 15/24] echo-cancel: Use anonymous unions for echo canceller params
On Mon, 2016-01-18 at 13:06 +0530, a...@accosted.net wrote: > From: Arun Raghavan > > Makes this part of the code just a little less verbose. > --- > src/modules/echo-cancel/adrian.c | 18 +- > src/modules/echo-cancel/echo-cancel.h | 3 +- > src/modules/echo-cancel/null.c| 4 +-- > src/modules/echo-cancel/speex.c | 50 +-- > src/modules/echo-cancel/webrtc.cc | 64 > +-- > 5 files changed, 70 insertions(+), 69 deletions(-) Looks good to me. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v3 16/24] echo-cancel: Make webrtc AGC start volume a modarg
On Mon, 2016-01-18 at 13:06 +0530, a...@accosted.net wrote: > From: Arun Raghavan > > Allows for tuning based on the target hardware. > --- > src/modules/echo-cancel/webrtc.cc | 19 --- > 1 file changed, 16 insertions(+), 3 deletions(-) Looks good to me. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
[pulseaudio-discuss] [PATCH] default.pa: remove X11 module examples
Loading X stuff from default.pa is a bad idea, since it doesn't work if there are multiple X sessions, or PulseAudio is started outside the X session. Since it's a bad idea, don't encourage it by including examples that do so. I also removed the sample loading examples. I don't think the examples are particularly useful, since nothing uses the loaded samples once module-x11-bell is removed from the configuration. BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=93109 --- src/daemon/default.pa.in | 31 --- 1 file changed, 31 deletions(-) diff --git a/src/daemon/default.pa.in b/src/daemon/default.pa.in index 783e326..7a68653 100755 --- a/src/daemon/default.pa.in +++ b/src/daemon/default.pa.in @@ -19,19 +19,6 @@ # (i.e. not in system mode) changequote(`[', `]')dnl Set up m4 quoting -.nofail - -### Load something into the sample cache -ifelse(@OS_IS_WIN32@, 1, [dnl -load-sample x11-bell %WINDIR%\Media\ding.wav -load-sample-dir-lazy %WINDIR%\Media\*.wav -], [dnl -#load-sample-lazy x11-bell /usr/share/sounds/freedesktop/stereo/bell.oga -#load-sample-lazy pulse-hotplug /usr/share/sounds/freedesktop/stereo/device-added.oga -#load-sample-lazy pulse-coldplug /usr/share/sounds/freedesktop/stereo/device-added.oga -#load-sample-lazy pulse-access /usr/share/sounds/freedesktop/stereo/message.oga -])dnl - .fail ### Automatically restore the volume of streams and devices @@ -174,24 +161,6 @@ load-module module-filter-heuristics load-module module-filter-apply ])dnl -ifelse(@HAVE_X11@, 1, [dnl -# X11 modules should not be started from default.pa so that one daemon -# can be shared by multiple sessions. - -### Load X11 bell module -#load-module module-x11-bell sample=x11-bell - -### Register ourselves in the X11 session manager -#load-module module-x11-xsmp - -### Publish connection data in the X11 root window -#.ifexists module-x11-publish@PA_SOEXT@ -#.nofail -#load-module module-x11-publish -#.fail -#.endif -])dnl - ### Make some devices default #set-default-sink output #set-default-source input -- 2.7.0 ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] Pulseaudio 8/Bluez5 fails to load module-bluez5-device when running in system-wide mode
On Sat, 2016-02-06 at 22:13 +, Vlad Nikolyukin wrote: > I'm running Jessie on Raspberry Pi2. I'm trying to install Pulseaudio 8 > from the source, but having problems running it in system mode with Bluez5 > with A2DP support. > > On startup in system-wide mode, it fails with `"module-bluez5-discover.c: > Failed to load module for device /org/bluez/hci0/dev_AC_CF_85_23_8C_78"` > error when I connect my phone to bluetooth. > > If I simply restart pulseaudio under normal user account (pi) using: > > pulseaudio - > > and connecting the phone, everything works fine and I can play music via > A2DP from the phone. > > Pulseaudio is setup to run as systemd service in > `/etc/systemd/system/pulseaudio.service`: > > /usr/local/bin/pulseaudio --system --disallow-exit --disable-shm > --exit-idle-time=-1 --disallow-module-loading --disallow-module-loading is probably the culprit. It prevents any modules from loading after the daemon initialization. (I think it should be changed to only prevent users from loading modules.) -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v3 17/24] echo-cancel: Fix webrtc canceller when rec channels != play channels
On Mon, 2016-01-18 at 13:06 +0530, a...@accosted.net wrote:
> From: Arun Raghavan
>
> The calculations around how many samples were sent to the canceller
> engine was not updated when we started supporting different channel
> counts for playback and capture.
> ---
> src/modules/echo-cancel/echo-cancel.h | 4 ++--
> src/modules/echo-cancel/webrtc.cc | 25 +
> 2 files changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/src/modules/echo-cancel/echo-cancel.h
> b/src/modules/echo-cancel/echo-cancel.h
> index 37f99c0..4693516 100644
> --- a/src/modules/echo-cancel/echo-cancel.h
> +++ b/src/modules/echo-cancel/echo-cancel.h
> @@ -64,8 +64,8 @@ struct pa_echo_canceller_params {
> /* This is a void* so that we don't have to convert this whole
> file
> * to C++ linkage. apm is a pointer to an AudioProcessing object
> */
> void *apm;
> -uint32_t blocksize;
> -pa_sample_spec sample_spec;
> +int32_t blocksize; /* in frames */
Why is the type changed from unsigned to signed? It doesn't look like
you need negative values.
> +pa_sample_spec rec_ss, play_ss;
> bool agc;
> bool trace;
> bool first;
> diff --git a/src/modules/echo-cancel/webrtc.cc
> b/src/modules/echo-cancel/webrtc.cc
> index ec0a383..2732b38 100644
> --- a/src/modules/echo-cancel/webrtc.cc
> +++ b/src/modules/echo-cancel/webrtc.cc
> @@ -327,9 +327,11 @@ bool pa_webrtc_ec_init(pa_core *c, pa_echo_canceller *ec,
> apm->voice_detection()->Enable(true);
>
> ec->params.webrtc.apm = apm;
> -ec->params.webrtc.sample_spec = *out_ss;
> -ec->params.webrtc.blocksize = (uint64_t)pa_bytes_per_second(out_ss) *
> BLOCK_SIZE_US / PA_USEC_PER_SEC;
> -*nframes = ec->params.webrtc.blocksize / pa_frame_size(out_ss);
> +ec->params.webrtc.rec_ss = *rec_ss;
> +ec->params.webrtc.play_ss = *play_ss;
> +ec->params.webrtc.blocksize =
> +(uint64_t) (pa_bytes_per_second(out_ss) / pa_frame_size(out_ss)) *
> BLOCK_SIZE_US / PA_USEC_PER_SEC;
pa_bytes_per_second(out_ss) / pa_frame_size(out_ss) calculates the
sample rate, so it can be replaced with out_ss->rate.
--
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 4/5] rtpoll: Implement pa_mainloop_api support
On Tue, 2016-02-09 at 15:47 +0530, Arun Raghavan wrote:
> On Fri, 2015-01-02 at 15:04 +0200, Tanu Kaskinen wrote:
> > The new tunnel sink and source use libpulse to talk to the remote
> > server, and libpulse requires a pa_mainloop_api implementation for
> > interacting with the event loop. The tunnel sink and source have so
> > far been using pa_mainloop, but there are some modules that assume
> > that all sinks and sources use pa_rtpoll in their IO threads, and
> > trying to use those modules together with the pa_mainloop based
> > tunnel sinks and sources will result in crashes (see [1]).
> >
> > I will switch the event loop implementation in the tunnel modules
> > to pa_rtpoll, but that requires a pa_mainloop_api implementation in
> > pa_rtpoll first - that's implemented here.
> >
> > pa_rtpoll_run() is changed so that it processes all defer events
> > first, and then all expired time events. The rest of pa_rtpoll_run()
> > works as before, except the poll timeout calculation now has to also
> > take the time events into account. IO events use the existing
> > pa_rtpoll_item interface.
> >
> > The time events are handled separately from the old timer
> > functionality, which is somewhat ugly. It might be a good idea to
> > remove the old timer functionality and only use the time events.
> > I didn't attempt to do that at this time, because I feared that
> > adapting the pa_rtpoll users to the new system would be difficult.
> >
> > [1] https://bugs.freedesktop.org/show_bug.cgi?id=73429
> > ---
>
> As a first comment, I'd like to ask that there be a test for this. It
> should be quite easy to refactor mainloop-test to run the same set of
> tests on different mainloop implementations.
Ok.
> > src/pulsecore/rtpoll.c | 504
> > -
> > src/pulsecore/rtpoll.h | 4 +
> > 2 files changed, 500 insertions(+), 8 deletions(-)
> >
> > diff --git a/src/pulsecore/rtpoll.c b/src/pulsecore/rtpoll.c
> > index 5f3ca8b..2e1907c 100644
> > --- a/src/pulsecore/rtpoll.c
> > +++ b/src/pulsecore/rtpoll.c
> > @@ -32,6 +32,7 @@
> > #include
> > #include
> >
> > +#include
> > #include
> > #include
> > #include
> > @@ -65,6 +66,18 @@ struct pa_rtpoll {
> > #endif
> >
> > PA_LLIST_HEAD(pa_rtpoll_item, items);
> > +
> > +pa_mainloop_api mainloop_api;
> > +
> > +pa_dynarray *io_events;
> > +
> > +pa_dynarray *time_events;
> > +pa_dynarray *enabled_time_events;
> > +pa_dynarray *expired_time_events;
> > +pa_time_event *cached_next_time_event;
> > +
> > +pa_dynarray *defer_events;
> > +pa_dynarray *enabled_defer_events;
> > };
> >
> > struct pa_rtpoll_item {
> > @@ -84,8 +97,321 @@ struct pa_rtpoll_item {
> > PA_LLIST_FIELDS(pa_rtpoll_item);
> > };
> >
> > +struct pa_io_event {
> > +pa_rtpoll *rtpoll;
> > +pa_rtpoll_item *rtpoll_item;
> > +pa_io_event_flags_t events;
> > +pa_io_event_cb_t callback;
> > +pa_io_event_destroy_cb_t destroy_callback;
> > +void *userdata;
> > +};
> > +
> > +static void io_event_enable(pa_io_event *event, pa_io_event_flags_t
> > events);
> > +
> > +struct pa_time_event {
> > +pa_rtpoll *rtpoll;
> > +pa_usec_t time;
> > +bool use_rtclock;
> > +bool enabled;
> > +pa_time_event_cb_t callback;
> > +pa_time_event_destroy_cb_t destroy_callback;
> > +void *userdata;
> > +};
> > +
> > +static void time_event_restart(pa_time_event *event, const struct
> > timeval *tv);
> > +
> > +struct pa_defer_event {
> > +pa_rtpoll *rtpoll;
> > +bool enabled;
> > +pa_defer_event_cb_t callback;
> > +pa_defer_event_destroy_cb_t destroy_callback;
> > +void *userdata;
> > +};
> > +
> > +static void defer_event_enable(pa_defer_event *event, int enable);
> > +
> > PA_STATIC_FLIST_DECLARE(items, 0, pa_xfree);
> >
> > +static short map_flags_to_libc(pa_io_event_flags_t flags) {
> > +return (short)
> > +((flags & PA_IO_EVENT_INPUT ? POLLIN : 0) |
> > + (flags & PA_IO_EVENT_OUTPUT ? POLLOUT : 0) |
> > + (flags & PA_IO_EVENT_ERROR ? POLLERR : 0) |
> > + (flags & PA_IO_EVENT_HANGUP ? POLLHUP : 0));
> > +}
> > +
> > +static pa_io_event_flags_t map_flags_from_libc(short flags) {
> > +
Re: [pulseaudio-discuss] [PATCH v3 18/24] echo-cancel: Improve webrtc canceller error handling a bit
On Mon, 2016-01-18 at 13:06 +0530, a...@accosted.net wrote:
> From: Arun Raghavan
>
> ---
> src/modules/echo-cancel/webrtc.cc | 14 ++
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/src/modules/echo-cancel/webrtc.cc
> b/src/modules/echo-cancel/webrtc.cc
> index 2732b38..5741f45 100644
> --- a/src/modules/echo-cancel/webrtc.cc
> +++ b/src/modules/echo-cancel/webrtc.cc
> @@ -284,7 +284,10 @@ bool pa_webrtc_ec_init(pa_core *c, pa_echo_canceller *ec,
> webrtc::StreamConfig(play_ss->rate, play_ss->channels, false), /*
> reverse input stream */
> webrtc::StreamConfig(play_ss->rate, play_ss->channels, false), /*
> reverse output stream */
> };
> -apm->Initialize(pconfig);
> +if (apm->Initialize(pconfig) != webrtc::AudioProcessing::kNoError) {
> +pa_log("Error initialising audio processing module");
> +goto fail;
> +}
>
> if (hpf)
> apm->high_pass_filter()->Enable(true);
> @@ -313,7 +316,8 @@ bool pa_webrtc_ec_init(pa_core *c, pa_echo_canceller *ec,
> ec->params.webrtc.agc = false;
> } else {
>
> apm->gain_control()->set_mode(webrtc::GainControl::kAdaptiveAnalog);
> -if (apm->gain_control()->set_analog_level_limits(0,
> WEBRTC_AGC_MAX_VOLUME) != apm->kNoError) {
> +if (apm->gain_control()->set_analog_level_limits(0,
> WEBRTC_AGC_MAX_VOLUME) !=
> +webrtc::AudioProcessing::kNoError) {
> pa_log("Failed to initialise AGC");
> goto fail;
> }
> @@ -361,7 +365,8 @@ void pa_webrtc_ec_play(pa_echo_canceller *ec, const
> uint8_t *play) {
> pa_assert(play_frame.samples_per_channel_ <=
> webrtc::AudioFrame::kMaxDataSizeSamples);
> memcpy(play_frame.data_, play, ec->params.webrtc.blocksize *
> pa_frame_size(ss));
>
> -apm->ProcessReverseStream(&play_frame);
> +if (apm->ProcessReverseStream(&play_frame) !=
> webrtc::AudioProcessing::kNoError)
> +pa_log("Failed to process playback stream");
> }
>
> void pa_webrtc_ec_record(pa_echo_canceller *ec, const uint8_t *rec, uint8_t
> *out) {
> @@ -387,7 +392,8 @@ void pa_webrtc_ec_record(pa_echo_canceller *ec, const
> uint8_t *rec, uint8_t *out
> }
>
> apm->set_stream_delay_ms(0);
> -apm->ProcessStream(&out_frame);
> +if (apm->ProcessStream(&out_frame) != webrtc::AudioProcessing::kNoError)
> +pa_log("Failed to process capture stream");
I don't like error messages that can spam the syslog. Would a hard
failure be acceptable here (that is, unload the module, or change to a
failure state where the module stays around but does no processing)?
--
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v3 18/24] echo-cancel: Improve webrtc canceller error handling a bit
On Wed, 2016-02-10 at 09:56 +0530, Arun Raghavan wrote:
> On 9 February 2016 at 23:41, Tanu Kaskinen wrote:
> > On Mon, 2016-01-18 at 13:06 +0530, a...@accosted.net wrote:
> > > @@ -387,7 +392,8 @@ void pa_webrtc_ec_record(pa_echo_canceller *ec, const
> > > uint8_t *rec, uint8_t *out
> > > }
> > >
> > > apm->set_stream_delay_ms(0);
> > > -apm->ProcessStream(&out_frame);
> > > +if (apm->ProcessStream(&out_frame) !=
> > > webrtc::AudioProcessing::kNoError)
> > > +pa_log("Failed to process capture stream");
> >
> > I don't like error messages that can spam the syslog. Would a hard
> > failure be acceptable here (that is, unload the module, or change to a
> > failure state where the module stays around but does no processing)?
>
> This particular case is extremely low probability, so I'd like to not
> change it if possible. I don't think the extra code to add a noop mode
> is justified since we're very unlikely to hit this.
>
> Maybe we could/should change the run/play/record vmethods to return an
> error code, but I'd prefer to do that separately if it's done.
So what are the extremely rare conditions where this error is
triggered? I'll assume that this error doesn't indicate a bug in our
code or in the webrtc library, and can really happen, so an assertion
is not appropriate here.
Adding a noop mode was just one idea, and I understand if that feels
overkill. But what about unloading the module? Other ideas: lower the
message level to info or debug, or prevent the error message being
printed more than once.
Flooding syslog is never a good error handling strategy. This looks
like if there is one error, there will probably be thousand.
--
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v3 19/24] echo-cancel: webrtc canceller supports different in/out channel counts
On Mon, 2016-01-18 at 13:06 +0530, a...@accosted.net wrote:
> From: Arun Raghavan
>
> Needed for upcoming beamforming code.
> ---
> src/modules/echo-cancel/echo-cancel.h | 2 +-
> src/modules/echo-cancel/webrtc.cc | 14 --
> 2 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/src/modules/echo-cancel/echo-cancel.h
> b/src/modules/echo-cancel/echo-cancel.h
> index 4693516..613f7e3 100644
> --- a/src/modules/echo-cancel/echo-cancel.h
> +++ b/src/modules/echo-cancel/echo-cancel.h
> @@ -65,7 +65,7 @@ struct pa_echo_canceller_params {
> * to C++ linkage. apm is a pointer to an AudioProcessing object
> */
> void *apm;
> int32_t blocksize; /* in frames */
> -pa_sample_spec rec_ss, play_ss;
> +pa_sample_spec rec_ss, play_ss, out_ss;
> bool agc;
> bool trace;
> bool first;
> diff --git a/src/modules/echo-cancel/webrtc.cc
> b/src/modules/echo-cancel/webrtc.cc
> index 5741f45..5f00286 100644
> --- a/src/modules/echo-cancel/webrtc.cc
> +++ b/src/modules/echo-cancel/webrtc.cc
> @@ -333,6 +333,7 @@ bool pa_webrtc_ec_init(pa_core *c, pa_echo_canceller *ec,
> ec->params.webrtc.apm = apm;
> ec->params.webrtc.rec_ss = *rec_ss;
> ec->params.webrtc.play_ss = *play_ss;
> +ec->params.webrtc.out_ss = *out_ss;
> ec->params.webrtc.blocksize =
> (uint64_t) (pa_bytes_per_second(out_ss) / pa_frame_size(out_ss)) *
> BLOCK_SIZE_US / PA_USEC_PER_SEC;
> *nframes = ec->params.webrtc.blocksize;
> @@ -372,17 +373,18 @@ void pa_webrtc_ec_play(pa_echo_canceller *ec, const
> uint8_t *play) {
> void pa_webrtc_ec_record(pa_echo_canceller *ec, const uint8_t *rec, uint8_t
> *out) {
> webrtc::AudioProcessing *apm =
> (webrtc::AudioProcessing*)ec->params.webrtc.apm;
> webrtc::AudioFrame out_frame;
> -const pa_sample_spec *ss = &ec->params.webrtc.rec_ss;
> +const pa_sample_spec *rec_ss = &ec->params.webrtc.rec_ss;
> +const pa_sample_spec *out_ss = &ec->params.webrtc.out_ss;
> pa_cvolume v;
> int old_volume, new_volume;
>
> -out_frame.num_channels_ = ss->channels;
> -out_frame.sample_rate_hz_ = ss->rate;
> +out_frame.num_channels_ = rec_ss->channels;
> +out_frame.sample_rate_hz_ = rec_ss->rate;
> out_frame.interleaved_ = true;
> out_frame.samples_per_channel_ = ec->params.webrtc.blocksize;
>
> pa_assert(out_frame.samples_per_channel_ <=
> webrtc::AudioFrame::kMaxDataSizeSamples);
Not directly related to this patch, but this assertion seems wrong. We
compare kMaxDataSizeSamples to the number of frames, but
kMaxDataSizeSamples is calculated as frames * channels.
Also, kMaxDataSizeSamples assumes that the maximum sample spec is 16-
bit, stereo, 32 kHz, and that the maximum buffer size is 60 ms.
However, we seem to allow using 48 kHz, and I didn't find any
limitations for the channel count. We seem to use 10 ms buffers, so
that gives some headroom. webrtc_ec_fixate_spec() should check all
this, and make sure that our parameters don't exceed
kMaxDataSizeSamples.
The changes in this patch all look good.
--
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
