[03/10] fmtconvert: Add a new method, int32_to_float_fmul_scalar_array

Message ID 1373980994-30628-3-git-send-email-martin@martin.st
State Superseded
Headers show

Commit Message

Martin Storsjö July 16, 2013, 1:23 p.m.
From: Ben Avison <bavison@riscosopen.org>

This is similar to int32_to_float_fmul_scalar, but
loads a new scalar multiplier every 8 input samples.
This enables the use of much larger input arrays, which
is important for pipelining on some CPUs (such as
ARMv6).
---
 libavcodec/fmtconvert.c |    7 +++++++
 libavcodec/fmtconvert.h |   14 ++++++++++++++
 2 files changed, 21 insertions(+)

Comments

Martin Storsjö July 16, 2013, 1:31 p.m. | #1
On Tue, 16 Jul 2013, Martin Storsjö wrote:

> From: Ben Avison <bavison@riscosopen.org>
>
> This is similar to int32_to_float_fmul_scalar, but
> loads a new scalar multiplier every 8 input samples.
> This enables the use of much larger input arrays, which
> is important for pipelining on some CPUs (such as
> ARMv6).
> ---
> libavcodec/fmtconvert.c |    7 +++++++
> libavcodec/fmtconvert.h |   14 ++++++++++++++
> 2 files changed, 21 insertions(+)
>
> diff --git a/libavcodec/fmtconvert.c b/libavcodec/fmtconvert.c
> index 5b7ab03..0ad54bb 100644
> --- a/libavcodec/fmtconvert.c
> +++ b/libavcodec/fmtconvert.c
> @@ -30,6 +30,12 @@ static void int32_to_float_fmul_scalar_c(float *dst, const int *src, float mul,
>         dst[i] = src[i] * mul;
> }
>
> +static void int32_to_float_fmul_scalar_array_c(FmtConvertContext *c, float *dst, const int *src, float *mul, int len){
> +    int i;
> +    for (i = 0; i < len; i += 8)
> +        c->int32_to_float_fmul_scalar(&dst[i], &src[i], *mul++, 8);
> +}
> +
> static av_always_inline int float_to_int16_one(const float *src){
>     return av_clip_int16(lrintf(*src));
> }
> @@ -79,6 +85,7 @@ void ff_float_interleave_c(float *dst, const float **src, unsigned int len,
> av_cold void ff_fmt_convert_init(FmtConvertContext *c, AVCodecContext *avctx)
> {
>     c->int32_to_float_fmul_scalar = int32_to_float_fmul_scalar_c;
> +    c->int32_to_float_fmul_scalar_array = int32_to_float_fmul_scalar_array_c;
>     c->float_to_int16             = float_to_int16_c;
>     c->float_to_int16_interleave  = float_to_int16_interleave_c;
>     c->float_interleave           = ff_float_interleave_c;
> diff --git a/libavcodec/fmtconvert.h b/libavcodec/fmtconvert.h
> index 93c3401..8a1395c 100644
> --- a/libavcodec/fmtconvert.h
> +++ b/libavcodec/fmtconvert.h
> @@ -38,6 +38,20 @@ typedef struct FmtConvertContext {
>     void (*int32_to_float_fmul_scalar)(float *dst, const int *src, float mul, int len);
>
>     /**
> +     * Convert an array of int32_t to float and multiply by a float value from another array,
> +     * stepping along the float array once for each 8 integers.
> +     * @param c   pointer to FmtConvertContext.
> +     * @param dst destination array of float.
> +     *            constraints: 16-byte aligned
> +     * @param src source array of int32_t.
> +     *            constraints: 16-byte aligned
> +     * @param mul source array of float multipliers.
> +     * @param len number of elements to convert.
> +     *            constraints: multiple of 8
> +     */
> +    void (*int32_to_float_fmul_scalar_array)(struct FmtConvertContext *c, float *dst, const int *src, float *mul, int len);

This function name sounds a little too generic to me, given that it has 
the assumption that mul is incremented once for each 8 samples (or would 
this be an assumption that would be suitable as such for many other codecs 
as well?). It think it might be better if the number 8 was part of the 
function name to make this a bit more explicit.

// Martin
Luca Barbato July 16, 2013, 3:15 p.m. | #2
On 07/16/2013 03:23 PM, Martin Storsjö wrote:
> From: Ben Avison <bavison@riscosopen.org>
> 
> This is similar to int32_to_float_fmul_scalar, but
> loads a new scalar multiplier every 8 input samples.
> This enables the use of much larger input arrays, which
> is important for pipelining on some CPUs (such as
> ARMv6).
> ---
>  libavcodec/fmtconvert.c |    7 +++++++
>  libavcodec/fmtconvert.h |   14 ++++++++++++++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/libavcodec/fmtconvert.c b/libavcodec/fmtconvert.c
> index 5b7ab03..0ad54bb 100644
> --- a/libavcodec/fmtconvert.c
> +++ b/libavcodec/fmtconvert.c
> @@ -30,6 +30,12 @@ static void int32_to_float_fmul_scalar_c(float *dst, const int *src, float mul,
>          dst[i] = src[i] * mul;
>  }
>  
> +static void int32_to_float_fmul_scalar_array_c(FmtConvertContext *c, float *dst, const int *src, float *mul, int len){
> +    int i;
> +    for (i = 0; i < len; i += 8)
> +        c->int32_to_float_fmul_scalar(&dst[i], &src[i], *mul++, 8);
> +}

Whoever pushes this please fix the style.

>  static av_always_inline int float_to_int16_one(const float *src){
>      return av_clip_int16(lrintf(*src));
>  }
> @@ -79,6 +85,7 @@ void ff_float_interleave_c(float *dst, const float **src, unsigned int len,
>  av_cold void ff_fmt_convert_init(FmtConvertContext *c, AVCodecContext *avctx)
>  {
>      c->int32_to_float_fmul_scalar = int32_to_float_fmul_scalar_c;
> +    c->int32_to_float_fmul_scalar_array = int32_to_float_fmul_scalar_array_c;
>      c->float_to_int16             = float_to_int16_c;
>      c->float_to_int16_interleave  = float_to_int16_interleave_c;
>      c->float_interleave           = ff_float_interleave_c;

Ditto.

> diff --git a/libavcodec/fmtconvert.h b/libavcodec/fmtconvert.h
> index 93c3401..8a1395c 100644
> --- a/libavcodec/fmtconvert.h
> +++ b/libavcodec/fmtconvert.h
> @@ -38,6 +38,20 @@ typedef struct FmtConvertContext {
>      void (*int32_to_float_fmul_scalar)(float *dst, const int *src, float mul, int len);
>  
>      /**
> +     * Convert an array of int32_t to float and multiply by a float value from another array,
> +     * stepping along the float array once for each 8 integers.
> +     * @param c   pointer to FmtConvertContext.
> +     * @param dst destination array of float.
> +     *            constraints: 16-byte aligned
> +     * @param src source array of int32_t.
> +     *            constraints: 16-byte aligned
> +     * @param mul source array of float multipliers.
> +     * @param len number of elements to convert.
> +     *            constraints: multiple of 8
> +     */
> +    void (*int32_to_float_fmul_scalar_array)(struct FmtConvertContext *c, float *dst, const int *src, float *mul, int len);

Ditto.

lu
Martin Storsjö July 16, 2013, 3:44 p.m. | #3
On Tue, 16 Jul 2013, Luca Barbato wrote:

> On 07/16/2013 03:23 PM, Martin Storsjö wrote:
>> From: Ben Avison <bavison@riscosopen.org>
>> 
>> This is similar to int32_to_float_fmul_scalar, but
>> loads a new scalar multiplier every 8 input samples.
>> This enables the use of much larger input arrays, which
>> is important for pipelining on some CPUs (such as
>> ARMv6).
>> ---
>>  libavcodec/fmtconvert.c |    7 +++++++
>>  libavcodec/fmtconvert.h |   14 ++++++++++++++
>>  2 files changed, 21 insertions(+)
>> 
>> diff --git a/libavcodec/fmtconvert.c b/libavcodec/fmtconvert.c
>> index 5b7ab03..0ad54bb 100644
>> --- a/libavcodec/fmtconvert.c
>> +++ b/libavcodec/fmtconvert.c
>> @@ -30,6 +30,12 @@ static void int32_to_float_fmul_scalar_c(float *dst, const int *src, float mul,
>>          dst[i] = src[i] * mul;
>>  }
>> 
>> +static void int32_to_float_fmul_scalar_array_c(FmtConvertContext *c, float *dst, const int *src, float *mul, int len){
>> +    int i;
>> +    for (i = 0; i < len; i += 8)
>> +        c->int32_to_float_fmul_scalar(&dst[i], &src[i], *mul++, 8);
>> +}
>
> Whoever pushes this please fix the style.

I did a few other small touchups to this one but apparently I missed the 
declaration line itself - thanks for reminding me.

>>  static av_always_inline int float_to_int16_one(const float *src){
>>      return av_clip_int16(lrintf(*src));
>>  }
>> @@ -79,6 +85,7 @@ void ff_float_interleave_c(float *dst, const float **src, unsigned int len,
>>  av_cold void ff_fmt_convert_init(FmtConvertContext *c, AVCodecContext *avctx)
>>  {
>>      c->int32_to_float_fmul_scalar = int32_to_float_fmul_scalar_c;
>> +    c->int32_to_float_fmul_scalar_array = int32_to_float_fmul_scalar_array_c;
>>      c->float_to_int16             = float_to_int16_c;
>>      c->float_to_int16_interleave  = float_to_int16_interleave_c;
>>      c->float_interleave           = ff_float_interleave_c;
>
> Ditto.

What's there to fix here - reindenting the other lines you mean?

>> diff --git a/libavcodec/fmtconvert.h b/libavcodec/fmtconvert.h
>> index 93c3401..8a1395c 100644
>> --- a/libavcodec/fmtconvert.h
>> +++ b/libavcodec/fmtconvert.h
>> @@ -38,6 +38,20 @@ typedef struct FmtConvertContext {
>>      void (*int32_to_float_fmul_scalar)(float *dst, const int *src, float mul, int len);
>>
>>      /**
>> +     * Convert an array of int32_t to float and multiply by a float value from another array,
>> +     * stepping along the float array once for each 8 integers.
>> +     * @param c   pointer to FmtConvertContext.
>> +     * @param dst destination array of float.
>> +     *            constraints: 16-byte aligned
>> +     * @param src source array of int32_t.
>> +     *            constraints: 16-byte aligned
>> +     * @param mul source array of float multipliers.
>> +     * @param len number of elements to convert.
>> +     *            constraints: multiple of 8
>> +     */
>> +    void (*int32_to_float_fmul_scalar_array)(struct FmtConvertContext *c, float *dst, const int *src, float *mul, int len);
>
> Ditto.

And what's to fix here - breaking the long line perhaps?

The mul pointer should be changed to const as well btw, I'll amend that 
locally.

// Martin
Luca Barbato July 16, 2013, 4:09 p.m. | #4
On 07/16/2013 05:44 PM, Martin Storsjö wrote:
> On Tue, 16 Jul 2013, Luca Barbato wrote:

>> Whoever pushes this please fix the style.
> 
> I did a few other small touchups to this one but apparently I missed the
> declaration line itself - thanks for reminding me.

Thanks for fixing it =)

>>>  static av_always_inline int float_to_int16_one(const float *src){
>>>      return av_clip_int16(lrintf(*src));
>>>  }
>>> @@ -79,6 +85,7 @@ void ff_float_interleave_c(float *dst, const float
>>> **src, unsigned int len,
>>>  av_cold void ff_fmt_convert_init(FmtConvertContext *c,
>>> AVCodecContext *avctx)
>>>  {
>>>      c->int32_to_float_fmul_scalar = int32_to_float_fmul_scalar_c;
>>> +    c->int32_to_float_fmul_scalar_array =
>>> int32_to_float_fmul_scalar_array_c;
>>>      c->float_to_int16             = float_to_int16_c;
>>>      c->float_to_int16_interleave  = float_to_int16_interleave_c;
>>>      c->float_interleave           = ff_float_interleave_c;
>>
>> Ditto.
> 
> What's there to fix here - reindenting the other lines you mean?

Would be nice.

>>> +    void (*int32_to_float_fmul_scalar_array)(struct
>>> FmtConvertContext *c, float *dst, const int *src, float *mul, int len);
>>
>> Ditto.
> 
> And what's to fix here - breaking the long line perhaps?
> 
> The mul pointer should be changed to const as well btw, I'll amend that
> locally.
> 

Yes.

lu
Christophe Gisquet July 16, 2013, 6:18 p.m. | #5
Hi,

2013/7/16 Martin Storsjö <martin@martin.st>:
> From: Ben Avison <bavison@riscosopen.org>
>
> This is similar to int32_to_float_fmul_scalar, but
>      /**
> +     * Convert an array of int32_t to float and multiply by a float value from another array,
> +     * stepping along the float array once for each 8 integers.
> +     * @param c   pointer to FmtConvertContext.
> +     * @param dst destination array of float.
> +     *            constraints: 16-byte aligned
> +     * @param src source array of int32_t.
> +     *            constraints: 16-byte aligned
> +     * @param mul source array of float multipliers.
> +     * @param len number of elements to convert.
> +     *            constraints: multiple of 8
> +     */
> +    void (*int32_to_float_fmul_scalar_array)(struct FmtConvertContext *c, float *dst, const int *src, float *mul, int len);

I don't remember completely when and where I last mentioned this, but
this new DSP function is propagating an issue right there in the
documentation above tihs prototype: the input is int32_t, not int.

I had a patch for this for the original DSP function:
https://github.com/kurosu/libav/commit/5a57bfa1a3024193503be0788a615e5abbe5bca8
It may be incomplete.

I have no time to submit it, so just FYI.
--
Christophe
Martin Storsjö July 16, 2013, 9:02 p.m. | #6
On Tue, 16 Jul 2013, Christophe Gisquet wrote:

> Hi,
>
> 2013/7/16 Martin Storsjö <martin@martin.st>:
>> From: Ben Avison <bavison@riscosopen.org>
>>
>> This is similar to int32_to_float_fmul_scalar, but
>>      /**
>> +     * Convert an array of int32_t to float and multiply by a float value from another array,
>> +     * stepping along the float array once for each 8 integers.
>> +     * @param c   pointer to FmtConvertContext.
>> +     * @param dst destination array of float.
>> +     *            constraints: 16-byte aligned
>> +     * @param src source array of int32_t.
>> +     *            constraints: 16-byte aligned
>> +     * @param mul source array of float multipliers.
>> +     * @param len number of elements to convert.
>> +     *            constraints: multiple of 8
>> +     */
>> +    void (*int32_to_float_fmul_scalar_array)(struct FmtConvertContext *c, float *dst, const int *src, float *mul, int len);
>
> I don't remember completely when and where I last mentioned this, but
> this new DSP function is propagating an issue right there in the
> documentation above tihs prototype: the input is int32_t, not int.

Yes, that's a good point. It probably doesn't really matter on any of the 
architectures we support, but it's better to get it right sooner rather 
than later.

> I had a patch for this for the original DSP function:
> https://github.com/kurosu/libav/commit/5a57bfa1a3024193503be0788a615e5abbe5bca8
> It may be incomplete.
>
> I have no time to submit it, so just FYI.

I can cherrypick and submit it - I'll probably rebase this patchset on top 
of it.

// Martin
Martin Storsjö July 17, 2013, 2:55 p.m. | #7
On Tue, 16 Jul 2013, Martin Storsjö wrote:

> On Tue, 16 Jul 2013, Martin Storsjö wrote:
>
>> From: Ben Avison <bavison@riscosopen.org>
>> 
>> This is similar to int32_to_float_fmul_scalar, but
>> loads a new scalar multiplier every 8 input samples.
>> This enables the use of much larger input arrays, which
>> is important for pipelining on some CPUs (such as
>> ARMv6).
>> ---
>> libavcodec/fmtconvert.c |    7 +++++++
>> libavcodec/fmtconvert.h |   14 ++++++++++++++
>> 2 files changed, 21 insertions(+)
>> 
>> diff --git a/libavcodec/fmtconvert.c b/libavcodec/fmtconvert.c
>> index 5b7ab03..0ad54bb 100644
>> --- a/libavcodec/fmtconvert.c
>> +++ b/libavcodec/fmtconvert.c
>> @@ -30,6 +30,12 @@ static void int32_to_float_fmul_scalar_c(float *dst, 
>> const int *src, float mul,
>>         dst[i] = src[i] * mul;
>> }
>> 
>> +static void int32_to_float_fmul_scalar_array_c(FmtConvertContext *c, float 
>> *dst, const int *src, float *mul, int len){
>> +    int i;
>> +    for (i = 0; i < len; i += 8)
>> +        c->int32_to_float_fmul_scalar(&dst[i], &src[i], *mul++, 8);
>> +}
>> +
>> static av_always_inline int float_to_int16_one(const float *src){
>>     return av_clip_int16(lrintf(*src));
>> }
>> @@ -79,6 +85,7 @@ void ff_float_interleave_c(float *dst, const float **src, 
>> unsigned int len,
>> av_cold void ff_fmt_convert_init(FmtConvertContext *c, AVCodecContext 
>> *avctx)
>> {
>>     c->int32_to_float_fmul_scalar = int32_to_float_fmul_scalar_c;
>> +    c->int32_to_float_fmul_scalar_array = 
>> int32_to_float_fmul_scalar_array_c;
>>     c->float_to_int16             = float_to_int16_c;
>>     c->float_to_int16_interleave  = float_to_int16_interleave_c;
>>     c->float_interleave           = ff_float_interleave_c;
>> diff --git a/libavcodec/fmtconvert.h b/libavcodec/fmtconvert.h
>> index 93c3401..8a1395c 100644
>> --- a/libavcodec/fmtconvert.h
>> +++ b/libavcodec/fmtconvert.h
>> @@ -38,6 +38,20 @@ typedef struct FmtConvertContext {
>>     void (*int32_to_float_fmul_scalar)(float *dst, const int *src, float 
>> mul, int len);
>>
>>     /**
>> +     * Convert an array of int32_t to float and multiply by a float value 
>> from another array,
>> +     * stepping along the float array once for each 8 integers.
>> +     * @param c   pointer to FmtConvertContext.
>> +     * @param dst destination array of float.
>> +     *            constraints: 16-byte aligned
>> +     * @param src source array of int32_t.
>> +     *            constraints: 16-byte aligned
>> +     * @param mul source array of float multipliers.
>> +     * @param len number of elements to convert.
>> +     *            constraints: multiple of 8
>> +     */
>> +    void (*int32_to_float_fmul_scalar_array)(struct FmtConvertContext *c, 
>> float *dst, const int *src, float *mul, int len);
>
> This function name sounds a little too generic to me, given that it has the 
> assumption that mul is incremented once for each 8 samples (or would this be 
> an assumption that would be suitable as such for many other codecs as well?). 
> It think it might be better if the number 8 was part of the function name to 
> make this a bit more explicit.

Justin or anyone else familiar with a number of audio codecs, can you give 
a comment on this - does this function make sense as such, or should it be 
renamed?

// Martin
Justin Ruggles July 17, 2013, 3:25 p.m. | #8
On 07/17/2013 10:55 AM, Martin Storsjö wrote:
> On Tue, 16 Jul 2013, Martin Storsjö wrote:
> 
>> On Tue, 16 Jul 2013, Martin Storsjö wrote:
>>
>>> From: Ben Avison <bavison@riscosopen.org>
>>>
>>> This is similar to int32_to_float_fmul_scalar, but
>>> loads a new scalar multiplier every 8 input samples.
>>> This enables the use of much larger input arrays, which
>>> is important for pipelining on some CPUs (such as
>>> ARMv6).
>>> ---
>>> libavcodec/fmtconvert.c |    7 +++++++
>>> libavcodec/fmtconvert.h |   14 ++++++++++++++
>>> 2 files changed, 21 insertions(+)
>>>
>>> diff --git a/libavcodec/fmtconvert.c b/libavcodec/fmtconvert.c
>>> index 5b7ab03..0ad54bb 100644
>>> --- a/libavcodec/fmtconvert.c
>>> +++ b/libavcodec/fmtconvert.c
>>> @@ -30,6 +30,12 @@ static void int32_to_float_fmul_scalar_c(float
>>> *dst, const int *src, float mul,
>>>         dst[i] = src[i] * mul;
>>> }
>>>
>>> +static void int32_to_float_fmul_scalar_array_c(FmtConvertContext *c,
>>> float *dst, const int *src, float *mul, int len){
>>> +    int i;
>>> +    for (i = 0; i < len; i += 8)
>>> +        c->int32_to_float_fmul_scalar(&dst[i], &src[i], *mul++, 8);
>>> +}
>>> +
>>> static av_always_inline int float_to_int16_one(const float *src){
>>>     return av_clip_int16(lrintf(*src));
>>> }
>>> @@ -79,6 +85,7 @@ void ff_float_interleave_c(float *dst, const float
>>> **src, unsigned int len,
>>> av_cold void ff_fmt_convert_init(FmtConvertContext *c, AVCodecContext
>>> *avctx)
>>> {
>>>     c->int32_to_float_fmul_scalar = int32_to_float_fmul_scalar_c;
>>> +    c->int32_to_float_fmul_scalar_array =
>>> int32_to_float_fmul_scalar_array_c;
>>>     c->float_to_int16             = float_to_int16_c;
>>>     c->float_to_int16_interleave  = float_to_int16_interleave_c;
>>>     c->float_interleave           = ff_float_interleave_c;
>>> diff --git a/libavcodec/fmtconvert.h b/libavcodec/fmtconvert.h
>>> index 93c3401..8a1395c 100644
>>> --- a/libavcodec/fmtconvert.h
>>> +++ b/libavcodec/fmtconvert.h
>>> @@ -38,6 +38,20 @@ typedef struct FmtConvertContext {
>>>     void (*int32_to_float_fmul_scalar)(float *dst, const int *src,
>>> float mul, int len);
>>>
>>>     /**
>>> +     * Convert an array of int32_t to float and multiply by a float
>>> value from another array,
>>> +     * stepping along the float array once for each 8 integers.
>>> +     * @param c   pointer to FmtConvertContext.
>>> +     * @param dst destination array of float.
>>> +     *            constraints: 16-byte aligned
>>> +     * @param src source array of int32_t.
>>> +     *            constraints: 16-byte aligned
>>> +     * @param mul source array of float multipliers.
>>> +     * @param len number of elements to convert.
>>> +     *            constraints: multiple of 8
>>> +     */
>>> +    void (*int32_to_float_fmul_scalar_array)(struct
>>> FmtConvertContext *c, float *dst, const int *src, float *mul, int len);
>>
>> This function name sounds a little too generic to me, given that it
>> has the assumption that mul is incremented once for each 8 samples (or
>> would this be an assumption that would be suitable as such for many
>> other codecs as well?). It think it might be better if the number 8
>> was part of the function name to make this a bit more explicit.
> 
> Justin or anyone else familiar with a number of audio codecs, can you
> give a comment on this - does this function make sense as such, or
> should it be renamed?

I agree with you on the name. Actually, I also had that same thought and
was going to say something about it until I read your email.

-Justin
Martin Storsjö July 17, 2013, 3:49 p.m. | #9
On Wed, 17 Jul 2013, Justin Ruggles wrote:

> On 07/17/2013 10:55 AM, Martin Storsjö wrote:
>> On Tue, 16 Jul 2013, Martin Storsjö wrote:
>>
>>> On Tue, 16 Jul 2013, Martin Storsjö wrote:
>>>
>>>> From: Ben Avison <bavison@riscosopen.org>
>>>>
>>>> This is similar to int32_to_float_fmul_scalar, but
>>>> loads a new scalar multiplier every 8 input samples.
>>>> This enables the use of much larger input arrays, which
>>>> is important for pipelining on some CPUs (such as
>>>> ARMv6).
>>>> ---
>>>> libavcodec/fmtconvert.c |    7 +++++++
>>>> libavcodec/fmtconvert.h |   14 ++++++++++++++
>>>> 2 files changed, 21 insertions(+)
>>>>
>>>> diff --git a/libavcodec/fmtconvert.c b/libavcodec/fmtconvert.c
>>>> index 5b7ab03..0ad54bb 100644
>>>> --- a/libavcodec/fmtconvert.c
>>>> +++ b/libavcodec/fmtconvert.c
>>>> @@ -30,6 +30,12 @@ static void int32_to_float_fmul_scalar_c(float
>>>> *dst, const int *src, float mul,
>>>>         dst[i] = src[i] * mul;
>>>> }
>>>>
>>>> +static void int32_to_float_fmul_scalar_array_c(FmtConvertContext *c,
>>>> float *dst, const int *src, float *mul, int len){
>>>> +    int i;
>>>> +    for (i = 0; i < len; i += 8)
>>>> +        c->int32_to_float_fmul_scalar(&dst[i], &src[i], *mul++, 8);
>>>> +}
>>>> +
>>>> static av_always_inline int float_to_int16_one(const float *src){
>>>>     return av_clip_int16(lrintf(*src));
>>>> }
>>>> @@ -79,6 +85,7 @@ void ff_float_interleave_c(float *dst, const float
>>>> **src, unsigned int len,
>>>> av_cold void ff_fmt_convert_init(FmtConvertContext *c, AVCodecContext
>>>> *avctx)
>>>> {
>>>>     c->int32_to_float_fmul_scalar = int32_to_float_fmul_scalar_c;
>>>> +    c->int32_to_float_fmul_scalar_array =
>>>> int32_to_float_fmul_scalar_array_c;
>>>>     c->float_to_int16             = float_to_int16_c;
>>>>     c->float_to_int16_interleave  = float_to_int16_interleave_c;
>>>>     c->float_interleave           = ff_float_interleave_c;
>>>> diff --git a/libavcodec/fmtconvert.h b/libavcodec/fmtconvert.h
>>>> index 93c3401..8a1395c 100644
>>>> --- a/libavcodec/fmtconvert.h
>>>> +++ b/libavcodec/fmtconvert.h
>>>> @@ -38,6 +38,20 @@ typedef struct FmtConvertContext {
>>>>     void (*int32_to_float_fmul_scalar)(float *dst, const int *src,
>>>> float mul, int len);
>>>>
>>>>     /**
>>>> +     * Convert an array of int32_t to float and multiply by a float
>>>> value from another array,
>>>> +     * stepping along the float array once for each 8 integers.
>>>> +     * @param c   pointer to FmtConvertContext.
>>>> +     * @param dst destination array of float.
>>>> +     *            constraints: 16-byte aligned
>>>> +     * @param src source array of int32_t.
>>>> +     *            constraints: 16-byte aligned
>>>> +     * @param mul source array of float multipliers.
>>>> +     * @param len number of elements to convert.
>>>> +     *            constraints: multiple of 8
>>>> +     */
>>>> +    void (*int32_to_float_fmul_scalar_array)(struct
>>>> FmtConvertContext *c, float *dst, const int *src, float *mul, int len);
>>>
>>> This function name sounds a little too generic to me, given that it
>>> has the assumption that mul is incremented once for each 8 samples (or
>>> would this be an assumption that would be suitable as such for many
>>> other codecs as well?). It think it might be better if the number 8
>>> was part of the function name to make this a bit more explicit.
>>
>> Justin or anyone else familiar with a number of audio codecs, can you
>> give a comment on this - does this function make sense as such, or
>> should it be renamed?
>
> I agree with you on the name. Actually, I also had that same thought and
> was going to say something about it until I read your email.

Right, so does e.g. int32_to_float_fmul_scalar_array_8 sound sensible as 
name for it?

// Martin
Justin Ruggles July 17, 2013, 4:21 p.m. | #10
On 07/17/2013 11:49 AM, Martin Storsjö wrote:
> On Wed, 17 Jul 2013, Justin Ruggles wrote:
> 
>> On 07/17/2013 10:55 AM, Martin Storsjö wrote:
>>> On Tue, 16 Jul 2013, Martin Storsjö wrote:
>>>
>>>> On Tue, 16 Jul 2013, Martin Storsjö wrote:
>>>>
>>>>> From: Ben Avison <bavison@riscosopen.org>
>>>>>
>>>>> This is similar to int32_to_float_fmul_scalar, but
>>>>> loads a new scalar multiplier every 8 input samples.
>>>>> This enables the use of much larger input arrays, which
>>>>> is important for pipelining on some CPUs (such as
>>>>> ARMv6).
>>>>> ---
>>>>> libavcodec/fmtconvert.c |    7 +++++++
>>>>> libavcodec/fmtconvert.h |   14 ++++++++++++++
>>>>> 2 files changed, 21 insertions(+)
>>>>>
>>>>> diff --git a/libavcodec/fmtconvert.c b/libavcodec/fmtconvert.c
>>>>> index 5b7ab03..0ad54bb 100644
>>>>> --- a/libavcodec/fmtconvert.c
>>>>> +++ b/libavcodec/fmtconvert.c
>>>>> @@ -30,6 +30,12 @@ static void int32_to_float_fmul_scalar_c(float
>>>>> *dst, const int *src, float mul,
>>>>>         dst[i] = src[i] * mul;
>>>>> }
>>>>>
>>>>> +static void int32_to_float_fmul_scalar_array_c(FmtConvertContext *c,
>>>>> float *dst, const int *src, float *mul, int len){
>>>>> +    int i;
>>>>> +    for (i = 0; i < len; i += 8)
>>>>> +        c->int32_to_float_fmul_scalar(&dst[i], &src[i], *mul++, 8);
>>>>> +}
>>>>> +
>>>>> static av_always_inline int float_to_int16_one(const float *src){
>>>>>     return av_clip_int16(lrintf(*src));
>>>>> }
>>>>> @@ -79,6 +85,7 @@ void ff_float_interleave_c(float *dst, const float
>>>>> **src, unsigned int len,
>>>>> av_cold void ff_fmt_convert_init(FmtConvertContext *c, AVCodecContext
>>>>> *avctx)
>>>>> {
>>>>>     c->int32_to_float_fmul_scalar = int32_to_float_fmul_scalar_c;
>>>>> +    c->int32_to_float_fmul_scalar_array =
>>>>> int32_to_float_fmul_scalar_array_c;
>>>>>     c->float_to_int16             = float_to_int16_c;
>>>>>     c->float_to_int16_interleave  = float_to_int16_interleave_c;
>>>>>     c->float_interleave           = ff_float_interleave_c;
>>>>> diff --git a/libavcodec/fmtconvert.h b/libavcodec/fmtconvert.h
>>>>> index 93c3401..8a1395c 100644
>>>>> --- a/libavcodec/fmtconvert.h
>>>>> +++ b/libavcodec/fmtconvert.h
>>>>> @@ -38,6 +38,20 @@ typedef struct FmtConvertContext {
>>>>>     void (*int32_to_float_fmul_scalar)(float *dst, const int *src,
>>>>> float mul, int len);
>>>>>
>>>>>     /**
>>>>> +     * Convert an array of int32_t to float and multiply by a float
>>>>> value from another array,
>>>>> +     * stepping along the float array once for each 8 integers.
>>>>> +     * @param c   pointer to FmtConvertContext.
>>>>> +     * @param dst destination array of float.
>>>>> +     *            constraints: 16-byte aligned
>>>>> +     * @param src source array of int32_t.
>>>>> +     *            constraints: 16-byte aligned
>>>>> +     * @param mul source array of float multipliers.
>>>>> +     * @param len number of elements to convert.
>>>>> +     *            constraints: multiple of 8
>>>>> +     */
>>>>> +    void (*int32_to_float_fmul_scalar_array)(struct
>>>>> FmtConvertContext *c, float *dst, const int *src, float *mul, int
>>>>> len);
>>>>
>>>> This function name sounds a little too generic to me, given that it
>>>> has the assumption that mul is incremented once for each 8 samples (or
>>>> would this be an assumption that would be suitable as such for many
>>>> other codecs as well?). It think it might be better if the number 8
>>>> was part of the function name to make this a bit more explicit.
>>>
>>> Justin or anyone else familiar with a number of audio codecs, can you
>>> give a comment on this - does this function make sense as such, or
>>> should it be renamed?
>>
>> I agree with you on the name. Actually, I also had that same thought and
>> was going to say something about it until I read your email.
> 
> Right, so does e.g. int32_to_float_fmul_scalar_array_8 sound sensible as
> name for it?

Or just int32_to_float_fmul_array_8

-Justin
Luca Barbato July 17, 2013, 4:49 p.m. | #11
On 07/17/2013 06:21 PM, Justin Ruggles wrote:
>> Right, so does e.g. int32_to_float_fmul_scalar_array_8 sound sensible as
>> name for it?
> 
> Or just int32_to_float_fmul_array_8

drop the last underscore while at it.

lu

Patch

diff --git a/libavcodec/fmtconvert.c b/libavcodec/fmtconvert.c
index 5b7ab03..0ad54bb 100644
--- a/libavcodec/fmtconvert.c
+++ b/libavcodec/fmtconvert.c
@@ -30,6 +30,12 @@  static void int32_to_float_fmul_scalar_c(float *dst, const int *src, float mul,
         dst[i] = src[i] * mul;
 }
 
+static void int32_to_float_fmul_scalar_array_c(FmtConvertContext *c, float *dst, const int *src, float *mul, int len){
+    int i;
+    for (i = 0; i < len; i += 8)
+        c->int32_to_float_fmul_scalar(&dst[i], &src[i], *mul++, 8);
+}
+
 static av_always_inline int float_to_int16_one(const float *src){
     return av_clip_int16(lrintf(*src));
 }
@@ -79,6 +85,7 @@  void ff_float_interleave_c(float *dst, const float **src, unsigned int len,
 av_cold void ff_fmt_convert_init(FmtConvertContext *c, AVCodecContext *avctx)
 {
     c->int32_to_float_fmul_scalar = int32_to_float_fmul_scalar_c;
+    c->int32_to_float_fmul_scalar_array = int32_to_float_fmul_scalar_array_c;
     c->float_to_int16             = float_to_int16_c;
     c->float_to_int16_interleave  = float_to_int16_interleave_c;
     c->float_interleave           = ff_float_interleave_c;
diff --git a/libavcodec/fmtconvert.h b/libavcodec/fmtconvert.h
index 93c3401..8a1395c 100644
--- a/libavcodec/fmtconvert.h
+++ b/libavcodec/fmtconvert.h
@@ -38,6 +38,20 @@  typedef struct FmtConvertContext {
     void (*int32_to_float_fmul_scalar)(float *dst, const int *src, float mul, int len);
 
     /**
+     * Convert an array of int32_t to float and multiply by a float value from another array,
+     * stepping along the float array once for each 8 integers.
+     * @param c   pointer to FmtConvertContext.
+     * @param dst destination array of float.
+     *            constraints: 16-byte aligned
+     * @param src source array of int32_t.
+     *            constraints: 16-byte aligned
+     * @param mul source array of float multipliers.
+     * @param len number of elements to convert.
+     *            constraints: multiple of 8
+     */
+    void (*int32_to_float_fmul_scalar_array)(struct FmtConvertContext *c, float *dst, const int *src, float *mul, int len);
+
+    /**
      * Convert an array of float to an array of int16_t.
      *
      * Convert floats from in the range [-32768.0,32767.0] to ints