[FFmpeg-devel] [PATCH 2/2] lavfi: make filter_frame non-recursive.

[James Almer]

On 11/13/2016 7:10 AM, Nicolas George wrote:
> A lot of changes happen at the same time:
> 
> - Add private_fields.h to devine private fields in AVFilterLink.
>   It allows to have the private fields directly in structured types
>   without needing an indirection for an opaque structure nor the
>   structure definition in public headers.

I don't really like this method. This kind of ifdeffery is very
uncommon and feels dirty. Just look at avutil/common.h to see how bad
a public header can get this way.
I'd prefer if we can stick with the usual internal opaque structs
instead.

The plan was to remove as many internal-but-not-really fields across
the codebase as possible on the next major bump. Ideally, we'd be
consistent with whatever method we choose to achieve this, and opaque
structs are already being used everywhere, including avfilter.h

[...]

> diff --git a/libavfilter/private_fields.h b/libavfilter/private_fields.h
> new file mode 100644
> index 0000000..5a3df68
> --- /dev/null
> +++ b/libavfilter/private_fields.h
> @@ -0,0 +1,43 @@
> +/*
> + * Generic frame queue
> + * Copyright (c) 2016 Nicolas George
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public License
> + * as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * 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 Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser 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
> + */
> +
> +#include "framequeue.h"
> +
> +#define AVFILTER_LINK_INTERNAL_FIELDS \
> +\
> +    FFFrameQueue fifo; \
> +\
> +    int frame_blocked_in; \
> +\
> +    /** \
> +     * Link input status. \
> +     */ \
> +    int status_in; \
> +    int64_t status_in_pts; \
> +\
> +    /** \
> +     * Link output status. \
> +     * If not zero, all attempts of request_frame will fail with the \
> +     * corresponding code.
> +     */ \
> +    int status_out; \
> + \

In any case, this is missing the usual header guards.

> +/* define AVFILTER_LINK_INTERNAL_FIELDS */
> diff --git a/libavfilter/split.c b/libavfilter/split.c
> index 6cf4ab1..89e3604 100644
> --- a/libavfilter/split.c
> +++ b/libavfilter/split.c
> @@ -30,6 +30,7 @@
>  #include "libavutil/mem.h"
>  #include "libavutil/opt.h"
>  
> +#include "private_fields.h"
>  #include "avfilter.h"
>  #include "audio.h"
>  #include "formats.h"
> @@ -78,7 +79,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
>      for (i = 0; i < ctx->nb_outputs; i++) {
>          AVFrame *buf_out;
>  
> -        if (ctx->outputs[i]->status)
> +        if (ctx->outputs[i]->status_in)
>              continue;
>          buf_out = av_frame_clone(frame);
>          if (!buf_out) {
> diff --git a/libavfilter/tests/filtfmts.c b/libavfilter/tests/filtfmts.c
> index 46a2d94..682af8c 100644
> --- a/libavfilter/tests/filtfmts.c
> +++ b/libavfilter/tests/filtfmts.c
> @@ -25,6 +25,7 @@
>  #include "libavutil/pixdesc.h"
>  #include "libavutil/samplefmt.h"
>  
> +#include "libavfilter/private_fields.h"
>  #include "libavfilter/avfilter.h"
>  #include "libavfilter/formats.h"
>  
> diff --git a/libavfilter/vf_extractplanes.c b/libavfilter/vf_extractplanes.c
> index a23f838..c4b01e0 100644
> --- a/libavfilter/vf_extractplanes.c
> +++ b/libavfilter/vf_extractplanes.c
> @@ -22,6 +22,7 @@
>  #include "libavutil/imgutils.h"
>  #include "libavutil/opt.h"
>  #include "libavutil/pixdesc.h"
> +#include "private_fields.h"
>  #include "avfilter.h"
>  #include "drawutils.h"
>  #include "internal.h"
> @@ -245,7 +246,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
>          const int idx = s->map[i];
>          AVFrame *out;
>  
> -        if (outlink->status)
> +        if (outlink->status_in)
>              continue;
>  
>          out = ff_get_video_buffer(outlink, outlink->w, outlink->h);
> diff --git a/tests/ref/fate/source b/tests/ref/fate/source
> index 4ee8a58..23ffd8a 100644
> --- a/tests/ref/fate/source
> +++ b/tests/ref/fate/source
> @@ -29,3 +29,4 @@ compat/cuda/nvcuvid.h
>  compat/float/float.h
>  compat/float/limits.h
>  compat/nvenc/nvEncodeAPI.h
> +libavfilter/private_fields.h

This shouldn't happen. Guess it's because of the missing guards?