Re: [FFmpeg-devel] Addition of encoder functionality in libavcodec/vpx_rac.h and corresponding commit in vpx_rac.c

Re: [FFmpeg-devel] Addition of encoder functionality in libavcodec/vpx_rac.h and corresponding commit in vpx_rac.c

[Peter Ross]

[-- Attachment #1.1: Type: text/plain, Size: 6900 bytes --]

Hi Amogh,

On Sun, Apr 27, 2025 at 10:38:57PM +0530, Amogh Kumar Sinha wrote:
>    Dear Sir,
>    I am sending this patch for adding encoder functionality, as the
>    Qualification Task for GSoC 2025. Kindly look into it:

Ok, this "diff" comes at the last minute and its not quite ready for
inclusion into FFmpeg. The deadline for GSoC student selection is in
~24hours, which makes this kind of difficult. I will give you some comments.

Firstly, patches need to be sent to ffmpeg-devel. You must be subscribed to
ffmpeg-devel before attempting to send to it. Instructions:
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Secondly, patches needs to be presented in git-format-patch style.
Instructions for setting up email/git here: 
https://ffmpeg.org/developer.html#Submitting-patches-1

I have managed to apply your diffs and test your code with a simple program.
Below is my test code. Something is not quite right with your encoder,
as the range decoder output does not match the input the encoder.
I don't have time tonight to dig deeper into why.

```
#include <stdint.h>
#include <stdio.h>
#include "libavcodec/vpx_rac.h"
int main()
{
    uint8_t buf[64] = {0};

    VPXRangeCoder e;
    ff_vpx_init_range_encoder(&e, buf, sizeof(buf));
    vpx_rac_put(&e, 1);
    vpx_rac_put(&e, 1);
    vpx_rac_put(&e, 1);
    vpx_rac_put(&e, 0);
    vpx_rac_put(&e, 1);
    vpx_rac_put(&e, 0);
    ff_vpx_range_encoder_flush(&e);

    printf("buf: %02x %02x %02x %02x\n", buf[0], buf[1], buf[2], buf[3]);

    VPXRangeCoder d;
    ff_vpx_init_range_decoder(&d, buf, sizeof(buf));
    printf("%d\n", vpx_rac_get(&d));
    printf("%d\n", vpx_rac_get(&d));
    printf("%d\n", vpx_rac_get(&d));
    printf("%d\n", vpx_rac_get(&d));
    printf("%d\n", vpx_rac_get(&d));
    printf("%d\n", vpx_rac_get(&d));

    return 0;
}
```

The program is expected to output 111010, but currently gives 111000.


>    Commit to vpx_rac.h:
>    +++ vpx_rac.h 2025-04-27 21:34:45
>    @@ -20,7 +20,7 @@
> 
>     /**
>      * @file
>    - * Common VP5-VP9 range decoder stuff
>    + * Common VP5-VP9 range decoder/encoder stuff
>      */
> 
>     #ifndef AVCODEC_VPX_RAC_H
>    \ No newline at end of file
>    @@ -40,10 +40,17 @@
>         const uint8_t *end;
>         unsigned int code_word;
>         int end_reached;
>    +
>    +    // For encoder
>    +    unsigned int low;
>    +    uint8_t *output_buffer;
>    +    uint8_t *output_start;
>    +    uint8_t *output_end;
>     } VPXRangeCoder;
> 
>     extern const uint8_t ff_vpx_norm_shift[256];
>     int ff_vpx_init_range_decoder(VPXRangeCoder *c, const uint8_t *buf,
>    int buf_size);
>    +int ff_vpx_init_range_encoder(VPXRangeCoder *c, uint8_t *buf, int
>    buf_size);
> 
>     /**
>      * returns 1 if the end of the stream has been reached, 0 otherwise.
>    \ No newline at end of file
>    @@ -72,6 +79,42 @@
>         return code_word;
>     }
> 
>    +// Encoder related function
>    +static av_always_inline int vpx_rac_renorm_encoder(VPXRangeCoder *c)
>    +{
>    +    while (c->high < 128) {
>    +        c->high <<= 1;
>    +
>    +        if (c->low & 0x8000) {
>    +            if (c->output_buffer > c->output_start) {
>    +                uint8_t *p = c->output_buffer - 1;
>    +                while (p > c->output_start && *p == 0xFF) {
>    +                    *p = 0;
>    +                    p--;
>    +                }
>    +                (*p)++;
>    +            }
>    +        }
>    +
>    +        c->low <<= 1;
>    +        c->low &= 0xFFFF;
>    +
>    +        c->bits++;
>    +        if (c->bits >= 0) {
>    +            if (c->output_buffer >= c->output_end)
>    +                return AVERROR(ENOSPC);
>    +
>    +            *c->output_buffer++ = (c->low >> 8) & 0xFF;
>    +            c->bits -= 8;
>    +        }
>    +    }
>    +
>    +    return 0;
>    +}
>    +
>    +// Encoder related function
>    +int ff_vpx_range_encoder_flush(VPXRangeCoder *c);
>    +
>     #if   ARCH_ARM
>     #include "arm/vpx_arith.h"
>     #elif ARCH_X86
>    \ No newline at end of file
>    @@ -132,4 +175,24 @@
>         return bit;
>     }
> 
>    -#endif /* AVCODEC_VPX_RAC_H */
>    +// Encoder related function
>    +static av_always_inline int vpx_rac_put(VPXRangeCoder *c, int bit)
>    +{
>    +    int ret;
>    +    unsigned int split = (c->high + 1) >> 1;
>    +
>    +    if (bit) {
>    +        c->low += split;
>    +        c->high -= split;
>    +    } else {
>    +        c->high = split;
>    +    }
>    +
>    +    ret = vpx_rac_renorm_encoder(c);
>    +    if (ret < 0)
>    +        return ret;
>    +
>    +    return 0;
>    +}
>    +
>    +#endif /* AVCODEC_VPX_RAC_H */
>    \ No newline at end of file
>    Commit to vpx_rac.c:
>    +++ vpx_rac.c 2025-04-27 22:31:48
>    @@ -51,3 +51,52 @@
>         c->code_word = bytestream_get_be24(&c->buffer);
>         return 0;
>     }
>    +
>    +//Encoder related
>    +int ff_vpx_init_range_encoder(VPXRangeCoder *c, uint8_t *buf, int
>    buf_size)
>    +{
>    +    if (!buf || buf_size < 2)
>    +        return AVERROR(EINVAL);
>    +
>    +    c->high = 255;             // Start with full range
>    +    c->low = 0;                // Initialize low value
>    +    c->bits = -8;              // Initialize bits (negative as in
>    decoder)
>    +    c->output_buffer = buf;
>    +    c->output_start = buf;
>    +    c->output_end = buf + buf_size;
>    +
>    +    // Reserve first 2 bytes for initial code word in decoder
>    +    c->output_buffer += 2;
>    +
>    +    if (c->output_buffer >= c->output_end)
>    +        return AVERROR(EINVAL);
>    +
>    +    return 0;
>    +}
>    +
>    +//Encoder related
>    +int ff_vpx_range_encoder_flush(VPXRangeCoder *c)
>    +{
>    +    int i, ret;
>    +
>    +    for (i = 0; i < 16; i++) {
>    +        ret = vpx_rac_put(c, 0);
>    +        if (ret < 0)
>    +            return ret;
>    +    }
>    +
>    +    if (c->output_start + 1 >= c->output_end)
>    +        return AVERROR(ENOSPC);
>    +
>    +    if (c->output_start + 3 < c->output_end) {
>    +        uint16_t initial_code = (c->output_start[2] << 8) |
>    c->output_start[3];
>    +        c->output_start[0] = initial_code >> 8;
>    +        c->output_start[1] = initial_code & 0xFF;
>    +    } else {
>    +        // Handle the case where we don't have enough encoded data
>    +        c->output_start[0] = 0;
>    +        c->output_start[1] = 0;
>    +    }
>    +
>    +    return c->output_buffer - c->output_start;
>    +}
>    \ No newline at end of file
>    Yours Sincerely,
>    Amogh Kumar Sinha

-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".