Message ID | 20190824161800.15365-1-lance.lmwang@gmail.com |
---|---|
State | Superseded |
Headers | show |
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 >
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 >
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 >
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 ... [...]
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".
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
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 --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 {