[alsa-devel] Conversion to int16_t and resolution loss with rate converter plugins

[Takashi Sakamoto]

Hi,

Sorry for delay.

On Apr 6 2018 06:10, Flavio Protasio Ribeiro wrote:
> My original proposal was to replace 'convert_s16' with 'convert_s32', while keeping 'convert'. Looking at your previous thread with Pavel on this topic, recognize your policy of not breaking binary compatibility with previous plugins.
> 
> So instead I propose adding a 'convert_s32' method (at the end of snd_pcm_rate_ops_t, with appropriate versioning) that gets called by default if the plugin implements it. The simplified code would look like:
> 
> (pcm_rate.c)
> static void do_convert(const snd_pcm_channel_area_t *dst_areas,
>                         snd_pcm_uframes_t dst_offset, unsigned int dst_frames,
>                         const snd_pcm_channel_area_t *src_areas,
>                         snd_pcm_uframes_t src_offset, unsigned int src_frames,
>                         unsigned int channels,
>                         snd_pcm_rate_t *rate)
> {
>          if (rate->ops.convert_s32) {
>                  convert_to_s32(rate, rate->src_buf, src_areas, src_offset, src_frames, channels);
>                  rate->ops.convert_s32(rate->obj, dst, dst_frames, src, src_frames);
>                  convert_from_s32(rate, rate->dst_buf, dst_areas, dst_offset,  dst_frames, channels);
>          } else if (rate->ops.convert_s16) {
>                  convert_to_s16(rate, rate->src_buf, src_areas, src_offset, src_frames, channels);
>                  rate->ops.convert_s16(rate->obj, dst, dst_frames, src, src_frames);
>                  convert_from_s16(rate, rate->dst_buf, dst_areas, dst_offset,  dst_frames, channels);
>          } else {
>                  rate->ops.convert(rate->obj, dst_areas, dst_offset, dst_frames,
>                                     src_areas, src_offset, src_frames);
>          }
> }
> 
> What do you think? If this looks good to you I can provide a patch.

Hm. In my opinion, in a point of existent
'struct snd_pcm_rate_ops.convert()', the new
'struct snd_pcm_rate_ops.convert_s32()' is not necessarily required.
Developers can implement conversion with data of any 32 bit physical
width as long as implementing buffer handling according to
'snd_pcm_channel_area_t' in backend of PCM plugin.

On the other hand, I recognize the new
'struct snd_pcm_rate_ops.convert_s32()' might be a fast path for cases 
that sample data is 32 bit physical width and aligned to interleaved
(packed) order. I can assume this can cover most use cases with
'.convert_s16()'.

However, at first place, it's better to utilize existent
'.convert_s32()' because no need to update interface version. When you
face performance issues, then work for the new API. Thus at present,
each plugin needs to parse 'snd_pcm_channel_area_t' and align sample
data according to alignment which used libraries; e.g. libsamplerate
requires.


Pavel's patch (0001-Support-for-float-samples-in-rate-converters.patch)
converts sample data to/from float/integer inner ALSA PCM rate plugin
according to API specification of libsamplerate/libspeex, as he said[1].
However, when considering that libsamplerate/libspeex have conversion
helper APIs or perform conversion internally, it's better to delegate
the task to judge the way to convert float/interger into each
implementation of PCM rate backend. As long as I know, below methods are
available for this purpose on below backends:

libsamplerate:
  * src_short_to_float_array()
  * src_float_to_short_array()
  * src_int_to_float_array()
  * src_float_to_int_array()
libspeex:
  * Below conversion API performs conversion of sample data to float as
    well.
   * speex_resampler_process_int()
   * speex_resampler_process_interleaved_int()
libswresample (obsoletes libavresample):
  * Below conversion API performs conversion of sample data to float as
    well if correctly configured.
   * swr_convert()

However, his patch is a good example for how to parse
'snd_pcm_channel_area_t', in my opinion. But I don't avoid to change the
other parts; e.g. code on pcm rate plugin itself.

On Apr 6 2018 15:32, Pavel Hofman wrote:
 > But the existing rate API will have to be modified since libsoxr
 > behaves  a bit different to speex or libresample - see the library
 > author discussing the issues
 > 
http://mailman.alsa-project.org/pipermail/alsa-devel/2013-September/066764.html

I get 404 from these links. Could I request more detail about a
requirement of modification for libsoxr?

[1] 
http://mailman.alsa-project.org/pipermail/alsa-devel/2018-April/134065.html


Regards

Takashi Sakamoto