> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of Lynne > Sent: 2021年4月8日 5:04 > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH V7 4/6] lavu: add side data > AV_FRAME_DATA_BOUNDING_BOXES > > Apr 7, 2021, 16:17 by yejun....@intel.com: > > > 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[]; > > + } *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; > > + > > + if (out_size) > > + *out_size = sizeof(*ret); > > + > > + return &ret->header; > > +} > > + > > +AVBoundingBoxHeader* av_bbox_create_side_data(AVFrame *frame, > uint32_t nb_bboxes) > > > > Fix this everywhere. We never put the * before the name. This is a misleading > coding style that makes absolutely no sense.
Do you mean change from: AVBoundingBoxHeader* av_bbox_create_side_data to: AVBoundingBoxHeader *av_bbox_create_side_data Will fix it, thanks. btw, I think the coding style is to put '*' just before the name, for example, AVFrame *frame; const AVClass *class; > > > > +{ > > + 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; > > + } > > + 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; > > +} > > diff --git a/libavutil/boundingbox.h b/libavutil/boundingbox.h > > new file mode 100644 > > index 0000000000..2b77c6c0dc > > --- /dev/null > > +++ b/libavutil/boundingbox.h > > @@ -0,0 +1,114 @@ > > +/* > > + * 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 > > + */ > > + > > +#ifndef AVUTIL_BOUNDINGBOX_H > > +#define AVUTIL_BOUNDINGBOX_H > > + > > +#include "rational.h" > > +#include "avassert.h" > > +#include "frame.h" > > + > > +typedef struct AVBoundingBox { > > + /** > > + * Distance in pixels from the top edge of the frame to top > > + * and bottom, and from the left edge of the frame to left and > > + * right, defining the bounding box. > > + */ > > + int top; > > + int left; > > + int bottom; > > + int right; > > > > Why not x, y, w, h? > It makes more sense, all of coordinates go like this. We want to keep it consistent with 'struct AVRegionOfInterest', we also have a plan to add a filter which converts from bounding box to RegionOfInterest which impacts the encoder. > > > > + > > +#define AV_BBOX_LABEL_NAME_MAX_SIZE 32 > > > > Better bump this to 64. It won't be enough, and will > require a major bump to increment. I usually don't see a very long label name, and you are right, we'd better bump it for safe. Will change, thanks. > > > > + > > + /** > > + * Detect result with confidence > > + */ > > + char detect_label[AV_BBOX_LABEL_NAME_MAX_SIZE]; > > + AVRational detect_confidence; > > + > > + /** > > + * At most 4 classifications based on the detected bounding box. > > + * For example, we can get max 4 different attributes with 4 different > > + * DNN models on one bounding box. > > + * classify_count is zero if no classification. > > + */ > > +#define AV_NUM_BBOX_CLASSIFY 4 > > + uint32_t classify_count; > > + char > classify_labels[AV_NUM_BBOX_CLASSIFY][AV_BBOX_LABEL_NAME_MAX_SIZE]; > > + AVRational classify_confidences[AV_NUM_BBOX_CLASSIFY]; > > +} AVBoundingBox; > > + > > +typedef struct AVBoundingBoxHeader { > > + /** > > + * Information about how the bounding box is generated. > > + * for example, the DNN model name. > > + */ > > + char source[128]; > > + > > + /** > > + * The size of frame when it is detected. > > + */ > > + int frame_width; > > + int frame_height; > > > > Why? This side data is attached to AVFrames only, where we > already have width and height. The detection result will be used by other filters, for example, dnn_classify (see https://github.com/guoyejun/ffmpeg/tree/classify). The filter dnn_detect detects all the objects (cat, dog, person ...) in a frame, while dnn_classify classifies one detected object (for example, person) for its attribute (for example, emotion, etc.) The filter dnn_classify have to check if the frame size is changed after it is detected, to handle the below filter chain: dnn_detect -> scale -> dnn_classify > > > > + > > + /** > > + * Number of bounding boxes in the array. > > + */ > > + uint32_t nb_bboxes; > > + > > + /** > > + * Offset in bytes from the beginning of this structure at which > > + * the array of bounding boxes starts. > > + */ > > + size_t bboxes_offset; > > + > > + /** > > + * Size of each bounding box in bytes. > > + */ > > + size_t bbox_size; > > +} AVBoundingBoxHeader; > > + > > +/* > > + * Get the bounding box at the specified {@code idx}. Must be between 0 > and nb_bboxes. > > + */ > > +static av_always_inline AVBoundingBox* > > +av_get_bounding_box(const AVBoundingBoxHeader *header, unsigned int > idx) > > +{ > > + av_assert0(idx < header->nb_bboxes); > > + return (AVBoundingBox *)((uint8_t *)header + header->bboxes_offset + > > + idx * header->bbox_size); > > +} > > + > > +/** > > + * Allocates memory for AVBoundingBoxHeader, plus an array of {@code > nb_bboxes} > > + * AVBoundingBox, and initializes the variables. > > + * Can be freed with a normal av_free() call. > > + * > > + * @param out_size if non-NULL, the size in bytes of the resulting data > > array > is > > + * written here. > > + */ > > +AVBoundingBoxHeader *av_bbox_alloc(uint32_t nb_bboxes, size_t > *out_size); > > + > > +/** > > + * Allocates memory for AVBoundingBoxHeader, plus an array of {@code > nb_bboxes} > > + * AVBoundingBox, in the given AVFrame {@code frame} as > AVFrameSideData of type > > + * AV_FRAME_DATA_BOUNDING_BOXES and initializes the variables. > > + */ > > +AVBoundingBoxHeader* av_bbox_create_side_data(AVFrame *frame, > uint32_t nb_bboxes); > > +#endif > > diff --git a/libavutil/frame.c b/libavutil/frame.c > > index fefb2b69c0..d8e86263af 100644 > > --- a/libavutil/frame.c > > +++ b/libavutil/frame.c > > @@ -853,6 +853,7 @@ const char *av_frame_side_data_name(enum > AVFrameSideDataType type) > > case AV_FRAME_DATA_VIDEO_ENC_PARAMS: return "Video > encoding parameters"; > > case AV_FRAME_DATA_SEI_UNREGISTERED: return "H.26[45] > User Data Unregistered SEI message"; > > case AV_FRAME_DATA_FILM_GRAIN_PARAMS: return "Film > grain parameters"; > > + case AV_FRAME_DATA_BOUNDING_BOXES: return > "Bounding boxes"; > > } > > return NULL; > > } > > diff --git a/libavutil/frame.h b/libavutil/frame.h > > index a5ed91b20a..41e22de02a 100644 > > --- a/libavutil/frame.h > > +++ b/libavutil/frame.h > > @@ -198,6 +198,13 @@ enum AVFrameSideDataType { > > * Must be present for every frame which should have film grain applied. > > */ > > AV_FRAME_DATA_FILM_GRAIN_PARAMS, > > + > > + /** > > + * Bounding boxes for object detection and classification, the data is > > a > AVBoundingBoxHeader > > + * followed with an array of AVBoudingBox, and > AVBoundingBoxHeader.nb_bboxes is the number > > + * of array element. > > + */ > > + AV_FRAME_DATA_BOUNDING_BOXES, > > }; > > > > Finally, why call it a Bounding Box? It's not descriptive at all. > How about "Object Classification"? It makes much more sense, it's > exactly what this is. So AVObjectClassification, AVObjectClassification, > AV_FRAME_DATA_OBJECT_CLASSIFICATION and so on. In object detection papers, bounding box is usually used. We'd better use the same term, imho, thanks. _______________________________________________ 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".