[FFmpeg-devel] [PATCH] H264 parser fix

[Michael Niedermayer]

On Wed, May 26, 2010 at 05:42:13PM +0200, Michael Niedermayer wrote:
> On Tue, May 25, 2010 at 08:49:13PM -0700, Howard Chu wrote:
> > Michael Niedermayer wrote:
> >> On Tue, May 25, 2010 at 09:31:51AM -0700, Howard Chu wrote:
> >>>>> It seems odd to me that the parser
> >>>>> entry points all require an AVCodecContext but av_parser_init() only
> >>>>> takes
> >>>>> a codec_id. If av_parser_init() took an AVCodecContext instead, all of
> >>>>> the
> >>>>> necessary information would be available.
> >>>>
> >>>> well, yes, its not given during init but just later, i dont see how this
> >>>> would be a problem
> >>>
> >>> This patch works. It just means adding an if (extradata_not_parsed_yet)
> >>> test in h264_parse(), instead of handling it unconditionally at init 
> >>> time.
> >>> Probably a trivial overhead. I've just taken the extradata parsing code
> >>> from ff_h264_decode_init() and broken it out into its own function so 
> >>> that
> >>> h264_parse can use it too.
> >
> >>> +    if (!h->sps_buffers[0]&&  avctx->extradata_size) {
> >>
> >> that looks wrong, i dont think anything gurantees that there is a sps
> >> with sps_id=0
> >
> > OK. Added an explicit flag to the H264Context for this instead. Still this 
> > feels clumsy, it should have just been done in the parser_init step. Also 
> > it feels clumsy that it has to be parsed again, when the H264Context 
> > hanging off the AVCodecContext may already have it (due to 
> > ff_h264_decode_init() having parsed it already).
> >
> 
> > There was a suggestion on IRC to add an av_parser_init2() API which 
> > provides the AVCodecContext, to be given to a new H264 parser_init2 
> > function. It could just fallback to passing the codec_id if a given parser 
> > doesn't implement the init2 function...
> 
> if that works iam nt against it but it will require apps to be updated for
> some files. not a big issue as far as iam concerned
> 
> 
> >
> >> no need to pass AVCodecContext if its in h->s.avctx
> >
> > OK.
> >
> >> also the factorization should be a seperate patch
> >
> > OK, split into two separate patches.
> 
> code factorization is ok

btw, if you want a svn write account (and agree to our devel policy)
then you can have one

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

He who knows, does not speak. He who speaks, does not know. -- Lao Tsu
-------------- 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/20100527/77c9f2d1/attachment.pgp>