On Fri, 11 Jul 2014 22:33:59 +0200
Stepan Bujnak <stepan.buj...@gmail.com> wrote:
>
> On 11.07.2014 22:25, wm4 wrote:
> > On Fri, 11 Jul 2014 22:13:54 +0200
> > Stepan Bujnak <stepan.buj...@gmail.com> wrote:
> >
> >> the libavfilter/vf_blackframe.c filter now not only logs detected
> >> values, but also sets frame metadata. The metadata have prefix
> >> 'lavfi.blackframe.*'.
> >>
> >> Signed-off-by: Stepan Bujnak <stepan.buj...@gmail.com>
> >> ---
> >> libavfilter/vf_blackframe.c | 29 +++++++++++++++++++++++++----
> >> 1 file changed, 25 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/libavfilter/vf_blackframe.c b/libavfilter/vf_blackframe.c
> >> index 0eeda61..d89a25e 100644
> >> --- a/libavfilter/vf_blackframe.c
> >> +++ b/libavfilter/vf_blackframe.c
> >> @@ -58,6 +58,10 @@ static int query_formats(AVFilterContext *ctx)
> >> return 0;
> >> }
> >>
> >> +#define SET_META(key, format, value) \
> >> + snprintf(buf, sizeof(buf), format, value); \
> >> + av_dict_set(metadata, key, buf, 0)
> >> +
> >> static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
> >> {
> >> AVFilterContext *ctx = inlink->dst;
> >> @@ -65,6 +69,10 @@ static int filter_frame(AVFilterLink *inlink, AVFrame
> >> *frame)
> >> int x, i;
> >> int pblack = 0;
> >> uint8_t *p = frame->data[0];
> >> + AVDictionary **metadata;
> >> + double t = -1;
> >> + char type;
> >> + char buf[32];
> >>
> >> for (i = 0; i < frame->height; i++) {
> >> for (x = 0; x < inlink->w; x++)
> >> @@ -76,12 +84,25 @@ static int filter_frame(AVFilterLink *inlink, AVFrame
> >> *frame)
> >> s->last_keyframe = s->frame;
> >>
> >> pblack = s->nblack * 100 / (inlink->w * inlink->h);
> >> - if (pblack >= s->bamount)
> >> + if (pblack >= s->bamount) {
> >> + metadata = avpriv_frame_get_metadatap(frame);
> >> + type = av_get_picture_type_char(frame->pict_type);
> >> +
> >> + if (frame->pts != AV_NOPTS_VALUE) {
> >> + t = frame->pts * av_q2d(inlink->time_base);
> >> + }
> >> +
> >> av_log(ctx, AV_LOG_INFO, "frame:%u pblack:%u pts:%"PRId64" t:%f "
> >> "type:%c last_keyframe:%d\n",
> >> - s->frame, pblack, frame->pts,
> >> - frame->pts == AV_NOPTS_VALUE ? -1 : frame->pts *
> >> av_q2d(inlink->time_base),
> >> - av_get_picture_type_char(frame->pict_type),
> >> s->last_keyframe);
> >> + s->frame, pblack, frame->pts, t, type, s->last_keyframe);
> >> +
> >> + SET_META("lavfi.blackframe.frame", "%u", s->frame);
> >> + SET_META("lavfi.blackframe.pblack", "%u", pblack);
> >> + SET_META("lavfi.blackframe.pts", "%"PRId64, frame->pts);
> > It shouldn't be needed to set the PTS - the frame already has it.
> >
> >> + SET_META("lavfi.blackframe.t", "%f", t);
> >> + SET_META("lavfi.blackframe.type", "%c", type);
> >> + SET_META("lavfi.blackframe.last_keyframe", "%d",
> >> s->last_keyframe);
> >> + }
> >>
> >> s->frame++;
> >> s->nblack = 0;
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> You are absolutely right. The same possibly applies to `t` and `type`
> values, but how about `s->frame` and `s->last_frame`? Are those necessary?
Neither of them look particularly useful (and they're all redundant),
so IMHO they should be dropped too.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
