[FFmpeg-devel] [PATCH] lavfi: add thumb video filter.

[Nicolas George]

Le nonidi 9 frimaire, an CCXX, Clément Bœsch a écrit :
> + at section thumb

Maybe thumbnail in full? Or even thumbnails, plural, depending on what it
does.

> +Select potential thumbnail frames.
> +
> +It accepts as argument the threshold of frames to analyze (default is 100). A
> +bigger value will result in a slower analysis and higher memory usage, but is
> +likely to be more efficient.

By reading that, I have no idea what the filter actually does, and I can not
fathom what the threshold is used for.

Also

> + * Copyright (C) 2011 Smartjog S.A.S, Clément Bœsch <clement.boesch at smartjog.com>

Apparently, the other files use lowercase C for copyright.

> + * Algorithm by Vadim Zaliva <lord at crocodile.org>.

Was this address openly written on the web until now?

> +    AVFilterBufferRef **frames;     ///< n_frames frames cache
> +    int *histogram;                 ///< array of RGB color distribution histograms

The calloc somewhat lower seems to indicate that:

struct thumb_frame {
    AVFilterBufferRef *buf;
    int hist[HIST_SZ];
};

and then:

    struct thumb_frame *frames;

would be more elegant throughout the code.

> +               "allocation failure, try to lower the frames threshold\n");

Apparently, they want capital on messages.

> +static float frame_rmse(const int *hist, const float *median)

Why a float rather than a double?

> +static av_cold void uninit(AVFilterContext *ctx)
> +{
> +    ThumbContext *thumb = ctx->priv;
> +    av_freep(&thumb->histogram);

You do not free thumb->frames?

> +    .inputs    = (const AVFilterPad[])  {{  .name             = "default",
> +                                            .type             = AVMEDIA_TYPE_VIDEO,

My narrow terminal finds that much of indentation not very readable, but
that is up to you.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20111130/ca335b84/attachment.asc>