| Message ID | 1552223806-32587-1-git-send-email-mypopydev@gmail.com |
|---|---|
| State | New |
| Headers | show |
> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > Of Jun Zhao > Sent: Sunday, March 10, 2019 9:17 PM > To: ffmpeg-devel@ffmpeg.org > Cc: Jun Zhao <barryjzhao@tencent.com> > Subject: [FFmpeg-devel] [PATCH V2] lavfi/showinfo: support regions of > interest sidedata > > From: Jun Zhao <barryjzhao@tencent.com> > > support regions of interest sidedata > > Signed-off-by: Jun Zhao <barryjzhao@tencent.com> > --- > libavfilter/vf_showinfo.c | 33 +++++++++++++++++++++++++++++++++ > 1 files changed, 33 insertions(+), 0 deletions(-) > > diff --git a/libavfilter/vf_showinfo.c b/libavfilter/vf_showinfo.c > index e41c330..d0e140f 100644 > --- a/libavfilter/vf_showinfo.c > +++ b/libavfilter/vf_showinfo.c > @@ -111,6 +111,36 @@ static void dump_stereo3d(AVFilterContext *ctx, > AVFrameSideData *sd) > av_log(ctx, AV_LOG_INFO, " (inverted)"); > } > > +static void dump_roi(AVFilterContext *ctx, AVFrameSideData *sd) > +{ > + AVRegionOfInterest *roi; > + int nb_rois; > + > + if (sd->size < sizeof(*roi)) { > + av_log(ctx, AV_LOG_INFO, "invalid data"); > + return; > + } it is possible. > + > + roi = (AVRegionOfInterest *)sd->data; > + if (roi->self_size == 0 || sd->size % roi->self_size != 0) { > + av_log(ctx, AV_LOG_INFO, "invalid ROI side data"); > + return; > + } > + nb_rois = sd->size / roi->self_size; > + > + av_log(ctx, AV_LOG_INFO, "Regions Of Interest(RoI) information: "); > + for (int index = 0; index < nb_rois; index++) { > + av_log(ctx, AV_LOG_INFO, "index: %d, region: (%d %d)/(%d %d), qp > offset: %d/%d", > + index, roi->left, roi->top, roi->right, roi->bottom, roi->qoffset.num, > roi->qoffset.den); > + > + if (roi->self_size == 0) > + av_log(ctx, AV_LOG_INFO, > + "AVRegionOfInterest.self_size must be set to > sizeof(AVRegionOfInterest).\n"); we can skip this since the first roi->self_size is tested. > + > + roi = (AVRegionOfInterest *)((char *)roi + roi->self_size); > + } > +} > + > static void dump_color_property(AVFilterContext *ctx, AVFrame *frame) > { > const char *color_range_str = av_color_range_name(frame- > >color_range); > @@ -246,6 +276,9 @@ static int filter_frame(AVFilterLink *inlink, AVFrame > *frame) > case AV_FRAME_DATA_AFD: > av_log(ctx, AV_LOG_INFO, "afd: value of %"PRIu8, sd->data[0]); > break; > + case AV_FRAME_DATA_REGIONS_OF_INTEREST: > + dump_roi(ctx, sd); > + break; > default: > av_log(ctx, AV_LOG_WARNING, "unknown side data type %d (%d > bytes)", > sd->type, sd->size); > -- > 1.7.1 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On Mon, Mar 11, 2019 at 10:51 AM Guo, Yejun <yejun.guo@intel.com> wrote: > > > > > -----Original Message----- > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > > Of Jun Zhao > > Sent: Sunday, March 10, 2019 9:17 PM > > To: ffmpeg-devel@ffmpeg.org > > Cc: Jun Zhao <barryjzhao@tencent.com> > > Subject: [FFmpeg-devel] [PATCH V2] lavfi/showinfo: support regions of > > interest sidedata > > > > From: Jun Zhao <barryjzhao@tencent.com> > > > > support regions of interest sidedata > > > > Signed-off-by: Jun Zhao <barryjzhao@tencent.com> > > --- > > libavfilter/vf_showinfo.c | 33 +++++++++++++++++++++++++++++++++ > > 1 files changed, 33 insertions(+), 0 deletions(-) > > > > diff --git a/libavfilter/vf_showinfo.c b/libavfilter/vf_showinfo.c > > index e41c330..d0e140f 100644 > > --- a/libavfilter/vf_showinfo.c > > +++ b/libavfilter/vf_showinfo.c > > @@ -111,6 +111,36 @@ static void dump_stereo3d(AVFilterContext *ctx, > > AVFrameSideData *sd) > > av_log(ctx, AV_LOG_INFO, " (inverted)"); > > } > > > > +static void dump_roi(AVFilterContext *ctx, AVFrameSideData *sd) > > +{ > > + AVRegionOfInterest *roi; > > + int nb_rois; > > + > > + if (sd->size < sizeof(*roi)) { > > + av_log(ctx, AV_LOG_INFO, "invalid data"); > > + return; > > + } > > it is possible. It's a surprised behavior for me, how to check the corrupted side data for this case? I don't like the self_size field in this case in fact. > > > + > > + roi = (AVRegionOfInterest *)sd->data; > > + if (roi->self_size == 0 || sd->size % roi->self_size != 0) { > > + av_log(ctx, AV_LOG_INFO, "invalid ROI side data"); > > + return; > > + } > > + nb_rois = sd->size / roi->self_size; > > + > > + av_log(ctx, AV_LOG_INFO, "Regions Of Interest(RoI) information: "); > > + for (int index = 0; index < nb_rois; index++) { > > + av_log(ctx, AV_LOG_INFO, "index: %d, region: (%d %d)/(%d %d), qp > > offset: %d/%d", > > + index, roi->left, roi->top, roi->right, roi->bottom, roi->qoffset.num, > > roi->qoffset.den); > > + > > + if (roi->self_size == 0) > > + av_log(ctx, AV_LOG_INFO, > > + "AVRegionOfInterest.self_size must be set to > > sizeof(AVRegionOfInterest).\n"); > > we can skip this since the first roi->self_size is tested. > If we have N ROIs, we just need to check the first roi entry for roi->self_size? It's the other surprised behavior for me > > + > > + roi = (AVRegionOfInterest *)((char *)roi + roi->self_size); > > + } > > +} > > + > > static void dump_color_property(AVFilterContext *ctx, AVFrame *frame) > > { > > const char *color_range_str = av_color_range_name(frame- > > >color_range); > > @@ -246,6 +276,9 @@ static int filter_frame(AVFilterLink *inlink, AVFrame > > *frame) > > case AV_FRAME_DATA_AFD: > > av_log(ctx, AV_LOG_INFO, "afd: value of %"PRIu8, sd->data[0]); > > break; > > + case AV_FRAME_DATA_REGIONS_OF_INTEREST: > > + dump_roi(ctx, sd); > > + break; > > default: > > av_log(ctx, AV_LOG_WARNING, "unknown side data type %d (%d > > bytes)", > > sd->type, sd->size); > > -- > > 1.7.1 > >
> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > Of mypopy@gmail.com > Sent: Monday, March 11, 2019 11:05 AM > To: FFmpeg development discussions and patches <ffmpeg- > devel@ffmpeg.org> > Cc: Jun Zhao <barryjzhao@tencent.com> > Subject: Re: [FFmpeg-devel] [PATCH V2] lavfi/showinfo: support regions of > interest sidedata > > On Mon, Mar 11, 2019 at 10:51 AM Guo, Yejun <yejun.guo@intel.com> > wrote: > > > > > > > > > -----Original Message----- > > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On > Behalf > > > Of Jun Zhao > > > Sent: Sunday, March 10, 2019 9:17 PM > > > To: ffmpeg-devel@ffmpeg.org > > > Cc: Jun Zhao <barryjzhao@tencent.com> > > > Subject: [FFmpeg-devel] [PATCH V2] lavfi/showinfo: support regions of > > > interest sidedata > > > > > > From: Jun Zhao <barryjzhao@tencent.com> > > > > > > support regions of interest sidedata > > > > > > Signed-off-by: Jun Zhao <barryjzhao@tencent.com> > > > --- > > > libavfilter/vf_showinfo.c | 33 > +++++++++++++++++++++++++++++++++ > > > 1 files changed, 33 insertions(+), 0 deletions(-) > > > > > > diff --git a/libavfilter/vf_showinfo.c b/libavfilter/vf_showinfo.c > > > index e41c330..d0e140f 100644 > > > --- a/libavfilter/vf_showinfo.c > > > +++ b/libavfilter/vf_showinfo.c > > > @@ -111,6 +111,36 @@ static void dump_stereo3d(AVFilterContext *ctx, > > > AVFrameSideData *sd) > > > av_log(ctx, AV_LOG_INFO, " (inverted)"); > > > } > > > > > > +static void dump_roi(AVFilterContext *ctx, AVFrameSideData *sd) > > > +{ > > > + AVRegionOfInterest *roi; > > > + int nb_rois; > > > + > > > + if (sd->size < sizeof(*roi)) { > > > + av_log(ctx, AV_LOG_INFO, "invalid data"); > > > + return; > > > + } > > > > it is possible. > It's a surprised behavior for me, how to check the corrupted side data > for this case? I don't like the self_size field in this case in fact. it is for ABI compatible, just imaging that we expand the struct, while the user is using an old version and there is only one roi in the sd buffer. For this case, sd->size is less than sizeof(*roi). > > > > > + > > > + roi = (AVRegionOfInterest *)sd->data; > > > + if (roi->self_size == 0 || sd->size % roi->self_size != 0) { > > > + av_log(ctx, AV_LOG_INFO, "invalid ROI side data"); > > > + return; > > > + } > > > + nb_rois = sd->size / roi->self_size; > > > + > > > + av_log(ctx, AV_LOG_INFO, "Regions Of Interest(RoI) information: "); > > > + for (int index = 0; index < nb_rois; index++) { > > > + av_log(ctx, AV_LOG_INFO, "index: %d, region: (%d %d)/(%d %d), > qp > > > offset: %d/%d", > > > + index, roi->left, roi->top, roi->right, roi->bottom, roi- > >qoffset.num, > > > roi->qoffset.den); > > > + > > > + if (roi->self_size == 0) > > > + av_log(ctx, AV_LOG_INFO, > > > + "AVRegionOfInterest.self_size must be set to > > > sizeof(AVRegionOfInterest).\n"); > > > > we can skip this since the first roi->self_size is tested. > > > If we have N ROIs, we just need to check the first roi entry for > roi->self_size? It's the other surprised behavior for me forgot to mention the next line together. let's save the first roi->self_size and so the next line will be: roi = (AVRegionOfInterest *)((char *)roi + self_size); The self_size here is to indicates the struct size when the user uses it. It is actually enough to just set the first roi. btw, there is an issue in my original code pushed, and Mark Thompson has given better code which has not been pushed yet, except saving the first roi->self_size. (I just realized this, and will comment MarkT again). > > > + > > > + roi = (AVRegionOfInterest *)((char *)roi + roi->self_size); > > > + } > > > +} > > > + > > > static void dump_color_property(AVFilterContext *ctx, AVFrame *frame) > > > { > > > const char *color_range_str = av_color_range_name(frame- > > > >color_range); > > > @@ -246,6 +276,9 @@ static int filter_frame(AVFilterLink *inlink, > AVFrame > > > *frame) > > > case AV_FRAME_DATA_AFD: > > > av_log(ctx, AV_LOG_INFO, "afd: value of %"PRIu8, sd->data[0]); > > > break; > > > + case AV_FRAME_DATA_REGIONS_OF_INTEREST: > > > + dump_roi(ctx, sd); > > > + break; > > > default: > > > av_log(ctx, AV_LOG_WARNING, "unknown side data type %d (%d > > > bytes)", > > > sd->type, sd->size); > > > -- > > > 1.7.1 > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
diff --git a/libavfilter/vf_showinfo.c b/libavfilter/vf_showinfo.c index e41c330..d0e140f 100644 --- a/libavfilter/vf_showinfo.c +++ b/libavfilter/vf_showinfo.c @@ -111,6 +111,36 @@ static void dump_stereo3d(AVFilterContext *ctx, AVFrameSideData *sd) av_log(ctx, AV_LOG_INFO, " (inverted)"); } +static void dump_roi(AVFilterContext *ctx, AVFrameSideData *sd) +{ + AVRegionOfInterest *roi; + int nb_rois; + + if (sd->size < sizeof(*roi)) { + av_log(ctx, AV_LOG_INFO, "invalid data"); + return; + } + + roi = (AVRegionOfInterest *)sd->data; + if (roi->self_size == 0 || sd->size % roi->self_size != 0) { + av_log(ctx, AV_LOG_INFO, "invalid ROI side data"); + return; + } + nb_rois = sd->size / roi->self_size; + + av_log(ctx, AV_LOG_INFO, "Regions Of Interest(RoI) information: "); + for (int index = 0; index < nb_rois; index++) { + av_log(ctx, AV_LOG_INFO, "index: %d, region: (%d %d)/(%d %d), qp offset: %d/%d", + index, roi->left, roi->top, roi->right, roi->bottom, roi->qoffset.num, roi->qoffset.den); + + if (roi->self_size == 0) + av_log(ctx, AV_LOG_INFO, + "AVRegionOfInterest.self_size must be set to sizeof(AVRegionOfInterest).\n"); + + roi = (AVRegionOfInterest *)((char *)roi + roi->self_size); + } +} + static void dump_color_property(AVFilterContext *ctx, AVFrame *frame) { const char *color_range_str = av_color_range_name(frame->color_range); @@ -246,6 +276,9 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame) case AV_FRAME_DATA_AFD: av_log(ctx, AV_LOG_INFO, "afd: value of %"PRIu8, sd->data[0]); break; + case AV_FRAME_DATA_REGIONS_OF_INTEREST: + dump_roi(ctx, sd); + break; default: av_log(ctx, AV_LOG_WARNING, "unknown side data type %d (%d bytes)", sd->type, sd->size);