[alsa-devel] [PATCH V2 1/7] BAT: Add initial functions

[Takashi Iwai]

On Wed, 23 Sep 2015 09:48:49 +0200,
han.lu at intel.com wrote:
> 
> From: "Lu, Han" <han.lu at intel.com>
> 
> Add main entrance, command line parsing, parameter initiating and
> thread initiating functions for BAT.
> 
> Signed-off-by: Lu, Han <han.lu at intel.com>
> Signed-off-by: Liam Girdwood <liam.r.girdwood at intel.com>
> Signed-off-by: Bernard Gautier <bernard.gautier at intel.com>
> 
> diff --git a/bat/bat.c b/bat/bat.c
> new file mode 100644
> index 0000000..54fe1ec
> --- /dev/null
> +++ b/bat/bat.c
> @@ -0,0 +1,603 @@
> +/*
> + * Copyright (C) 2013-2015 Intel Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program 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.
> + *
> + */
> +
> +#include <unistd.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <stdbool.h>
> +#include <string.h>
> +#include <errno.h>
> +#include <pthread.h>
> +#include <getopt.h>
> +#include <math.h>
> +#include <limits.h>
> +
> +#include "aconfig.h"
> +#include "gettext.h"
> +#include "version.h"
> +
> +#include "common.h"
> +
> +#include "alsa.h"
> +#include "convert.h"
> +#include "analyze.h"
> +
> +static int get_duration(struct bat *bat)
> +{
> +	float duration_f;
> +	long duration_i;
> +	char *ptrf, *ptri;
> +
> +	duration_f = strtof(bat->narg, &ptrf);
> +	if (duration_f == HUGE_VALF || duration_f == -HUGE_VALF) {
> +		fprintf(bat->err, _("duration float overflow: %f %d\n"),
> +				duration_f, -errno);
> +		return -errno;
> +	} else if (duration_f == 0.0 && errno != 0) {
> +		fprintf(bat->err, _("duration float underflow: %f %d\n"),
> +				duration_f, -errno);
> +		return -errno;
> +	}
> +
> +	duration_i = strtol(bat->narg, &ptri, 10);
> +	if (duration_i == LONG_MAX) {
> +		fprintf(bat->err, _("duration long overflow: %ld %d\n"),
> +				duration_i, -errno);
> +		return -errno;
> +	} else if (duration_i == LONG_MIN) {
> +		fprintf(bat->err, _("duration long underflow: %ld %d\n"),
> +				duration_i, -errno);
> +		return -errno;
> +	}
> +
> +	if (*ptrf == 's') {
> +		bat->frames = duration_f * bat->rate;
> +	} else if (*ptri == 0) {
> +		bat->frames = duration_i;
> +	} else {
> +		fprintf(bat->err, _("invalid duration: %s\n"), bat->narg);
> +		return -EINVAL;
> +	}
> +
> +	if (bat->frames <= 0 || bat->frames > MAX_FRAMES) {
> +		fprintf(bat->err, _("duration out of range: (0, %d(%ds))\n"),
> +				MAX_FRAMES, (bat->frames / bat->rate));

The messages strings could be unified to a single error message.
Otherwise you'll annoy translators, as they will need to work on each
different variant.

> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static void get_sine_frequencies(struct bat *bat, char *freq)
> +{
> +	char *tmp1;
> +
> +	tmp1 = strchr(freq, ',');
> +	if (tmp1 == NULL) {
> +		bat->target_freq[1] = bat->target_freq[0] = atof(optarg);
> +	} else {
> +		*tmp1 = '\0';
> +		bat->target_freq[0] = atof(optarg);
> +		bat->target_freq[1] = atof(tmp1 + 1);
> +	}

Note that parsing a floting point string is tricky.  For example, if
you run it in a locale that handles comma as the decimal point, this
gets messy.  Did you check it, e.g. with LANG=en_GB or such?


> +static void usage(struct bat *bat, char *argv[])
> +{
> +	fprintf(bat->log,
> +_("Usage:%s [Option]...\n"
> +"\n"
> +"-h, --help             help\n"
> +"-D                     sound card\n"
> +"-P                     playback pcm\n"
> +"-C                     capture pcm\n"
> +"-f                     sample size\n"
> +"-c                     number of channels\n"
> +"-r                     sampling rate\n"
> +"-n                     frames to capture\n"
> +"-k                     sigma k\n"
> +"-F                     target frequency\n"
> +"-p                     total number of periods to play/capture\n"
> +"    --log=#            path of log file. if not set, logs be put to stdout,\n"
> +"                       and errors be put to stderr.\n"
> +"    --file=#           input file\n"
> +"    --saveplay=#       save playback content to target file, for debug\n"
> +"    --local            internal loop, bypass hardware\n"

Why there are options with and without indentation?

> +), argv[0]);

The program name should be a const string.


Takashi