On Thu, 2011-11-24 at 09:51 +0100, Takashi Iwai wrote:
If you don't mind we can keep it as is for now, and once we have capture, mmap and other support built and tested we can merge these?
Well, such a decision should be done as early as possible. Changing the device file assignment means to change the interface to the user-space. So, this is not allowed once when the code is merge to the upstream.
Somehow I was anticipating this
That said, if you want to fiddle it in your tree, I'm fine. But, until the API is fixed, the code can be never merged.
I'm OK to keep the direction-separated devices if it's mandatory, but note that it's a special handling only for ALSA PCM. (OSS PCM doesn't need it, too.)
Well I am planning to get it merged ASAP, not keep in my tree.
Only reason why I asked is since we don't have mmap support and capture support, if it all we need to differentiate we should have a way. Given that, today I don't have a reason why we cant do that if we know the direction. Something like if (playback) do_this_way; else do_another_way;
So I will take your advice and fix it once for now.
Hm, but this also change other pr_*(). If you want only the debug prints, you can use snd_printdd(). This is compiled conditionally, and can show the file, the function and the line number.
I wanted for all prints, its nicer to have this info as well for error and info prints.
Do you mean the race of opening the same device or among (all) different devices? For the first case, the lock should be device-specific. For the latter case, I don't understand why.
former case, yes makes sense to use device lock rather than global. I will split locking now.
But snd_compr_calc_avail() may return without filling avail->avail, e.g.
if (stream->runtime->bytes_written == 0 && stream->runtime->state == SNDRV_PCM_STATE_SETUP) { pr_debug("detected init and someone forgot to do a write\n"); return stream->runtime->buffer_size; }
So, this leaks the uninitialized space.
got it :)
The biggest problem is the conflict of terminology. What do you mean as "frame" and what do you mean as "fragment" in your implementation? At least, the term "frame" is already used in ALSA PCM, and I'm not sure whether you use this term for the very same meaning in the above context...
fragment is some random number of bytes after which driver will callback. This is typically used for pushing data to device.
For capture, since application may need to get data on frame basis (think video recording with encoded video usage, where application needs audio compressed data on "frame" basis for encapsulation). The DSP is supposed callback after every encoded frame. I think only difference between PCM and this is encoding format, otherwise in terms of decoded data and time they would mean the same.. I am not expert here so maybe wrong.
And, another question is how you can guarantee that the capture works in that way. What happens if a hardware updates the data only in the fragments level even for the capture?
That would be a buggy driver and I would ask them to read the documentation. At least it wont work for apps which need data on frame basis
Yes, but the substitution of cmd is useless in your code:
- case _IOC_NR(SNDRV_COMPRESS_DRAIN):
cmd = SND_COMPR_TRIGGER_DRAIN;retval = snd_compr_drain(stream);break;
I should start wearing glasses now