Guo, Yejun: > > >> -----Original Message----- >> From: ffmpeg-devel <[email protected]> On Behalf Of >> Andreas Rheinhardt >> Sent: 2021年3月1日 20:24 >> To: [email protected] >> Subject: Re: [FFmpeg-devel] [PATCH V3 3/3] libavfilter: add filter dnn_detect >> for object detection >> >> Guo, Yejun: >>> >>> >>>> -----Original Message----- >>>> From: ffmpeg-devel <[email protected]> On Behalf Of >>>> Andreas Rheinhardt >>>> Sent: 2021年3月1日 20:13 >>>> To: [email protected] >>>> Subject: Re: [FFmpeg-devel] [PATCH V3 3/3] libavfilter: add filter >>>> dnn_detect >>>> for object detection >>>> >>>> Guo, Yejun: >>>>> >>>>> >>>>>> -----Original Message----- >>>>>> From: ffmpeg-devel <[email protected]> On Behalf Of >>>>>> Andreas Rheinhardt >>>>>> Sent: 2021年3月1日 16:31 >>>>>> To: [email protected] >>>>>> Subject: Re: [FFmpeg-devel] [PATCH V3 3/3] libavfilter: add filter >>>>>> dnn_detect for object detection >>>>>> >>>>>> Guo, Yejun: >>>>>>> >>>>>>> >>>>>>>> -----Original Message----- >>>>>>>> From: ffmpeg-devel <[email protected]> On Behalf Of >>>>>>>> Andreas Rheinhardt >>>>>>>> Sent: 2021年3月1日 12:50 >>>>>>>> To: [email protected] >>>>>>>> Subject: Re: [FFmpeg-devel] [PATCH V3 3/3] libavfilter: add filter >>>>>>>> dnn_detect for object detection >>>>>>>> >>>>>>>> Guo, Yejun: >>>>>>>>> Signed-off-by: Guo, Yejun <[email protected]> >>>>>>>>> --- >>>>>>>>> configure | 1 + >>>>>>>>> doc/filters.texi | 40 +++ >>>>>>>>> libavfilter/Makefile | 1 + >>>>>>>>> libavfilter/allfilters.c | 1 + >>>>>>>>> libavfilter/dnn/dnn_backend_openvino.c | 12 + >>>>>>>>> libavfilter/dnn_filter_common.c | 7 + >>>>>>>>> libavfilter/dnn_filter_common.h | 1 + >>>>>>>>> libavfilter/dnn_interface.h | 6 +- >>>>>>>>> libavfilter/vf_dnn_detect.c | 462 >>>>>>>> +++++++++++++++++++++++++ >>>>>>>>> 9 files changed, 529 insertions(+), 2 deletions(-) create mode >>>>>>> >>>>>>>>> b/libavfilter/dnn_interface.h index d3a0c58a61..90a08129f4 100644 >>>>>>>>> --- a/libavfilter/dnn_interface.h >>>>>>>>> +++ b/libavfilter/dnn_interface.h >>>>>>>>> @@ -63,6 +63,8 @@ typedef struct DNNData{ >>>>>>>>> DNNColorOrder order; >>>>>>>>> } DNNData; >>>>>>>>> >>>>>>>>> +typedef int (*PRE_POST_PROC)(AVFrame *frame, DNNData *model, >>>>>>>>> +AVFilterContext *filter_ctx); >>>>>>>> >>>>>>>> Why uppercase? It is a typedef, not a macro. >>>>>>> >>>>>>> will change to CamelCase, thanks. >>>>>>> >>>>>>>> >>>>>>>>> + >>>>>>>>> typedef struct DNNModel{ >>>>>>>>> // Stores model that can be different for different backends. >>>>>>>>> void *model; >>>>>>>>> @@ -80,10 +82,10 @@ typedef struct DNNModel{ >>>>>>>>> const char *output_name, int >>>>>>>> *output_width, int *output_height); >>>>>>>>> // set the pre process to transfer data from AVFrame to >> DNNData >>>>>>>>> // the default implementation within DNN is used if it is not >>>>>>>>> provided >>>>>>>> by the filter >>>>>>>>> - int (*pre_proc)(AVFrame *frame_in, DNNData *model_input, >>>>>>>> AVFilterContext *filter_ctx); >>>>>>>>> + PRE_POST_PROC pre_proc; >>>>>>>>> // set the post process to transfer data from DNNData to >>>> AVFrame >>>>>>>>> // the default implementation within DNN is used if it is not >>>>>>>>> provided >>>>>>>> by the filter >>>>>>>>> - int (*post_proc)(AVFrame *frame_out, DNNData >> *model_output, >>>>>>>> AVFilterContext *filter_ctx); >>>>>>>>> + PRE_POST_PROC post_proc; >>>>>>>> >>>>>>>> Spurious change. >>>>>>> >>>>>>> sorry, not quite understand this comment. It is used for code refine. >>>>>>> Maybe I need to let it in a separate patch. >>>>>>> >>>>>>>> >>>>>>>>> } DNNModel; >>>>>>>>> >>>>>>>>> + >>>>>>>>> + frame->private_ref = av_buffer_alloc(sizeof(*header) + >>>>>>>>> + sizeof(*bbox) * >>>>>>>> nb_bbox); >>>>>>>>> + if (!frame->private_ref) { >>>>>>>>> + av_log(filter_ctx, AV_LOG_ERROR, "failed to allocate >>>>>>>>> + buffer for %d >>>>>>>> bboxes\n", nb_bbox); >>>>>>>>> + return AVERROR(ENOMEM);; >>>>>>>> >>>>>>>> Double ; >>>>>>> >>>>>>> thanks, will remove it. >>>>>>> >>>>>>>> >>>>>>>>> + } >>>>>>>>> + >>>>>>>>> + header = (BoundingBoxHeader *)frame->private_ref->data; >>>>>>>>> + strncpy(header->source, ctx->dnnctx.model_filename, >>>>>>>> sizeof(header->source)); >>>>>>>>> + header->source[sizeof(header->source) - 1] = '\0'; >>>>>>>>> + header->bbox_size = sizeof(*bbox); >>>>>>>>> + >>>>>>>>> + bbox = (BoundingBox *)(header + 1); >>>>>>>> >>>>>>>> This does not guarantee proper alignment. You could use a flexible >>>>>>>> array member for that. >>>>>>> >>>>>>> The memory layout of frame->private_ref->data is: >>>>>>> sizeof(*header) + sizeof(*bbox) + sizeof(*bbox) + ... >>>>>>> >>>>>>> I think 'header + 1' goes correctly to the first bbox. thanks. >>>>>>> >>>>>> >>>>>> header + 1 points to the position where another BoundingBoxHeader >>>>>> would be if header were an array of BoundingBoxHeaders (i.e. it >>>>>> points >>>>>> sizeof(BoundingBoxHeader) bytes after header). So this guarantees >>>>>> that there is no overlap between your header and your actual boxes, >>>>>> but this does not guarantee proper alignment. This will be a problem >>>>>> on platforms where int is 64bit and requires eight byte alignment >>>>>> (unlikely) or if you add something that requires alignment of eight >>>>>> bytes (like a 64bit integer or a pointer on a 64bit >>>>>> system) to BoundingBox. >>>>> >>>>> got the point, thanks. Will add 'BoundingBox bboxes[0]' at the end of >> struct >>>> BoundingBoxHeader. >>>>> >>>> >>>> An array with zero elements is not allowed in ISO C (see 6.7.6.2 1 in >>>> C11 or 6.7.5.2 1 in C99), although many compilers support it. >>> >>> thanks for the info, this struct is expected to be in side_data in the >>> future, >>> I'll add 'bboxes[1]' in it, and allocate sizeof(*header) + (nb_bbox - 1) * >> sizeof(*bbox). >> >> Notice that in this case it is undefined behaviour to access any of the >> boxes outside of BoundingBoxHeader (i.e. when using header->bboxes[i], >> the compiler is allowed to infer that i == 0 as all other cases would be >> undefined behaviour). >> >> - Andreas > > Thanks for let me know it. I'll not use header->bboxes[i], but will use: > bbox = header->bboxes; > bbox++; > > and i'll also add comment in code to warn: don't use header->bboxes[i].
Well, the spec-compliant and most elegant way to deal with such situations is a flexible array member: Add "BoundingBox boxes[];" to BoundingBoxHeader and allocate the memory the way you do now. You could then access these via header->boxes[i]. But this is a C99 feature that apparently older versions of MSVC don't support. So I don't know whether we are allowed to use it or not. Just do whatever works. Apologies for wasting your time here. - Andreas _______________________________________________ ffmpeg-devel mailing list [email protected] https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email [email protected] with subject "unsubscribe".
