Quoting Diego Biurrun (2015-03-11 19:15:37) > On Wed, Mar 11, 2015 at 02:32:57PM +0100, Niels Möller wrote: > > Diego Biurrun <[email protected]> writes: > > > > > I don't see that saving our skin, what if chset->downmix_ncoeffs is equal > > > to DCA_XLL_DMIX_NCOEFFS_MAX? Why shouldn't it overflow on the second > > > iteration if chset->channels is 16? > > > > The thing is that chset->downmix_ncoeffs is the total number of > > coefficients which will be read in the loop (and they are stored > > consecutively, even if that's not entirely obvious), and if that value > > happens to equal DCA_XLL_DMIX_NCOEFFS_MAX, that just means that the > > values fit precisely in the buffer. > > > > And I don't think it is possible (in the !chset->primary_ch_set case, > > which I think is what we're discussing) to have chset->channels == 16 > > and downmix_ncoeffs <= DCA_XLL_DMIX_NCOEFFS_MAX. Since xll_channels is > > the total number of channels in all channel sets, it will also be at > > least 16, so chset->channels == 16 implies that downmix_ncoeffs >= 16 * > > (16 + 1) = 272. > > Yes, that is the case we are discussing. For context: > > #define DCA_XLL_DMIX_NCOEFFS_MAX (18) > > typedef struct XllChSetSubHeader { > int channels; ///< number of channels in channel set, at > most 16 > int downmix_coeffs[DCA_XLL_DMIX_NCOEFFS_MAX]; > > and let me paste the actual block of code: > > unsigned c, r; > for (c = 0, i = 0; c < s->xll_channels; c++, i += chset->channels + 1) { > if ((chset->downmix_coeffs[i] = dca_get_inv_dmix_coeff(s)) == -1) > return AVERROR_INVALIDDATA; > for (r = 1; r <= chset->channels; r++) { > int32_t coeff = dca_get_dmix_coeff(s); > if (coeff == -1) > return AVERROR_INVALIDDATA; > chset->downmix_coeffs[i + r] = > (chset->downmix_coeffs[i] * (int64_t) coeff + (1 << 15)) >> > 16; > } > } > > 'i' and 'i + r' can grow past 18 easily unless I'm missing something if the > outer > for-loop is traversed more than once. s->xll_channels can be larger than 1, > so > please enlighten me how this does not overflow the downmix_coeffs[18] array > ...
I am satisfied with Niels' original explanation and agree that this code doesn't overwrite the array. In this branch, chset->downmix_ncoeffs is equal to (chset->channels + 1) * s->xll_channels and it's verified that this number is not larger than the array size i here is at most (s->xll_channels - 1) * (chset->channels + 1) r goes up to chset->channels so (i + r) <= s->xll_channels * (chset->channels + 1) - 1, which exactly fits in the array -- Anton Khirnov _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
