diff mbox

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

Message ID 20190824161800.15365-1-lance.lmwang@gmail.com
State Superseded
Headers show

Commit Message

Lance Wang Aug. 24, 2019, 4:17 p.m. UTC 
From: Limin Wang <lance.lmwang@gmail.com>

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavfilter/f_sidedata.c | 10 +++++-----
 libavutil/frame.h        | 10 ++++++++++
 2 files changed, 15 insertions(+), 5 deletions(-)

Comments

Lance Wang Sept. 1, 2019, 1:42 p.m. UTC | #1 
ping 1 and 2, please ignore patchset 3.


On Sun, Aug 25, 2019 at 12:17:58AM +0800, lance.lmwang@gmail.com wrote:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Signed-off-by: Limin Wang <lance.lmwang@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
>  };
>  
>  enum AVActiveFormatDescription {
> -- 
> 2.9.5
>
Lance Wang Sept. 8, 2019, 10:01 a.m. UTC | #2 
ping, #patch 2 is pushed already.

On Sun, Aug 25, 2019 at 12:17:58AM +0800, lance.lmwang@gmail.com wrote:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Signed-off-by: Limin Wang <lance.lmwang@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
>  };
>  
>  enum AVActiveFormatDescription {
> -- 
> 2.9.5
>
Lance Wang Sept. 18, 2019, 2:21 p.m. UTC | #3 
ping for this patch, other patchset are merged.

On Sun, Aug 25, 2019 at 12:17:58AM +0800, lance.lmwang@gmail.com wrote:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Signed-off-by: Limin Wang <lance.lmwang@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
>  };
>  
>  enum AVActiveFormatDescription {
> -- 
> 2.9.5
>
Michael Niedermayer Sept. 18, 2019, 6:23 p.m. UTC | #4 
On Sun, Aug 25, 2019 at 12:17:58AM +0800, lance.lmwang@gmail.com wrote:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Signed-off-by: Limin Wang <lance.lmwang@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 ...


[...]
Lance Wang Sept. 19, 2019, 1:25 a.m. UTC | #5 
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@gmail.com wrote:
> > From: Limin Wang <lance.lmwang@gmail.com>
> > 
> > Signed-off-by: Limin Wang <lance.lmwang@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@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Marton Balint Sept. 19, 2019, 7:58 a.m. UTC | #6 
On Thu, 19 Sep 2019, Limin Wang wrote:

> 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@gmail.com wrote:
>> > From: Limin Wang <lance.lmwang@gmail.com>
>> > 
>> > Signed-off-by: Limin Wang <lance.lmwang@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.

That won't help, as the issue is caused by using non-public API/ABI of 
libavutil in another library (libavfilter).

Also it is not very good idea to change the value of the unset entry from 
-1, as the user might use that to explicitly set the parameter to unset.

If you want to fix the warnings maybe it is better if you simply define 
something like this in f_sidedata.c:

#define UNSET ((enum AVFrameSideDataType)-1)

Regards,
Marton
Lance Wang Sept. 19, 2019, 2:49 p.m. UTC | #7 
On Thu, Sep 19, 2019 at 09:58:46AM +0200, Marton Balint wrote:
> 
> On Thu, 19 Sep 2019, Limin Wang wrote:
> 
> >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@gmail.com wrote:
> >>> From: Limin Wang <lance.lmwang@gmail.com>
> >>> > Signed-off-by: Limin Wang <lance.lmwang@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.
> 
> That won't help, as the issue is caused by using non-public API/ABI
> of libavutil in another library (libavfilter).
> 
> Also it is not very good idea to change the value of the unset entry
> from -1, as the user might use that to explicitly set the parameter
> to unset.
> 
> If you want to fix the warnings maybe it is better if you simply
> define something like this in f_sidedata.c:
> 
> #define UNSET ((enum AVFrameSideDataType)-1)
Thanks for the detailed comments, it's very helpful. I'll try with your
method.

> 
> Regards,
> Marton
> _______________________________________________
> 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".
diff mbox

Patch

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
 };
 
 enum AVActiveFormatDescription {