| Message ID | 20200820145006.341018-4-george@nsup.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [FFmpeg-devel,1/4] lavfi: regroup formats lists in a single structure. | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
On Thu, Aug 20, 2020 at 04:50:05PM +0200, Nicolas George wrote: > The channel_layouts and channel_counts options set what buffersink > is supposed to accept. If channel_counts contains 2, then stereo is > already accepted, there is no point in having it in channel_layouts > too. This was not properly documented until now, so only print a > warning. > > Signed-off-by: Nicolas George <george@nsup.org> > --- > libavfilter/buffersink.c | 23 +++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > > > Added an explanation requested by Paul. > > > diff --git a/libavfilter/buffersink.c b/libavfilter/buffersink.c > index 76a46f6678..c58daf6124 100644 > --- a/libavfilter/buffersink.c > +++ b/libavfilter/buffersink.c > @@ -62,6 +62,28 @@ typedef struct BufferSinkContext { > > #define NB_ITEMS(list) (list ## _size / sizeof(*list)) > > +static void cleanup_redundant_layouts(AVFilterContext *ctx) > +{ > + BufferSinkContext *buf = ctx->priv; > + int nb_layouts = NB_ITEMS(buf->channel_layouts); > + int nb_counts = NB_ITEMS(buf->channel_counts); > + int l, lc, c, n; > + > + for (l = lc = 0; l < nb_layouts; l++) { > + n = av_get_channel_layout_nb_channels(buf->channel_layouts[l]); > + for (c = 0; c < nb_counts; c++) > + if (n == buf->channel_counts[c]) > + break; this would be O(nb_layouts * nb_counts) but can be done in O(nb_layouts + nb_counts) as in for (c = 0; c < nb_counts; c++) has_channel_count[ buf->channel_counts[c] ] = 1; for (l = lc = 0; l < nb_layouts; l++) { n = av_get_channel_layout_nb_channels(buf->channel_layouts[l]); if (has_channel_count[ n ]) break; not sure this is faster or how much it is in practice or how much that matters but its asymptotically better and its simple thx [...]
diff --git a/libavfilter/buffersink.c b/libavfilter/buffersink.c index 76a46f6678..c58daf6124 100644 --- a/libavfilter/buffersink.c +++ b/libavfilter/buffersink.c @@ -62,6 +62,28 @@ typedef struct BufferSinkContext { #define NB_ITEMS(list) (list ## _size / sizeof(*list)) +static void cleanup_redundant_layouts(AVFilterContext *ctx) +{ + BufferSinkContext *buf = ctx->priv; + int nb_layouts = NB_ITEMS(buf->channel_layouts); + int nb_counts = NB_ITEMS(buf->channel_counts); + int l, lc, c, n; + + for (l = lc = 0; l < nb_layouts; l++) { + n = av_get_channel_layout_nb_channels(buf->channel_layouts[l]); + for (c = 0; c < nb_counts; c++) + if (n == buf->channel_counts[c]) + break; + if (c < nb_counts) + av_log(ctx, AV_LOG_WARNING, + "Removing channel layout 0x%"PRIx64", redundant with %d channels\n", + buf->channel_layouts[l], buf->channel_counts[c]); + else + buf->channel_layouts[lc++] = buf->channel_layouts[l]; + } + buf->channel_layouts_size = lc * sizeof(*buf->channel_layouts); +} + int attribute_align_arg av_buffersink_get_frame(AVFilterContext *ctx, AVFrame *frame) { return av_buffersink_get_frame_flags(ctx, frame, 0); @@ -253,6 +275,7 @@ static int asink_query_formats(AVFilterContext *ctx) if (buf->channel_layouts_size || buf->channel_counts_size || buf->all_channel_counts) { + cleanup_redundant_layouts(ctx); for (i = 0; i < NB_ITEMS(buf->channel_layouts); i++) if ((ret = ff_add_channel_layout(&layouts, buf->channel_layouts[i])) < 0) return ret;
The channel_layouts and channel_counts options set what buffersink is supposed to accept. If channel_counts contains 2, then stereo is already accepted, there is no point in having it in channel_layouts too. This was not properly documented until now, so only print a warning. Signed-off-by: Nicolas George <george@nsup.org> --- libavfilter/buffersink.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) Added an explanation requested by Paul.