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.lmw...@gmail.com wrote:
> >>> From: Limin Wang <lance.lmw...@gmail.com>
> >>> > Signed-off-by: Limin Wang <lance.lmw...@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-requ...@ffmpeg.org with subject "unsubscribe".
_______________________________________________
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".
