Please wrap your emails within 80 chars, you have been repeatedly asked to do so on this on list. I don't understand how hard it is to follow basic list etiquettes. Its difficult to read screwed up formatting
Yes sir. Apologize for the mistake.
If someone changes the device mode, this is screwed though there are two types of (PLAYBACK and CAPTURE) devices are registered? Would be better if there are two separate playback and capture operations registered separately.
Again, please read the related discussion on the list.
Compressed devices will not have direction associated with them . They can be opened in any mode depending on the mode the DSP supports. So /dev/snd/comprC0D0 can be either playback or capture device. So a device can be ONLY opened with O_WRONLY or O_RDONLY.
Nice, Does it look like a hack hack hack...
O_NONBLOCK is not tested here for non-blocking support.
Why should it be?
- Any blocking and non blocking should be handled in user space
- Nevertheless, that is not supported, HINT: read the documentation
This is not as per the write system call definition as I understand. Look like we redefine the system call here. Support it in the standard way.
- if (stream->runtime->state != SNDRV_PCM_STATE_SETUP &&
stream->runtime->state != SNDRV_PCM_STATE_RUNNING){
Cant we call write again when the stream is in SNDRV_PCM_STATE_PREPARED state? This code allows only one write after the stream is in SNDRV_PCM_STATE_SETUP state. What if someone wants to write multiple smaller fragments before the start?
Yes currently write is allowed only once and we *normally* expect a START call. I am not sure why someone would need to call "smaller" writes multiple times. Remember this is compressed offload where we ideally want larger chunks of data otherwise there is no point in using this.
The code allows this to happen. Is there any minimum size documented anywhere?
+static unsigned int snd_compr_poll(struct file *f, poll_table *wait) {
- struct snd_compr_file *data = f->private_data;
- struct snd_compr_stream *stream;
- size_t avail;
- int retval = 0;
- BUG_ON(!data);
- stream = &data->stream;
Can you validate O_NONBLOCK before going into poll_wait? I guess this has to return -EINVAL if the device is not opened with O_NONBLOCK.
See above
Not a write implementation.
What is the maximum number of fragments supported?
Whatever dsp supports
IMHO, compress-core needs to query it from each driver and allocate memory only if the supported fragments is greater than requested fragments.
Can we check the fragments to that limit?
Yes, can be checked here as well but I prefer to do these checks in userspace library
Think the driver needs to check and return error if it is not within the maximum supported by the driver.
<Sing>: Can you check stream->ops->set_params for NULL as it is done in the get_params function?
No set_params is mandatory function to be implemented
Ok. It is my miss.
-- ~Vinod