On Sun, 10 Feb 2013 00:40:56 -0800, Alexander Strange <[email protected]> 
wrote:
> 
> On Feb 6, 2013, at 10:40 AM, Anton Khirnov <[email protected]> wrote:
> 
> > Most of the changes are just trivial are just trivial replacements of
> > fields from MpegEncContext with equivalent fields in H264Context.
> > Everything in h264* other than h264.c are those trivial changes.
> > 
> > The nontrivial parts are:
> > 1) extracting a simplified version of the frame management code from
> > mpegvideo.c. We don't need last/next_picture anymore, since h264 uses
> > its own more complex system already and those were set only to appease
> > the mpegvideo parts.
> 
> Could you call 'cur' current_picture still?

cur corresponds to current_picture_ptr
Keeping that name would not reduce the diff, since I still have to do the s => h
replacement and IMO 'current_picture_ptr' is a bit too long.
But I'm not all that opposed to keeping it if people think it's better.

> 
> > 2) some tables that need to be allocated/freed in appropriate places.
> > 3) hwaccels -- mostly trivial replacements.
> > 4) svq3 -- it does not use h264 complex reference system, so I just
> > added some very simplistic frame management instead and dropped the use
> > of ff_h264_frame_start(). Because of this I also had to move some
> > initialization code to svq3.
> > ---
> > [...]
> > -        /* update linesize on resize for h264. The h264 decoder doesn't
> > +        /* update linesize on resize The decoder doesn't
> >          * necessarily call ff_MPV_frame_start in the new thread */
> > -        s->linesize   = s1->linesize;
> > -        s->uvlinesize = s1->uvlinesize;
> > +        h->linesize   = h1->linesize;
> > +        h->uvlinesize = h1->uvlinesize;
> 
> This comment is missing period after 'resize'. And do you still call 
> ff_MPV_frame_start with no MpegEncContext?
> 
> That's a lot of change but I have no other objections if it all works.

Right, fixed the comment locally. ff_MPV_frame_start is replaced with
ff_h264_frame_start() now.

Thanks for the review.
I've also noticed that i forgot a call to ff_mpeg_flush() in there. I'll fix
that and send a new patch.

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

Reply via email to