Andreas Rheinhardt (12020-08-13):
> The channel layouts accepted by ff_merge_channel_layouts() are of two
> types: Ordinary channel layouts and generic channel layouts. These are
> layouts that match all layouts with a certain number of channels.
> Therefore parsing these channel layouts is not done in one go; instead
> first the intersection of the ordinary layouts of the first input
> list of channel layouts with the ordinary layouts of the second list is
> determined, then the intersection of the ordinary layouts of the first
> one and the generic layouts of the second one etc. In order to mark the
> ordinary channel layouts that have already been matched as used they are
> zeroed. The inner loop that does this is as follows:
>
> 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];
> a->channel_layouts[i] = b->channel_layouts[j] = 0;
> }
> }
>
> (Here ret->channel_layouts is the array containing the intersection of
> the two input arrays.)
>
> Yet the problem with this code is that after a match has been found, the
> loop continues the search with the new value a->channel_layouts[i].
> The intention of zeroing these elements was to make sure that elements
> already paired at this stage are ignored later. And while they are indeed
> ignored when pairing ordinary and generic channel layouts later, it has
> the exact opposite effect when pairing ordinary channel layouts.
>
> To see this consider the channel layouts A B C D E and E D C B A. In the
> first round, A and A will be paired and added to ret->channel_layouts.
> In the second round, the input arrays are 0 B C D E and E D C B 0.
> At first B and B will be matched and zeroed, but after doing so matching
> continues, but this time it will search for 0, which will match with the
> last entry of the second array. ret->channel_layouts now contains A B 0.
> In the third round, C 0 0 will be added to ret->channel_layouts etc.
> This gives a quadratic amount of elements, yet the amount of elements
> allocated for said array is only the sum of the sizes of a and b.
>
> This issue can e.g. be reproduced by
> ffmpeg -f lavfi -i anullsrc=cl=7.1 \
> -af 'aformat=cl=mono|stereo|2.1|3.0|4.0,aformat=cl=4.0|3.0|2.1|stereo|mono' \
> -f null -Oh, good catch. It is a bug indeed. > The fix is easy: break out of the inner loop after having found a match. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@gmail.com> > --- > 1. This patch is intended to be applied before > https://ffmpeg.org/pipermail/ffmpeg-devel/2020-August/267790.html > > 2. After this patch the code will stay within the buffer provided that > no list contains one and the same generic channel layout multiple times > (ordinary channel layouts may appear arbitrary often and there may even > be ordinary channel layouts and generic channel layouts of the same > channel count in the same list). But is this actually ensured? > (Given that the concept of generic channel layouts is not part of the > public API, but just valid in AVFilterChannelLayouts the question can be > rephrased as: Is it possible for the (API) user to somehow create > generic channel layouts?) I thought it was ensured by the rest of the code, where the lists of supported formats are very limited. But I forgot what you found: a few filters accept them from the application or the user. In particular, bufferink can inject generic layouts. I will make a patch that checks the validity after calling query_formats. > > libavfilter/formats.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/libavfilter/formats.c b/libavfilter/formats.c > index 00d050e439..f39396ed81 100644 > --- a/libavfilter/formats.c > +++ b/libavfilter/formats.c > @@ -219,6 +219,7 @@ AVFilterChannelLayouts > *ff_merge_channel_layouts(AVFilterChannelLayouts *a, > 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; > + break; > } > } > } I think you can add the break to the other two loops too. Regards, -- Nicolas George
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
