From: "Dai, Jianhui J" <jianhui.j.dai-at-intel.com@ffmpeg.org>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH V1] avcodec/cbs_vp8: Add support for VP8 codec bitstream READ methods
Date: Wed, 17 May 2023 00:40:29 +0000
Message-ID: <DM6PR11MB26817AAEAE52071CFEF3B685B17E9@DM6PR11MB2681.namprd11.prod.outlook.com> (raw)
In-Reply-To: <GV1P250MB0737DD781C971EF0288F7AD18F789@GV1P250MB0737.EURP250.PROD.OUTLOOK.COM>
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Andreas Rheinhardt
> Sent: Monday, May 15, 2023 4:10 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH V1] avcodec/cbs_vp8: Add support for VP8
> codec bitstream READ methods
>
> Dai, Jianhui J:
> > This commit adds VP8 into cbs supported codec list, and enables the
> > `trace_headers` bitstream filters to support VP8, besides existing
> > AV1, H.264, H.265 and VP9. It can be useful to debug VP8 stream issues.
> >
> > Only the READ methods `read_unit` and `split_fragment` are
> > implemented, the WRITE methods `write_unit` and `assemble_fragment`
> > return `AVERROR_PATCHWELCOME` error code. It is because the CBS VP8
> > WRITE is unlikely used by any applications at the moment. The WRITE
> > methods can be added later if there are real requirments.
> >
> > TESTS: ffmpeg -i fate-suite/vp8/frame_size_change.webm -vcodec copy
> > -bsf:v trace_headers -f null -
> >
> > Signed-off-by: Jianhui Dai <jianhui.j.dai@intel.com>
> > ---
> > configure | 4 +-
> > libavcodec/Makefile | 1 +
> > libavcodec/cbs.c | 6 +
> > libavcodec/cbs_internal.h | 1 +
> > libavcodec/cbs_vp8.c | 259 +++++++++++++++++++++++
> > libavcodec/cbs_vp8.h | 116 +++++++++++
> > libavcodec/cbs_vp8_syntax_template.c | 294
> > +++++++++++++++++++++++++++
> > 7 files changed, 680 insertions(+), 1 deletion(-) create mode 100644
> > libavcodec/cbs_vp8.c create mode 100644 libavcodec/cbs_vp8.h create
> > mode 100644 libavcodec/cbs_vp8_syntax_template.c
> >
> > diff --git a/configure b/configure
> > index bb7be67676..6ba7759e34 100755
> > --- a/configure
> > +++ b/configure
> > @@ -2433,6 +2433,7 @@ CONFIG_EXTRA="
> > cbs_jpeg
> > cbs_mpeg2
> > cbs_vp9
> > + cbs_vp8
>
> vp8 should be before vp9 in all these lists.
Thank for your review. ACK.
>
> > deflate_wrapper
> > dirac_parse
> > dnn
> > @@ -2714,6 +2715,7 @@ cbs_h265_select="cbs"
> > cbs_jpeg_select="cbs"
> > cbs_mpeg2_select="cbs"
> > cbs_vp9_select="cbs"
> > +cbs_vp8_select="cbs"
> > dct_select="rdft"
> > deflate_wrapper_deps="zlib"
> > dirac_parse_select="golomb"
> > @@ -3284,7 +3286,7 @@ h264_redundant_pps_bsf_select="cbs_h264"
> > hevc_metadata_bsf_select="cbs_h265"
> > mjpeg2jpeg_bsf_select="jpegtables"
> > mpeg2_metadata_bsf_select="cbs_mpeg2"
> > -trace_headers_bsf_select="cbs"
> > +trace_headers_bsf_select="cbs cbs_vp8"
> > vp9_metadata_bsf_select="cbs_vp9"
> >
> > # external libraries
> > diff --git a/libavcodec/Makefile b/libavcodec/Makefile index
> > 3cf4444b7e..a4e5d7d7a5 100644
> > --- a/libavcodec/Makefile
> > +++ b/libavcodec/Makefile
> > @@ -79,6 +79,7 @@ OBJS-$(CONFIG_CBS_H265) += cbs_h2645.o
> cbs_sei.o h2645_parse.o
> > OBJS-$(CONFIG_CBS_JPEG) += cbs_jpeg.o
> > OBJS-$(CONFIG_CBS_MPEG2) += cbs_mpeg2.o
> > OBJS-$(CONFIG_CBS_VP9) += cbs_vp9.o
> > +OBJS-$(CONFIG_CBS_VP8) += cbs_vp8.o vpx_rac.o
> > OBJS-$(CONFIG_CRYSTALHD) += crystalhd.o
> > OBJS-$(CONFIG_DCT) += dct.o dct32_fixed.o dct32_float.o
> > OBJS-$(CONFIG_DEFLATE_WRAPPER) += zlib_wrapper.o
> > diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c index
> > 504197e06d..f4ff05181f 100644
> > --- a/libavcodec/cbs.c
> > +++ b/libavcodec/cbs.c
> > @@ -49,6 +49,9 @@ static const CodedBitstreamType *const
> > cbs_type_table[] = { #if CONFIG_CBS_VP9
> > &ff_cbs_type_vp9,
> > #endif
> > +#if CONFIG_CBS_VP8
> > + &ff_cbs_type_vp8,
> > +#endif
> > };
> >
> > const enum AVCodecID ff_cbs_all_codec_ids[] = { @@ -69,6 +72,9 @@
> > const enum AVCodecID ff_cbs_all_codec_ids[] = { #endif #if
> > CONFIG_CBS_VP9
> > AV_CODEC_ID_VP9,
> > +#endif
> > +#if CONFIG_CBS_VP8
> > + AV_CODEC_ID_VP8,
> > #endif
> > AV_CODEC_ID_NONE
> > };
> > diff --git a/libavcodec/cbs_internal.h b/libavcodec/cbs_internal.h
> > index e585c77934..9ea6393a9f 100644
> > --- a/libavcodec/cbs_internal.h
> > +++ b/libavcodec/cbs_internal.h
> > @@ -248,6 +248,7 @@ extern const CodedBitstreamType ff_cbs_type_h265;
> > extern const CodedBitstreamType ff_cbs_type_jpeg; extern const
> > CodedBitstreamType ff_cbs_type_mpeg2; extern const CodedBitstreamType
> > ff_cbs_type_vp9;
> > +extern const CodedBitstreamType ff_cbs_type_vp8;
> >
> >
> > #endif /* AVCODEC_CBS_INTERNAL_H */
> > diff --git a/libavcodec/cbs_vp8.c b/libavcodec/cbs_vp8.c new file mode
> > 100644 index 0000000000..ee5c5cbceb
> > --- /dev/null
> > +++ b/libavcodec/cbs_vp8.c
> > @@ -0,0 +1,259 @@
> > +/*
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * FFmpeg is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with FFmpeg; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> > +02110-1301 USA */
> > +
> > +#include "libavutil/avassert.h"
> > +
> > +#include "cbs.h"
> > +#include "cbs_internal.h"
> > +#include "cbs_vp8.h"
> > +
> > +#include "vp8data.h"
> > +
> > +static av_unused int
> > +cbs_vp8_trace_syntax_element(CodedBitstreamContext *ctx,
>
> Why is this marked as av_unused?
ACK
>
> > + int pos, const char *name,
> > + const int *subscripts,
> > + int32_t value, int
> > +width) {
> > + if (ctx->trace_enable) {
> > + char bits[33] = "-";
> > +
> > + av_assert0(width >= 0 && width <= 24);
> > + if (width > 0) {
> > + int i;
> > + for (i = 0; i < width; i++)
> > + bits[i] = value >> (width - i - 1) & 1 ? '1' : '0';
> > + bits[i] = 0;
> > + }
> > +
> > + ff_cbs_trace_syntax_element(ctx, pos, name, subscripts, bits, value);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int cbs_vp8_read(CodedBitstreamContext *ctx, GetBitContext *gbc,
> > + int width, int32_t *write_to) {
> > + uint32_t value;
> > +
> > + av_assert0(width > 0 && width <= 24);
> > +
> > + if (get_bits_left(gbc) < width + 1) {
>
> Why the +1?
ACK
>
> > + av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid value: bitstream
> ended.\n");
> > + return AVERROR_INVALIDDATA;
> > + }
> > +
> > + value = get_bits_le(gbc, width);
>
> You are reading from a bitstream reader that has not been opened as little-
> endian. This will work, but it is not how it is supposed to be.
ACK
>
> > + *write_to = value;
> > + return 0;
> > +}
> > +
> > +static av_unused int cbs_vp8_rac_get(VPXRangeCoder *c, int bits) {
> > + int value = 0;
> > +
> > + while (bits--)
> > + value = (value << 1) | vpx_rac_get_prob(c, 128);
> > +
> > + return value;
> > +}
> > +
> > +static int cbs_vp8_rac_get_bits_count(GetBitContext *gbc,
> > +VPXRangeCoder *c) {
> > + int bits_count = (c->buffer - gbc->buffer) * 8;
> > + bits_count += c->bits;
>
> Your mixing of VPXRangeCoder and GetBitContext is incredibly hacky; if I see
> this correctly, then you are not even advancing the GetBitContext position for all
> the rac stuff that you are reading (meaning that the byte-alignment code in
> trailing_bits() is nonsense).
Right.
VP8 bitstream comprise 3 parts: Frame Tag (uncompressed header), Frame Header (Boolean Entropy Encoder) and MBs.
The `Frame Tag` is parsed by GetBitContext, ` Frame Header` is parsed by VPXRangeCoder.
I should check trailing_bits() after FrameHeader. Will fix it
> Furthermore, the rac functions do not report when they run out of input, as is
> common for CBS. You should probably write CBS versions of them that closely
> follow the spec (read: that need not be optimized for speed).
I would like the cbs_vp8 to be lightweight, that reuse existing VPXRangeCoder.
I will check and fix it.
>
> > + return bits_count;
> > +}
> > +
> > +#define HEADER(name) \
> > + do { \
> > + ff_cbs_trace_header(ctx, name); \
> > + } while (0)
> > +
> > +#define CHECK(call) \
> > + do { \
> > + err = (call); \
> > + if (err < 0) \
> > + return err; \
> > + } while (0)
> > +
> > +#define FUNC_NAME(rw, codec, name) cbs_##codec##_##rw##_##name
> > +#define FUNC_VP8(rw, name) FUNC_NAME(rw, vp8, name) #define
> > +FUNC(name) FUNC_VP8(READWRITE, name)
> > +
> > +#define SUBSCRIPTS(subs, ...) (subs > 0 ? ((int[subs + 1]){ subs,
> > +__VA_ARGS__ }) : NULL)
> > +
> > +#define vp8_f(width, name) vp8_xf(width, name, current->name)
> > +
> > +#define vp8_rac_unsigned(width, name) \
> > + do { \
> > + uint32_t value; \
> > + int pos = cbs_vp8_rac_get_bits_count(rw, c); \
> > + value = cbs_vp8_rac_get(c, width); \
> > + cbs_vp8_trace_syntax_element(ctx, pos, #name, 0, value, 0); \
>
> IMO it is better to have a function cbs_vp8_rac_read_unsigned() for this;
> vp8_rac_unsigned and vp8_rac_unsigned_subs would then just be wrappers
> around this function like it is done for xs() in cbs_vp9.
ACK
>
> > + current->name = value; \
> > + } while (0)
> > +
> > +#define vp8_rac_unsigned_subs(width, name, subs, ...) \
> > + do { \
> > + uint32_t value; \
> > + int pos = cbs_vp8_rac_get_bits_count(rw, c); \
> > + value = cbs_vp8_rac_get(c, width); \
> > + cbs_vp8_trace_syntax_element(ctx, pos, #name, \
> > + SUBSCRIPTS(subs, __VA_ARGS__), value, 0); \
> > + current->name = value; \
> > + } while (0)
> > +
> > +#define vp8_rac_signed(width, name) \
> > + do { \
> > + uint32_t value; \
>
> Why is this not signed? Same below.
ACK
>
> > + int pos = cbs_vp8_rac_get_bits_count(rw, c); \
> > + value = cbs_vp8_rac_get(c, width); \
> > + if (cbs_vp8_rac_get(c, 1)) \
> > + value = -value; \
> > + cbs_vp8_trace_syntax_element(ctx, pos, #name, 0, value, 0); \
>
> Same as above for vp8_rac_unsigned.
ACK
>
> > + current->name = value; \
> > + } while (0)
> > +
> > +#define vp8_rac_signed_subs(width, name, subs, ...) \
> > + do { \
> > + uint32_t value; \
> > + int pos = cbs_vp8_rac_get_bits_count(rw, c); \
> > + value = cbs_vp8_rac_get(c, width); \
> > + if (cbs_vp8_rac_get(c, 1)) \
> > + value = -value; \
> > + cbs_vp8_trace_syntax_element(ctx, pos, #name, \
> > + SUBSCRIPTS(subs, __VA_ARGS__), value, 0); \
> > + current->name = value; \
> > + } while (0)
> > +
> > +#define READ
> > +#define READWRITE read
> > +#define RWContext GetBitContext
> > +
> > +#define vp8_xf(width, name, var) \
> > + do { \
> > + uint32_t value; \
> > + int pos = get_bits_count(rw); \
> > + CHECK(cbs_vp8_read(ctx, rw, width, &value)); \
> > + var = value; \
> > + cbs_vp8_trace_syntax_element(ctx, pos, #name, 0, value, width); \
> > + } while (0)
> > +
> > +#define fixed(width, name, value) \
> > + do { \
> > + av_unused uint32_t fixed_value; \
> > + CHECK(ff_cbs_read_unsigned(ctx, rw, width, #name, 0, &fixed_value, \
> > + value, value)); \
> > + } while (0)
> > +
> > +#define byte_alignment(rw) (get_bits_count(rw) % 8)
> > +
> > +#include "cbs_vp8_syntax_template.c"
> > +
> > +static int cbs_vp8_split_fragment(CodedBitstreamContext *ctx,
> > + CodedBitstreamFragment *frag, int
> > +header) {
> > + int err;
> > +
> > + if (frag->data_size == 0)
> > + return AVERROR_INVALIDDATA;
> > +
> > + err = ff_cbs_append_unit_data(frag, 0, frag->data, frag->data_size,
> > + frag->data_ref);
> > + if (err < 0)
> > + return err;
> > +
> > + return 0;
> > +}
> > +
> > +static int cbs_vp8_read_unit(CodedBitstreamContext *ctx,
> > + CodedBitstreamUnit *unit) {
> > + VP8RawFrame *frame;
> > + GetBitContext gbc;
> > + int err, pos;
> > +
> > + err = init_get_bits(&gbc, unit->data, 8 * unit->data_size);
> > + if (err < 0)
> > + return err;
> > +
> > + err = ff_cbs_alloc_unit_content(ctx, unit);
> > + if (err < 0)
> > + return err;
> > + frame = unit->content;
> > +
> > + err = cbs_vp8_read_frame(ctx, &gbc, frame);
> > + if (err < 0)
> > + return err;
> > +
> > + pos = get_bits_count(&gbc);
> > + av_assert0(pos % 8 == 0);
> > + pos /= 8;
> > + av_assert0(pos <= unit->data_size);
> > +
> > + if (pos == unit->data_size)
> > + return AVERROR_INVALIDDATA;
> > +
> > + frame->data_ref = av_buffer_ref(unit->data_ref);
> > + if (!frame->data_ref)
> > + return AVERROR(ENOMEM);
> > +
> > + frame->data = unit->data + pos;
> > + frame->data_size = unit->data_size - pos;
> > +
> > + return 0;
> > +}
> > +
> > +static int cbs_vp8_write_unit(CodedBitstreamContext *ctx,
> > + CodedBitstreamUnit *unit, PutBitContext
> > +*pbc) {
> > + return AVERROR_PATCHWELCOME;
> > +}
> > +
> > +static int cbs_vp8_assemble_fragment(CodedBitstreamContext *ctx,
> > + CodedBitstreamFragment *frag) {
> > + return AVERROR_PATCHWELCOME;
> > +}
> > +
> > +static void cbs_vp8_flush(CodedBitstreamContext *ctx) {
> > + return;
> > +}
>
> No need for a useless flush. Just leave the function empty.
> (One could also do the same for the write_unit and assemble_fragment
> function: Nothing should call these functions at all.)
ACK
>
> > +
> > +static const CodedBitstreamUnitTypeDescriptor cbs_vp8_unit_types[] = {
> > + CBS_UNIT_TYPE_INTERNAL_REF(0, VP8RawFrame, data),
> > + CBS_UNIT_TYPE_END_OF_LIST
> > +};
> > +
> > +const CodedBitstreamType ff_cbs_type_vp8 = {
> > + .codec_id = AV_CODEC_ID_VP8,
> > +
> > + .priv_data_size = sizeof(CodedBitstreamVP8Context),
> > +
> > + .unit_types = cbs_vp8_unit_types,
> > +
> > + .split_fragment = &cbs_vp8_split_fragment,
> > + .read_unit = &cbs_vp8_read_unit,
> > + .write_unit = &cbs_vp8_write_unit,
> > +
> > + .flush = &cbs_vp8_flush,
> > +
> > + .assemble_fragment = &cbs_vp8_assemble_fragment, };
> > diff --git a/libavcodec/cbs_vp8.h b/libavcodec/cbs_vp8.h new file mode
> > 100644 index 0000000000..176d95853b
> > --- /dev/null
> > +++ b/libavcodec/cbs_vp8.h
> > @@ -0,0 +1,116 @@
> > +/*
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * FFmpeg is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with FFmpeg; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> > +02110-1301 USA */
> > +
> > +#ifndef AVCODEC_CBS_VP8_H
> > +#define AVCODEC_CBS_VP8_H
> > +
> > +#include <stddef.h>
> > +#include <stdint.h>
> > +
> > +#include "cbs.h"
> > +#include "vpx_rac.h"
> > +
> > +enum {
> > + VP8_START_CODE_0 = 0x9D,
> > + VP8_START_CODE_1 = 0x01,
> > + VP8_START_CODE_2 = 0x2A,
> > +};
> > +
> > +enum {
> > + VP8_KEY_FRAME = 0,
> > + VP8_NON_KEY_FRAME = 1,
> > +};
> > +
> > +typedef struct VP8RawFrameHeader {
> > + // frame tag
> > + uint8_t frame_type;
> > + uint8_t profile;
> > + uint8_t show_frame;
> > + uint32_t first_partition_length_in_bytes;
> > +
> > + uint32_t start_code_0;
> > + uint32_t start_code_1;
> > + uint32_t start_code_2;
> > +
> > + uint16_t width;
> > + uint16_t horizontal_scale;
> > + uint16_t height;
> > + uint16_t vertical_scale;
> > +
> > + // frame header
> > + uint8_t color_space;
> > + uint8_t clamping_type;
> > +
> > + // segmentation
> > + uint8_t segmentation_enabled;
> > + uint8_t update_segment_map;
> > + uint8_t update_segment_feature_data;
> > + uint8_t segment_feature_mode;
> > + int8_t segment_update_qp[4];
> > + int8_t segment_update_loop_filter_level[4];
> > + uint8_t segment_update_probs[3];
> > +
> > + // loop filter
> > + uint8_t loop_filter_type;
> > + uint8_t loop_filter_level;
> > + uint8_t loop_filter_sharpness;
> > + uint8_t mode_ref_lf_delta_enabled;
> > + int8_t ref_lf_deltas[4];
> > + int8_t mode_lf_deltas[4];
> > +
> > + uint8_t log2_token_partitions;
> > +
> > + // qp
> > + uint8_t base_qindex;
> > + int8_t y1dc_delta_q;
> > + int8_t y2dc_delta_q;
> > + int8_t y2ac_delta_q;
> > + int8_t uvdc_delta_q;
> > + int8_t uvac_delta_q;
> > +
> > + // ref
> > + uint8_t refresh_golden_frame;
> > + uint8_t refresh_alternate_frame;
> > + uint8_t ref_frame_sign_bias_golden;
> > + uint8_t ref_frame_sign_bias_alternate;
> > + uint8_t refresh_last_frame;
> > +
> > + // token probs
> > + uint8_t refresh_entropy_probs;
> > +
> > + uint8_t mb_no_skip_coeff;
> > + uint8_t prob_skip_false;
> > +
> > + uint8_t prob_intra;
> > + uint8_t prob_last;
> > + uint8_t prob_golden;
> > +} VP8RawFrameHeader;
> > +
> > +typedef struct VP8RawFrame {
> > + VP8RawFrameHeader header;
> > +
> > + uint8_t *data;
> > + AVBufferRef *data_ref;
> > + size_t data_size;
> > +} VP8RawFrame;
> > +
> > +typedef struct CodedBitstreamVP8Context {
> > + VPXRangeCoder c;
> > +} CodedBitstreamVP8Context;
> > +
> > +#endif /* AVCODEC_CBS_VP8_H */
> > diff --git a/libavcodec/cbs_vp8_syntax_template.c
> > b/libavcodec/cbs_vp8_syntax_template.c
> > new file mode 100644
> > index 0000000000..24f4075dab
> > --- /dev/null
> > +++ b/libavcodec/cbs_vp8_syntax_template.c
> > @@ -0,0 +1,294 @@
> > +/*
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * FFmpeg is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with FFmpeg; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> > +02110-1301 USA */
> > +
> > +static int FUNC(update_segmentation)(CodedBitstreamContext *ctx,
> RWContext *rw,
> > + VPXRangeCoder *c,
> > + VP8RawFrameHeader *current) {
> > +#ifdef READ
> > + int i;
>
> We allow (and prefer) C99 for loops with variable declarations.
ACK
>
> > +
> > + vp8_rac_unsigned(1, update_segment_map);
> > + vp8_rac_unsigned(1, update_segment_feature_data);
> > +
> > + if (current->update_segment_feature_data) {
> > + vp8_rac_unsigned(1, segment_feature_mode);
> > + // quantizer
> > + for (i = 0; i < 4; i++)
> > + if (cbs_vp8_rac_get(c, 1))
> > + vp8_rac_signed_subs(7, segment_update_qp[i], 1, i);
> > + // loop filter
> > + for (i = 0; i < 4; i++) {
> > + if (cbs_vp8_rac_get(c, 1)) {
> > + vp8_rac_signed_subs(6, segment_update_loop_filter_level[i], 1,
> > + i);
> > + }
> > + }
> > + }
> > +
> > + if (current->update_segment_map) {
> > + for (i = 0; i < 3; i++)
> > + if (cbs_vp8_rac_get(c, 1))
> > + vp8_rac_unsigned_subs(8, segment_update_probs[i], 1, i);
> > + }
> > +
> > + return 0;
> > +#else
> > + return AVERROR_PATCHWELCOME;
> > +#endif
>
> IMO there is no need for these #else branches given that this file is not included
> for anything but reading.
The intention is that possible we want the cbs_vp8 WRITE methods in near future.
If the WRITE methods are unlikely needed, I will remove it.
>
> > +}
> > +
> > +static int FUNC(mode_ref_lf_deltas)(CodedBitstreamContext *ctx,
> > + RWContext *rw, VPXRangeCoder *c,
> > + VP8RawFrameHeader
> > +*current) { #ifdef READ
> > + vp8_rac_unsigned(1, mode_ref_lf_delta_enabled);
> > + if (current->mode_ref_lf_delta_enabled) {
> > + if (cbs_vp8_rac_get(c, 1)) {
> > + int i;
> > + // ref_lf_deltas
> > + for (i = 0; i < 4; i++)
> > + if (cbs_vp8_rac_get(c, 1))
> > + vp8_rac_signed_subs(6, ref_lf_deltas[i], 1, i);
> > + // mode_lf_deltas
> > + for (i = 0; i < 4; i++)
> > + if (cbs_vp8_rac_get(c, 1))
> > + vp8_rac_signed_subs(6, mode_lf_deltas[i], 1, i);
> > + }
> > + }
> > +
> > + return 0;
> > +#else
> > + return AVERROR_PATCHWELCOME;
> > +#endif
> > +}
> > +
> > +static int FUNC(quantization_params)(CodedBitstreamContext *ctx,
> RWContext *rw,
> > + VPXRangeCoder *c,
> > +VP8RawFrameHeader *current) { #ifdef READ
> > + vp8_rac_unsigned(7, base_qindex);
> > +
> > + if (cbs_vp8_rac_get(c, 1))
> > + vp8_rac_signed(4, y1dc_delta_q);
> > +
> > + if (cbs_vp8_rac_get(c, 1))
> > + vp8_rac_signed(4, y2dc_delta_q);
> > +
> > + if (cbs_vp8_rac_get(c, 1))
> > + vp8_rac_signed(4, y2ac_delta_q);
> > +
> > + if (cbs_vp8_rac_get(c, 1))
> > + vp8_rac_signed(4, uvdc_delta_q);
> > +
> > + if (cbs_vp8_rac_get(c, 1))
> > + vp8_rac_signed(4, uvac_delta_q);
> > +
> > + return 0;
> > +#else
> > + return AVERROR_PATCHWELCOME;
> > +#endif
> > +}
> > +
> > +static int FUNC(update_token_probs)(CodedBitstreamContext *ctx,
> RWContext *rw,
> > + VPXRangeCoder *c,
> > + VP8RawFrameHeader *current) {
> > +#ifdef READ
> > + int i, j, k, l;
> > +
> > + for (i = 0; i < 4; ++i) {
> > + for (j = 0; j < 8; ++j) {
> > + for (k = 0; k < 3; ++k) {
> > + for (l = 0; l < 11; ++l)
> > + if (vpx_rac_get_prob_branchy(
> > + c, vp8_token_update_probs[i][j][k][l]))
> > + cbs_vp8_rac_get(c, 8);
> > + }
> > + }
> > + }
> > +
> > + return 0;
> > +#else
> > + return AVERROR_PATCHWELCOME;
> > +#endif
> > +}
> > +
> > +static int FUNC(update_mv_probs)(CodedBitstreamContext *ctx, RWContext
> *rw,
> > + VPXRangeCoder *c, VP8RawFrameHeader
> > +*current) { #ifdef READ
> > + int i, j;
> > +
> > + for (i = 0; i < 2; ++i) {
> > + for (j = 0; j < 19; ++j)
> > + if (cbs_vp8_rac_get(c, 1))
> > + cbs_vp8_rac_get(c, 7);
> > + }
> > +
> > + return 0;
> > +#else
> > + return AVERROR_PATCHWELCOME;
> > +#endif
> > +}
> > +
> > +static int FUNC(frame_tag)(CodedBitstreamContext *ctx, RWContext *rw,
> > + VP8RawFrameHeader *current) { #ifdef READ
> > + int err;
> > +
> > + vp8_f(1, frame_type);
> > + vp8_f(3, profile);
> > + vp8_f(1, show_frame);
> > + vp8_f(19, first_partition_length_in_bytes);
> > +
> > + if (current->frame_type == VP8_KEY_FRAME) {
> > + fixed(8, start_code_0, VP8_START_CODE_0);
> > + fixed(8, start_code_1, VP8_START_CODE_1);
> > + fixed(8, start_code_2, VP8_START_CODE_2);
> > +
> > + vp8_f(14, width);
> > + vp8_f(2, horizontal_scale);
> > + vp8_f(14, height);
> > + vp8_f(2, vertical_scale);
> > + }
> > +
> > + return 0;
> > +#else
> > + return AVERROR_PATCHWELCOME;
> > +#endif
> > +}
> > +
> > +static int FUNC(frame_header)(CodedBitstreamContext *ctx, RWContext *rw,
> > + VP8RawFrameHeader *current) { #ifdef
> > +READ
> > + CodedBitstreamVP8Context *vp8 = ctx->priv_data;
> > + VPXRangeCoder *c = &vp8->c;
> > + int header_size;
> > + int pos;
> > + int err;
> > +
> > + pos = get_bits_count(rw);
> > + av_assert0(pos % 8 == 0);
> > + pos /= 8;
> > +
> > + header_size = current->first_partition_length_in_bytes;
> > +
> > + err = ff_vpx_init_range_decoder(c, rw->buffer + pos, header_size);
> > + if (err < 0)
> > + return err;
> > +
> > + if (current->frame_type == VP8_KEY_FRAME) {
> > + vp8_rac_unsigned(1, color_space);
> > + vp8_rac_unsigned(1, clamping_type);
> > + }
> > +
> > + vp8_rac_unsigned(1, segmentation_enabled);
> > + if (current->segmentation_enabled)
> > + CHECK(FUNC(update_segmentation)(ctx, rw, c, current));
> > +
> > + vp8_rac_unsigned(1, loop_filter_type);
> > + vp8_rac_unsigned(6, loop_filter_level);
> > + vp8_rac_unsigned(3, loop_filter_sharpness);
> > +
> > + CHECK(FUNC(mode_ref_lf_deltas)(ctx, rw, c, current));
> > +
> > + vp8_rac_unsigned(2, log2_token_partitions);
> > +
> > + CHECK(FUNC(quantization_params)(ctx, rw, c, current));
> > +
> > + if (current->frame_type != VP8_KEY_FRAME) {
> > + vp8_rac_unsigned(1, refresh_golden_frame);
> > + vp8_rac_unsigned(1, refresh_alternate_frame);
> > + if (!current->refresh_golden_frame)
> > + cbs_vp8_rac_get(c, 2);
> > + if (!current->refresh_alternate_frame)
> > + cbs_vp8_rac_get(c, 2);
> > + vp8_rac_unsigned(1, ref_frame_sign_bias_golden);
> > + vp8_rac_unsigned(1, ref_frame_sign_bias_alternate);
> > + }
> > + vp8_rac_unsigned(1, refresh_entropy_probs);
> > + if (current->frame_type != VP8_KEY_FRAME)
> > + vp8_rac_unsigned(1, refresh_last_frame);
> > +
> > + CHECK(FUNC(update_token_probs)(ctx, rw, c, current));
> > +
> > + vp8_rac_unsigned(1, mb_no_skip_coeff);
> > + if (current->mb_no_skip_coeff)
> > + vp8_rac_unsigned(8, prob_skip_false);
> > +
> > + if (current->frame_type != VP8_KEY_FRAME) {
> > + int i;
> > +
> > + vp8_rac_unsigned(8, prob_intra);
> > + vp8_rac_unsigned(8, prob_last);
> > + vp8_rac_unsigned(8, prob_golden);
> > +
> > + // intra_16x16_prob
> > + if (cbs_vp8_rac_get(c, 1))
> > + for (i = 0; i < 4; i++)
> > + cbs_vp8_rac_get(c, 8);
> > +
> > + // intra_chroma_prob
> > + if (cbs_vp8_rac_get(c, 1))
> > + for (i = 0; i < 4; i++)
> > + cbs_vp8_rac_get(c, 8);
> > +
> > + CHECK(FUNC(update_mv_probs)(ctx, rw, c, current));
> > + }
> > +
> > + return 0;
> > +#else
> > + return AVERROR_PATCHWELCOME;
> > +#endif
> > +}
> > +
> > +static int FUNC(trailing_bits)(CodedBitstreamContext *ctx, RWContext
> > +*rw) { #ifdef READ
> > + int err;
> > +
> > + while (byte_alignment(rw) != 0)
> > + fixed(1, zero_bit, 0);
> > +
> > + return 0;
> > +#else
> > + return AVERROR_PATCHWELCOME;
> > +#endif
> > +}
> > +
> > +static int FUNC(frame)(CodedBitstreamContext *ctx, RWContext *rw,
> > + VP8RawFrame *current) { #ifdef READ
> > + int err;
> > +
> > + HEADER("Frame");
> > +
> > + CHECK(FUNC(frame_tag)(ctx, rw, ¤t->header));
> > + CHECK(FUNC(frame_header)(ctx, rw, ¤t->header));
> > + CHECK(FUNC(trailing_bits)(ctx, rw));
> > +
> > + return 0;
> > +#else
> > + return AVERROR_PATCHWELCOME;
> > +#endif
> > +}
>
> _______________________________________________
> 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".
_______________________________________________
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".
prev parent reply other threads:[~2023-05-17 0:40 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-15 5:56 Dai, Jianhui J
2023-05-15 8:09 ` Andreas Rheinhardt
2023-05-17 0:40 ` Dai, Jianhui J [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DM6PR11MB26817AAEAE52071CFEF3B685B17E9@DM6PR11MB2681.namprd11.prod.outlook.com \
--to=jianhui.j.dai-at-intel.com@ffmpeg.org \
--cc=ffmpeg-devel@ffmpeg.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git