[FFmpeg-devel] [PATCH] Channel Layout Negotiation

Stefano Sabatini stefano.sabatini-lala at poste.it
Fri Jun 17 17:45:36 CEST 2011


On date Friday 2011-06-17 18:07:39 +0300, MNZ encoded:
> Another iteration over this.
> 
> This time an avfilter_make_format64_list() was added and the original was
> retained. Also instead of avfilter_set_common_formats() there are now separate
> functions for pixel_formats, sample_formats and channel_layouts. The names are a
> bit too long though.
> 
> --
> Mina

> From 8de1ad310758db13fea3e2f5fc093b52de178c75 Mon Sep 17 00:00:00 2001
> From: Mina Nagy Zaki <mnzaki at gmail.com>
> Date: Tue, 7 Jun 2011 21:17:23 +0300
> Subject: [PATCH 1/2] lavfi: Use int64_t lists in AVFilteFormats.
> 
> The list type was changed to int64_t to be able to hold
> channel layouts.
> 
> avfilter_make_format_list() still takes a int32_t array and converts
> it to int64_t. A new function, avfilter_make_format64_list, that
> takes int64_t arrays has been added.
> ---
>  libavfilter/avfilter.h |    5 +++--
>  libavfilter/formats.c  |   36 ++++++++++++++++++++++++------------
>  2 files changed, 27 insertions(+), 14 deletions(-)
> 
> diff --git a/libavfilter/avfilter.h b/libavfilter/avfilter.h
> index 7628cd5..9651d86 100644
> --- a/libavfilter/avfilter.h
> +++ b/libavfilter/avfilter.h
> @@ -223,7 +223,7 @@ void avfilter_unref_buffer(AVFilterBufferRef *ref);
>   */
>  typedef struct AVFilterFormats {
>      unsigned format_count;      ///< number of formats
> -    int *formats;               ///< list of media formats
> +    int64_t *formats;           ///< list of media formats
>  
>      unsigned refcount;          ///< number of references to this list
>      struct AVFilterFormats ***refs; ///< references to this list
> @@ -238,6 +238,7 @@ typedef struct AVFilterFormats {
>   * @return the format list, with no existing references
>   */
>  AVFilterFormats *avfilter_make_format_list(const int *fmts);
> +AVFilterFormats *avfilter_make_format64_list(const int64_t *fmts);
>  
>  /**
>   * Add fmt to the list of media formats contained in *avff.
> @@ -247,7 +248,7 @@ AVFilterFormats *avfilter_make_format_list(const int *fmts);
>   * @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_format(AVFilterFormats **avff, int fmt);
> +int avfilter_add_format(AVFilterFormats **avff, int64_t fmt);
>  
>  /**
>   * Return a list of all formats supported by FFmpeg for the given media type.
> diff --git a/libavfilter/formats.c b/libavfilter/formats.c
> index 58593fc..40254a0 100644
> --- a/libavfilter/formats.c
> +++ b/libavfilter/formats.c
> @@ -72,28 +72,40 @@ AVFilterFormats *avfilter_merge_formats(AVFilterFormats *a, AVFilterFormats *b)
>      return ret;
>  }
>  

> +#define MAKE_FORMAT_LIST()                                              \
> +    AVFilterFormats *formats;                                           \
> +    int count = 0;                                                      \
> +do {                                                                    \
> +    if (fmts)                                                           \
> +        for (count = 0; fmts[count] != -1; count++)                     \
> +            ;                                                           \
> +    formats = av_mallocz(sizeof(AVFilterFormats));                      \
> +    formats->format_count = count;                                      \
> +    if (count)                                                          \
> +        formats->formats  = av_malloc(sizeof(*formats->formats)*count); \
> +} while (0)

Missing NULL check => return NULL

Alternatively, you could create a function
static AVFilterFormats *make_format_list(int count);
which returns NULL in case of failure.

> +
>  AVFilterFormats *avfilter_make_format_list(const int *fmts)
>  {
> -    AVFilterFormats *formats;
> -    int count = 0;
> +    MAKE_FORMAT_LIST();
> +    while (count--)
> +        formats->formats[count] = fmts[count];
>  
> -    if (fmts)
> -        for (count = 0; fmts[count] != -1; count++)
> -            ;
> +    return formats;
> +}
>  
> -    formats               = av_mallocz(sizeof(AVFilterFormats));
> -    formats->format_count = count;
> -    if (count) {
> -        formats->formats  = av_malloc(sizeof(*formats->formats) * count);
> +AVFilterFormats *avfilter_make_format64_list(const int64_t *fmts)
> +{
> +    MAKE_FORMAT_LIST();
> +    if (count)
>          memcpy(formats->formats, fmts, sizeof(*formats->formats) * count);
> -    }
>  
>      return formats;
>  }
>  
> -int avfilter_add_format(AVFilterFormats **avff, int fmt)
> +int avfilter_add_format(AVFilterFormats **avff, int64_t fmt)
>  {
> -    int *fmts;
> +    int64_t *fmts;
>  
>      if (!(*avff) && !(*avff = av_mallocz(sizeof(AVFilterFormats))))
>          return AVERROR(ENOMEM);
> -- 
> 1.7.4.4

Looks OK otherwise. 

> From 60e9b4e924e177fd089094176e3ab19005896b3f Mon Sep 17 00:00:00 2001
> From: Mina Nagy Zaki <mnzaki at gmail.com>
> Date: Tue, 7 Jun 2011 21:46:12 +0300
> Subject: [PATCH 2/2] lavfi: add layout negotiation fields and helper
>  functions.
> 
> ---
>  cmdutils.c                  |    2 +-
>  ffplay.c                    |    2 +-
>  libavfilter/avfilter.c      |    3 ++
>  libavfilter/avfilter.h      |   32 +++++++++++++------
>  libavfilter/avfiltergraph.c |   10 +++++-
>  libavfilter/defaults.c      |   71 ++++++++++++++++++++++++-------------------
>  libavfilter/formats.c       |   22 +++++++++++++
>  libavfilter/vf_blackframe.c |    2 +-
>  libavfilter/vf_crop.c       |    2 +-
>  libavfilter/vf_cropdetect.c |    2 +-
>  libavfilter/vf_drawbox.c    |    2 +-
>  libavfilter/vf_drawtext.c   |    2 +-
>  libavfilter/vf_fade.c       |    2 +-
>  libavfilter/vf_format.c     |    4 +-
>  libavfilter/vf_frei0r.c     |    2 +-
>  libavfilter/vf_gradfun.c    |    2 +-
>  libavfilter/vf_hflip.c      |    2 +-
>  libavfilter/vf_hqdn3d.c     |    2 +-
>  libavfilter/vf_libopencv.c  |    2 +-
>  libavfilter/vf_mp.c         |    2 +-
>  libavfilter/vf_pad.c        |    2 +-
>  libavfilter/vf_transpose.c  |    2 +-
>  libavfilter/vf_unsharp.c    |    2 +-
>  libavfilter/vf_yadif.c      |    2 +-
>  libavfilter/vsrc_buffer.c   |    2 +-
>  libavfilter/vsrc_color.c    |    2 +-
>  libavfilter/vsrc_movie.c    |    2 +-
>  27 files changed, 119 insertions(+), 65 deletions(-)
> 
> diff --git a/cmdutils.c b/cmdutils.c
> index 1f7ecc4..91caa14 100644
> --- a/cmdutils.c
> +++ b/cmdutils.c
> @@ -901,7 +901,7 @@ static int ffsink_query_formats(AVFilterContext *ctx)
>      FFSinkContext *priv = ctx->priv;
>      enum PixelFormat pix_fmts[] = { priv->pix_fmt, PIX_FMT_NONE };
>  
> -    avfilter_set_common_formats(ctx, avfilter_make_format_list(pix_fmts));
> +    avfilter_set_common_pixel_formats(ctx, avfilter_make_format_list(pix_fmts));
>      return 0;
>  }
>  
> diff --git a/ffplay.c b/ffplay.c
> index 31a6832..c2f3df6 100644
> --- a/ffplay.c
> +++ b/ffplay.c
> @@ -1643,7 +1643,7 @@ static int input_query_formats(AVFilterContext *ctx)
>          priv->is->video_st->codec->pix_fmt, PIX_FMT_NONE
>      };
>  
> -    avfilter_set_common_formats(ctx, avfilter_make_format_list(pix_fmts));
> +    avfilter_set_common_pixel_formats(ctx, avfilter_make_format_list(pix_fmts));
>      return 0;
>  }
>  
> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
> index 3b2e3ca..44dd515 100644
> --- a/libavfilter/avfilter.c
> +++ b/libavfilter/avfilter.c
> @@ -217,6 +217,9 @@ int avfilter_insert_filter(AVFilterLink *link, AVFilterContext *filt,
>      if (link->out_formats)
>          avfilter_formats_changeref(&link->out_formats,
>                                     &filt->outputs[filt_dstpad_idx]->out_formats);
> +    if (link->out_chlayouts)
> +        avfilter_formats_changeref(&link->out_chlayouts,
> +                                   &filt->outputs[filt_dstpad_idx]->out_chlayouts);
>  
>      return 0;
>  }
> diff --git a/libavfilter/avfilter.h b/libavfilter/avfilter.h
> index 9651d86..f25ebfa 100644
> --- a/libavfilter/avfilter.h
> +++ b/libavfilter/avfilter.h
> @@ -256,6 +256,11 @@ int avfilter_add_format(AVFilterFormats **avff, int64_t fmt);
>  AVFilterFormats *avfilter_all_formats(enum AVMediaType type);
>  
>  /**
> + * Return a list of all channel layouts supported by FFmpeg.
> + */
> +AVFilterFormats *avfilter_all_channel_layouts(void);
> +
> +/**
>   * Return a format list which contains the intersection of the formats of
>   * a and b. Also, all the references of a, all the references of b, and
>   * a and b themselves will be deallocated.
> @@ -466,11 +471,13 @@ AVFilterBufferRef *avfilter_default_get_audio_buffer(AVFilterLink *link, int per
>                                                       int64_t channel_layout, int planar);
>  
>  /**
> - * A helper for query_formats() which sets all links to the same list of
> - * formats. If there are no links hooked to this filter, the list of formats is
> - * freed.
> + * Helpers for query_formats() which set all links to the same list of
> + * formats/layouts. If there are no links hooked to this filter, the list
> + * of formats is freed.
>   */
> -void avfilter_set_common_formats(AVFilterContext *ctx, AVFilterFormats *formats);
> +void avfilter_set_common_pixel_formats(AVFilterContext *ctx, AVFilterFormats *formats);
> +void avfilter_set_common_sample_formats(AVFilterContext *ctx, AVFilterFormats *formats);
> +void avfilter_set_common_channel_layouts(AVFilterContext *ctx, AVFilterFormats *formats);
>  
>  /** Default handler for query_formats() */
>  int avfilter_default_query_formats(AVFilterContext *ctx);
> @@ -521,9 +528,9 @@ typedef struct AVFilter {
>      void (*uninit)(AVFilterContext *ctx);
>  
>      /**
> -     * Queries formats supported by the filter and its pads, and sets the
> -     * in_formats for links connected to its output pads, and out_formats
> -     * for links connected to its input pads.
> +     * Queries formats/layouts supported by the filter and its pads, and sets
> +     * the in_formats/in_chlayouts for links connected to its output pads,
> +     * and out_formats/out_chlayouts for links connected to its input pads.
>       *
>       * @return zero on success, a negative value corresponding to an
>       * AVERROR code otherwise
> @@ -593,13 +600,18 @@ struct AVFilterLink {
>      int format;                 ///< agreed upon media format
>  
>      /**
> -     * Lists of formats 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 member, above, when chosen.
> +     * 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;
>  
> +    AVFilterFormats *in_chlayouts;
> +    AVFilterFormats *out_chlayouts;
> +
>      /**
>       * The buffer reference currently being sent across the link by the source
>       * filter. This is used internally by the filter system to allow
> diff --git a/libavfilter/avfiltergraph.c b/libavfilter/avfiltergraph.c
> index 60d529b..19909a3 100644
> --- a/libavfilter/avfiltergraph.c
> +++ b/libavfilter/avfiltergraph.c
> @@ -195,9 +195,17 @@ static void pick_format(AVFilterLink *link)
>  
>      link->in_formats->format_count = 1;
>      link->format = link->in_formats->formats[0];
> -
>      avfilter_formats_unref(&link->in_formats);
>      avfilter_formats_unref(&link->out_formats);
> +
> +    if (link->type == AVMEDIA_TYPE_AUDIO) {
> +        link->in_chlayouts->format_count = 1;
> +        link->channel_layout = link->in_chlayouts->formats[0];
> +        avfilter_formats_unref(&link->in_chlayouts);
> +        avfilter_formats_unref(&link->out_chlayouts);

> +

Nit+++: superfluous empty line

> +    }
> +
>  }
>  
>  static void pick_formats(AVFilterGraph *graph)
> diff --git a/libavfilter/defaults.c b/libavfilter/defaults.c
> index c39ed64..f75c0c6 100644
> --- a/libavfilter/defaults.c
> +++ b/libavfilter/defaults.c
> @@ -197,45 +197,54 @@ int avfilter_default_config_output_link(AVFilterLink *link)
>      return 0;
>  }
>  
> -/**
> - * A helper for query_formats() which sets all links to the same list of
> - * formats. If there are no links hooked to this filter, the list of formats is
> - * freed.
> - *
> - * FIXME: this will need changed for filters with a mix of pad types
> - * (video + audio, etc)
> - */
> -void avfilter_set_common_formats(AVFilterContext *ctx, AVFilterFormats *formats)

> +static void set_common(AVFilterContext *ctx, AVFilterFormats *fmts,

Nit+: maybe set_common_formats() is more explicative.

> +                             enum AVMediaType type, int offin, int offout)

Nit+++: align

>  {
> -    int count = 0, i;
> -
> -    for (i = 0; i < ctx->input_count; i++) {
> -        if (ctx->inputs[i]) {
> -            avfilter_formats_ref(formats, &ctx->inputs[i]->out_formats);
> -            count++;
> -        }
> -    }
> -    for (i = 0; i < ctx->output_count; i++) {
> -        if (ctx->outputs[i]) {
> -            avfilter_formats_ref(formats, &ctx->outputs[i]->in_formats);
> -            count++;
> -        }
> +    int i;
> +    for (i = 0; i < ctx->input_count; i++)
> +        if (ctx->inputs[i] && ctx->inputs[i]->type == type)
> +            avfilter_formats_ref(fmts,
> +                                 (AVFilterFormats**)((void*)ctx->inputs[i]+offout));
> +
> +    for (i = 0; i < ctx->output_count; i++)
> +        if (ctx->outputs[i] && ctx->outputs[i]->type == type)
> +            avfilter_formats_ref(fmts,
> +                                 (AVFilterFormats**)((void*)ctx->outputs[i]+offin));
> +
> +    if (!fmts->refcount) {
> +        av_free(fmts->formats);
> +        av_free(fmts->refs);
> +        av_free(fmts);
>      }
> +}
>  
> -    if (!count) {
> -        av_free(formats->formats);
> -        av_free(formats->refs);
> -        av_free(formats);
> -    }
> +void avfilter_set_common_pixel_formats(AVFilterContext *ctx, AVFilterFormats *formats)
> +{
> +    set_common(ctx, formats, AVMEDIA_TYPE_VIDEO,
> +               offsetof(AVFilterLink, in_formats),
> +               offsetof(AVFilterLink, out_formats));
> +}
> +
> +void avfilter_set_common_sample_formats(AVFilterContext *ctx, AVFilterFormats *formats)
> +{
> +    set_common(ctx, formats, AVMEDIA_TYPE_AUDIO,
> +               offsetof(AVFilterLink, in_formats),
> +               offsetof(AVFilterLink, out_formats));
> +}
> +
> +void avfilter_set_common_channel_layouts(AVFilterContext *ctx, AVFilterFormats *formats)
> +{
> +    set_common(ctx, formats, AVMEDIA_TYPE_AUDIO,
> +               offsetof(AVFilterLink, in_chlayouts),
> +               offsetof(AVFilterLink, out_chlayouts));
>  }
>  
>  int avfilter_default_query_formats(AVFilterContext *ctx)
>  {
> -    enum AVMediaType type = ctx->inputs  && ctx->inputs [0] ? ctx->inputs [0]->type :
> -                            ctx->outputs && ctx->outputs[0] ? ctx->outputs[0]->type :
> -                            AVMEDIA_TYPE_VIDEO;
> +    avfilter_set_common_pixel_formats(ctx, avfilter_all_formats(AVMEDIA_TYPE_VIDEO));
> +    avfilter_set_common_sample_formats(ctx, avfilter_all_formats(AVMEDIA_TYPE_AUDIO));
> +    avfilter_set_common_channel_layouts(ctx, avfilter_all_channel_layouts());
>  
> -    avfilter_set_common_formats(ctx, avfilter_all_formats(type));
>      return 0;
>  }
>  
> diff --git a/libavfilter/formats.c b/libavfilter/formats.c
> index 40254a0..dd4da80 100644
> --- a/libavfilter/formats.c
> +++ b/libavfilter/formats.c
> @@ -20,6 +20,7 @@
>   */
>  
>  #include "libavutil/pixdesc.h"
> +#include "libavutil/audioconvert.h"
>  #include "avfilter.h"
>  
>  /**
> @@ -135,6 +136,27 @@ AVFilterFormats *avfilter_all_formats(enum AVMediaType type)
>      return ret;
>  }
>  
> +AVFilterFormats *avfilter_all_channel_layouts(void)
> +{
> +    static int64_t chlayouts[] = {
> +        AV_CH_LAYOUT_MONO,
> +        AV_CH_LAYOUT_STEREO,
> +        AV_CH_LAYOUT_4POINT0,
> +        AV_CH_LAYOUT_QUAD,
> +        AV_CH_LAYOUT_5POINT0,
> +        AV_CH_LAYOUT_5POINT0_BACK,
> +        AV_CH_LAYOUT_5POINT1,
> +        AV_CH_LAYOUT_5POINT1_BACK,
> +        AV_CH_LAYOUT_5POINT1|AV_CH_LAYOUT_STEREO_DOWNMIX,
> +        AV_CH_LAYOUT_7POINT1,
> +        AV_CH_LAYOUT_7POINT1_WIDE,
> +        AV_CH_LAYOUT_7POINT1|AV_CH_LAYOUT_STEREO_DOWNMIX,
> +        -1,
> +    };
> +
> +    return avfilter_make_format64_list(chlayouts);
> +}
> +
>  void avfilter_formats_ref(AVFilterFormats *f, AVFilterFormats **ref)
>  {
>      *ref = f;

Looks fine to me.
-- 
FFmpeg = Free and Free Most Perennial Elastic God


More information about the ffmpeg-devel mailing list