[FFmpeg-devel] [PATCH 02/10] lavfi: support unknown channel layouts.

Stefano Sabatini stefasab at gmail.com
Tue Jan 1 23:10:08 CET 2013


On date Monday 2012-12-31 18:57:59 +0100, Nicolas George encoded:
> 
> Signed-off-by: Nicolas George <nicolas.george at normalesup.org>
> ---
>  libavfilter/audio.c         |    2 +-
>  libavfilter/avcodec.c       |    3 --
>  libavfilter/avfiltergraph.c |   86 ++++++++++++++++++++++++++++++++++++++-----
>  libavfilter/formats.c       |   76 +++++++++++++++++++++++++++++++++-----
>  libavfilter/formats.h       |   31 ++++++++++++++++
>  5 files changed, 176 insertions(+), 22 deletions(-)
> 
> 
> Slightly changed the data structure, inspired by Stefano's comments.
> Some code simplification, a few bug fixes. FATE passes.
> 
> 
> diff --git a/libavfilter/audio.c b/libavfilter/audio.c
> index ca076fc..9cecd79 100644
> --- a/libavfilter/audio.c
> +++ b/libavfilter/audio.c
> @@ -44,7 +44,7 @@ AVFilterBufferRef *ff_default_get_audio_buffer(AVFilterLink *link, int perms,
>      AVFilterBufferRef *samplesref = NULL;
>      uint8_t **data;
>      int planar      = av_sample_fmt_is_planar(link->format);
> -    int nb_channels = av_get_channel_layout_nb_channels(link->channel_layout);
> +    int nb_channels = link->channels;
>      int planes      = planar ? nb_channels : 1;
>      int linesize;
>      int full_perms = AV_PERM_READ | AV_PERM_WRITE | AV_PERM_PRESERVE |
> diff --git a/libavfilter/avcodec.c b/libavfilter/avcodec.c
> index dcd6bb6..b7d369b 100644
> --- a/libavfilter/avcodec.c
> +++ b/libavfilter/avcodec.c
> @@ -96,9 +96,6 @@ AVFilterBufferRef *avfilter_get_audio_buffer_ref_from_frame(const AVFrame *frame
>      int channels = av_frame_get_channels(frame);
>      int64_t layout = av_frame_get_channel_layout(frame);
>  
> -    if(av_frame_get_channels(frame) > 8) // libavfilter does not suport more than 8 channels FIXME, remove once libavfilter is fixed
> -        return NULL;
> -
>      if (layout && av_get_channel_layout_nb_channels(layout) != av_frame_get_channels(frame)) {
>          av_log(0, AV_LOG_ERROR, "Layout indicates a differnt number of channels than actually present\n");
>          return NULL;
> diff --git a/libavfilter/avfiltergraph.c b/libavfilter/avfiltergraph.c
> index f144624..ac76423 100644
> --- a/libavfilter/avfiltergraph.c
> +++ b/libavfilter/avfiltergraph.c
> @@ -185,9 +185,24 @@ AVFilterContext *avfilter_graph_get_filter(AVFilterGraph *graph, char *name)
>      return NULL;
>  }
>  
> +static void sanitize_channel_layouts(void *log, AVFilterChannelLayouts *l)
> +{
> +    if (!l)
> +        return;
> +    if (l->nb_channel_layouts) {
> +        if (l->all_layouts || l->all_counts)
> +            av_log(log, AV_LOG_WARNING, "All layouts set on non-empty list\n");
> +        l->all_layouts = l->all_counts = 0;
> +    } else {
> +        if (l->all_counts && !l->all_layouts)
> +            av_log(log, AV_LOG_WARNING, "All counts without all layouts\n");
> +        l->all_layouts = 1;
> +    }
> +}
> +
>  static int filter_query_formats(AVFilterContext *ctx)
>  {
> -    int ret;
> +    int ret, i;
>      AVFilterFormats *formats;
>      AVFilterChannelLayouts *chlayouts;
>      AVFilterFormats *samplerates;
> @@ -201,6 +216,11 @@ static int filter_query_formats(AVFilterContext *ctx)
>          return ret;
>      }
>  
> +    for (i = 0; i < ctx->nb_inputs; i++)
> +        sanitize_channel_layouts(ctx, ctx->inputs[i]->out_channel_layouts);
> +    for (i = 0; i < ctx->nb_outputs; i++)
> +        sanitize_channel_layouts(ctx, ctx->outputs[i]->in_channel_layouts);
> +
>      formats = ff_all_formats(type);
>      if (!formats)
>          return AVERROR(ENOMEM);
> @@ -470,7 +490,7 @@ static int pick_format(AVFilterLink *link, AVFilterLink *ref)
>          link->in_samplerates->format_count = 1;
>          link->sample_rate = link->in_samplerates->formats[0];
>  
> -        if (!link->in_channel_layouts->nb_channel_layouts) {

> +        if (link->in_channel_layouts->all_layouts) {

is this logic still valid? It looks backward to me.

>              av_log(link->src, AV_LOG_ERROR, "Cannot select channel layout for"
>                     "the link between filters %s and %s.\n", link->src->name,
>                     link->dst->name);
> @@ -478,7 +498,10 @@ static int pick_format(AVFilterLink *link, AVFilterLink *ref)
>          }
>          link->in_channel_layouts->nb_channel_layouts = 1;
>          link->channel_layout = link->in_channel_layouts->channel_layouts[0];
> -        link->channels = av_get_channel_layout_nb_channels(link->channel_layout);
> +        if ((link->channels = FF_LAYOUT2CHAN(link->channel_layout)))
> +            link->channel_layout = 0;
> +        else
> +            link->channels = av_get_channel_layout_nb_channels(link->channel_layout);
>      }
>  
>      ff_formats_unref(&link->in_formats);
> @@ -534,8 +557,39 @@ static int reduce_formats_on_filter(AVFilterContext *filter)
>                     format_count, ff_add_format);
>      REDUCE_FORMATS(int,      AVFilterFormats,        samplerates,     formats,
>                     format_count, ff_add_format);
> -    REDUCE_FORMATS(uint64_t, AVFilterChannelLayouts, channel_layouts,
> -                   channel_layouts, nb_channel_layouts, ff_add_channel_layout);
> +
> +    /* reduce channel layouts */
> +    for (i = 0; i < filter->nb_inputs; i++) {
> +        AVFilterLink *link = filter->inputs[i];

inlink?

> +        uint64_t fmt;
> +
> +        if (!link->out_channel_layouts ||
> +            link->out_channel_layouts->nb_channel_layouts != 1)
> +            continue;
> +        fmt = link->out_channel_layouts->channel_layouts[0];
> +
> +        for (j = 0; j < filter->nb_outputs; j++) {
> +            AVFilterLink *out_link = filter->outputs[j];

Nit: outlink?

> +            AVFilterChannelLayouts *fmts;
> +
> +            fmts = out_link->in_channel_layouts;
> +            if (link->type != out_link->type || fmts->nb_channel_layouts == 1)
> +                continue;
> +
> +            if (fmts->all_layouts) {
> +                ff_add_channel_layout(&out_link->in_channel_layouts, fmt);
> +                break;
> +            }
> +

> +            for (k = 0; k < out_link->in_channel_layouts->nb_channel_layouts; k++)
> +                if (fmts->channel_layouts[k] == fmt) {
> +                    fmts->channel_layouts[0]  = fmt;
> +                    fmts->nb_channel_layouts = 1;
> +                    ret = 1;
> +                    break;
> +                }

nit+++: for (...) {...}

> +        }
> +    }
>  
>      return ret;
>  }
> @@ -663,7 +717,18 @@ static void swap_channel_layouts_on_filter(AVFilterContext *filter)
>              int out_channels      = av_get_channel_layout_nb_channels(out_chlayout);
>              int count_diff        = out_channels - in_channels;
>              int matched_channels, extra_channels;
> -            int score = 0;
> +            int score = 100000;
> +
> +            if (FF_LAYOUT2CHAN(in_chlayout) || FF_LAYOUT2CHAN(out_chlayout)) {
> +                if (FF_LAYOUT2CHAN(in_chlayout))
> +                    in_channels = FF_LAYOUT2CHAN(in_chlayout);
> +                if (FF_LAYOUT2CHAN(out_chlayout))
> +                    out_channels = FF_LAYOUT2CHAN(out_chlayout);
> +                score -= 10000 + FFABS(out_channels - in_channels) +
> +                         (in_channels > out_channels ? 10000 : 0);
> +                /* let the remaining computation run and get 0 */
> +                in_chlayout = out_chlayout = 0;
> +            }
>  
>              /* channel substitution */
>              for (k = 0; k < FF_ARRAY_ELEMS(ch_subst); k++) {

> @@ -795,7 +860,8 @@ static int pick_formats(AVFilterGraph *graph)
>              if (filter->nb_inputs){
>                  for (j = 0; j < filter->nb_inputs; j++){
>                      if(filter->inputs[j]->in_formats && filter->inputs[j]->in_formats->format_count == 1) {
> -                        pick_format(filter->inputs[j], NULL);
> +                        if ((ret = pick_format(filter->inputs[j], NULL)) < 0)
> +                            return ret;
>                          change = 1;
>                      }
>                  }
> @@ -803,7 +869,8 @@ static int pick_formats(AVFilterGraph *graph)
>              if (filter->nb_outputs){
>                  for (j = 0; j < filter->nb_outputs; j++){
>                      if(filter->outputs[j]->in_formats && filter->outputs[j]->in_formats->format_count == 1) {
> -                        pick_format(filter->outputs[j], NULL);
> +                        if ((ret = pick_format(filter->outputs[j], NULL)) < 0)
> +                            return ret;
>                          change = 1;
>                      }
>                  }
> @@ -811,7 +878,8 @@ static int pick_formats(AVFilterGraph *graph)
>              if (filter->nb_inputs && filter->nb_outputs && filter->inputs[0]->format>=0) {
>                  for (j = 0; j < filter->nb_outputs; j++) {
>                      if(filter->outputs[j]->format<0) {
> -                        pick_format(filter->outputs[j], filter->inputs[0]);
> +                        if ((ret = pick_format(filter->outputs[j], filter->inputs[0])) < 0)
> +                            return ret;
>                          change = 1;
>                      }
>                  }

Unrelated, can be pushed apart.

> diff --git a/libavfilter/formats.c b/libavfilter/formats.c
> index ea9a184..90b61f5 100644
> --- a/libavfilter/formats.c
> +++ b/libavfilter/formats.c
> @@ -28,6 +28,8 @@
>  #include "internal.h"
>  #include "formats.h"
>  
> +#define KNOWN(l) (!FF_LAYOUT2CHAN(l)) /* for readability */
> +
>  /**
>   * Add all refs from a to ret and destroy a.
>   */
> @@ -136,21 +138,74 @@ AVFilterChannelLayouts *ff_merge_channel_layouts(AVFilterChannelLayouts *a,
>                                                   AVFilterChannelLayouts *b)
>  {
>      AVFilterChannelLayouts *ret = NULL;
> +    unsigned a_all = a->all_layouts + a->all_counts;
> +    unsigned b_all = b->all_layouts + b->all_counts;
> +    int ret_max, ret_nb = 0, i, j, round;
>  
>      if (a == b) return a;
>  
> -    if (a->nb_channel_layouts && b->nb_channel_layouts) {
> -        MERGE_FORMATS(ret, a, b, channel_layouts, nb_channel_layouts,
> -                      AVFilterChannelLayouts, fail);
> -    } else if (a->nb_channel_layouts) {
> -        MERGE_REF(a, b, channel_layouts, AVFilterChannelLayouts, fail);
> -        ret = a;
> -    } else {

> +    if (a_all < b_all) {
> +        FFSWAP(AVFilterChannelLayouts *, a, b);
> +        FFSWAP(unsigned, a_all, b_all);
> +    }

please add a comment here to clarify the logic

> +    if (a_all) {
> +        if (a_all == 1 && !b_all) {
> +            /* keep only known layouts in b; works also for b_all = 1 */
> +            for (i = j = 0; i < b->nb_channel_layouts; i++)
> +                if (KNOWN(b->channel_layouts[i]))
> +                    b->channel_layouts[j++] = b->channel_layouts[i];
> +            b->nb_channel_layouts = j;
> +        }
>          MERGE_REF(b, a, channel_layouts, AVFilterChannelLayouts, fail);
> -        ret = b;
> +        return b;
>      }
>  
> +    ret_max = a->nb_channel_layouts + b->nb_channel_layouts;
> +    if (!(ret = av_mallocz(sizeof(*ret))) ||
> +        !(ret->channel_layouts = av_malloc(sizeof(*ret->channel_layouts) *
> +                                           ret_max)))
> +        goto fail;
> +

> +    /* a[known] inter b[known] */

nit: inter -> intersect

> +    for (i = 0; i < a->nb_channel_layouts; i++) {

> +        if (!KNOWN(a->channel_layouts[i]))
> +            continue;

since we suppose that known channels are stored before the unknown
channels, couldn't you just break here?

> +        for (j = 0; j < a->nb_channel_layouts; j++) {

b->nb_channel_layouts?

> +            if (a->channel_layouts[i] == b->channel_layouts[j]) {
> +                ret->channel_layouts[ret_nb++] = a->channel_layouts[i];
> +                a->channel_layouts[i] = b->channel_layouts[j] = 0;
> +            }
> +        }
> +    }
> +    /* a[known] inter b[generic] + a[generic] inter b[known] */
> +    for (round = 0; round < 2; round++) {
> +        for (i = 0; i < a->nb_channel_layouts; i++) {
> +            uint64_t fmt = a->channel_layouts[i], bfmt;
> +            if (!fmt || !KNOWN(fmt))
> +                continue;
> +            bfmt = FF_CHAN2LAYOUT(av_get_channel_layout_nb_channels(fmt));
> +            for (j = 0; j < b->nb_channel_layouts; j++)
> +                if (b->channel_layouts[j] == bfmt)
> +                    ret->channel_layouts[ret_nb++] = a->channel_layouts[i];
> +        }
> +        FFSWAP(AVFilterChannelLayouts *, a, b);
> +    }
> +    /* a[generic] inter b[generic] */
> +    for (i = 0; i < a->nb_channel_layouts; i++) {
> +        if (KNOWN(a->channel_layouts[i]))
> +            continue;
> +        for (j = 0; j < b->nb_channel_layouts; j++)
> +            if (a->channel_layouts[i] == b->channel_layouts[j])
> +                ret->channel_layouts[ret_nb++] = a->channel_layouts[i];
> +    }
> +
> +    ret->nb_channel_layouts = ret_nb;
> +    if (!ret->nb_channel_layouts)
> +        goto fail;
> +    MERGE_REF(ret, a, channel_layouts, AVFilterChannelLayouts, fail);
> +    MERGE_REF(ret, b, channel_layouts, AVFilterChannelLayouts, fail);
>      return ret;

I have the feeling that the code is overcomplicated to perform the
merge and I'm having an hard time at grabbing the logic.

I suppose it may be as simple as:
- merge known layouts
- merge counts
- normalize

Maybe storing layouts and counts in separate fields may simplify the
iterative logic.

> +
>  fail:
>      if (ret) {
>          av_freep(&ret->refs);
> @@ -248,17 +303,19 @@ do {                                                        \
>                                                              \
>      (*f)->list = fmts;                                      \
>      (*f)->list[(*f)->nb++] = fmt;                           \
> -    return 0;                                               \
>  } while (0)
>  
>  int ff_add_format(AVFilterFormats **avff, int64_t fmt)
>  {
>      ADD_FORMAT(avff, fmt, int, formats, format_count);
> +    return 0;
>  }
>  
>  int ff_add_channel_layout(AVFilterChannelLayouts **l, uint64_t channel_layout)
>  {
>      ADD_FORMAT(l, channel_layout, uint64_t, channel_layouts, nb_channel_layouts);
> +    (*l)->all_layouts = (*l)->all_counts = 0;
> +    return 0;
>  }
>  
>  AVFilterFormats *ff_all_formats(enum AVMediaType type)
> @@ -309,6 +366,7 @@ AVFilterFormats *ff_all_samplerates(void)
>  AVFilterChannelLayouts *ff_all_channel_layouts(void)
>  {
>      AVFilterChannelLayouts *ret = av_mallocz(sizeof(*ret));
> +    ret->all_layouts = 1;
>      return ret;
>  }
>  
> diff --git a/libavfilter/formats.h b/libavfilter/formats.h
> index c5a4e3d..9fbed21 100644
> --- a/libavfilter/formats.h
> +++ b/libavfilter/formats.h
> @@ -69,15 +69,46 @@ struct AVFilterFormats {
>      struct AVFilterFormats ***refs; ///< references to this list
>  };
>  
> +/**
> + * A list of supported channel layouts.
> + *
> + * The list works the same as AVFilterFormats, except for the following
> + * differences:
> + * - A list with all_layouts = 1 means all channel layouts with a known
> + *   disposition; nb_channel_layouts must then be 0.
> + * - A list with all_counts = 1 means all channel counts, with a known or
> + *   unknown disposition ; nb_channel_layouts must then be 0 and all_layouts 1.
> + * - The list must not contain a layout with a known disposition and a
> + *   channel count with unknown disposition with the same number of channels
> + *   (e.g. AV_CH_LAYOUT_STEREO and FF_CHAN2LAYOUT(2).
> + */
>  typedef struct AVFilterChannelLayouts {
>      uint64_t *channel_layouts;  ///< list of channel layouts
>      int    nb_channel_layouts;  ///< number of channel layouts
> +    char all_layouts;           ///< accept any known channel layout
> +    char all_counts;            ///< accept any channel layout or count
>  
>      unsigned refcount;          ///< number of references to this list
>      struct AVFilterChannelLayouts ***refs; ///< references to this list
>  } AVFilterChannelLayouts;
> 
>  /**
> + * Encode a channel count as a channel layout.
> + * FF_CHAN2LAYOUT(c) means any channel layout with c channels, with a known
> + * or unknown disposition.
> + * The result is only valid inside AVFilterChannelLayouts and immediately
> + * related functions.
> + */
> +#define FF_CHAN2LAYOUT(c) (0x8000000000000000ULL | (c))

CHANNELS2LAYOUT?

> +
> +/**
> + * Decode a channel count encoded as a channel layout.
> + * Return 0 is the channel layout was a real one.
> + */
> +#define FF_LAYOUT2CHAN(l) (((l) & 0x8000000000000000ULL) ? \
> +                           (int)((l) & 0x7FFFFFFF) : 0)

LAYOUT2CHANNELS?

[...]
-- 
FFmpeg = Foolish and Fabulous Murdering Pacific Eager Ghost


More information about the ffmpeg-devel mailing list