[FFmpeg-devel] 答复: [PATCH]Filter: Add snapshot filter. It is enable to save snapshot.

Nicolas George george at nsup.org
Fri May 27 13:49:31 CEST 2016


Le nonidi 9 prairial, an CCXXIV, kl222 a écrit :
> 发件人: ffmpeg-devel-bounces at ffmpeg.org [mailto:ffmpeg-devel-bounces at ffmpeg.org] 代表 Michael Niedermayer
> 发送时间: 2016年5月26日 19:24
> 收件人: FFmpeg development discussions and patches
> 主题: Re: [FFmpeg-devel] Filter: Add snapshot filter. It is enable to save snapshot.

Plese remember that top-posting is not welcome on this mailing-list. See
comments below on the patch itself.

> From: KangLin <kl222 at 126.com>
> Date: Wed, 25 May 2016 16:29:45 +0800
> Subject: [PATCH] avfilter/vf_snapshot: Add snapshot filter. It is enable to
>  save snapshot.

For possible future evolutions, we need a long-term contact. Who would we
call if the discussion of a global license change in the project were
started?

> 
> Signed-off-by: KangLin <kl222 at 126.com>
> ---
>  doc/filters.texi          |  23 +++++
>  libavfilter/Makefile      |   1 +
>  libavfilter/allfilters.c  |   1 +
>  libavfilter/vf_snapshot.c | 256 ++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 281 insertions(+)
>  create mode 100644 libavfilter/vf_snapshot.c
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index aa208f1..a75005a 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -12037,6 +12037,29 @@ in [-30,0] will filter edges. Default value is 0.
>  If a chroma option is not explicitly set, the corresponding luma value
>  is set.
>  
> + at section snapshot
> +
> +it can save a snapshot picture. Supports .png, .jpg, .bmp formats
> +Snapshot with process_command api.

Can you explain your use case for this filter?

I ask because I am very uncomfortable with filters that perform direct
output instead of producing frames for the rest of the graph. They need to
duplicate the encoding and muxing code, usually with fewer options and
features, and that duplicated code becomes itself a maintenance burden in
case of API changes and such.

IMHO, a better solution would be some kind of "interactive select" (or just
an option of plain select) filter, and encode its output as image2.

> +
> +It accepts the following options:
> +
> + at table @option
> + at item directory
> +Save the snapshot directory. Default value is snapshot.
> + at item filename
> +Snapshot file name
> + at end table
> +
> + at example
> +After ffmpeg start, when you want a snapshot, enter the following
> +command at the command console:
> +
> +csnapshot -1 filename snapshot.png
> +
> +Save the snapshot to the snapshot directory.
> + at end example
> +
>  @section ssim
>  
>  Obtain the SSIM (Structural SImilarity Metric) between two input videos.
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index 65a831e..aceeed6 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -292,6 +292,7 @@ OBJS-$(CONFIG_YADIF_FILTER)                  += vf_yadif.o
>  OBJS-$(CONFIG_ZMQ_FILTER)                    += f_zmq.o
>  OBJS-$(CONFIG_ZOOMPAN_FILTER)                += vf_zoompan.o
>  OBJS-$(CONFIG_ZSCALE_FILTER)                 += vf_zscale.o
> +OBJS-$(CONFIG_SNAPSHOT_FILTER)               += vf_snapshot.o
>  
>  OBJS-$(CONFIG_ALLRGB_FILTER)                 += vsrc_testsrc.o
>  OBJS-$(CONFIG_ALLYUV_FILTER)                 += vsrc_testsrc.o
> diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
> index d0d491e..b2e3b3c 100644
> --- a/libavfilter/allfilters.c
> +++ b/libavfilter/allfilters.c
> @@ -307,6 +307,7 @@ void avfilter_register_all(void)
>      REGISTER_FILTER(ZMQ,            zmq,            vf);
>      REGISTER_FILTER(ZOOMPAN,        zoompan,        vf);
>      REGISTER_FILTER(ZSCALE,         zscale,         vf);
> +    REGISTER_FILTER(SNAPSHOT,       snapshot,       vf);
>  
>      REGISTER_FILTER(ALLRGB,         allrgb,         vsrc);
>      REGISTER_FILTER(ALLYUV,         allyuv,         vsrc);
> diff --git a/libavfilter/vf_snapshot.c b/libavfilter/vf_snapshot.c
> new file mode 100644
> index 0000000..0f210e3
> --- /dev/null
> +++ b/libavfilter/vf_snapshot.c
> @@ -0,0 +1,256 @@
> +/*

> +* Copyright (c) 2002 Michael Niedermayer <michaelni at gmx.at>
> +* Copyright (c) 2012 Jeremy Tran

This looks nonsensical.

> +*
> +* This file is part of FFmpeg.
> +*

> +* FFmpeg is free software; you can redistribute it and/or modify
> +* it under the terms of the GNU General Public License as published by
> +* the Free Software Foundation; either version 2 of the License, or
> +* (at your option) any later version.

Most of FFmpeg code is under LGPL. Is the choice of GPL intentional? If so,
you need to update the dependencies in configure.

> +*
> +* FFmpeg is distributed in the hope that it will be useful,
> +* but WITHOUT ANY WARRANTY; without even the implied warranty of
> +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +* GNU General Public License for more details.
> +*
> +* You should have received a copy of the GNU General Public License along
> +* with FFmpeg; if not, write to the Free Software Foundation, Inc.,
> +* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> +*/
> +
> +/**
> + * @file
> + * Snapshot video filter, it can save a snapshot picture.
> + * Author:KangLin<kl222 at 126.com>
> + */
> +
> +#include <float.h>
> +#include <stdint.h>
> +
> +#include "libavutil/attributes.h"
> +#include "libavutil/avstring.h"
> +#include "libavutil/avassert.h"
> +#include "libavutil/opt.h"
> +#include "libavutil/imgutils.h"
> +#include "libavutil/internal.h"
> +#include "libavutil/timestamp.h"
> +#include "libavformat/avformat.h"
> +#include "avfilter.h"
> +#include "formats.h"
> +#include "internal.h"
> +#include "video.h"
> +
> +typedef struct SnapshotContext {
> +    /* common A/V fields */
> +    const AVClass *class;
> +
> +    char * pDir;
> +    char * pFileName;

Nit: "type *var", not "type* var" nor "type * var".

> +
> +    int bEnable;
> +    AVFormatContext *ofmt_ctx;

> +    AVCodecContext *pOutCodeCtx;
> +    AVCodec *pEncodec;
> +    AVPacket outPacket;

This is not FFmpeg coding style at all: noCamelCasePleaseItIsUnreadable. And
we do not include the type of the variables in their name. Your various 

(My understanding is that practice is meant to prevent mediocre developers
from getting confused by their own code too easily. I want to assume you are
not a mediocre developer.)

> +
> +}SnapshotContext;
> +
> +#define OFFSET(x) offsetof(SnapshotContext, x)
> +#define FLAGS AV_OPT_FLAG_FILTERING_PARAM | AV_OPT_FLAG_VIDEO_PARAM
> +
> +static const AVOption snapshot_options[] = {
> +    { "directory", NULL, OFFSET(pDir), AV_OPT_TYPE_STRING, { .str = "snapshot" }, .flags = FLAGS },
> +    { "filename", NULL, OFFSET(pFileName), AV_OPT_TYPE_STRING, { .str = "snapshot.png" }, .flags = FLAGS },
> +    { NULL },
> +};
> +
> +static int snapshot_close(AVFilterContext *ctx){
> +    int ret = 0;
> +    SnapshotContext *s = ctx->priv;
> +

> +    if (s->pOutCodeCtx)
> +    {
> +        avcodec_free_context(&s->pOutCodeCtx);
> +        s->pOutCodeCtx = NULL;
> +    }

avcodec_free_context() already does the check and clearing.

> +    if (s->ofmt_ctx){
> +        av_free_packet(&s->outPacket);
> +        avformat_free_context(s->ofmt_ctx);
> +        s->ofmt_ctx = NULL;
> +    }
> +    return ret;
> +}
> +

> +static int snapshot_open(AVFilterContext *ctx, AVFrame* frame){

Nit: brace for function body in a separate line.

> +    int ret = 0;
> +    SnapshotContext *s = ctx->priv;
> +    AVStream *out_stream = NULL;

> +    char f[1024] = { 0 };

You may want to look at the BPrint API.

> +    if (s->ofmt_ctx)
> +        return 0;
> +
> +    if (s->pDir){
> +        av_strlcpy(f, s->pDir, 1024);
> +        av_strlcat(f, "/", 1024);
> +    }
> +    if (s->pFileName)
> +        av_strlcat(f, s->pFileName, 1024);
> +    else{
> +        av_log(ctx, AV_LOG_ERROR, "please set filename.\n");
> +        return AVERROR(EPERM);
> +    }
> +
> +    ret = avformat_alloc_output_context2(&s->ofmt_ctx, NULL, NULL, f);
> +    if (ret < 0){
> +        av_log(ctx, AV_LOG_ERROR, "open file is fail:%d;filename:%s\n", ret, f);
> +        return ret;
> +    }
> +    if (!s->ofmt_ctx) {
> +        av_log(ctx, AV_LOG_ERROR, "open file is fail:%d\n", ret);
> +        return ret;
> +    }
> +
> +    av_init_packet(&s->outPacket);
> +
> +    do{
> +        s->pEncodec = avcodec_find_encoder(s->ofmt_ctx->oformat->video_codec);
> +        if (!s->pEncodec){

> +            av_log(ctx, AV_LOG_ERROR, "encodec isn't found.codec id:%d\n",
> +                   s->ofmt_ctx->oformat->video_codec);
> +            break;

ret is still 0 at this point and will be returned like that, while an error
occurred. Same below.

> +        }
> +
> +        out_stream = avformat_new_stream(s->ofmt_ctx, s->pEncodec);
> +        if (!out_stream) {
> +            av_log(ctx, AV_LOG_ERROR, "Failed allocating output stream\n");
> +            ret = AVERROR_UNKNOWN;
> +            break;
> +        }
> +
> +        //Write file header
> +        ret = avformat_write_header(s->ofmt_ctx, NULL);
> +        if (ret < 0) {
> +            av_log(ctx, AV_LOG_ERROR, "avformat_write_header is fail:%d\n", ret);
> +            break;
> +        }
> +
> +        s->pOutCodeCtx = avcodec_alloc_context3(s->pEncodec);
> +        if (!s->pOutCodeCtx)
> +            break;
> +        s->pOutCodeCtx->width = frame->width;
> +        s->pOutCodeCtx->height = frame->height;
> +        s->pOutCodeCtx->pix_fmt = s->pEncodec->pix_fmts[0];
> +        s->pOutCodeCtx->time_base = (AVRational){ 1, 1 };
> +
> +        if ((ret = avcodec_open2(s->pOutCodeCtx, s->pEncodec, NULL)) < 0) {
> +            av_log(ctx, AV_LOG_ERROR, "Cannot open video encoder:%d\n", ret);
> +            break;
> +        }
> +
> +        return ret;
> +    }while (0);
> +
> +    snapshot_close(ctx);
> +
> +    return ret;
> +}
> +
> +static int snapshot_save(AVFilterContext *ctx, AVFrame* frame){
> +    int ret = 0;
> +    SnapshotContext *s = ctx->priv;
> +    int get_out_frame = 0;
> +
> +    ret = snapshot_open(ctx, frame);
> +    if (ret < 0)
> +        return ret;
> +
> +    ret = avcodec_encode_video2(s->pOutCodeCtx, &s->outPacket, frame, &get_out_frame);
> +    if (ret < 0){
> +        av_log(ctx, AV_LOG_ERROR, "Cannot open video encoder:%d\n", ret);
> +        snapshot_close(ctx);
> +        return ret;
> +    }
> +
> +    if (get_out_frame)
> +    {
> +        ret = av_interleaved_write_frame(s->ofmt_ctx, &s->outPacket);
> +        if (ret < 0) {
> +            av_log(ctx, AV_LOG_ERROR, "Error muxing packet\n");
> +        }
> +        //Write file trailer
> +        av_write_trailer(s->ofmt_ctx);
> +        s->bEnable = 0;
> +        snapshot_close(ctx);
> +    }
> +
> +    return ret;
> +}
> +
> +static int snapshot_filter_frame(AVFilterLink *inlink, AVFrame *frame){
> +    int ret = 0;
> +    AVFilterContext *ctx = inlink->dst;
> +    SnapshotContext *s = ctx->priv;
> +    if (s->bEnable) {
> +        ret = snapshot_save(ctx, frame);
> +        if (ret) {
> +            av_log(ctx, AV_LOG_ERROR, "snapshot save fail:%d\n", ret);
> +            s->bEnable = 0;
> +        }
> +    }
> +    ret = ff_filter_frame(inlink->dst->outputs[0], frame);
> +    return ret;
> +}
> +
> +static av_cold void snapshot_uninit(AVFilterContext *ctx){
> +    snapshot_close(ctx);
> +}
> +
> +static av_cold int snapshot_init(AVFilterContext *ctx){
> +    SnapshotContext *snapshot = ctx->priv;
> +    snapshot->bEnable = 0;
> +    return 0;
> +}
> +
> +static int snapshot_process_command(AVFilterContext *ctx, const char *cmd, const char *args,
> +                                    char *res, int res_len, int flags)
> +{
> +    int ret = 0;
> +    SnapshotContext *snapshot = ctx->priv;
> +
> +    if (!strcmp(cmd, "filename")){
> +        ret = av_opt_set(snapshot, "filename", args, 0);
> +        snapshot->bEnable = 1;
> +    }
> +    return ret;
> +}
> +
> +static const AVFilterPad avfilter_snapshot_inputs[] = {
> +    {
> +        .name = "default",
> +        .type = AVMEDIA_TYPE_VIDEO,
> +        .filter_frame = snapshot_filter_frame,
> +    },
> +    { NULL }
> +};
> +
> +static const AVFilterPad avfilter_snapshot_outputs[] = {
> +    {
> +        .name = "default",
> +        .type = AVMEDIA_TYPE_VIDEO,
> +    },
> +    { NULL }
> +};
> +
> +AVFILTER_DEFINE_CLASS(snapshot);
> +AVFilter ff_vf_snapshot = {
> +    .name = "snapshot",

> +    .description = NULL_IF_CONFIG_SMALL("Snapshot filter, it can save a snapshot picture. Supports .png, .jpg, .bmp formats"),

The description is usually not that verbose.

> +    .priv_size = sizeof(SnapshotContext),
> +    .priv_class = &snapshot_class,
> +    .init = snapshot_init,
> +    .uninit = snapshot_uninit,
> +    .process_command = snapshot_process_command,
> +    .inputs = avfilter_snapshot_inputs,
> +    .outputs = avfilter_snapshot_outputs,
> +};

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160527/110cf9d8/attachment.sig>


More information about the ffmpeg-devel mailing list