Hi Takashi,
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Friday, October 2, 2015 6:56 PM To: Lu, Han Cc: Girdwood, Liam R; Gautier, Bernard; caleb@crome.org; alsa-devel@alsa- project.org Subject: Re: [PATCH V2 1/7] BAT: Add initial functions
On Wed, 23 Sep 2015 09:48:49 +0200, han.lu@intel.com wrote:
From: "Lu, Han" han.lu@intel.com
Add main entrance, command line parsing, parameter initiating and thread initiating functions for BAT.
Signed-off-by: Lu, Han han.lu@intel.com Signed-off-by: Liam Girdwood liam.r.girdwood@intel.com Signed-off-by: Bernard Gautier bernard.gautier@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.
[Han] may I just change the strings to " duration overflow ..." " duration underflow ..." " duration overflow ..." " duration underflow ..." " invalid duration ..." " duration out of range ..." Or change the sequence and keep 2 or 3 strings like: "duration overflow/underflow ..." "invalid duration ..." "duration out of range ..." Since they would be clearer to developer/user?
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?
[Han] OK, I'll check the implementation.
+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.
[Han] OK. I followed the indentation style of aplay, to align with multiple-character options such as "--log=#". Shall I remove indentation for these options?
Takashi
BR, Han Lu