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 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_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; }
BTW, one another thing:
@@ -1386,7 +1414,7 @@ int snd_pcm_rate_open(snd_pcm_t **pcmp, const char *name, #else type = "linear"; open_func = SND_PCM_RATE_PLUGIN_ENTRY(linear);
- err = open_func(SND_PCM_RATE_PLUGIN_VERSION, &rate->obj, &rate->ops);
- err = open_func(SND_PCM_RATE_PLUGIN_VERSION, &rate->obj, &rate->ops, 0);
Is this really correct? I thought SND_PCM_RATE_PLUGIN_ENTRY() refers to the old style?
Yes, left over from earlier version where I just changed the old func's signature.
Alan.