Re: [PATCH] [updated] pcm: rate: Add capability to pass configuration node to plugins

[Takashi Iwai <tiwai@suse.de>]

On Tue, 21 Feb 2017 13:02:49 +0100,
Alan Young wrote:
> 
> On 17/02/17 17:59, Takashi Iwai wrote:
> > It becomes too complex by mixing up things in a single loop.
> > Let's take it just simple like:
> >
> > 	if (is_string_array(converter)) {
> > 		snd_config_for_each(i, next, converter) {
> > 			// like the old way
> > 		}
> > 	} else {
> > 		// handle for the new compound type
> > 	}
> 
> 
> Like this?
> 
> 	else if (snd_config_get_type(converter) == SND_CONFIG_TYPE_COMPOUND) {
> 		/*
> 		 * If the convertor compound is an array of alternatives then the id of
> 		 * the first element will be "0" (or maybe NULL). Otherwise assume it is
> 		 * a structure and must have a "name" id to identify the converter type.
> 		 */
> 		snd_config_iterator_t i, next;
> 		i = snd_config_iterator_first(converter);
> 		if (i && i != snd_config_iterator_end(converter)) {
> 			snd_config_t *n = snd_config_iterator_entry(i);
> 			const char *id;
> 			snd_config_get_id(n, &id);
> 			if (!id || strcmp(id, "0") == 0) {

Make this a function, e.g. is_string_array(), as I mentioned.


> 				snd_config_for_each(i, next, converter) {
> 					n = snd_config_iterator_entry(i);
> 					if (snd_config_get_string(n, &type) < 0)
> 						break;
> 					err = rate_open_func(rate, type, NULL, 0);
> 					if (!err)
> 						break;
> 				}

> 			} else {
> 				snd_config_for_each(i, next, converter) {
> 					snd_config_t *n = snd_config_iterator_entry(i);
> 					const char *id;
> 					snd_config_get_id(n, &id);
> 					if (strcmp(id, "name") != 0)
> 						continue;
> 					if (snd_config_get_string(n, &type) < 0)
> 						break;
> 					err = rate_open_func(rate, type, converter, 0);
> 					if (!err)
> 						break;
> 				}

In the second case, calling the open function inside the loop makes
the code unclear.  The loop is only for obtaining the name string.
The open func should be called outside the loop once when you get the
name string (or bail out as an error if no name is found beforehand).


thanks,

Takashi