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

2024-04-03 Thread Andreas Rheinhardt 
Anton Khirnov:
> Quoting Andreas Rheinhardt (2024-03-31 07:30:26)
>> Signed-off-by: Andreas Rheinhardt 
>> ---
>>  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) &&
> 
> Weren't we moving towards getting rid of our dependency on DCE?
> I recall some discussions about this in recent years, though I don't
> remember if there was a consensus.
> 

This patch does not add any dependency on DCE, because the code block
currently #if'ed away does not contain anything that would cause a
linking failure with a compiler not performing DCE.

- Andreas

___
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".

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

2024-04-03 Thread Anton Khirnov 
Quoting Andreas Rheinhardt (2024-03-31 07:30:26)
> Signed-off-by: Andreas Rheinhardt 
> ---
>  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) &&

Weren't we moving towards getting rid of our dependency on DCE?
I recall some discussions about this in recent years, though I don't
remember if there was a consensus.

-- 
Anton Khirnov
___
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".

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

2024-04-02 Thread James Zern via ffmpeg-devel 
On Mon, Apr 1, 2024 at 11:29 AM Andreas Rheinhardt
 wrote:
>
> James Zern via ffmpeg-devel:
> > On Sat, Mar 30, 2024 at 10:30 PM Andreas Rheinhardt
> >  wrote:
> >>
> >> Signed-off-by: Andreas Rheinhardt 
> >> ---
> >>  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.
___
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".

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

2024-04-01 Thread Andreas Rheinhardt 
James Zern via ffmpeg-devel:
> On Sat, Mar 30, 2024 at 10:30 PM Andreas Rheinhardt
>  wrote:
>>
>> Signed-off-by: Andreas Rheinhardt 
>> ---
>>  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.

> 
>>  // Keep HDR10+ if it has bit depth higher than 8 and
>>  // it has PQ trc (SMPTE2084).
>>  enccfg->g_bit_depth > 8 && avctx->color_trc == AVCOL_TRC_SMPTE2084) 
>> {
>> @@ -372,7 +374,6 @@ static int frame_data_submit(AVCodecContext *avctx, 
>> AVFifo *fifo,
>>  return AVERROR(ENOMEM);
>>  }
>>  }
>> -#endif
>>
>>  fd.duration = frame->duration;
>>  fd.frame_opaque = frame->opaque;
>> --
>> 2.40.1

___
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".

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

2024-04-01 Thread James Zern via ffmpeg-devel 
On Sat, Mar 30, 2024 at 10:30 PM Andreas Rheinhardt
 wrote:
>
> Signed-off-by: Andreas Rheinhardt 
> ---
>  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.

>  // Keep HDR10+ if it has bit depth higher than 8 and
>  // it has PQ trc (SMPTE2084).
>  enccfg->g_bit_depth > 8 && avctx->color_trc == AVCOL_TRC_SMPTE2084) {
> @@ -372,7 +374,6 @@ static int frame_data_submit(AVCodecContext *avctx, 
> AVFifo *fifo,
>  return AVERROR(ENOMEM);
>  }
>  }
> -#endif
>
>  fd.duration = frame->duration;
>  fd.frame_opaque = frame->opaque;
> --
> 2.40.1
>
___
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".