On Fri, 17 Feb 2017 18:44:47 +0100, Alan Young wrote:
On 17/02/17 16:53, Takashi Iwai wrote:
The problem is that you pass always the compound object as the converter argument. As you already suggested, there are two cases: one is a string array and another is a compound with optional args. In your code, the first iteration doesn't check which type is. So it always fails if the string array is passed.
Well, it does actually work in both cases but I guess that the plugin could get passed an unexpected type of compound config converter parameter in some cases.
That is, the right implementation is to check whether the given compound is a string array is. If yes, it goes to the old style loop (and you can check "name" argument properly). If not, it goes with the new compound argument. That's simple enough, and more understandable the condition you used for the loop termination.
The following makes the difference more explicit What do you think?
else if (snd_config_get_type(converter) == SND_CONFIG_TYPE_COMPOUND) { snd_config_iterator_t i, next; int pos = 0, is_array = 0; /* * If the convertor compound is an array of alternatives thenthe 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_for_each(i, next, converter) { snd_config_t *n = snd_config_iterator_entry(i); const char *id; snd_config_get_id(n, &id); if (pos++ == 0 && (!id || strcmp(id, "0") == 0)) is_array = 1; if (!is_array && strcmp(id, "name") != 0) continue; if (snd_config_get_string(n, &type) < 0) break; err = rate_open_func(rate, type, is_array ? NULL : converter, 0); if (!err || !is_array) break; }
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 }
Takashi