[FFmpeg-devel] h264.c avcC file reading error on PPS with trailing zeros, with tiny patch attached

[Michael Niedermayer]

On Thu, Aug 14, 2008 at 09:27:03AM +0200, Benoit Fouet wrote:
> Hi Michael,
> 
> Michael Niedermayer wrote:
> > On Wed, Aug 13, 2008 at 03:41:05PM -0400, Francois Oligny-Lemieux wrote:
> >   
> >> Hi,
> >>
> >> In .mp4 files are saved the SPS and PPS. When ffmpeg decodes the .mp4 video
> >> extradata, it should allow trailing zeros at the end of those NAL units.
> >>
> >> In original code, the SPS is decoded, then the PPS
> >>
> >>         // Decode sps from avcC
> >>         cnt = *(p+5) & 0x1f; // Number of sps
> >>         p += 6;
> >>         for (i = 0; i < cnt; i++) {
> >>             nalsize = AV_RB16(p) + 2;
> >>             if(decode_nal_units(h, p, nalsize) < 0) {
> >>                 av_log(avctx, AV_LOG_ERROR, "Decoding sps %d from avcC
> >> failed\n", i);
> >>                 return -1;
> >>             }
> >>             p += nalsize;
> >>         }
> >>         // Decode pps from avcC
> >>         cnt = *(p++); // Number of pps
> >>         for (i = 0; i < cnt; i++) {
> >>             nalsize = AV_RB16(p) + 2;
> >>             if(decode_nal_units(h, p, nalsize)   != nalsize) {
> >>                 av_log(avctx, AV_LOG_ERROR, "Decoding pps %d from avcC
> >> failed\n", i);
> >>                 return -1;
> >>             }
> >>             p += nalsize;
> >>         }
> >>
> >> The SPS part is OK because it just checks if decode_nal_units didn't fail.
> >> The second part is different and less flexible because it requires that
> >> decode_nal_unit consumes the same amount of bytes that were passed. I don't
> >> know why both part were not coded the same. The problem is that this doesn't
> >> pass when there are trailing zeros. So just making the second part's check
> >> like the first one allows the decoding of .mp4 that have trailing zeros in
> >> PPS (like the file I uploaded on
> >> upload.mplayerhq.hu/MPlayer/incoming/HD4ED_au_720p60.03.mp4). Note
> >> that QuickTime is able to
> >> decode it, while FFmpeg fails; but with the patch, FFmpeg succeeds.
> >>     
> >
> > patch ok (not that i think that a file needing it would be valid) but its
> > more robust with the patch
> >
> > [...]
> >   
> >   
> 
> as you're working on h.264 these days, do you want to apply it yourself
> or do you want me to do it ?

feel free to apply, iam too busy with SOC patches ATM for any h.264 work, not
that its not fun to spend hours staring at the spec, the code and
pitures that are +-1 off what they should be. ;)

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080814/4c137a7f/attachment.pgp>