[Sound-open-firmware] [PATCH 2/2] SRC: Bug fix for issuing xru and xro errors

[Seppo Ingalsuo]

On 21.11.2017 02:59, Liam Girdwood wrote:
> On Mon, 2017-11-20 at 19:56 +0200, Seppo Ingalsuo wrote:
>> This patch prevents SRC to stall due to too demanding criteria of
>> function comp_buffer_can_copy_bytes. Since the input and output rate and
>> period length of SRC are different for source and sink both buffers do
>> not need to meet both criteria.
> These functions only check that there is enough avail bytes in source
> and free bytes in sink before we do any copy, what errors did you see ?

The stall happens in SRC test bench. The pipeline cannot be processed 
because SRC can't fill e.g. the sink and therefore cannot drain the 
source. The trace is flooded by xru or xro and nothing is processed.

As example the number of free/avail bytes in source 2x 1ms buffer @ 48 
kHz  and sink 2x 1ms buffer @ 8 kHz should not be compared with same 
number of bytes. SRC resizes in params() the sink buffer and it becomes 
quite small with low sample rates. The risk of this to trigger is the 
higher the larger the ratio of rates is.

Another way to fix this would to add to the function a parameter and 
have different thresholds for source and sink.


>
> Liam
>
>> Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo at linux.intel.com>
>> ---
>>   src/audio/src.c | 27 +++++++++++----------------
>>   1 file changed, 11 insertions(+), 16 deletions(-)
>>
>> diff --git a/src/audio/src.c b/src/audio/src.c
>> index 3378065..0103e2e 100644
>> --- a/src/audio/src.c
>> +++ b/src/audio/src.c
>> @@ -496,7 +496,6 @@ static int src_copy(struct comp_dev *dev)
>>   	int need_sink;
>>   	size_t consumed = 0;
>>   	size_t produced = 0;
>> -	int res;
>>   
>>   	trace_src("SRC");
>>   
>> @@ -519,30 +518,26 @@ static int src_copy(struct comp_dev *dev)
>>   	/* make sure source component buffer has enough data available and that
>>   	 * the sink component buffer has enough free bytes for copy. Also
>>   	 * check for XRUNs */
>> -	res = comp_buffer_can_copy_bytes(source, sink, need_source);
>> -	if (res) {
>> +	if (source->avail < need_source) {
>>   		trace_src_error("xru");
>>   		return -EIO;	/* xrun */
>>   	}
>> -	res = comp_buffer_can_copy_bytes(source, sink, need_sink);
>> -	if (res) {
>> +	if (sink->free < need_sink) {
>>   		trace_src_error("xro");
>>   		return -EIO;	/* xrun */
>>   	}
>>   
>> -	/* Run SRC function if buffers avail and free allow */
>> -	if (((int) source->avail >= need_source) && ((int) sink->free >= need_sink)) {
>> -		cd->src_func(dev, source, sink, &consumed, &produced);
>> +	cd->src_func(dev, source, sink, &consumed, &produced);
>>   
>> -		/* Calc new free and available if data was processed. These
>> -		 * functions must not be called with 0 consumed/produced.
>> -		 */
>> -		if (consumed > 0)
>> -			comp_update_buffer_consume(source, consumed);
>> +	/* Calc new free and available if data was processed. These
>> +	 * functions must not be called with 0 consumed/produced.
>> +	 */
>> +	if (consumed > 0)
>> +		comp_update_buffer_consume(source, consumed);
>> +
>> +	if (produced > 0)
>> +		comp_update_buffer_produce(sink, produced);
>>   
>> -		if (produced > 0)
>> -			comp_update_buffer_produce(sink, produced);
>> -	}
>>   	return 0;
>>   }
>>   
>
> _______________________________________________
> Sound-open-firmware mailing list
> Sound-open-firmware at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
>