[PATCH v3 0/1] ALSA: compress: allow pause and resume during draining

[PATCH v3 0/1] ALSA: compress: allow pause and resume during draining

[Gyeongtaek Lee]

On Wed, 25 Nov 2020 16:28:05 +0530, Vinod Koul wrote:
>On 27-10-20, 10:57, Gyeongtaek Lee wrote:
>> diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
>> index 0e53f6f31916..a071485383ed 100644
>> --- a/sound/core/compress_offload.c
>> +++ b/sound/core/compress_offload.c
>> @@ -708,11 +708,24 @@ static int snd_compr_pause(struct snd_compr_stream *stream)
>>  {
>>  	int retval;
>>  
>> -	if (stream->runtime->state != SNDRV_PCM_STATE_RUNNING)
>> +	switch (stream->runtime->state) {
>> +	case SNDRV_PCM_STATE_RUNNING:
>> +		retval = stream->ops->trigger(stream,
>> +			SNDRV_PCM_TRIGGER_PAUSE_PUSH);
>
>this seems to fit single line with new 100char limit and makes it a
>better read, can we please do that here and few places below .. The line
>split is making it look bit ugly IMO
>
>> +		if (!retval)
>> +			stream->runtime->state = SNDRV_PCM_STATE_PAUSED;
>> +		break;
>> +	case SNDRV_PCM_STATE_DRAINING:
>> +		if (!stream->device->use_pause_in_draining)
>> +			return -EPERM;
>
>I am expecting patches to tinycompress to handle pause while drain. Esp
>this case..
>
>> +		retval = stream->ops->trigger(stream,
>> +			SNDRV_PCM_TRIGGER_PAUSE_PUSH);
>> +		if (!retval)
>> +			stream->pause_in_draining = true;
>> +		break;
>> +	default:
>>  		return -EPERM;
>> -	retval = stream->ops->trigger(stream, SNDRV_PCM_TRIGGER_PAUSE_PUSH);
>> -	if (!retval)
>> -		stream->runtime->state = SNDRV_PCM_STATE_PAUSED;
>> +	}
>>  	return retval;
>>  }
>>  
>> @@ -720,11 +733,25 @@ static int snd_compr_resume(struct snd_compr_stream *stream)
>>  {
>>  	int retval;
>>  
>> -	if (stream->runtime->state != SNDRV_PCM_STATE_PAUSED)
>> +	switch (stream->runtime->state) {
>> +	case SNDRV_PCM_STATE_PAUSED:
>> +		retval = stream->ops->trigger(stream,
>> +			SNDRV_PCM_TRIGGER_PAUSE_RELEASE);
>> +		if (!retval)
>> +			stream->runtime->state = SNDRV_PCM_STATE_RUNNING;
>> +		break;
>> +	case SNDRV_PCM_STATE_DRAINING:
>> +		if (!stream->device->use_pause_in_draining ||
>> +		    !stream->pause_in_draining)
>> +			return -EPERM;
>
>does this condition make sense for resume part..? We have already
>checked for this while going into pause. I am not expecting drain state
>to change while we are paused :)
>

I have fixed the patch as your review comment and send version 3.
For your comment about tinycompress, I'll try to make a patch after study about tinycompress.

Thanks,
Gyeongtaek


Gyeongtaek Lee (1):
  ALSA: compress: allow pause and resume during draining

 include/sound/compress_driver.h | 16 ++++++++++++++
 sound/core/compress_offload.c   | 39 ++++++++++++++++++++++++++-------
 2 files changed, 47 insertions(+), 8 deletions(-)


base-commit: 418baf2c28f3473039f2f7377760bd8f6897ae18
-- 
2.21.0