[FFmpeg-devel] [PATCH] Generalize pixel format enum fields to int formats
Stefano Sabatini
stefano.sabatini-lala
Tue Jul 20 11:54:13 CEST 2010
On date Monday 2010-07-19 21:35:58 -0700, S.N. Hemanth Meenakshisundaram encoded:
> Almost same patch as earlier. Bringing some doxy changes corresponding
> to PixelFormat format -> int format change from the lavfi audio patch.
> Used the phrase 'media formats' in place of pixel formats or colorspace
> since it is generic and does not talk about sample formats or audio
> which hasn't been added as of this patch.
>
> ---
> libavfilter/avfilter.c | 5 +++--
> libavfilter/avfilter.h | 22 ++++++++++++----------
> libavfilter/avfiltergraph.c | 4 ++--
> libavfilter/defaults.c | 5 ++++-
> libavfilter/formats.c | 35 ++++++++++++++++++++---------------
> libavfilter/vf_scale.c | 4 ++--
> 6 files changed, 43 insertions(+), 32 deletions(-)
>
>
> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
> index f442cdc..411551b 100644
> --- a/libavfilter/avfilter.c
> +++ b/libavfilter/avfilter.c
> @@ -97,7 +97,8 @@ int avfilter_link(AVFilterContext *src, unsigned srcpad,
> link->dst = dst;
> link->srcpad = srcpad;
> link->dstpad = dstpad;
> - link->format = PIX_FMT_NONE;
> + link->type = src->output_pads[srcpad].type;
> + link->format = -1;
>
> return 0;
> }
> @@ -121,7 +122,7 @@ int avfilter_insert_filter(AVFilterLink *link, AVFilterContext *filt,
> link->dstpad = in;
> filt->inputs[in] = link;
>
> - /* if any information on supported colorspaces already exists on the
> + /* if any information on supported media formats already exists on the
> * link, we need to preserve that */
> if(link->out_formats)
> avfilter_formats_changeref(&link->out_formats,
> diff --git a/libavfilter/avfilter.h b/libavfilter/avfilter.h
> index 3737d46..066bdc3 100644
> --- a/libavfilter/avfilter.h
> +++ b/libavfilter/avfilter.h
> @@ -68,7 +68,7 @@ typedef struct AVFilterBuffer
> {
> uint8_t *data[4]; ///< buffer data for each plane
> int linesize[4]; ///< number of bytes per line
> - enum PixelFormat format; ///< colorspace
> + int format; ///< media format
>
> unsigned refcount; ///< number of references to this buffer
>
> @@ -190,7 +190,7 @@ typedef struct AVFilterFormats AVFilterFormats;
> struct AVFilterFormats
> {
> unsigned format_count; ///< number of formats
> - enum PixelFormat *formats; ///< list of pixel formats
> + int *formats; ///< list of media formats
>
> unsigned refcount; ///< number of references to this list
> AVFilterFormats ***refs; ///< references to this list
> @@ -199,25 +199,25 @@ struct AVFilterFormats
> /**
> * Create a list of supported formats. This is intended for use in
> * AVFilter->query_formats().
> - * @param pix_fmts list of pixel formats, terminated by PIX_FMT_NONE
> + * @param fmts list of media formats, terminated by -1
> * @return the format list, with no existing references
> */
> -AVFilterFormats *avfilter_make_format_list(const enum PixelFormat *pix_fmts);
> +AVFilterFormats *avfilter_make_format_list(const int *fmts);
>
> /**
> - * Add pix_fmt to the list of pixel formats contained in *avff.
> + * Add fmt to the list of media formats contained in *avff.
> * If *avff is NULL the function allocates the filter formats struct
> * and puts its pointer in *avff.
> *
> * @return a non negative value in case of success, or a negative
> * value corresponding to an AVERROR code in case of error
> */
> -int avfilter_add_colorspace(AVFilterFormats **avff, enum PixelFormat pix_fmt);
> +int avfilter_add_format(AVFilterFormats **avff, int fmt);
>
> /**
> - * Return a list of all colorspaces supported by FFmpeg.
> + * Return a list of all formats supported by FFmpeg for the given media type.
> */
> -AVFilterFormats *avfilter_all_colorspaces(void);
> +AVFilterFormats *avfilter_all_formats(enum AVMediaType type);
>
> /**
> * Return a format list which contains the intersection of the formats of
> @@ -507,9 +507,11 @@ struct AVFilterLink
> AVLINK_INIT ///< complete
> } init_state;
>
> + enum AVMediaType type; ///< filter media type
> +
> int w; ///< agreed upon image width
> int h; ///< agreed upon image height
> - enum PixelFormat format; ///< agreed upon image colorspace
> + int format; ///< agreed upon media format
>
> /**
> * Lists of formats supported by the input and output filters respectively.
> @@ -544,7 +546,7 @@ int avfilter_link(AVFilterContext *src, unsigned srcpad,
> AVFilterContext *dst, unsigned dstpad);
>
> /**
> - * Negotiate the colorspace, dimensions, etc of all inputs to a filter.
> + * Negotiate the media format, dimensions, etc of all inputs to a filter.
> * @param filter the filter to negotiate the properties for its inputs
> * @return zero on successful negotiation
> */
> diff --git a/libavfilter/avfiltergraph.c b/libavfilter/avfiltergraph.c
> index 9ad6536..6f219a6 100644
> --- a/libavfilter/avfiltergraph.c
> +++ b/libavfilter/avfiltergraph.c
> @@ -111,7 +111,7 @@ static int query_formats(AVFilterGraph *graph, AVClass *log_ctx)
> int scaler_count = 0;
> char inst_name[30];
>
> - /* ask all the sub-filters for their supported colorspaces */
> + /* ask all the sub-filters for their supported media formats */
> for(i = 0; i < graph->filter_count; i ++) {
> if(graph->filters[i]->filter->query_formats)
> graph->filters[i]->filter->query_formats(graph->filters[i]);
> @@ -197,7 +197,7 @@ int avfilter_graph_config_formats(AVFilterGraph *graph, AVClass *log_ctx)
> return -1;
>
> /* Once everything is merged, it's possible that we'll still have
> - * multiple valid colorspace choices. We pick the first one. */
> + * multiple valid media format choices. We pick the first one. */
> pick_formats(graph);
>
> return 0;
> diff --git a/libavfilter/defaults.c b/libavfilter/defaults.c
> index ed1db94..8c63e5a 100644
> --- a/libavfilter/defaults.c
> +++ b/libavfilter/defaults.c
> @@ -160,7 +160,10 @@ void avfilter_set_common_formats(AVFilterContext *ctx, AVFilterFormats *formats)
>
> int avfilter_default_query_formats(AVFilterContext *ctx)
> {
> - avfilter_set_common_formats(ctx, avfilter_all_colorspaces());
> + enum AVMediaType type = AVMEDIA_TYPE_VIDEO;
> + if (ctx->inputs[0]) type = ctx->inputs[0]->type;
> + else if (ctx->outputs[0]) type = ctx->outputs[0]->type;
Maybe a bit more compact:
enum AVMediaType type = ctx->inputs [0] ? ctx->inputs [0]->type :
ctx->outputs[0] ? ctx->outputs[0]->type : AVMEDIA_TYPE_VIDEO;
> + avfilter_set_common_formats(ctx, avfilter_all_formats(type));
> return 0;
This looks potentially troublesome in case of a trans-media filter,
but if it works with what we have now is fine.
> }
>
> diff --git a/libavfilter/formats.c b/libavfilter/formats.c
> index 2a9bdb0..b870358 100644
> --- a/libavfilter/formats.c
> +++ b/libavfilter/formats.c
> @@ -70,47 +70,52 @@ AVFilterFormats *avfilter_merge_formats(AVFilterFormats *a, AVFilterFormats *b)
> return ret;
> }
>
> -AVFilterFormats *avfilter_make_format_list(const enum PixelFormat *pix_fmts)
> +AVFilterFormats *avfilter_make_format_list(const int *fmts)
> {
> AVFilterFormats *formats;
> int count;
>
> - for (count = 0; pix_fmts[count] != PIX_FMT_NONE; count++)
> + for (count = 0; fmts[count] != -1; count++)
> ;
>
> formats = av_mallocz(sizeof(AVFilterFormats));
> formats->formats = av_malloc(sizeof(*formats->formats) * count);
> formats->format_count = count;
> - memcpy(formats->formats, pix_fmts, sizeof(*formats->formats) * count);
> + memcpy(formats->formats, fmts, sizeof(*formats->formats) * count);
>
> return formats;
> }
>
> -int avfilter_add_colorspace(AVFilterFormats **avff, enum PixelFormat pix_fmt)
> +int avfilter_add_format(AVFilterFormats **avff, int fmt)
> {
> - enum PixelFormat *pix_fmts;
> + int *fmts;
>
> if (!(*avff) && !(*avff = av_mallocz(sizeof(AVFilterFormats))))
> return AVERROR(ENOMEM);
>
> - pix_fmts = av_realloc((*avff)->formats,
> - sizeof((*avff)->formats) * ((*avff)->format_count+1));
> - if (!pix_fmts)
> + fmts = av_realloc((*avff)->formats,
> + sizeof((*avff)->formats) * ((*avff)->format_count+1));
> + if (!fmts)
> return AVERROR(ENOMEM);
>
> - (*avff)->formats = pix_fmts;
> - (*avff)->formats[(*avff)->format_count++] = pix_fmt;
> + (*avff)->formats = fmts;
> + (*avff)->formats[(*avff)->format_count++] = fmt;
> return 0;
> }
>
> -AVFilterFormats *avfilter_all_colorspaces(void)
> +AVFilterFormats *avfilter_all_formats(enum AVMediaType type)
> {
> AVFilterFormats *ret = NULL;
> - enum PixelFormat pix_fmt;
> + int fmt;
> + int num_formats = 0;
These two can be merged, num_formats can be initialized during
declaration with something like:
num_formats = type == AVMEDIA_TYPE_VIDEO ? PIX_FMT_NB : 0;
>
> - for (pix_fmt = 0; pix_fmt < PIX_FMT_NB; pix_fmt++)
> - if (!(av_pix_fmt_descriptors[pix_fmt].flags & PIX_FMT_HWACCEL))
> - avfilter_add_colorspace(&ret, pix_fmt);
> + if (type == AVMEDIA_TYPE_VIDEO)
> + num_formats = PIX_FMT_NB;
> +
> + for (fmt = 0; fmt < num_formats; fmt++)
> + if ((type != AVMEDIA_TYPE_VIDEO) ||
> + !(av_pix_fmt_descriptors[fmt].flags & PIX_FMT_HWACCEL))
> + avfilter_add_format(&ret, fmt);
This logic looks a bit weird, better use positive logic (less
bug-prone), like this:
if ( (type == AVMEDIA_TYPE_VIDEO && !(av_pix_fmt_..)) ||
(type == AVMEDIA_TYPE_AUDIO ...))
>
> return ret;
> }
> diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
> index 29fd3fe..56b1cf6 100644
> --- a/libavfilter/vf_scale.c
> +++ b/libavfilter/vf_scale.c
> @@ -83,7 +83,7 @@ static int query_formats(AVFilterContext *ctx)
> formats = NULL;
> for (pix_fmt = 0; pix_fmt < PIX_FMT_NB; pix_fmt++)
> if ( sws_isSupportedInput(pix_fmt)
> - && (ret = avfilter_add_colorspace(&formats, pix_fmt)) < 0) {
> + && (ret = avfilter_add_format(&formats, pix_fmt)) < 0) {
> avfilter_formats_unref(&formats);
> return ret;
> }
> @@ -93,7 +93,7 @@ static int query_formats(AVFilterContext *ctx)
> formats = NULL;
> for (pix_fmt = 0; pix_fmt < PIX_FMT_NB; pix_fmt++)
> if ( sws_isSupportedOutput(pix_fmt)
> - && (ret = avfilter_add_colorspace(&formats, pix_fmt)) < 0) {
> + && (ret = avfilter_add_format(&formats, pix_fmt)) < 0) {
> avfilter_formats_unref(&formats);
> return ret;
> }
Patch looks OK apart the comments above, I suppose it has been tested
through make lavfitest and make test.
While at it please also write an APIchanges entry, leave in blank
revision and lib numbers, I'll fill that after the commit.
Regards.
--
FFmpeg = Fundamental Fabulous Multipurpose Power Enhanced God
More information about the ffmpeg-devel
mailing list