On 12/27/16, James Almer <jamr...@gmail.com> wrote:
> 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.
>
Yes, what's point of that?
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
