[FFmpeg-devel] [PATCH v1 1/3] avfilter/f_sidedata: try to fix warning: comparison of constant -1 with expression of type 'enum AVFrameSideDataType'

[Michael Niedermayer]

On Sun, Aug 25, 2019 at 12:17:58AM +0800, lance.lmwang at gmail.com wrote:
> From: Limin Wang <lance.lmwang at gmail.com>
> 
> Signed-off-by: Limin Wang <lance.lmwang at gmail.com>
> ---
>  libavfilter/f_sidedata.c | 10 +++++-----
>  libavutil/frame.h        | 10 ++++++++++
>  2 files changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/libavfilter/f_sidedata.c b/libavfilter/f_sidedata.c
> index 381da5a..3082aa6 100644
> --- a/libavfilter/f_sidedata.c
> +++ b/libavfilter/f_sidedata.c
> @@ -49,7 +49,7 @@ static const AVOption filt_name##_options[] = { \
>      { "mode", "set a mode of operation", OFFSET(mode),   AV_OPT_TYPE_INT,    {.i64 = 0 }, 0, SIDEDATA_NB-1, FLAGS, "mode" }, \
>      {   "select", "select frame",        0,              AV_OPT_TYPE_CONST,  {.i64 = SIDEDATA_SELECT }, 0, 0, FLAGS, "mode" }, \
>      {   "delete", "delete side data",    0,              AV_OPT_TYPE_CONST,  {.i64 = SIDEDATA_DELETE }, 0, 0, FLAGS, "mode" }, \
> -    { "type",   "set side data type",    OFFSET(type),   AV_OPT_TYPE_INT,    {.i64 = -1 }, -1, INT_MAX, FLAGS, "type" }, \
> +    { "type",   "set side data type",    OFFSET(type),   AV_OPT_TYPE_INT,    {.i64 = AV_FRAME_DATA_NB }, 0, AV_FRAME_DATA_NB, FLAGS, "type" }, \
>      {   "PANSCAN",                    "", 0,             AV_OPT_TYPE_CONST,  {.i64 = AV_FRAME_DATA_PANSCAN                    }, 0, 0, FLAGS, "type" }, \
>      {   "A53_CC",                     "", 0,             AV_OPT_TYPE_CONST,  {.i64 = AV_FRAME_DATA_A53_CC                     }, 0, 0, FLAGS, "type" }, \
>      {   "STEREO3D",                   "", 0,             AV_OPT_TYPE_CONST,  {.i64 = AV_FRAME_DATA_STEREO3D                   }, 0, 0, FLAGS, "type" }, \
> @@ -79,7 +79,7 @@ static const AVOption filt_name##_options[] = { \
>      { "mode", "set a mode of operation", OFFSET(mode),   AV_OPT_TYPE_INT,    {.i64 = 0 }, 0, SIDEDATA_NB-1, FLAGS, "mode" }, \
>      {   "select", "select frame",        0,              AV_OPT_TYPE_CONST,  {.i64 = SIDEDATA_SELECT }, 0, 0, FLAGS, "mode" }, \
>      {   "delete", "delete side data",    0,              AV_OPT_TYPE_CONST,  {.i64 = SIDEDATA_DELETE }, 0, 0, FLAGS, "mode" }, \
> -    { "type",   "set side data type",    OFFSET(type),   AV_OPT_TYPE_INT,    {.i64 = -1 }, -1, INT_MAX, FLAGS, "type" }, \
> +    { "type",   "set side data type",    OFFSET(type),   AV_OPT_TYPE_INT,    {.i64 = AV_FRAME_DATA_NB }, 0, AV_FRAME_DATA_NB, FLAGS, "type" }, \
>      {   "PANSCAN",                    "", 0,             AV_OPT_TYPE_CONST,  {.i64 = AV_FRAME_DATA_PANSCAN                    }, 0, 0, FLAGS, "type" }, \
>      {   "A53_CC",                     "", 0,             AV_OPT_TYPE_CONST,  {.i64 = AV_FRAME_DATA_A53_CC                     }, 0, 0, FLAGS, "type" }, \
>      {   "STEREO3D",                   "", 0,             AV_OPT_TYPE_CONST,  {.i64 = AV_FRAME_DATA_STEREO3D                   }, 0, 0, FLAGS, "type" }, \
> @@ -107,7 +107,7 @@ static av_cold int init(AVFilterContext *ctx)
>  {
>      SideDataContext *s = ctx->priv;
>  
> -    if (s->type == -1 && s->mode != SIDEDATA_DELETE) {
> +    if (s->type == AV_FRAME_DATA_NB && s->mode != SIDEDATA_DELETE) {
>          av_log(ctx, AV_LOG_ERROR, "Side data type must be set\n");
>          return AVERROR(EINVAL);
>      }
> @@ -122,7 +122,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
>      SideDataContext *s = ctx->priv;
>      AVFrameSideData *sd = NULL;
>  
> -    if (s->type != -1)
> +    if (s->type != AV_FRAME_DATA_NB)
>         sd = av_frame_get_side_data(frame, s->type);
>  
>      switch (s->mode) {
> @@ -132,7 +132,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
>          }
>          break;
>      case SIDEDATA_DELETE:
> -        if (s->type == -1) {
> +        if (s->type == AV_FRAME_DATA_NB) {
>              while (frame->nb_side_data)
>                  av_frame_remove_side_data(frame, frame->side_data[0]->type);
>          } else if (sd) {
> diff --git a/libavutil/frame.h b/libavutil/frame.h
> index 5d3231e..4b83125 100644
> --- a/libavutil/frame.h
> +++ b/libavutil/frame.h
> @@ -179,6 +179,16 @@ enum AVFrameSideDataType {
>       * array element is implied by AVFrameSideData.size / AVRegionOfInterest.self_size.
>       */
>      AV_FRAME_DATA_REGIONS_OF_INTEREST,
> +
> +    /**
> +     * The number of side data types.
> +     * This is not part of the public API/ABI in the sense that it may
> +     * change when new side data types are added.
> +     * This must stay the last enum value.
> +     * If its value becomes huge, some code using it
> +     * needs to be updated as it assumes it to be smaller than other limits.
> +     */
> +    AV_FRAME_DATA_NB
>  };

This looks not really correct
this defines a constant in libavutil that can change with libavutils
minor version and then use it outside in libavfilter
it could then mismatch what is linked at runtime ...


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

"You are 36 times more likely to die in a bathtub than at the hands of a
terrorist. Also, you are 2.5 times more likely to become a president and
2 times more likely to become an astronaut, than to die in a terrorist
attack." -- Thoughty2

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190918/26f13c20/attachment.sig>