[FFmpeg-devel] [PATCH] h264 bitstream filter

[Michael Niedermayer]

Hi

On Fri, Aug 31, 2007 at 12:31:59PM +0200, Benoit Fouet wrote:
> Benoit Fouet wrote:
> > Benoit Fouet wrote:
> >   
> >> M?ns Rullg?rd wrote:
> >>   
> >>     
> >>> This is wrong.  Maybe you misunderstood what I said about where to
> >>> insert SPS and PPS.
> >>>     
> >>>       
> >> it seems so...
> >>
> >>   
> >>     
> >>>   They should be added before the first type 5 NAL
> >>> unit of an IDR picture, after whatever SEI or AUD units that picture
> >>> has.  A multislice IDR picture only needs SPS and PPS before the first
> >>> slice, and non-IDR pictures should not have SPS or PPS added.
> >>>
> >>>   
> >>>     
> >>>       
> >> i'm not sure i get it, here is what i understood, please correct me if
> >> i'm wrong
> >> i receive NALU in the filter
> >> if it's the first coded slice of an IDR picture (type 5 NAL), i prepend
> >> sps and pps NALUs (i don't really have to care about SEI or AUD in the
> >> bitstream filter case, right?)
> >> else, i don't add sps and pps
> >>
> >> this would give, in pseudo code:
> >>
> >> if (not sps and pps data)
> >>   retrieve sps and pps
> >>   first_idr=1
> >>
> >> if (first_idr && nal_unit_type == 5)
> >>   prepend sps and pps
> >>   first_idr=0
> >> else
> >>   don't prepend sps and pps
> >>   if( nal_unit_type != 5)
> >>   
> >>     
> >
> > when i think about it, i'd put:
> > if (nal_unit_type == 1) instead...
> >
> >   
> >>     first_idr=1
> >>
> >> is this right ?
> >>     
> 
> updated patch attached, which should address all problems (apart from
> the above one, if i misunderstood once again, and the naming)

[...]
> +    if (!ctx->sps_pps_data) {
> +        uint16_t unit_size;
> +        uint32_t total_size = 0;
> +        uint8_t *out = NULL, unit_nb, sps_done = 0;
> +        const uint8_t *extradata = avctx->extradata+4;
> +        static const uint8_t nalu_header[4] = {0, 0, 0, 1};
> +
> +        /* retrieve length coded size */
> +        ctx->length_size = (*extradata++ & 0x03) + 1;
> +
> +        /* retrieve sps and pps unit(s) */
> +        unit_nb = *extradata++ & 0x1f; /* number of sps unit(s) */
> +        if (!unit_nb) {
> +            unit_nb = *extradata++; /* number of pps unit(s) */
> +            sps_done++;
> +        }
> +        while (unit_nb--) {
> +            unit_size = AV_RB16(extradata);
> +            total_size += unit_size+4;
> +            if (extradata+2+unit_size > avctx->extradata+avctx->extradata_size) {
> +                av_free(out);
> +                return -1;
> +            }
> +            out = av_realloc(out, total_size);
> +            memcpy(out+total_size-unit_size-4, nalu_header, 4);

out==NULL should be checked as we dont start writing at out but later
theres a small risk that this could be exploitable

except that it looks ok
[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The educated differ from the uneducated as much as the living from the
dead. -- Aristotle 
-------------- 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/20070831/10c9403e/attachment.pgp>