From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alan Young Subject: Re: [PATCH] [updated] pcm: rate: Add capability to pass configuration node to plugins Date: Fri, 17 Feb 2017 17:44:47 +0000 Message-ID: <7b3c655f-4b7a-e10a-90ff-5cb480332f3f@IEE.org> References: <6d7c74b7-da90-5ae9-c355-c6884b2edf09@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-wm0-f54.google.com (mail-wm0-f54.google.com [74.125.82.54]) by alsa0.perex.cz (Postfix) with ESMTP id B69A7267205 for ; Fri, 17 Feb 2017 18:44:49 +0100 (CET) Received: by mail-wm0-f54.google.com with SMTP id v186so21973789wmd.0 for ; Fri, 17 Feb 2017 09:44:49 -0800 (PST) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Takashi Iwai Cc: alsa-devel@alsa-project.org List-Id: alsa-devel@alsa-project.org 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.