[alsa-devel] AK4114 - capturing spdif input stops the stream

[Takashi Iwai]

At Thu, 13 Mar 2008 13:49:20 +0100,
Pavel Hofman wrote:
> 
> Takashi Iwai napsal(a):
> > At Wed, 12 Mar 2008 23:50:08 +0100,
> > Pavel Hofman wrote:
> >> Hi,
> >>
> >> here is my scenario:
> >>
> >> ICE1724 card, trying to record from SPDIF input via AK4114, ICE1724 
> >> slaved to SPDIF clock from the receiver.
> >>
> >> The card detects SPDIF input rate correctly (in my case ESI Juli where 
> >> AK4114 is provided with independent clock signal to enable the rate 
> >> detection). Let's say it is 192000.
> >>
> >> Now I want to arecord the input stream, let's say in CD quality, using 
> >> the plug plugin. The command fails, because of the check in ak4114.c:
> >>
> >> res = external_rate(rcs1);
> >> 	if (!(flags & AK4114_CHECK_NO_RATE) && runtime && runtime->rate != res) {
> >> 		snd_pcm_stream_lock_irqsave(ak4114->capture_substream, _flags);
> >> 		if (snd_pcm_running(ak4114->capture_substream)) {
> >> 			printk(KERN_DEBUG "rate changed (%i <- %i)\n", runtime->rate, res);
> >> 			snd_pcm_stop(ak4114->capture_substream, SNDRV_PCM_STATE_DRAINING);
> >> 			res = 1;
> >> 		}
> >> 		snd_pcm_stream_unlock_irqrestore(ak4114->capture_substream, _flags);
> >> 	}
> >>
> >> The stream gets stopped because res = 192000 and runtime->rate = 44100.
> >>
> >> The problem is that the capture device still offers all the available 
> >> rates, instead of the single SPDIF input one. Thus, the plug plugin is 
> >> not forced to convert from 192000 to 44100 and runtime->rate is 44100 
> >> instead of 192000.
> >>
> >> Of course recording at 192000 works fine.
> >>
> >> What would be the best way to force the SPDIF capture device to offer 
> >> (advertise) only the current rate? This functionality would have to be 
> >> applicable only to the few cards correctly detecting incoming rate 
> >> (unlike most ice1724 cards without independent clock in slave mode).
> > 
> > What about to call snd_ak4114_check_rate_and_errors() at PCM open?
> > You can pass AK4114_CHECK_NO_RATE to flags argument to skip the check
> > there, at least.
> > 
> > 
> 
> Well, I would not want to skip the check for cards with functioning rate 
> detection. Plus snd_ak4114_check_rate_and_errors gets called 
> periodically afterwards.

Yes, but the check there is simply useless at open.  It's only for
running states.  snd_ak4114_external_rate() would be simler, then.

> It would be great if the driver in slaved-clock mode cut its list of 
> native sample rates to the only one currently fed to SPDIF input and 
> detected by AK4114. A routine doing this would be called when switching 
> clock to the slaved-clock mode and called again in 
> snd_ak4114_check_rate_and_errors() when any change in input rate is 
> detected (and the stream is not running). Going back to master-clock 
> mode would restore the existing list of all the rates.
> 
> In case of input sample rate change when the PCM stream is running, the 
> stream would be stopped. This is already implemented in the current 
> version of snd_ak4114_check_rate_and_errors()
> 
> Unfortunately, I do not know how to change the rates list properly not 
> to break something. If it is only about changing the HW params struct, 
> that would be trivial.

Suppose that the slave <-> master mode change doesn't happen during
the stream is opened, it's relatively easy.  So far, ice1724.c doesn't
call spdif.ops.open/close callbacks.  Call them in *_spdif_open/close
functions with NULL check. Then add open hook in juli.c so that you
can change rates_min and rates_max to the currently detected rate at
open...

Well, it's faster to write a patch than texts.


Takashi


diff -r 547e051a88b5 pci/ice1712/ice1724.c
--- a/pci/ice1712/ice1724.c	Wed Mar 12 13:12:15 2008 +0100
+++ b/pci/ice1712/ice1724.c	Thu Mar 13 15:03:14 2008 +0100
@@ -970,6 +970,8 @@ static int snd_vt1724_playback_spdif_ope
 				   VT1724_BUFFER_ALIGN);
 	snd_pcm_hw_constraint_step(runtime, 0, SNDRV_PCM_HW_PARAM_BUFFER_BYTES,
 				   VT1724_BUFFER_ALIGN);
+	if (ice->spdif.ops.open)
+		ice->spdif.ops.open(ice, substream);
 	return 0;
 }
 
@@ -980,6 +982,8 @@ static int snd_vt1724_playback_spdif_clo
 	if (PRO_RATE_RESET)
 		snd_vt1724_set_pro_rate(ice, PRO_RATE_DEFAULT, 0);
 	ice->playback_con_substream = NULL;
+	if (ice->spdif.ops.close)
+		ice->spdif.ops.close(ice, substream);
 
 	return 0;
 }
@@ -1002,6 +1006,8 @@ static int snd_vt1724_capture_spdif_open
 				   VT1724_BUFFER_ALIGN);
 	snd_pcm_hw_constraint_step(runtime, 0, SNDRV_PCM_HW_PARAM_BUFFER_BYTES,
 				   VT1724_BUFFER_ALIGN);
+	if (ice->spdif.ops.open)
+		ice->spdif.ops.open(ice, substream);
 	return 0;
 }
 
@@ -1012,6 +1018,8 @@ static int snd_vt1724_capture_spdif_clos
 	if (PRO_RATE_RESET)
 		snd_vt1724_set_pro_rate(ice, PRO_RATE_DEFAULT, 0);
 	ice->capture_con_substream = NULL;
+	if (ice->spdif.ops.close)
+		ice->spdif.ops.close(ice, substream);
 
 	return 0;
 }
diff -r 547e051a88b5 pci/ice1712/juli.c
--- a/pci/ice1712/juli.c	Wed Mar 12 13:12:15 2008 +0100
+++ b/pci/ice1712/juli.c	Thu Mar 13 15:03:14 2008 +0100
@@ -75,6 +75,22 @@ static unsigned char juli_ak4114_read(vo
 static unsigned char juli_ak4114_read(void *private_data, unsigned char reg)
 {
 	return snd_vt1724_read_i2c((struct snd_ice1712 *)private_data, AK4114_ADDR, reg);
+}
+
+static void juli_spdif_in_open(struct snd_ice1712 *ice,
+			       struct snd_pcm_substream *substream)
+{
+	struct juli_spec *spec = ice->spec;
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	int rate;
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		return;
+	rate = snd_ak4114_external_rate(spec->ak4114);
+	if (rate >= runtime->hw.rate_min && rate <= runtime->hw.rate_max) {
+		runtime->hw.rate_min = rate;
+		runtime->hw.rate_max = rate;
+	}
 }
 
 /*
@@ -210,6 +226,7 @@ static int __devinit juli_init(struct sn
 			return err;
 	}
 	
+	ice->spdif.ops.open = juli_spdif_in_open;
 	return 0;
 }