> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of
> Andreas Rheinhardt
> Sent: 2021年4月7日 22:44
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH V7 4/6] lavu: add side data
> AV_FRAME_DATA_BOUNDING_BOXES
>
> Guo, Yejun:
> > Signed-off-by: Guo, Yejun <yejun....@intel.com>
> > ---
> > doc/APIchanges | 2 +
> > libavutil/Makefile | 2 +
> > libavutil/boundingbox.c | 73 +++++++++++++++++++++++++
> > libavutil/boundingbox.h | 114
> ++++++++++++++++++++++++++++++++++++++++
> > libavutil/frame.c | 1 +
> > libavutil/frame.h | 7 +++
> > 6 files changed, 199 insertions(+)
> > create mode 100644 libavutil/boundingbox.c
> > create mode 100644 libavutil/boundingbox.h
> >
> > diff --git a/doc/APIchanges b/doc/APIchanges
> > index 9dfcc97d5c..81d01aac1e 100644
> > --- a/doc/APIchanges
> > +++ b/doc/APIchanges
> > @@ -14,6 +14,8 @@ libavutil: 2017-10-21
> >
> >
> > API changes, most recent first:
> > +2021-04-xx - xxxxxxxxxx - lavu 56.xx.100 - frame.h boundingbox.h
> > + Add AV_FRAME_DATA_BOUNDING_BOXES
> >
> > 2021-04-06 - xxxxxxxxxx - lavf 58.78.100 - avformat.h
> > Add avformat_index_get_entries_count(), avformat_index_get_entry(),
> > diff --git a/libavutil/Makefile b/libavutil/Makefile
> > index 27bafe9e12..f6d21bb5ce 100644
> > --- a/libavutil/Makefile
> > +++ b/libavutil/Makefile
> > @@ -11,6 +11,7 @@ HEADERS = adler32.h
> \
> > avutil.h
> \
> > base64.h
> \
> > blowfish.h
> \
> > + boundingbox.h
> \
> > bprint.h
> \
> > bswap.h
> \
> > buffer.h
> \
> > @@ -104,6 +105,7 @@ OBJS = adler32.o
> \
> > avsscanf.o
> \
> > base64.o
> \
> > blowfish.o
> \
> > + boundingbox.o
> \
> > bprint.o
> \
> > buffer.o
> \
> > cast5.o
> \
> > diff --git a/libavutil/boundingbox.c b/libavutil/boundingbox.c
> > new file mode 100644
> > index 0000000000..881661ec18
> > --- /dev/null
> > +++ b/libavutil/boundingbox.c
> > @@ -0,0 +1,73 @@
> > +/*
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * FFmpeg is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with FFmpeg; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
> USA
> > + */
> > +
> > +#include "boundingbox.h"
> > +
> > +AVBoundingBoxHeader *av_bbox_alloc(uint32_t nb_bboxes, size_t
> *out_size)
> > +{
> > + size_t size;
> > + struct {
> > + AVBoundingBoxHeader header;
> > + AVBoundingBox boxes[];
>
> Hopefully all compilers accept a flexible array member; if not, using
> boxes[1] (or just boxes) would be sufficient.
patchwork is passed, but I'm not 100% sure all the compilers accept it,
I'll change to boxes[1].
>
> > + } *ret;
> > +
> > + size = sizeof(*ret);
> > + if (nb_bboxes > (SIZE_MAX - size) / sizeof(*ret->boxes))
> > + return NULL;
> > + size += sizeof(*ret->boxes) * nb_bboxes;
> > +
> > + ret = av_mallocz(size);
> > + if (!ret)
> > + return NULL;
> > +
> > + ret->header.nb_bboxes = nb_bboxes;
> > + ret->header.bbox_size = sizeof(*ret->boxes);
> > + ret->header.bboxes_offset = (char *)&ret->boxes - (char *)&ret->header;
>
> Using offsetof would be clearer (for this you have to declare a proper
> type).
maybe both are ok.
>
> > +
> > + if (out_size)
> > + *out_size = sizeof(*ret);
> > +
> > + return &ret->header;
> > +}
> > +
> > +AVBoundingBoxHeader* av_bbox_create_side_data(AVFrame *frame,
> uint32_t nb_bboxes)
> > +{
> > + AVBufferRef *buf;
> > + AVBoundingBoxHeader *header;
> > + size_t size;
> > +
> > + header = av_bbox_alloc(nb_bboxes, &size);
> > + if (!header)
> > + return NULL;
> > + if (size > INT_MAX) {
> > + av_freep(&header);
> > + return NULL;
> > + }
>
> Will be obsolete soon.
will remove the check, thanks.
>
> > + buf = av_buffer_create((uint8_t *)header, size, NULL, NULL, 0);
> > + if (!buf) {
> > + av_freep(&header);
> > + return NULL;
> > + }
> > +
> > + if (!av_frame_new_side_data_from_buf(frame,
> AV_FRAME_DATA_BOUNDING_BOXES, buf)) {
> > + av_buffer_unref(&buf);
> > + return NULL;
> > + }
> > +
> > + return header;
> > +}
>
> Overall, the code duplication with video_enc_params.c should be factored
> out.
Yes, there's some code duplication. But some code are relative to different
struct (AVBoundingBoxHeader and AVVideoEncParams), and we could not
unify them without type template. The left common code is about av_buffer_create
and av_frame_new_side_data_from_buf, do we have necessarity to put them
together? My current answer is no, I'll continue to think about it tomorrow.
I'll push the first 3 patches in this patch set tomorrow if there's no
objection.
_______________________________________________
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".
