[FFmpeg-devel] [PATCH 01/10] avcodec/libvpxenc: Avoid unused-variable warning if VP9 enc is disabled

[James Zern]

On Mon, Apr 1, 2024 at 11:29 AM Andreas Rheinhardt
<andreas.rheinhardt at outlook.com> wrote:
>
> James Zern via ffmpeg-devel:
> > On Sat, Mar 30, 2024 at 10:30 PM Andreas Rheinhardt
> > <andreas.rheinhardt at outlook.com> wrote:
> >>
> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
> >> ---
> >>  libavcodec/libvpxenc.c | 7 ++++---
> >>  1 file changed, 4 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
> >> index 635cdf7a0e..bcbdc4981e 100644
> >> --- a/libavcodec/libvpxenc.c
> >> +++ b/libavcodec/libvpxenc.c
> >> @@ -49,6 +49,9 @@
> >>  #include "libavutil/opt.h"
> >>  #include "libavutil/pixdesc.h"
> >>
> >> +#define IS_VP9(avctx) (CONFIG_LIBVPX_VP9_ENCODER && avctx->codec_id == AV_CODEC_ID_VP9)
> >> +#define IS_VP8(avctx) (CONFIG_LIBVPX_VP8_ENCODER && avctx->codec_id == AV_CODEC_ID_VP8)
> >> +
> >>  /**
> >>   * Portion of struct vpx_codec_cx_pkt from vpx_encoder.h.
> >>   * One encoded frame returned from the library.
> >> @@ -359,8 +362,7 @@ static int frame_data_submit(AVCodecContext *avctx, AVFifo *fifo,
> >>      FrameData fd = { .pts = frame->pts };
> >>      int ret;
> >>
> >> -#if CONFIG_LIBVPX_VP9_ENCODER
> >> -    if (avctx->codec_id == AV_CODEC_ID_VP9 &&
> >> +    if (IS_VP9(avctx) &&
> >
> > This works and I think the style is largely prevalent in other code.
> > Given the current structure you could move the enccfg declaration to
> > this block as an alternative.
>
> The latter would entail either opening a new block for the #if part or
> hoping (due to -Wdeclaration-after-statement) that this #if block stays
> at the start of this function. I prefer my approach above to either of
> these alternatives (a third alternative would be to avoid the enccfg
> variable altogether and to check ctx->encoder.config.enc->g_bit_depth
> instead; another alternative is av_unused). Just tell me which
> alternative you prefer.
>

I only made the comment because I didn't think it would need a new
block as it is, but you're right, code moves around. This is fine and
simpler than having a debate about the warning given the adoption of
other C99/C11 features.