[pulseaudio-discuss] [PATCH 2/6] tests: Add a test for the stream volume API

[Tanu Kaskinen]

On Tue, 2015-12-29 at 09:03 +0530, arun at accosted.net wrote:
> From: Arun Raghavan <git at arunraghavan.net>
> 
> This currently only checks the stream volume API, but can be extended in
> the future to validate other bits of the API too.
> ---
>  src/Makefile.am      |   8 ++-
>  src/tests/api-test.c | 184 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 191 insertions(+), 1 deletion(-)
>  create mode 100644 src/tests/api-test.c
> 
> diff --git a/src/Makefile.am b/src/Makefile.am
> index b0ca2bc..6785ef2 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -286,7 +286,8 @@ TESTS_daemon = \
>  		connect-stress \
>  		extended-test \
>  		interpol-test \
> -		sync-playback
> +		sync-playback \
> +		api-test
>  
>  if !OS_IS_WIN32
>  TESTS_default += \
> @@ -563,6 +564,11 @@ lfe_filter_test_LDADD = $(AM_LDADD) libpulsecore- at PA_MAJORMINOR@.la libpulse.la
>  lfe_filter_test_CFLAGS = $(AM_CFLAGS) $(LIBCHECK_CFLAGS)
>  lfe_filter_test_LDFLAGS = $(AM_LDFLAGS) $(BINLDFLAGS) $(LIBCHECK_LIBS)
>  
> +api_test_SOURCES = tests/api-test.c
> +api_test_LDADD = $(AM_LDADD) libpulse.la libpulsecommon- at PA_MAJORMINOR@.la
> +api_test_CFLAGS = $(AM_CFLAGS) $(LIBCHECK_CFLAGS) $(libpulse_la_CFLAGS)

Why is libpulse_la_CFLAGS here? It's not used in any other similar
context.

> +api_test_LDFLAGS = $(AM_LDFLAGS) $(BINLDFLAGS) $(LIBCHECK_LIBS)
> +
>  rtstutter_SOURCES = tests/rtstutter.c
>  rtstutter_LDADD = $(AM_LDADD) libpulsecore- at PA_MAJORMINOR@.la libpulse.la libpulsecommon- at PA_MAJORMINOR@.la
>  rtstutter_CFLAGS = $(AM_CFLAGS)
> diff --git a/src/tests/api-test.c b/src/tests/api-test.c
> new file mode 100644
> index 0000000..5d75607
> --- /dev/null
> +++ b/src/tests/api-test.c
> @@ -0,0 +1,184 @@
> +/***
> +  This file is part of PulseAudio.
> +
> +  Copyright 2015 Arun Raghavan <mail at arunraghavan.net>
> +
> +  PulseAudio is free software; you can redistribute it and/or modify
> +  it under the terms of the GNU Lesser General Public License as published
> +  by the Free Software Foundation; either version 2.1 of the License,
> +  or (at your option) any later version.
> +
> +  PulseAudio is distributed in the hope that it will be useful, but
> +  WITHOUT ANY WARRANTY; without even the implied warranty of
> +  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +  General Public License for more details.
> +
> +  You should have received a copy of the GNU Lesser General Public License
> +  along with PulseAudio; if not, see <http://www.gnu.org/licenses/>.
> +***/
> +
> +#ifdef HAVE_CONFIG_H
> +#include 
> +#endif
> +
> +#include 
> +#include 
> +#include 
> +
> +#include <pulse/internal.h> /* for pa_context->mainloop */

Is this just to avoid defining a userdata struct for holding the
mainloop pointer? I'd prefer defining the userdata struct over
accessing private fields of pa_context.

> +#include <pulse/pulseaudio.h>
> +
> +#include 
> +
> +#define CHANNELS 2
> +
> +static void stream_volume_callback(pa_stream *s, int success, void *userdata) {
> +    pa_cvolume v;
> +    bool disconnect = PA_PTR_TO_UINT(userdata);
> +
> +    fail_unless(success == 1);
> +    fail_unless(pa_stream_get_volume(s, &v) == 0);

pa_stream_get_volume() is not defined.

> +
> +    if (disconnect) {
> +        fail_unless(pa_cvolume_avg(&v) == (PA_VOLUME_NORM / 3));
> +        pa_stream_disconnect(s);
> +    } else {
> +        fail_unless(pa_cvolume_avg(&v) == (PA_VOLUME_NORM / 2));
> +    }
> +}
> +
> +static void stream_state_callback(pa_stream *s, void *userdata) {
> +    pa_cvolume v;
> +
> +    fail_unless(s != NULL);
> +
> +    switch (pa_stream_get_state(s)) {
> +        case PA_STREAM_UNCONNECTED:
> +        case PA_STREAM_CREATING:
> +            break;
> +
> +        case PA_STREAM_TERMINATED:
> +            pa_stream_unref(s);
> +            pa_context_disconnect(pa_stream_get_context(s));
> +            break;
> +
> +        case PA_STREAM_READY:
> +            pa_cvolume_set(&v, CHANNELS, PA_VOLUME_NORM / 3);
> +            fail_unless(pa_stream_set_volume(s, &v, stream_volume_callback, PA_UINT_TO_PTR(true)) == 0);
> +            break;
> +
> +        default:
> +        case PA_STREAM_FAILED:
> +            fprintf(stderr, "Stream error: %s\n", pa_strerror(pa_context_errno(pa_stream_get_context(s))));
> +            fail();

Now that the echo-cancel patch set is merged, the fail() macro doesn't
work any more.

> +    }
> +}
> +
> +static void context_state_callback(pa_context *c, void *userdata) {
> +    pa_stream *s;
> +    pa_cvolume v;
> +    pa_sample_spec ss = {
> +        .format = PA_SAMPLE_S16NE,
> +        .rate = 44100,
> +        .channels = CHANNELS,
> +    };
> +    bool playback = PA_PTR_TO_UINT(userdata);
> +
> +    fail_unless(c != NULL);
> +
> +    switch (pa_context_get_state(c)) {
> +        case PA_CONTEXT_CONNECTING:
> +        case PA_CONTEXT_AUTHORIZING:
> +        case PA_CONTEXT_SETTING_NAME:
> +            break;
> +
> +        case PA_CONTEXT_READY:
> +            s = pa_stream_new(c, "api-test", &ss, NULL);
> +            fail_unless(s != NULL);
> +
> +            pa_cvolume_set(&v, CHANNELS, PA_VOLUME_NORM / 2);
> +            fail_unless(pa_stream_set_volume(s, &v, stream_volume_callback, PA_UINT_TO_PTR(false)) == 0);
> +
> +            pa_stream_set_state_callback(s, stream_state_callback, NULL);
> +            if (playback)
> +                fail_unless(pa_stream_connect_playback(s, NULL, NULL, PA_STREAM_START_CORKED, NULL, NULL) == 0);
> +            else
> +                fail_unless(pa_stream_connect_record(s, NULL, NULL, PA_STREAM_START_CORKED) == 0);
> +
> +            break;
> +
> +        case PA_CONTEXT_TERMINATED:
> +            c->mainloop->quit(c->mainloop, 0);
> +            break;
> +
> +        case PA_CONTEXT_FAILED:
> +        default:
> +            fprintf(stderr, "Context error: %s\n", pa_strerror(pa_context_errno(c)));
> +            fail();

I don't think this handler makes sense for the default case. If the
context state is something crazy, there's no reason to assume that
pa_context_errno() returns anything relevant.

-- 
Tanu