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

[Limin Wang]

On Wed, Sep 18, 2019 at 08:23:46PM +0200, Michael Niedermayer wrote:
> 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 ...

Thanks for the comments, I'll update to bump the minor version of libavutils.

> 
> 
> [...]
> 
> -- 
> 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
> 



> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".