Re: [PATCH] crec: Add option to specify codec ID

[Richard Fitzgerald <rf@opensource.wolfsonmicro.com>]

On Wed, 2016-11-16 at 18:35 +0530, Vinod Koul wrote:
> On Wed, Nov 16, 2016 at 11:44:09AM +0000, Richard Fitzgerald wrote:
> > This patch adds a -I command line option to set the codec ID,
> > either from a defined set of string values or as a number.
> 
> Can you explain why you want to add this? The utility cant really record a
> mp3 file!
> 
> 

You need to be able to pass a codec ID that the driver supports, and to
indicate which codec you're trying to use. It's not useful to only be
able to open the "PCM" codec. It doesn't really matter whether crec
understands the content of the data, we're just pulling raw data, most
likely for test/debug.

The wm_adsp driver on Wolfson/Cirrus codecs uses the new BESPOKE stream
ID so we need a way to pass that. And we'd also need it for any drivers
that had streams using other codec IDs.

> > 
> > Signed-off-by: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
> > ---
> >  src/utils/crec.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++------
> >  1 file changed, 59 insertions(+), 7 deletions(-)
> > 
> > diff --git a/src/utils/crec.c b/src/utils/crec.c
> > index 8d5b7b0..a586fc4 100644
> > --- a/src/utils/crec.c
> > +++ b/src/utils/crec.c
> > @@ -83,6 +83,27 @@ static bool streamed;
> >  static const unsigned int DEFAULT_CHANNELS = 1;
> >  static const unsigned int DEFAULT_RATE = 44100;
> >  static const unsigned int DEFAULT_FORMAT = SNDRV_PCM_FORMAT_S16_LE;
> > +static const unsigned int DEFAULT_CODEC_ID = SND_AUDIOCODEC_PCM;
> > +
> > +static const struct {
> > +	const char *name;
> > +	unsigned int id;
> > +} codec_ids[] = {
> > +	{ "PCM", SND_AUDIOCODEC_PCM },
> > +	{ "MP3", SND_AUDIOCODEC_MP3 },
> > +	{ "AMR", SND_AUDIOCODEC_AMR },
> > +	{ "AMRWB", SND_AUDIOCODEC_AMRWB },
> > +	{ "ARMWBPLUS", SND_AUDIOCODEC_AMRWBPLUS },
> > +	{ "AAC", SND_AUDIOCODEC_AAC },
> > +	{ "WMA", SND_AUDIOCODEC_WMA },
> > +	{ "REAL", SND_AUDIOCODEC_REAL },
> > +	{ "VORBIS", SND_AUDIOCODEC_VORBIS },
> > +	{ "FLAC", SND_AUDIOCODEC_FLAC },
> > +	{ "IEC61937", SND_AUDIOCODEC_IEC61937 },
> > +	{ "G723_1", SND_AUDIOCODEC_G723_1 },
> > +	{ "G729", SND_AUDIOCODEC_G729 },
> > +};
> > +#define CREC_NUM_CODEC_IDS (sizeof(codec_ids) / sizeof(codec_ids[0]))
> >  
> >  struct riff_chunk {
> >  	char desc[4];
> > @@ -153,6 +174,8 @@ static void size_wave_header(struct wave_header *header, uint32_t size)
> >  
> >  static void usage(void)
> >  {
> > +	int i;
> > +
> >  	fprintf(stderr, "usage: crec [OPTIONS] [filename]\n"
> >  		"-c\tcard number\n"
> >  		"-d\tdevice node\n"
> > @@ -163,14 +186,22 @@ static void usage(void)
> >  		"-h\tPrints this help list\n\n"
> >  		"-C\tSpecify the number of channels (default %u)\n"
> >  		"-R\tSpecify the sample rate (default %u)\n"
> > -		"-F\tSpecify the format: S16_LE, S32_LE (default S16_LE)\n\n"
> > +		"-F\tSpecify the format: S16_LE, S32_LE (default S16_LE)\n"
> > +		"-I\tSpecify codec ID (default PCM)\n\n"
> >  		"If filename is not given the output is\n"
> >  		"written to stdout\n\n"
> >  		"Example:\n"
> >  		"\tcrec -c 1 -d 2 test.wav\n"
> > -		"\tcrec -f 5 test.wav\n",
> > +		"\tcrec -f 5 test.wav\n\n"
> > +		"Valid codec IDs:\n",
> >  		DEFAULT_CHANNELS, DEFAULT_RATE);
> >  
> > +	for (i = 0; i < CREC_NUM_CODEC_IDS; ++i)
> > +		fprintf(stderr, "%s%c", codec_ids[i].name,
> > +			(i % 8) ? ' ' : '\n');
> > +
> > +	fprintf(stderr, "\nor the value in decimal or hex\n");
> > +
> >  	exit(EXIT_FAILURE);
> >  }
> >  
> > @@ -239,7 +270,8 @@ static int finish_record(void)
> >  static void capture_samples(char *name, unsigned int card, unsigned int device,
> >  		     unsigned long buffer_size, unsigned int frag,
> >  		     unsigned int length, unsigned int rate,
> > -		     unsigned int channels, unsigned int format)
> > +		     unsigned int channels, unsigned int format,
> > +		     unsigned int codec_id)
> >  {
> >  	struct compr_config config;
> >  	struct snd_codec codec;
> > @@ -288,7 +320,7 @@ static void capture_samples(char *name, unsigned int card, unsigned int device,
> >  
> >  	memset(&codec, 0, sizeof(codec));
> >  	memset(&config, 0, sizeof(config));
> > -	codec.id = SND_AUDIOCODEC_PCM;
> > +	codec.id = codec_id;
> 
> 
> So we are going to dump raw encoded data. I am not sure thats a smart
> choice.. We should really support format headers and save proper files
> 
>