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

[James Almer <jamrial@gmail.com>]

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

> +    if (ret < 0)
> +        return ret;
> +
> +    FF_FILTER_FORWARD_STATUS(inlink, outlink);
> +    FF_FILTER_FORWARD_WANTED(outlink, inlink);
> +
> +    return FFERROR_NOT_READY;
> +}
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".