[FFmpeg-devel] [PATCH] Implement PAFF in H.264
Michael Niedermayer
michaelni
Mon Oct 8 19:20:28 CEST 2007
On Mon, Oct 08, 2007 at 11:50:48AM -0400, Jeff Downs wrote:
> On Mon, 8 Oct 2007, Michael Niedermayer wrote:
>
> > On Thu, Oct 04, 2007 at 03:07:09PM -0400, Jeff Downs wrote:
>
> [...]
>
> > > 8. paff-picmgmt.patch
> > > Manage Picture buffers for fields as well as frames. Pair complementary
> > > fields into one MPV Picture. Part of PAFF implementation.
> > >
>
> [...]
>
> > > @@ -3991,8 +3998,50 @@
> > > }
> > >
> > > if(h0->current_slice == 0){
> > > - if(frame_start(h) < 0)
> > > + /* See if we have a decoded first field looking for a pair... */
> > > + if (s0->first_field) {
> > > + assert(s0->current_picture_ptr);
> > > + assert(s0->current_picture_ptr->data[0]);
> > > + assert(s0->current_picture_ptr->reference != DELAYED_PIC_REF);
> > > +
> > > + /* figure out if we have a complementary field pair */
> > > + if (!FIELD_PICTURE || s->picture_structure == last_pic_structure) {
> > > + /*
> > > + * Previous field is unmatched. Don't display it, but let it
> > > + * remain for reference if marked as such.
> > > + */
> > > + s0->current_picture_ptr = NULL;
> > > + s0->first_field = FIELD_PICTURE;
> >
> > first field is set to 0, 1, FIELD_PICTURE and tested against 0/not 0
> > this is inconsistent
> > also it should be here !=0 already
>
> Hmm not sure I follow your comment. This block is dealing with any sort
> of unmatched field pair. That is,
>
> Had an initial (unmatched as of yet) field.
> Now decoding either a frame picture (!FIELD_PICTURE) or a field of same
> parity.
>
> FIELD_PICTURE is 0/1. If desired for clarity, it can easily be changed to:
>
> first_field = FIELD_PICTURE ? 1 : 0
>
> or
>
> if (!FIELD_PICTURE)
> first_field = 0
forget my comment ... i have confused FIELD_PICTURE with a constant
but this brings up another issue, i and probably other devels as well
tend to think of upper case names as constants
maybe it would be cleaner if we would use code like
#define FRAME_MBAFF(h) h->mb_aff_frame
instead of the current
#define FRAME_MBAFF h->mb_aff_frame
that of course is seperate from this patch ...
ill retry reviewing it :)
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable
-------------- 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/20071008/3f337420/attachment.pgp>
More information about the ffmpeg-devel
mailing list