[FFmpeg-devel] [PATCH] Blackframe video filter now sets frame metadata accordingly.

[Stepan Bujnak]

On 11.07.2014 22:25, wm4 wrote:
> On Fri, 11 Jul 2014 22:13:54 +0200
> Stepan Bujnak <stepan.bujnak at 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.bujnak at 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 at 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?