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)
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
