[MPlayer-dev-eng] [PATCH] latm faad handling

[Dan Oscarsson]

On 2010-06-21 at 18:48 +0200 Reimar Döffinger wrote:
> > Fixed in the attached updated patch.
> 
> Applied the demux_ts part.
> Does someone know what this patch will cause to happen if the LATM
> demuxing support is enabled in the libavformat demuxer and it does
> not use a separate codec id for LATM?

If you call lavf demuxer on a mpeg-ts stream that calls this code, it
will works somewhat better then without the patch as it first probes for
LATM but it will still fail for those LATM files that need to be probed
deeper to find the LATM tags. So unless I am misstaken it should work as
without the patch or better.

> 
> > --- libmpcodecs/ad_faad.c.org	2010-06-12 17:25:38.999522927 +0200
> > +++ libmpcodecs/ad_faad.c	2010-06-21 18:00:21.204759059 +0200
> > @@ -126,6 +126,27 @@
> >      faacDecSetConfiguration(faac_hdec, faac_conf);
> >  
> >      sh->a_in_buffer_len = demux_read_data(sh->ds, sh->a_in_buffer, sh->a_in_buffer_size);
> > +    /* init the codec, look for LATM */
> > +    faac_init = faacDecInit(faac_hdec, sh->a_in_buffer,
> > +                            sh->a_in_buffer_len, &faac_samplerate, &faac_channels,1);
> > +    if (faac_init < 0 && sh->format == mmioFOURCC('M', 'P', '4', 'L')) {
> > +        // working LATM not found at first try, look further on in stream
> > +        int i;
> > +
> > +        for (i = 0; i < 5 && sh->a_in_buffer_len >= 3; i++) {
> > +            pos = sh->a_in_buffer_len-3;
> > +            memmove(sh->a_in_buffer, &(sh->a_in_buffer[pos]), 3);
> > +            sh->a_in_buffer_len  = 3;
> 
> sh->a_in_buffer_len >= 3 should always be true except possibly during the first iteration,
> so this check can go into the first if (though it might actually be better to call demux_read_data
> in a loop untile we get enough data, and I really suspect 3 bytes are not enough for faacDecInit
> in general...)

Yes, I could move it to the if. 3 bytes is not enough so in those cases
FaacDecInit will fail and the code will loop and try to get more data. I
suspect the cases where demux_read fails are eof or faulty data. The
normal case would be for demux_read to fill buffer? If so I do not think
we need a read loop to fill buffer for the special cases.

The 3 bytes is because a LATM tag/sync word is encoded in 3 bytes.

> 
> > +            sh->a_in_buffer_len += demux_read_data(sh->ds,&sh->a_in_buffer[sh->a_in_buffer_len],
> > +                                                   sh->a_in_buffer_size - sh->a_in_buffer_len);
> 
> This may cause an endless loop if demux_read_data always returns 0 due to reaching eof.

No, the loop is bounded by i reaching 5.

> 
> > +            pos = 0;
> 
> Setting this to 0 inside the loop seems pointless...

Agreed. I just did as the original code did (and still do later on). But
looking at the code I cannot see any need to have it here or in original
code.

   Dan