[FFmpeg-devel] [PATCH] lavc/cfhd: added alpha decompanding in rgba12

[Aurelien Jacobs]

On Wed, Mar 21, 2018 at 10:19:58PM +0530, Gagandeep Singh wrote:
> alpha decompanding curve added to post process the decoded alpha channel
> ---
>  libavcodec/cfhd.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/libavcodec/cfhd.c b/libavcodec/cfhd.c
> index fd5555834b..e35732df45 100644
> --- a/libavcodec/cfhd.c
> +++ b/libavcodec/cfhd.c
> @@ -37,6 +37,9 @@
>  #include "thread.h"
>  #include "cfhd.h"
>  
> +#define ALPHA_COMPAND_DC_OFFSET 256
> +#define ALPHA_COMPAND_GAIN 9400
> +
>  enum CFHDParam {
>      ChannelCount     =  12,
>      SubbandCount     =  14,
> @@ -94,6 +97,20 @@ static inline int dequant_and_decompand(int level, int quantisation)
>             FFSIGN(level) * quantisation;
>  }
>  
> +static inline void process_alpha(int16_t *alpha, int width)
> +{
> +    int i, channel;
> +    for (i = 0; i < width; i++) {
> +        channel   = alpha[i];
> +        channel  -= ALPHA_COMPAND_DC_OFFSET;
> +        channel <<= 3;
> +        channel  *= ALPHA_COMPAND_GAIN;

Any reason why you can't merge the << 3 (ie. * 8) with the
* ALPHA_COMPAND_GAIN ?

> +        channel >>= 16;

> +        channel   = av_clip_uintp2(channel, 12);
> +        alpha[i]  = channel;

Here you should affect the result of av_clip_uintp2 directly to alpha[i].

Actually, I think it would be more readable by dropping the channel
intermediate variable entirely. You could write this function like this
(untested):

static inline void process_alpha(int16_t *alpha, int width)
{
    for (int i = 0; i < width; i++)
        alpha[i] = av_clip_uintp2(((alpha[i] - 256) * 75200) >> 16, 12);
}

Of course, you can use DC_OFFSET and GAIN constants in there if you
think it is more readable.