All of lore.kernel.org
 help / color / mirror / Atom feed
* AK4114 - capturing spdif input stops the stream
@ 2008-03-12 22:50 Pavel Hofman
  2008-03-13  7:50 ` Takashi Iwai
  0 siblings, 1 reply; 8+ messages in thread
From: Pavel Hofman @ 2008-03-12 22:50 UTC (permalink / raw)
  To: Clemens Ladisch; +Cc: Takashi Iwai, ALSA development

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).

If the card cannot detect input frequency reliably (the driver developer 
should know), I guess it should not do the check in ak4114.c at all - I 
can prepare a simple patch for that.

The same will most probably apply to cards with AK4117 as the code looks 
the same.

I know it is a minor issue but if a user hits this problem, it will take 
him a lot of effort to fix his setup appropriately.

Thanks,

Pavel.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: AK4114 - capturing spdif input stops the stream
  2008-03-12 22:50 AK4114 - capturing spdif input stops the stream Pavel Hofman
@ 2008-03-13  7:50 ` Takashi Iwai
  2008-03-13 12:49   ` Pavel Hofman
  0 siblings, 1 reply; 8+ messages in thread
From: Takashi Iwai @ 2008-03-13  7:50 UTC (permalink / raw)
  To: Pavel Hofman; +Cc: ALSA development, Clemens Ladisch

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.


Takashi

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: AK4114 - capturing spdif input stops the stream
  2008-03-13  7:50 ` Takashi Iwai
@ 2008-03-13 12:49   ` Pavel Hofman
  2008-03-13 13:57     ` Takashi Iwai
  0 siblings, 1 reply; 8+ messages in thread
From: Pavel Hofman @ 2008-03-13 12:49 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: ALSA Developers, Clemens Ladisch

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.

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.

Thanks for suggestions,

Pavel.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: AK4114 - capturing spdif input stops the stream
  2008-03-13 12:49   ` Pavel Hofman
@ 2008-03-13 13:57     ` Takashi Iwai
  2008-03-13 14:11       ` Pavel Hofman
  2008-03-13 19:18       ` Pavel Hofman
  0 siblings, 2 replies; 8+ messages in thread
From: Takashi Iwai @ 2008-03-13 13:57 UTC (permalink / raw)
  To: Pavel Hofman; +Cc: ALSA Developers, Clemens Ladisch

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;
 }

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: AK4114 - capturing spdif input stops the stream
  2008-03-13 13:57     ` Takashi Iwai
@ 2008-03-13 14:11       ` Pavel Hofman
  2008-03-13 19:18       ` Pavel Hofman
  1 sibling, 0 replies; 8+ messages in thread
From: Pavel Hofman @ 2008-03-13 14:11 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: ALSA Developers

Takashi Iwai napsal(a):
> 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

Thanks a lot. I will test it in the evening. The code looks simple now 
but without your help I would be stuck forever :)

Pavel.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: AK4114 - capturing spdif input stops the stream
  2008-03-13 13:57     ` Takashi Iwai
  2008-03-13 14:11       ` Pavel Hofman
@ 2008-03-13 19:18       ` Pavel Hofman
  2008-03-14 16:18         ` Takashi Iwai
  1 sibling, 1 reply; 8+ messages in thread
From: Pavel Hofman @ 2008-03-13 19:18 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: ALSA Developers



Takashi Iwai wrote:
> 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, thanks for the patch, it works flawlessly.

Another issue - when the external clock changes rate and the stream is 
running (typically when starting the source playback which switches the 
source card (and its SPDIF-OUT) to a different frequency), the target 
card detects the change and the capture stream is stopped in 
snd_ak4114_check_rate_and_errors() by

snd_pcm_stop(ak4114->capture_substream, SNDRV_PCM_STATE_DRAINING);

How should recording applications behave with stream in this mode? I 
would expect arecord to close, but it does nothing. Is this correct 
behaviour?

Thanks,

Pavel.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: AK4114 - capturing spdif input stops the stream
  2008-03-13 19:18       ` Pavel Hofman
@ 2008-03-14 16:18         ` Takashi Iwai
  2008-03-14 17:15           ` Pavel Hofman
  0 siblings, 1 reply; 8+ messages in thread
From: Takashi Iwai @ 2008-03-14 16:18 UTC (permalink / raw)
  To: Pavel Hofman; +Cc: ALSA Developers

At Thu, 13 Mar 2008 20:18:34 +0100,
Pavel Hofman wrote:
> 
> Takashi Iwai wrote:
> > 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, thanks for the patch, it works flawlessly.

OK, now committed to HG tree.

> Another issue - when the external clock changes rate and the stream is 
> running (typically when starting the source playback which switches the 
> source card (and its SPDIF-OUT) to a different frequency), the target 
> card detects the change and the capture stream is stopped in 
> snd_ak4114_check_rate_and_errors() by
> 
> snd_pcm_stop(ak4114->capture_substream, SNDRV_PCM_STATE_DRAINING);
> 
> How should recording applications behave with stream in this mode? I 
> would expect arecord to close, but it does nothing. Is this correct 
> behaviour?

It's supposed to be drained then stopped.  Maybe something wrong in
the handling of DRAINING state in the capture direction.

Anyway, it's a bit strange situation as is.  Since the sample rate is
changed, you can't call simply prepare again like normal errors...


Takashi

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: AK4114 - capturing spdif input stops the stream
  2008-03-14 16:18         ` Takashi Iwai
@ 2008-03-14 17:15           ` Pavel Hofman
  0 siblings, 0 replies; 8+ messages in thread
From: Pavel Hofman @ 2008-03-14 17:15 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: ALSA Developers

Takashi Iwai wrote:
>> Takashi, thanks for the patch, it works flawlessly.
> 
> OK, now committed to HG tree.

Thanks a lot. I will just resolve the conflicts now :)


>> Another issue - when the external clock changes rate and the stream is 
>> running (typically when starting the source playback which switches the 
>> source card (and its SPDIF-OUT) to a different frequency), the target 
>> card detects the change and the capture stream is stopped in 
>> snd_ak4114_check_rate_and_errors() by
>>
>> snd_pcm_stop(ak4114->capture_substream, SNDRV_PCM_STATE_DRAINING);
>>
>> How should recording applications behave with stream in this mode? I 
>> would expect arecord to close, but it does nothing. Is this correct 
>> behaviour?
> 
> It's supposed to be drained then stopped.  Maybe something wrong in
> the handling of DRAINING state in the capture direction.
> 
> Anyway, it's a bit strange situation as is.  Since the sample rate is
> changed, you can't call simply prepare again like normal errors...

I would expect arecord to quit with some relevant message. In larger 
recording apps, e.g. Audacity, it should probably stop recording and 
display a warning message. No idea if the controlled stop with an 
appropriate error message is feasible with the existing API and code.

Thanks,

Pavel Hofman.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2008-03-14 17:15 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-03-12 22:50 AK4114 - capturing spdif input stops the stream Pavel Hofman
2008-03-13  7:50 ` Takashi Iwai
2008-03-13 12:49   ` Pavel Hofman
2008-03-13 13:57     ` Takashi Iwai
2008-03-13 14:11       ` Pavel Hofman
2008-03-13 19:18       ` Pavel Hofman
2008-03-14 16:18         ` Takashi Iwai
2008-03-14 17:15           ` Pavel Hofman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.