Diego Biurrun <[email protected]> writes:
> I have a bunch of mostly small comments below. If you cannot be bothered
> to address them, I will.
If you can take care of it, I'm grateful.
>> @@ -1461,6 +1660,730 @@ static void
>> dca_exss_skip_mix_coeffs(GetBitContext *gb, int channels, int out_ch
>> +
>> +/* Returns -1 on error */
>> +static int32_t dca_get_dmix_coeff(DCAContext *s)
>> +{
>> +}
>> +
>> +/* Returns -1 on error */
>> +static int32_t dca_get_inv_dmix_coeff(DCAContext *s)
>> +{
>> +}
>
> Returning a more descriptive error code would be better here, probably
> AVERROR_INVALLIDDATA.
Returning an error code might collide with a possible dmix value. The
particular value -1 is not a possible output for valid input. So I
think it makes sense to have the caller check for this and return
AVERROR_INVALIDDATA (which is the only possible reason for failure
here).
Or one could change the function to return an error code and have a
separate pointer argument for the output value. In my, not very strong,
opinion, that would be useless clutter.
>> + /* Layout: First the sample buffer for one segment per channel,
>> + * followed by history buffers of DCA_XLL_AORDER_MAX samples for
>> + * each channel. */
>> + av_fast_malloc(&s->xll_sample_buf, &s->xll_sample_buf_size,
>> + (s->xll_smpl_in_seg + DCA_XLL_AORDER_MAX) *
>> + s->xll_channels * sizeof(*s->xll_sample_buf));
>> + if (!s->xll_sample_buf)
>> + return AVERROR(ENOMEM);
>
> Where does this memory get freed?
There's an av_freep(&s->xll_sample_buf) in dca_decode_end. Is that the
right way to do it?
Regards,
/Niels
--
Niels Möller. PGP-encrypted email is preferred. Keyid C0B98E26.
Internet email is subject to wholesale government surveillance.
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
