Bugfix: Fix resampling when client and slave both use format float

Commit Message

Maarten Baert Feb. 24, 2014, 8:52 p.m. UTC

On 24/02/14 14:37, Takashi Iwai wrote:

> Looking at the code again, the PCM rate plugin version checks are done
> slightly differently from ext-plugin or io-plugin; it's rather done in
> the plugin side.  pcm_rate.c repeats to trying to hook a plugin until
> it matches by degrading the version.  In the plugin side, it provides
> additional ops depending on the version it's asked.

That complicates things a lot. Currently the format conversion is
decided before the rate conversion is done. If not all rate conversion
plugins support float, then the format conversion plugin needs to know
this ahead of time to make the right decision. Or the plugin insertion
code needs to be changed so the plugins aren't created in a fixed order.
They are currently created back-to-front, it would probably make more
sense to choose a rate conversion and channel remapping plugin first,
and then insert format conversions where needed.

Re-submitting my original patch as requested.

Maarten Baert

Comments

Takashi Iwai Feb. 26, 2014, 7:28 a.m. UTC | #1

At Mon, 24 Feb 2014 21:52:56 +0100,
Maarten Baert wrote:
> 
> On 24/02/14 14:37, Takashi Iwai wrote:
> 
> > Looking at the code again, the PCM rate plugin version checks are done
> > slightly differently from ext-plugin or io-plugin; it's rather done in
> > the plugin side.  pcm_rate.c repeats to trying to hook a plugin until
> > it matches by degrading the version.  In the plugin side, it provides
> > additional ops depending on the version it's asked.
> 
> That complicates things a lot. Currently the format conversion is
> decided before the rate conversion is done. If not all rate conversion
> plugins support float, then the format conversion plugin needs to know
> this ahead of time to make the right decision. Or the plugin insertion
> code needs to be changed so the plugins aren't created in a fixed order.
> They are currently created back-to-front, it would probably make more
> sense to choose a rate conversion and channel remapping plugin first,
> and then insert format conversions where needed.

Yeah, this might work better.

> Re-submitting my original patch as requested.

It's not applicable via git am.  Please resubmit with the right format
(that is cleanly applied via git am).


thanks,

Takashi

Patch

snd_pcm_plug_change_format: Insert linear-to-float conversion when rate
or channel count is incorrect.

This fixes a bug where snd_pcm_plug_insert_plugins fails when both
client and slave use format float, but the rate or channel count does
not match. I also removed some redundant code.

Signed-off-by: Maarten Baert <[e-mail]>

diff --git a/src/pcm/pcm_plug.c b/src/pcm/pcm_plug.c
index fa84eaa..ede9c15 100644
--- a/src/pcm/pcm_plug.c
+++ b/src/pcm/pcm_plug.c
@@ -522,15 +522,13 @@  static int snd_pcm_plug_change_format(snd_pcm_t *pcm, snd_pcm_t **new, snd_pcm_p
 		}
 #ifdef BUILD_PCM_PLUGIN_LFLOAT
 	} else if (snd_pcm_format_float(slv->format)) {
-		/* Conversion is done in another plugin */
-		if (clt->format == slv->format &&
-		    clt->rate == slv->rate &&
-		    clt->channels == slv->channels)
-			return 0;
-		cfmt = clt->format;
-		if (snd_pcm_format_linear(clt->format))
+		if (snd_pcm_format_linear(clt->format)) {
+			cfmt = clt->format;
 			f = snd_pcm_lfloat_open;
-		else
+		} else if (clt->rate != slv->rate || clt->channels != slv->channels) {
+			cfmt = SND_PCM_FORMAT_S16;
+			f = snd_pcm_lfloat_open;
+		} else
 			return -EINVAL;
 #endif
 #ifdef BUILD_PCM_NONLINEAR