[FFmpeg-devel] [PATCH v3 1/2] avfilter: Add fsync filter

[Thilo Borgmann]

Am 06.01.24 um 18:39 schrieb James Almer:
> On 12/16/2023 5:13 AM, Thilo Borgmann via ffmpeg-devel wrote:
>> ---
>>   Changelog                |   1 +
>>   MAINTAINERS              |   1 +
>>   configure                |   2 +
>>   doc/filters.texi         |  33 +++++
>>   libavfilter/Makefile     |   1 +
>>   libavfilter/allfilters.c |   1 +
>>   libavfilter/version.h    |   2 +-
>>   libavfilter/vf_fsync.c   | 286 +++++++++++++++++++++++++++++++++++++++
>>   8 files changed, 326 insertions(+), 1 deletion(-)
>>   create mode 100644 libavfilter/vf_fsync.c
> 
> [...]
> 
>> +static int activate(AVFilterContext *ctx)
>> +{
>> +    FsyncContext *s       = ctx->priv;
>> +    AVFilterLink *inlink  = ctx->inputs[0];
>> +    AVFilterLink *outlink = ctx->outputs[0];
>> +
>> +    int ret, line_count;
>> +    AVFrame *frame;
>> +
>> +    FF_FILTER_FORWARD_STATUS_BACK(outlink, inlink);
>> +
>> +    buf_skip_eol(s);
>> +    line_count = buf_get_line_count(s);
>> +    if (line_count < 0) {
>> +        line_count = buf_reload(s);
>> +        if (line_count < 0)
>> +            return line_count;
>> +        line_count = buf_get_line_count(s);
>> +        if (line_count < 0)
>> +            return line_count;
>> +    }
>> +
>> +    if (avio_feof(s->avio_ctx) && buf_get_zero(s) < 3) {
>> +        av_log(ctx, AV_LOG_DEBUG, "End of file. To zero = %i\n", buf_get_zero(s));
>> +        goto end;
>> +    }
>> +
>> +    if (s->last_frame) {
>> +        ret = av_sscanf(s->cur, "%"PRId64" %"PRId64" %d/%d", &s->ptsi, &s->pts, &s->tb_num, &s->tb_den);
>> +        if (ret != 4) {
>> +            av_log(ctx, AV_LOG_ERROR, "Unexpected format found (%i / 4).\n", ret);
>> +            ff_outlink_set_status(outlink, AVERROR_INVALIDDATA, AV_NOPTS_VALUE);
>> +            return AVERROR_INVALIDDATA;
>> +        }
>> +
>> +        av_log(ctx, AV_LOG_DEBUG, "frame %lli ", s->last_frame->pts);
>> +
>> +        if (s->last_frame->pts >= s->ptsi) {
>> +            av_log(ctx, AV_LOG_DEBUG, ">= %lli: DUP LAST with pts = %lli\n", s->ptsi, s->pts);
>> +
>> +            // clone frame
>> +            frame = av_frame_clone(s->last_frame);
>> +            if (!frame) {
>> +                ff_outlink_set_status(outlink, AVERROR(ENOMEM), AV_NOPTS_VALUE);
>> +                return AVERROR(ENOMEM);
>> +            }
>> +
>> +            // set output pts and timebase
>> +            frame->pts = s->pts;
>> +            frame->time_base = av_make_q((int)s->tb_num, (int)s->tb_den);
>> +
>> +            // advance cur to eol, skip over eol in the next call
>> +            s->cur += line_count;
>> +
>> +            // call again
>> +            if (ff_inoutlink_check_flow(inlink, outlink))
>> +                ff_filter_set_ready(ctx, 100);
>> +
>> +            // filter frame
>> +            return ff_filter_frame(outlink, frame);
>> +        } else if (s->last_frame->pts < s->ptsi) {
>> +            av_log(ctx, AV_LOG_DEBUG, "<  %lli: DROP\n", s->ptsi);
>> +            av_frame_free(&s->last_frame);
>> +
>> +            // call again
>> +            if (ff_inoutlink_check_flow(inlink, outlink))
>> +                ff_filter_set_ready(ctx, 100);
>> +
>> +            return 0;
>> +        }
>> +    }
>> +
>> +end:
>> +    ret = ff_inlink_consume_frame(inlink, &s->last_frame);
> 
> There's a leak in this filter, and i suspect it's here. If s->last_frame is not NULL, the pointer will be rewritten and the frame will leak.
> 
> http://fate.ffmpeg.org/history.cgi?slot=x86_64-archlinux-gcc-valgrind-no-undef

Indeed - fixed & pushed.

Thanks,
Thilo