On 12/27/2016 3:02 PM, Nicolas George wrote:
> AVFilterLink contains fields of internal types that are better
> isolated. Furthermore, with the prospect of inter-filters
> threading, it is better if filters do not access links directly.
> 
> Signed-off-by: Nicolas George <geo...@nsup.org>
> ---
>  libavfilter/Makefile       |   1 +
>  libavfilter/avfilter.h     | 217 +--------------------------------------
>  libavfilter/avfilterlink.h | 248 
> +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 250 insertions(+), 216 deletions(-)
>  create mode 100644 libavfilter/avfilterlink.h
> 
> 
> That way, making it public or private is just a matter of changing one line
> in the Makefile.
> 

[...]

> diff --git a/libavfilter/avfilterlink.h b/libavfilter/avfilterlink.h
> new file mode 100644
> index 0000000000..51ab322ae9
> --- /dev/null
> +++ b/libavfilter/avfilterlink.h
> @@ -0,0 +1,248 @@
> +/*
> + * AVFilterLink definition
> + * Copyright (c) 2007 Bobby Bingham
> + *
> + * 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
> + */
> +
> +#ifndef AVFILTER_AVFILTERLINK_H
> +#define AVFILTER_AVFILTERLINK_H
> +
> +#include "avfilter.h"
> +
> +#ifdef FF_INTERNAL_FIELDS
> +# include "framequeue.h"
> +#endif
> +
> +/**
> + * A link between two filters. This contains pointers to the source and
> + * destination filters between which this link exists, and the indexes of
> + * the pads involved. In addition, this link also contains the parameters
> + * which have been negotiated and agreed upon between the filter, such as
> + * image dimensions, format, etc.
> + */
> +struct AVFilterLink {
> +    AVFilterContext *src;       ///< source filter
> +    AVFilterPad *srcpad;        ///< output pad on the source filter
> +
> +    AVFilterContext *dst;       ///< dest filter
> +    AVFilterPad *dstpad;        ///< input pad on the dest filter
> +
> +    enum AVMediaType type;      ///< filter media type
> +
> +    /* These parameters apply only to video */
> +    int w;                      ///< agreed upon image width
> +    int h;                      ///< agreed upon image height
> +    AVRational sample_aspect_ratio; ///< agreed upon sample aspect ratio
> +    /* These parameters apply only to audio */
> +    uint64_t channel_layout;    ///< channel layout of current buffer (see 
> libavutil/channel_layout.h)
> +    int sample_rate;            ///< samples per second
> +
> +    int format;                 ///< agreed upon media format
> +
> +    /**
> +     * Define the time base used by the PTS of the frames/samples
> +     * which will pass through this link.
> +     * During the configuration stage, each filter is supposed to
> +     * change only the output timebase, while the timebase of the
> +     * input link is assumed to be an unchangeable property.
> +     */
> +    AVRational time_base;
> +
> +    /*****************************************************************
> +     * All fields below this line are not part of the public API. They
> +     * may not be used outside of libavfilter and can be changed and
> +     * removed at will.
> +     * New public fields should be added right above.
> +     *****************************************************************
> +     */
> +    /**
> +     * Lists of formats and channel layouts supported by the input and output
> +     * filters respectively. These lists are used for negotiating the format
> +     * to actually be used, which will be loaded into the format and
> +     * channel_layout members, above, when chosen.
> +     *
> +     */
> +    AVFilterFormats *in_formats;
> +    AVFilterFormats *out_formats;
> +
> +    /**
> +     * Lists of channel layouts and sample rates used for automatic
> +     * negotiation.
> +     */
> +    AVFilterFormats  *in_samplerates;
> +    AVFilterFormats *out_samplerates;
> +    struct AVFilterChannelLayouts  *in_channel_layouts;
> +    struct AVFilterChannelLayouts *out_channel_layouts;
> +
> +    /**
> +     * Audio only, the destination filter sets this to a non-zero value to
> +     * request that buffers with the given number of samples should be sent 
> to
> +     * it. AVFilterPad.needs_fifo must also be set on the corresponding input
> +     * pad.
> +     * Last buffer before EOF will be padded with silence.
> +     */
> +    int request_samples;
> +
> +    /** stage of the initialization of the link properties (dimensions, etc) 
> */
> +    enum {
> +        AVLINK_UNINIT = 0,      ///< not started
> +        AVLINK_STARTINIT,       ///< started, but incomplete
> +        AVLINK_INIT             ///< complete
> +    } init_state;
> +
> +    /**
> +     * Graph the filter belongs to.
> +     */
> +    struct AVFilterGraph *graph;
> +
> +    /**
> +     * Current timestamp of the link, as defined by the most recent
> +     * frame(s), in link time_base units.
> +     */
> +    int64_t current_pts;
> +
> +    /**
> +     * Current timestamp of the link, as defined by the most recent
> +     * frame(s), in AV_TIME_BASE units.
> +     */
> +    int64_t current_pts_us;
> +
> +    /**
> +     * Index in the age array.
> +     */
> +    int age_index;
> +
> +    /**
> +     * Frame rate of the stream on the link, or 1/0 if unknown or variable;
> +     * if left to 0/0, will be automatically copied from the first input
> +     * of the source filter if it exists.
> +     *
> +     * Sources should set it to the best estimation of the real frame rate.
> +     * If the source frame rate is unknown or variable, set this to 1/0.
> +     * Filters should update it if necessary depending on their function.
> +     * Sinks can use it to set a default output frame rate.
> +     * It is similar to the r_frame_rate field in AVStream.
> +     */
> +    AVRational frame_rate;
> +
> +    /**
> +     * Buffer partially filled with samples to achieve a fixed/minimum size.
> +     */
> +    AVFrame *partial_buf;
> +
> +    /**
> +     * Size of the partial buffer to allocate.
> +     * Must be between min_samples and max_samples.
> +     */
> +    int partial_buf_size;
> +
> +    /**
> +     * Minimum number of samples to filter at once. If filter_frame() is
> +     * called with fewer samples, it will accumulate them in partial_buf.
> +     * This field and the related ones must not be changed after filtering
> +     * has started.
> +     * If 0, all related fields are ignored.
> +     */
> +    int min_samples;
> +
> +    /**
> +     * Maximum number of samples to filter at once. If filter_frame() is
> +     * called with more samples, it will split them.
> +     */
> +    int max_samples;
> +
> +    /**
> +     * Number of channels.
> +     */
> +    int channels;
> +
> +    /**
> +     * Link processing flags.
> +     */
> +    unsigned flags;
> +
> +    /**
> +     * Number of past frames sent through the link.
> +     */
> +    int64_t frame_count_in, frame_count_out;
> +
> +    /**
> +     * A pointer to a FFVideoFramePool struct.
> +     */
> +    void *video_frame_pool;
> +
> +    /**
> +     * True if a frame is currently wanted on the output of this filter.
> +     * Set when ff_request_frame() is called by the output,
> +     * cleared when a frame is filtered.
> +     */
> +    int frame_wanted_out;
> +
> +    /**
> +     * For hwaccel pixel formats, this should be a reference to the
> +     * AVHWFramesContext describing the frames.
> +     */
> +    AVBufferRef *hw_frames_ctx;
> +
> +#ifndef FF_INTERNAL_FIELDS
> +
> +    /**
> +     * Internal structure members.
> +     * The fields below this limit are internal for libavfilter's use
> +     * and must in no way be accessed by applications.

How is this any different than the above notice? Everything below time_base
is also meant for internal use and shouldn't be accessed by applications or
other ffmpeg libraries.
Is extending the scope of FF_INTERNAL_FIELDS to cover the above fields your
intention for the next major bump?

> +     */
> +    char reserved[0xF000];

What's the point in reserving space like this? And isn't 60kb a bit overkill?

I insist this is just ugly and unconventional.

> +
> +#else /* FF_INTERNAL_FIELDS */
> +
> +    /**
> +     * Queue of frames waiting to be filtered.
> +     */
> +    FFFrameQueue fifo;
> +
> +    /**
> +     * If set, the source filter can not generate a frame as is.
> +     * The goal is to avoid repeatedly calling the request_frame() method on
> +     * the same link.
> +     */
> +    int frame_blocked_in;
> +
> +    /**
> +     * Link input status.
> +     * If not zero, all attempts of filter_frame will fail with the
> +     * corresponding code.
> +     */
> +    int status_in;
> +
> +    /**
> +     * Timestamp of the input status change.
> +     */
> +    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;
> +
> +#endif /* FF_INTERNAL_FIELDS */
> +
> +};
> +
> +#endif /* AVFILTER_AVFILTERLINK_H */
> 

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to