On Mon, Mar 02, 2015 at 10:42:27PM +0100, Anton Khirnov wrote:
> Quoting Diego Biurrun (2015-02-25 15:28:14)
> > From: Niels Möller <[email protected]>
> > 
> > ---
> > Review questions from last round addressed.
> 
> The commit message and the changelog still claim it's lossless. They
> should both clearly say that it's not full support.
> 
> > +                } else {
> > +                    if (chset->primary_ch_set) {
> > +                        for (i = 0; i < chset->downmix_ncoeffs; i++)
> > +                            if ((chset->downmix_coeffs[i] = 
> > dca_get_dmix_coeff(s)) == -1)
> 
> You're checking for -1, but the function returns a proper error code.
> Same below twice.
> 
> > +    if (data_start + 8 * s->xll_navi.band_size[0] > asset_end) {
> > +        av_log(s->avctx, AV_LOG_ERROR,
> > +               "XLL: Data in NAVI table exceeds containing asset\n"
> > +               "start: %d (bit), size %u (bytes), end %d (bit), error 
> > %u\n",
> > +               data_start, s->xll_navi.band_size[0], asset_end,
> > +               data_start + 8 * s->xll_navi.band_size[0] - asset_end);
> 
> So shouldn't this return an error?
> 
> > +            if (s->avctx->sample_rate != chset->sampling_frequency) {
> > +                av_log(s->avctx, AV_LOG_WARNING,
> > +                       "XLL: unexected chset sample rate %d, expected 
> > %d\n",
> 
> nit: unexpected
> 
> > @@ -1050,8 +1159,19 @@ static int dca_decode_frame(AVCodecContext *avctx, 
> > void *data,
> > +    if (!avpkt->pos)
> > +        s->frame_index  =
> > +        s->sample_index = 0;
> >  
> > -    s->xch_present = 0;
> > +    av_log(avctx, AV_LOG_DEBUG,
> > +           "%s: stream_index %d, pos %"PRId64", frame index %u, sample 
> > index %u\n",
> > +           __func__, avpkt->stream_index, avpkt->pos,
> > +           s->frame_index, s->sample_index);
> > +    s->frame_index++;
> 
> I think this debugging log should be dropped, along with
> frame_index/sample_index.

All fixed locally.

Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to