[FFmpeg-devel] [PATCH] Implement PAFF in H.264

Jeff Downs heydowns
Wed Oct 3 23:03:13 CEST 2007


On Wed, 3 Oct 2007, Michael Niedermayer wrote:

> Hi
> 
> On Tue, Oct 02, 2007 at 11:57:15AM -0400, Jeff Downs wrote:
> > --- ../ffmpeg-cosmetics/libavcodec/h264.c	2007-10-01 14:02:06.000000000 -0400
> > +++ libavcodec/h264.c	2007-10-01 15:56:22.000000000 -0400
> > @@ -175,11 +175,11 @@
> >      if(for_deblock && (h->slice_num == 1 || h->slice_table[mb_xy] == h->slice_table[mb_xy-s->mb_stride]) && !FRAME_MBAFF)
> >          return;
> >  
> >      //wow what a mess, why didn't they simplify the interlacing&intra stuff, i can't imagine that these complex rules are worth it
> >  
> > -    top_xy     = mb_xy  - s->mb_stride;
> > +    top_xy     = mb_xy  - (s->mb_stride << FIELD_PICTURE);
> 
> could mb_stride itself be <<=1 ?
> or are there many uses which need it like it is even for field pics?

Most uses need it as-is; most times it is used to compute current 
macroblock location (mb_x + mb_y * mb_stride).  y is in frame units all 
the time, currently, which is really convenient for supporting mbaff.  

mb_stride is setup in MPV_common_init (essentially) once.  We'd have to 
move it to a per-frame setting to make it work right with frame & field 
intermingling, and change how mb_y, height are incremented/tested.


> >      topleft_xy = top_xy - 1;
> >      topright_xy= top_xy + 1;
> >      left_xy[1] = left_xy[0] = mb_xy-1;
> >      left_block[0]= 0;
> >      left_block[1]= 1;
> > @@ -1701,11 +1701,11 @@
> >      int extra_height= h->emu_edge_height;
> >      int emu=0;
> >      const int full_mx= mx>>2;
> >      const int full_my= my>>2;
> >      const int pic_width  = 16*s->mb_width;
> > -    const int pic_height = 16*s->mb_height >> MB_MBAFF;
> > +    const int pic_height = 16*s->mb_height >> (MB_MBAFF || FIELD_PICTURE);
> >  
> >      if(!pic->data[0]) //FIXME this is unacceptable, some senseable error concealment must be done for missing reference frames
> >          return;
> >  
> >      if(mx&7) extra_width -= 3;
> > @@ -1725,11 +1725,11 @@
> >          qpix_op[luma_xy](dest_y + delta, src_y + delta, h->mb_linesize);
> >      }
> >  
> >      if(ENABLE_GRAY && s->flags&CODEC_FLAG_GRAY) return;
> >  
> > -    if(MB_MBAFF){
> > +    if(MB_MBAFF || FIELD_PICTURE){
> >          // chroma offset when predicting from a field of opposite parity
> >          my += 2 * ((s->mb_y & 1) - (h->ref_cache[list][scan8[n]] & 1));
> >          emu |= (my>>3) < 0 || (my>>3) + 8 >= (pic_height>>1);
> >      }
> >      src_cb= pic->data[1] + (mx>>3) + (my>>3)*h->mb_uvlinesize;
> > @@ -1760,11 +1760,11 @@
> >  
> >      dest_y  += 2*x_offset + 2*y_offset*h->  mb_linesize;
> >      dest_cb +=   x_offset +   y_offset*h->mb_uvlinesize;
> >      dest_cr +=   x_offset +   y_offset*h->mb_uvlinesize;
> >      x_offset += 8*s->mb_x;
> > -    y_offset += 8*(s->mb_y >> MB_MBAFF);
> > +    y_offset += 8*(s->mb_y >> (MB_MBAFF || FIELD_PICTURE));
> >  
> >      if(list0){
> >          Picture *ref= &h->ref_list[0][ h->ref_cache[0][ scan8[n] ] ];
> >          mc_dir_part(h, ref, n, square, chroma_height, delta, 0,
> >                             dest_y, dest_cb, dest_cr, x_offset, y_offset,
> > @@ -1793,11 +1793,11 @@
> >  
> >      dest_y  += 2*x_offset + 2*y_offset*h->  mb_linesize;
> >      dest_cb +=   x_offset +   y_offset*h->mb_uvlinesize;
> >      dest_cr +=   x_offset +   y_offset*h->mb_uvlinesize;
> >      x_offset += 8*s->mb_x;
> > -    y_offset += 8*(s->mb_y >> MB_MBAFF);
> > +    y_offset += 8*(s->mb_y >> (MB_MBAFF || FIELD_PICTURE));
> >  
> >      if(list0 && list1){
> >          /* don't optimize for luma-only case, since B-frames usually
> >           * use implicit weights => chroma too. */
> >          uint8_t *tmp_cb = s->obmc_scratchpad;
> 
> these can be commited if we have a FIELD_PICTURE=0 

Assume you mean with a #define FIELD_PICTURE 0 until rest of the overall
patch is incorportated. If so, patch doing so for application attached 
(paff-mbdecode.patch).

> > @@ -2247,10 +2247,11 @@
> >      int i;
> >  
> >      if(MPV_frame_start(s, s->avctx) < 0)
> >          return -1;
> >      ff_er_frame_start(s);
> > +    s->current_picture_ptr->key_frame= 0;
> >  
> 
> this line could benefit from a comment explaining how it is/has to be
> initalized for field_pics

Added.

> [...]
> > +static int split_field_copy(Picture *dest, Picture *src,
> > +                            int parity, int id_add){
> > +    int match = (src->reference & parity) != 0;
> 
> !!(src->reference & parity);

Changed.

> [...]
> > +    int same_i, opp_i;
> > +    int i;
> > +    int same;
> > +    int out_i;
> > +
> > +    same_i = 0;
> > +    opp_i = 0;
> > +    same = 1;
> > +    out_i = 0;
> 
> declaration and initalization can be merged

Changed.

> > +
> > +    while (out_i < dest_len) {
> > +        if (same && same_i < src_len) {
> > +            i = split_field_copy(dest + out_i, src + same_i, parity, 1);
> > +            same = !i;
> > +            same_i++;
> > +
> > +        } else if (opp_i < src_len) {
> > +            i = split_field_copy(dest + out_i, src + opp_i,
> > +                                 PICT_FRAME - parity, 0);
> > +            same = i;
> > +            opp_i++;
> > +
> > +        } else {
> > +            break;
> > +        }
> > +        out_i += i;
> > +    }
> 
> for(out_i = 0; out_i < dest_len; out_i += i)

Changed.

> [...]
> > +    if (!FIELD_PICTURE) {
> > +        structure_sel = 0;
> > +        frame_list[0] = h->default_ref_list[0];
> > +        frame_list[1] = h->default_ref_list[1];
> > +    } else {
> > +        structure_sel = PICT_FRAME;
> > +        frame_list[0] = field_entry_list[0];
> > +        frame_list[1] = field_entry_list[1];
> > +    }
> 
> if(FIELD_PICTURE){
> }else{
> }
> is a "not" less and IMHO a tiny bit easier to understand
> 

Sure. Changed.

> [...]
> > +                        if (FIELD_PICTURE) {
> > +                            frame_num = pred >> 1;
> > +                            if (pred & 1) {
> > +                                pic_structure = s->picture_structure;
> > +                                bot = (s->picture_structure == PICT_BOTTOM_FIELD);
> > +                            } else {
> > +                                pic_structure = PICT_FRAME - s->picture_structure;
> > +                                bot = (s->picture_structure == PICT_TOP_FIELD);
> > +                            }
> > +                        } else {
> > +                            frame_num = pred;
> > +                            pic_structure = PICT_FRAME;
> > +                            bot = 0;
> > +                        }
> > +
> >                          for(i= h->short_ref_count-1; i>=0; i--){
> >                              ref = h->short_ref[i];
> > -                            assert(ref->reference == 3);
> > +                            assert(ref->reference);
> >                              assert(!ref->long_ref);
> > -                            if(ref->data[0] != NULL && ref->frame_num == pred && ref->long_ref == 0) // ignore non existing pictures by testing data[0] pointer
> > +                            if(ref->data[0] != NULL &&
> > +                                   ref->frame_num == frame_num &&
> > +                                   (ref->reference & pic_structure) &&
> > +                                   ref->long_ref == 0) // ignore non existing pictures by testing data[0] pointer
> >                                  break;
> >                          }
> >                          if(i>=0)
> > -                            ref->pic_id= ref->frame_num;
> > +                            ref->pic_id= pred;
> >                      }else{
> > +                        int long_idx;
> >                          pic_id= get_ue_golomb(&s->gb); //long_term_pic_idx
> > -                        if(pic_id>31){
> > +
> > +                        if (FIELD_PICTURE) {
> > +                            long_idx = pic_id >> 1;
> > +                            if (pic_id & 1) {
> > +                                pic_structure = s->picture_structure;
> > +                                bot = (s->picture_structure == PICT_BOTTOM_FIELD);
> > +                            } else {
> > +                                pic_structure = PICT_FRAME - s->picture_structure;
> > +                                bot = (s->picture_structure == PICT_TOP_FIELD);
> > +                            }
> > +                        } else {
> > +                            long_idx = pic_id;
> > +                            pic_structure = PICT_FRAME;
> > +                            bot = 0;
> > +                        }
> 
> this looks duplicated

Assuming you mean derivation of pic_structure, long_idx/frame_num, 
then its now pulled out to a function.

> [...]
> > -static inline void unreference_pic(H264Context *h, Picture *pic){
> > +/**
> > + * Mark a picture as no longer needed for reference. The refmask
> > + * argument allows unreferencing of individual fields or the whole frame.
> > + * If the picture becomes entirely unreferenced, but is being held for
> > + * display purposes, it is marked as such.
> > + * @param refmask mask of fields to unreference; the mask is bitwise
> > + *                anded with the reference marking of pic
> > + * @return non-zero if pic becomes entirely unreferenced (except possibly
> > + *         for display purposes) zero if one of the fields remains in
> > + *         reference
> > + */
> > +static inline int unreference_pic(H264Context *h, Picture *pic, int refmask){
> >      int i;
> > -    pic->reference=0;
> > +    if (pic->reference &= refmask) {
> > +        return 0;
> > +    } else {
> >      if(pic == h->delayed_output_pic)
> >          pic->reference=DELAYED_PIC_REF;
> >      else{
> >          for(i = 0; h->delayed_pic[i]; i++)
> >              if(pic == h->delayed_pic[i]){
> >                  pic->reference=DELAYED_PIC_REF;
> >                  break;
> >              }
> >      }
> > +        return 1;
> > +    }
> >  }
> 
> this looks ok, if it where in a seperate patch with the related
> unreference_pic() call changes that could be commited

Separated. paff-unreference.patch


> [...]
> >  /**
> > - *
> > - * @return the removed picture or NULL if an error occurs
> > + * Find a Picture in the short term reference list by frame number.
> > + * @param frame_num frame number to search for
> > + * @param idx the index into h->short_ref where returned picture is found
> > + *            undefined if no picture found.
> > + * @return pointer to the found picture, or NULL if no pic with the provided
> > + *                 frame number is found
> >   */
> > -static Picture * remove_short(H264Context *h, int frame_num){
> > +static Picture * find_short(H264Context *h, int frame_num, int *idx){
> >      MpegEncContext * const s = &h->s;
> >      int i;
> >  
> > -    if(s->avctx->debug&FF_DEBUG_MMCO)
> > -        av_log(h->s.avctx, AV_LOG_DEBUG, "remove short %d count %d\n", frame_num, h->short_ref_count);
> > -
> >      for(i=0; i<h->short_ref_count; i++){
> >          Picture *pic= h->short_ref[i];
> >          if(s->avctx->debug&FF_DEBUG_MMCO)
> >              av_log(h->s.avctx, AV_LOG_DEBUG, "%d %d %p\n", i, pic->frame_num, pic);
> > -        if(pic->frame_num == frame_num){
> > -            h->short_ref[i]= NULL;
> > -            if (--h->short_ref_count)
> > -                memmove(&h->short_ref[i], &h->short_ref[i+1], (h->short_ref_count - i)*sizeof(Picture*));
> > +        if(pic->frame_num == frame_num) {
> > +            *idx = i;
> >              return pic;
> >          }
> >      }
> >      return NULL;
> >  }
> 
> this split out could also be in its own patch

With associated changes to remove_short I assume. Separated into 
paff-shortrefmgmt.patch

> [...]
> > -                    unsigned int long_index= get_ue_golomb(gb);
> > -                    if(/*h->mmco[i].long_arg >= h->long_ref_count || h->long_ref[ h->mmco[i].long_arg ] == NULL*/ long_index >= 16){
> > +                    unsigned int long_arg= get_ue_golomb(gb);
> > +                    if(long_arg >= 32 || (long_arg >= 16 && !(opcode == MMCO_LONG2UNUSED && FIELD_PICTURE))){
> >                          av_log(h->s.avctx, AV_LOG_ERROR, "illegal long ref in memory management control operation %d\n", opcode);
> >                          return -1;
> >                      }
> > -                    h->mmco[i].long_arg= long_index;
> > +                    h->mmco[i].long_arg= long_arg;
> 
> this renaming should be in a seperate patch

paff-longoprename.patch

> [...]
> >      }
> > +
> >      if(h != h0)
> 
> cosmetic

Byproduct of diff; with the new block above it, thought the extra line 
helped readability. Anyway, removed.

> [...]
> > -    //FIXME do something with unavailable reference frames
> [...]
> > +        //FIXME do something with unavailable reference frames
> 
> cosmetic

Didn't belong in the first place. Removed - sorry.




Separated patches to be applied in order, w/ suggested commit messages:

1. paff-mbdecode.patch 
Partial PAFF implementation at macroblock level.
PAFF support disabled until implementation complete.

2. paff-unreference.patch 
Modify unreference_pic implementation with PAFF in mind.

3. paff-unreference-indent.patch
Re-indent unreference_pic.

4. paff-shortrefmgmt.patch
Further modularize short reference list management for upcoming PAFF 
implementation.

5. paff-longoprename.patch
Rename variable to make sense in both field and frame contexts 
(support of PAFF implementation).



Then there is the remainder that you didn't specifically ask to be split, 
but that I've tried to split further with the intention of making it 
easier to review.

6. paff-currpicnum.patch
Fix h->curr_pic_num for field pictures. Necessary for proper PAFF support.

7. paff-keyframe.patch
Fix Picture.key_frame setting to be compatible with frame and field 
contexts. Part of PAFF implementation. Contributed in part by Neil Brown.

8. paff-longrefmgmt.patch
Reorganize long reference management to minimize code duplication in 
upcoming PAFF implementation.

9. paff-defreflist.patch
Support functions and changes to default reference list creation for PAFF.

10. paff-defreflist-indent.patch
Reindent fill_default_ref_list after changes for PAFF

11. paff-reordering.patch
Support function and changes to reference picture reordering for PAFF.



The remainder of the original patch is in paff-noident.patch. 
Bulk is MMCO for fields and interleaving of fields into one AVPicture.

Figure we can try to get through the above, then separate even further 
still if desired.

	-Jeff
-------------- next part --------------
Index: libavcodec/h264.c
===================================================================
--- libavcodec/h264.c	(revision 10656)
+++ libavcodec/h264.c	(working copy)
@@ -1703,7 +1703,7 @@
     const int full_mx= mx>>2;
     const int full_my= my>>2;
     const int pic_width  = 16*s->mb_width;
-    const int pic_height = 16*s->mb_height >> MB_MBAFF;
+    const int pic_height = 16*s->mb_height >> (MB_MBAFF || FIELD_PICTURE);
 
     if(!pic->data[0]) //FIXME this is unacceptable, some senseable error concealment must be done for missing reference frames
         return;
@@ -1727,7 +1727,7 @@
 
     if(ENABLE_GRAY && s->flags&CODEC_FLAG_GRAY) return;
 
-    if(MB_MBAFF){
+    if(MB_MBAFF || FIELD_PICTURE){
         // chroma offset when predicting from a field of opposite parity
         my += 2 * ((s->mb_y & 1) - (h->ref_cache[list][scan8[n]] & 1));
         emu |= (my>>3) < 0 || (my>>3) + 8 >= (pic_height>>1);
@@ -1762,7 +1762,7 @@
     dest_cb +=   x_offset +   y_offset*h->mb_uvlinesize;
     dest_cr +=   x_offset +   y_offset*h->mb_uvlinesize;
     x_offset += 8*s->mb_x;
-    y_offset += 8*(s->mb_y >> MB_MBAFF);
+    y_offset += 8*(s->mb_y >> (MB_MBAFF || FIELD_PICTURE));
 
     if(list0){
         Picture *ref= &h->ref_list[0][ h->ref_cache[0][ scan8[n] ] ];
@@ -1795,7 +1795,7 @@
     dest_cb +=   x_offset +   y_offset*h->mb_uvlinesize;
     dest_cr +=   x_offset +   y_offset*h->mb_uvlinesize;
     x_offset += 8*s->mb_x;
-    y_offset += 8*(s->mb_y >> MB_MBAFF);
+    y_offset += 8*(s->mb_y >> (MB_MBAFF || FIELD_PICTURE));
 
     if(list0 && list1){
         /* don't optimize for luma-only case, since B-frames usually
Index: libavcodec/h264.h
===================================================================
--- libavcodec/h264.h	(revision 10656)
+++ libavcodec/h264.h	(working copy)
@@ -59,10 +59,12 @@
 #define MB_MBAFF h->mb_mbaff
 #define MB_FIELD h->mb_field_decoding_flag
 #define FRAME_MBAFF h->mb_aff_frame
+#define FIELD_PICTURE 0
 #else
 #define MB_MBAFF 0
 #define MB_FIELD 0
 #define FRAME_MBAFF 0
+#define FIELD_PICTURE 0
 #undef  IS_INTERLACED
 #define IS_INTERLACED(mb_type) 0
 #endif
-------------- next part --------------
--- ../ffmpeg-mbdecode/libavcodec/h264.c	2007-10-03 15:07:39.000000000 -0400
+++ libavcodec/h264.c	2007-10-03 15:12:11.000000000 -0400
@@ -3093,9 +3093,22 @@
     }
 }
 
-static inline void unreference_pic(H264Context *h, Picture *pic){
+/**
+ * Mark a picture as no longer needed for reference. The refmask
+ * argument allows unreferencing of individual fields or the whole frame.
+ * If the picture becomes entirely unreferenced, but is being held for
+ * display purposes, it is marked as such.
+ * @param refmask mask of fields to unreference; the mask is bitwise
+ *                anded with the reference marking of pic
+ * @return non-zero if pic becomes entirely unreferenced (except possibly
+ *         for display purposes) zero if one of the fields remains in
+ *         reference
+ */
+static inline int unreference_pic(H264Context *h, Picture *pic, int refmask){
     int i;
-    pic->reference=0;
+    if (pic->reference &= refmask) {
+        return 0;
+    } else {
     if(pic == h->delayed_output_pic)
         pic->reference=DELAYED_PIC_REF;
     else{
@@ -3105,6 +3118,8 @@
                 break;
             }
     }
+        return 1;
+    }
 }
 
 /**
@@ -3115,14 +3130,14 @@
 
     for(i=0; i<16; i++){
         if (h->long_ref[i] != NULL) {
-            unreference_pic(h, h->long_ref[i]);
+            unreference_pic(h, h->long_ref[i], 0);
             h->long_ref[i]= NULL;
         }
     }
     h->long_ref_count=0;
 
     for(i=0; i<h->short_ref_count; i++){
-        unreference_pic(h, h->short_ref[i]);
+        unreference_pic(h, h->short_ref[i], 0);
         h->short_ref[i]= NULL;
     }
     h->short_ref_count=0;
@@ -3234,13 +3249,13 @@
         case MMCO_SHORT2UNUSED:
             pic= remove_short(h, mmco[i].short_pic_num);
             if(pic)
-                unreference_pic(h, pic);
+                unreference_pic(h, pic, 0);
             else if(s->avctx->debug&FF_DEBUG_MMCO)
                 av_log(h->s.avctx, AV_LOG_DEBUG, "mmco: remove_short() failure\n");
             break;
         case MMCO_SHORT2LONG:
             pic= remove_long(h, mmco[i].long_arg);
-            if(pic) unreference_pic(h, pic);
+            if(pic) unreference_pic(h, pic, 0);
 
             h->long_ref[ mmco[i].long_arg ]= remove_short(h, mmco[i].short_pic_num);
             if (h->long_ref[ mmco[i].long_arg ]){
@@ -3251,13 +3266,13 @@
         case MMCO_LONG2UNUSED:
             pic= remove_long(h, mmco[i].long_arg);
             if(pic)
-                unreference_pic(h, pic);
+                unreference_pic(h, pic, 0);
             else if(s->avctx->debug&FF_DEBUG_MMCO)
                 av_log(h->s.avctx, AV_LOG_DEBUG, "mmco: remove_long() failure\n");
             break;
         case MMCO_LONG:
             pic= remove_long(h, mmco[i].long_arg);
-            if(pic) unreference_pic(h, pic);
+            if(pic) unreference_pic(h, pic, 0);
 
             h->long_ref[ mmco[i].long_arg ]= s->current_picture_ptr;
             h->long_ref[ mmco[i].long_arg ]->long_ref=1;
@@ -3270,17 +3285,17 @@
             // just remove the long term which index is greater than new max
             for(j = mmco[i].long_arg; j<16; j++){
                 pic = remove_long(h, j);
-                if (pic) unreference_pic(h, pic);
+                if (pic) unreference_pic(h, pic, 0);
             }
             break;
         case MMCO_RESET:
             while(h->short_ref_count){
                 pic= remove_short(h, h->short_ref[0]->frame_num);
-                if(pic) unreference_pic(h, pic);
+                if(pic) unreference_pic(h, pic, 0);
             }
             for(j = 0; j < 16; j++) {
                 pic= remove_long(h, j);
-                if(pic) unreference_pic(h, pic);
+                if(pic) unreference_pic(h, pic, 0);
             }
             break;
         default: assert(0);
@@ -3290,7 +3305,7 @@
     if(!current_is_long){
         pic= remove_short(h, s->current_picture_ptr->frame_num);
         if(pic){
-            unreference_pic(h, pic);
+            unreference_pic(h, pic, 0);
             av_log(h->s.avctx, AV_LOG_ERROR, "illegal short term buffer state detected\n");
         }
 
-------------- next part --------------
--- ../ffmpeg-unreference/libavcodec/h264.c	2007-10-03 15:17:17.000000000 -0400
+++ libavcodec/h264.c	2007-10-03 15:18:37.000000000 -0400
@@ -3109,15 +3109,15 @@
     if (pic->reference &= refmask) {
         return 0;
     } else {
-    if(pic == h->delayed_output_pic)
-        pic->reference=DELAYED_PIC_REF;
-    else{
-        for(i = 0; h->delayed_pic[i]; i++)
-            if(pic == h->delayed_pic[i]){
-                pic->reference=DELAYED_PIC_REF;
-                break;
-            }
-    }
+        if(pic == h->delayed_output_pic)
+            pic->reference=DELAYED_PIC_REF;
+        else{
+            for(i = 0; h->delayed_pic[i]; i++)
+                if(pic == h->delayed_pic[i]){
+                    pic->reference=DELAYED_PIC_REF;
+                    break;
+                }
+        }
         return 1;
     }
 }
-------------- next part --------------
--- ../ffmpeg-unreference-indent/libavcodec/h264.c	2007-10-03 15:19:31.000000000 -0400
+++ libavcodec/h264.c	2007-10-03 15:24:01.000000000 -0400
@@ -3161,24 +3161,23 @@
 }
 
 /**
- *
- * @return the removed picture or NULL if an error occurs
+ * Find a Picture in the short term reference list by frame number.
+ * @param frame_num frame number to search for
+ * @param idx the index into h->short_ref where returned picture is found
+ *            undefined if no picture found.
+ * @return pointer to the found picture, or NULL if no pic with the provided
+ *                 frame number is found
  */
-static Picture * remove_short(H264Context *h, int frame_num){
+static Picture * find_short(H264Context *h, int frame_num, int *idx){
     MpegEncContext * const s = &h->s;
     int i;
 
-    if(s->avctx->debug&FF_DEBUG_MMCO)
-        av_log(h->s.avctx, AV_LOG_DEBUG, "remove short %d count %d\n", frame_num, h->short_ref_count);
-
     for(i=0; i<h->short_ref_count; i++){
         Picture *pic= h->short_ref[i];
         if(s->avctx->debug&FF_DEBUG_MMCO)
             av_log(h->s.avctx, AV_LOG_DEBUG, "%d %d %p\n", i, pic->frame_num, pic);
-        if(pic->frame_num == frame_num){
-            h->short_ref[i]= NULL;
-            if (--h->short_ref_count)
-                memmove(&h->short_ref[i], &h->short_ref[i+1], (h->short_ref_count - i)*sizeof(Picture*));
+        if(pic->frame_num == frame_num) {
+            *idx = i;
             return pic;
         }
     }
@@ -3186,6 +3185,38 @@
 }
 
 /**
+ * Remove a picture from the short term reference list by its index in
+ * that list.  This does no checking on the provided index; it is assumed
+ * to be valid. Other list entries are shifted down.
+ * @param i index into h->short_ref of picture to remove.
+ */
+static void remove_short_at_index(H264Context *h, int i){
+    assert(i > 0 && i < h->short_ref_count);
+    h->short_ref[i]= NULL;
+    if (--h->short_ref_count)
+        memmove(&h->short_ref[i], &h->short_ref[i+1], (h->short_ref_count - i)*sizeof(Picture*));
+}
+
+/**
+ *
+ * @return the removed picture or NULL if an error occurs
+ */
+static Picture * remove_short(H264Context *h, int frame_num){
+    MpegEncContext * const s = &h->s;
+    Picture *pic;
+    int i;
+
+    if(s->avctx->debug&FF_DEBUG_MMCO)
+        av_log(h->s.avctx, AV_LOG_DEBUG, "remove short %d count %d\n", frame_num, h->short_ref_count);
+
+    pic = find_short(h, frame_num, &i);
+    if (pic)
+        remove_short_at_index(h, i);
+
+    return pic;
+}
+
+/**
  *
  * @return the removed picture or NULL if an error occurs
  */
-------------- next part --------------
--- ../ffmpeg-shortrefmgmt/libavcodec/h264.c	2007-10-03 15:26:49.000000000 -0400
+++ libavcodec/h264.c	2007-10-03 15:30:46.000000000 -0400
@@ -3380,12 +3380,12 @@
                     }*/
                 }
                 if(opcode==MMCO_SHORT2LONG || opcode==MMCO_LONG2UNUSED || opcode==MMCO_LONG || opcode==MMCO_SET_MAX_LONG){
-                    unsigned int long_index= get_ue_golomb(gb);
-                    if(/*h->mmco[i].long_arg >= h->long_ref_count || h->long_ref[ h->mmco[i].long_arg ] == NULL*/ long_index >= 16){
+                    unsigned int long_arg= get_ue_golomb(gb);
+                    if(/*h->mmco[i].long_arg >= h->long_ref_count || h->long_ref[ h->mmco[i].long_arg ] == NULL*/ long_arg >= 16){
                         av_log(h->s.avctx, AV_LOG_ERROR, "illegal long ref in memory management control operation %d\n", opcode);
                         return -1;
                     }
-                    h->mmco[i].long_arg= long_index;
+                    h->mmco[i].long_arg= long_arg;
                 }
 
                 if(opcode > (unsigned)MMCO_LONG){
-------------- next part --------------
--- ../ffmpeg-longoprename/libavcodec/h264.c	2007-10-03 15:31:47.000000000 -0400
+++ libavcodec/h264.c	2007-10-03 15:54:51.000000000 -0400
@@ -3742,7 +3742,7 @@
         h->curr_pic_num=   h->frame_num;
         h->max_pic_num= 1<< h->sps.log2_max_frame_num;
     }else{
-        h->curr_pic_num= 2*h->frame_num;
+        h->curr_pic_num= 2*h->frame_num + 1;
         h->max_pic_num= 1<<(h->sps.log2_max_frame_num + 1);
     }
 
--- ../ffmpeg-longoprename/libavcodec/h264.h	2007-10-03 15:31:47.000000000 -0400
+++ libavcodec/h264.h	2007-10-03 15:54:20.000000000 -0400
@@ -285,7 +285,7 @@
     int prev_frame_num;           ///< frame_num of the last pic for POC type 1/2
 
     /**
-     * frame_num for frames or 2*frame_num for field pics.
+     * frame_num for frames or 2*frame_num+1 for field pics.
      */
     int curr_pic_num;
 
-------------- next part --------------
--- ../ffmpeg-currpicnum/libavcodec/h264.c	2007-10-03 15:57:38.000000000 -0400
+++ libavcodec/h264.c	2007-10-03 16:00:27.000000000 -0400
@@ -2249,6 +2249,13 @@
     if(MPV_frame_start(s, s->avctx) < 0)
         return -1;
     ff_er_frame_start(s);
+    /*
+     * MPV_frame_start uses pict_type to derive key_frame. 
+     * This is incorrect for H.264; IDR markings must be used.
+     * Zero here; IDR markings per slice in frame or fields are OR'd in later.
+     * See decode_nal_units().
+     */
+    s->current_picture_ptr->key_frame= 0;
 
     assert(s->linesize && s->uvlinesize);
 
@@ -7189,7 +7196,7 @@
             if((err = decode_slice_header(hx, h)))
                break;
 
-            s->current_picture_ptr->key_frame= (hx->nal_unit_type == NAL_IDR_SLICE);
+            s->current_picture_ptr->key_frame|= (hx->nal_unit_type == NAL_IDR_SLICE);
             if(hx->redundant_pic_count==0 && hx->s.hurry_up < 5
                && (avctx->skip_frame < AVDISCARD_NONREF || hx->nal_ref_idc)
                && (avctx->skip_frame < AVDISCARD_BIDIR  || hx->slice_type!=B_TYPE)
-------------- next part --------------
--- ../ffmpeg-keyframe/libavcodec/h264.c	2007-10-03 16:01:56.000000000 -0400
+++ libavcodec/h264.c	2007-10-03 16:12:21.000000000 -0400
@@ -3224,6 +3224,17 @@
 }
 
 /**
+ * Remove a picture from the long term reference list by its index in
+ * that list.  This does no checking on the provided index; it is assumed
+ * to be valid. The removed entry is set to NULL. Other entries are unaffected.
+ * @param i index into h->long_ref of picture to remove.
+ */
+static void remove_long_at_index(H264Context *h, int i){
+    h->long_ref[i]= NULL;
+    h->long_ref_count--;
+}
+
+/**
  *
  * @return the removed picture or NULL if an error occurs
  */
@@ -3231,8 +3242,8 @@
     Picture *pic;
 
     pic= h->long_ref[i];
-    h->long_ref[i]= NULL;
-    if(pic) h->long_ref_count--;
+    if (pic)
+        remove_long_at_index(h, i);
 
     return pic;
 }
-------------- next part --------------
--- ../ffmpeg-longref/libavcodec/h264.c	2007-10-03 16:15:14.000000000 -0400
+++ libavcodec/h264.c	2007-10-03 16:27:26.000000000 -0400
@@ -2764,6 +2764,103 @@
     else hl_decode_mb_simple(h);
 }
 
+static void pic_as_field(Picture *pic, const int bottom){
+    int i;
+    for (i = 0; i < 4; ++i) {
+        if (bottom)
+            pic->data[i] += pic->linesize[i];
+        pic->linesize[i] *= 2;
+    }
+}
+
+static int split_field_copy(Picture *dest, Picture *src,
+                            int parity, int id_add){
+    int match = !!(src->reference & parity);
+
+    if (match) {
+        *dest = *src;
+        pic_as_field(dest, parity == PICT_BOTTOM_FIELD);
+        dest->pic_id *= 2;
+        dest->pic_id += id_add;
+    }
+
+    return match;
+}
+
+/**
+ * Split one reference list into field parts, interleaving by parity
+ * as per H.264 spec section 8.2.4.2.5. Output fields have their data pointers
+ * set to look at the actual start of data for that field.
+ *
+ * @param dest output list
+ * @param dest_len maximum number of fields to put in dest
+ * @param src the source reference list containing fields and/or field pairs
+ *            (aka short_ref/long_ref, or
+ *             refFrameListXShortTerm/refFrameListLongTerm in spec-speak)
+ * @param src_len number of Picture's in source (pairs and unmatched fields)
+ * @param parity the parity of the picture being decoded/needing
+ *        these ref pics (PICT_{TOP,BOTTOM}_FIELD)
+ * @return number of fields placed in dest
+ */
+static int split_field_half_ref_list(Picture *dest, int dest_len,
+                                     Picture *src,  int src_len,  int parity){
+    int same   = 1;
+    int same_i = 0;
+    int opp_i  = 0;
+    int out_i;
+    int i;
+
+    for (out_i = 0; out_i < dest_len; out_i += i) {
+        if (same && same_i < src_len) {
+            i = split_field_copy(dest + out_i, src + same_i, parity, 1);
+            same = !i;
+            same_i++;
+
+        } else if (opp_i < src_len) {
+            i = split_field_copy(dest + out_i, src + opp_i,
+                                 PICT_FRAME - parity, 0);
+            same = i;
+            opp_i++;
+
+        } else {
+            break;
+        }
+    }
+
+    return out_i;
+}
+
+/**
+ * Split the reference frame list into a reference field list.
+ * This implements H.264 spec 8.2.4.2.5 for a combined input list.
+ * The input list contains both reference field pairs and
+ * unmatched reference fields; it is ordered as spec describes
+ * RefPicListX for frames in 8.2.4.2.1 and 8.2.4.2.3, except that
+ * unmatched field pairs are also present. Conceptually this is equivalent
+ * to concatenation of refFrameListXShortTerm with refFrameListLongTerm.
+ *
+ * @param dest output reference list where ordered fields are to be placed
+ * @param dest_len max number of fields to place at dest
+ * @param src source reference list, as described above
+ * @param src_len number of pictures (pairs and unmatched fields) in src
+ * @param parity parity of field being currently decoded
+ *        (one of PICT_{TOP,BOTTOM}_FIELD)
+ * @param long_i index into src array that holds first long reference picture,
+ *        or src_len if no long refs present.
+ */
+static int split_field_ref_list(Picture *dest, int dest_len,
+                                Picture *src,  int src_len,
+                                int parity,    int long_i){
+
+    int i = split_field_half_ref_list(dest, dest_len, src, long_i, parity);
+    dest += i;
+    dest_len -= i;
+
+    i += split_field_half_ref_list(dest, dest_len, src + long_i,
+                                   src_len - long_i, parity);
+    return i;
+}
+
 /**
  * fills the default_ref_list.
  */
@@ -2771,9 +2868,25 @@
     MpegEncContext * const s = &h->s;
     int i;
     int smallest_poc_greater_than_current = -1;
+    int structure_sel;
     Picture sorted_short_ref[32];
+    Picture field_entry_list[2][32];
+    Picture *frame_list[2];
+
+    if (FIELD_PICTURE) {
+        structure_sel = PICT_FRAME;
+        frame_list[0] = field_entry_list[0];
+        frame_list[1] = field_entry_list[1];
+    } else {
+        structure_sel = 0;
+        frame_list[0] = h->default_ref_list[0];
+        frame_list[1] = h->default_ref_list[1];
+    }
 
     if(h->slice_type==B_TYPE){
+        int list;
+        int len[2];
+        int short_len[2];
         int out_i;
         int limit= INT_MIN;
 
@@ -2801,11 +2914,7 @@
                 }
             }
         }
-    }
 
-    if(s->picture_structure == PICT_FRAME){
-        if(h->slice_type==B_TYPE){
-            int list;
             tprintf(h->s.avctx, "current poc: %d, smallest_poc_greater_than_current: %d\n", s->current_picture_ptr->poc, smallest_poc_greater_than_current);
 
             // find the largest poc
@@ -2815,57 +2924,83 @@
                 int step= list ? -1 : 1;
 
                 for(i=0; i<h->short_ref_count && index < h->ref_count[list]; i++, j+=step) {
+                    int sel;
                     while(j<0 || j>= h->short_ref_count){
                         if(j != -99 && step == (list ? -1 : 1))
                             return -1;
                         step = -step;
                         j= smallest_poc_greater_than_current + (step>>1);
                     }
-                    if(sorted_short_ref[j].reference != 3) continue;
-                    h->default_ref_list[list][index  ]= sorted_short_ref[j];
-                    h->default_ref_list[list][index++].pic_id= sorted_short_ref[j].frame_num;
+                    sel = sorted_short_ref[j].reference | structure_sel;
+                    if(sel != PICT_FRAME) continue;
+                    frame_list[list][index  ]= sorted_short_ref[j];
+                    frame_list[list][index++].pic_id= sorted_short_ref[j].frame_num;
                 }
+                short_len[list] = index;
 
                 for(i = 0; i < 16 && index < h->ref_count[ list ]; i++){
+                    int sel;
                     if(h->long_ref[i] == NULL) continue;
-                    if(h->long_ref[i]->reference != 3) continue;
+                    sel = h->long_ref[i]->reference | structure_sel;
+                    if(sel != PICT_FRAME) continue;
 
-                    h->default_ref_list[ list ][index  ]= *h->long_ref[i];
-                    h->default_ref_list[ list ][index++].pic_id= i;;
+                    frame_list[ list ][index  ]= *h->long_ref[i];
+                    frame_list[ list ][index++].pic_id= i;;
                 }
+                len[list] = index;
 
                 if(list && (smallest_poc_greater_than_current<=0 || smallest_poc_greater_than_current>=h->short_ref_count) && (1 < index)){
                     // swap the two first elements of L1 when
                     // L0 and L1 are identical
-                    Picture temp= h->default_ref_list[1][0];
-                    h->default_ref_list[1][0] = h->default_ref_list[1][1];
-                    h->default_ref_list[1][1] = temp;
+                    Picture temp= frame_list[1][0];
+                    frame_list[1][0] = frame_list[1][1];
+                    frame_list[1][1] = temp;
                 }
 
-                if(index < h->ref_count[ list ])
-                    memset(&h->default_ref_list[list][index], 0, sizeof(Picture)*(h->ref_count[ list ] - index));
             }
+
+            for(list=0; list<2; list++){
+                if (FIELD_PICTURE)
+                    len[list] = split_field_ref_list(h->default_ref_list[list],
+                                                     h->ref_count[list],
+                                                     frame_list[list],
+                                                     len[list],
+                                                     s->picture_structure,
+                                                     short_len[list]);
+
+                if(len[list] < h->ref_count[ list ])
+                    memset(&h->default_ref_list[list][len[list]], 0, sizeof(Picture)*(h->ref_count[ list ] - len[list]));
+            }
+
+
         }else{
             int index=0;
+            int short_len;
             for(i=0; i<h->short_ref_count; i++){
-                if(h->short_ref[i]->reference != 3) continue; //FIXME refernce field shit
-                h->default_ref_list[0][index  ]= *h->short_ref[i];
-                h->default_ref_list[0][index++].pic_id= h->short_ref[i]->frame_num;
+                int sel;
+                sel = h->short_ref[i]->reference | structure_sel;
+                if(sel != PICT_FRAME) continue;
+                frame_list[0][index  ]= *h->short_ref[i];
+                frame_list[0][index++].pic_id= h->short_ref[i]->frame_num;
             }
+            short_len = index;
             for(i = 0; i < 16; i++){
+                int sel;
                 if(h->long_ref[i] == NULL) continue;
-                if(h->long_ref[i]->reference != 3) continue;
-                h->default_ref_list[0][index  ]= *h->long_ref[i];
-                h->default_ref_list[0][index++].pic_id= i;;
+                sel = h->long_ref[i]->reference | structure_sel;
+                if(sel != PICT_FRAME) continue;
+                frame_list[0][index  ]= *h->long_ref[i];
+                frame_list[0][index++].pic_id= i;;
             }
+
+            if (FIELD_PICTURE)
+                index = split_field_ref_list(h->default_ref_list[0],
+                                             h->ref_count[0], frame_list[0],
+                                             index, s->picture_structure,
+                                             short_len);
+
             if(index < h->ref_count[0])
                 memset(&h->default_ref_list[0][index], 0, sizeof(Picture)*(h->ref_count[0] - index));
-        }
-    }else{ //FIELD
-        if(h->slice_type==B_TYPE){
-        }else{
-            //FIXME second field balh
-        }
     }
 #ifdef TRACE
     for (i=0; i<h->ref_count[0]; i++) {
-------------- next part --------------
--- ../ffmpeg-defreflist/libavcodec/h264.c	2007-10-03 16:34:30.000000000 -0400
+++ libavcodec/h264.c	2007-10-03 16:35:49.000000000 -0400
@@ -2915,92 +2915,92 @@
             }
         }
 
-            tprintf(h->s.avctx, "current poc: %d, smallest_poc_greater_than_current: %d\n", s->current_picture_ptr->poc, smallest_poc_greater_than_current);
+        tprintf(h->s.avctx, "current poc: %d, smallest_poc_greater_than_current: %d\n", s->current_picture_ptr->poc, smallest_poc_greater_than_current);
 
-            // find the largest poc
-            for(list=0; list<2; list++){
-                int index = 0;
-                int j= -99;
-                int step= list ? -1 : 1;
-
-                for(i=0; i<h->short_ref_count && index < h->ref_count[list]; i++, j+=step) {
-                    int sel;
-                    while(j<0 || j>= h->short_ref_count){
-                        if(j != -99 && step == (list ? -1 : 1))
-                            return -1;
-                        step = -step;
-                        j= smallest_poc_greater_than_current + (step>>1);
-                    }
-                    sel = sorted_short_ref[j].reference | structure_sel;
-                    if(sel != PICT_FRAME) continue;
-                    frame_list[list][index  ]= sorted_short_ref[j];
-                    frame_list[list][index++].pic_id= sorted_short_ref[j].frame_num;
-                }
-                short_len[list] = index;
-
-                for(i = 0; i < 16 && index < h->ref_count[ list ]; i++){
-                    int sel;
-                    if(h->long_ref[i] == NULL) continue;
-                    sel = h->long_ref[i]->reference | structure_sel;
-                    if(sel != PICT_FRAME) continue;
-
-                    frame_list[ list ][index  ]= *h->long_ref[i];
-                    frame_list[ list ][index++].pic_id= i;;
-                }
-                len[list] = index;
-
-                if(list && (smallest_poc_greater_than_current<=0 || smallest_poc_greater_than_current>=h->short_ref_count) && (1 < index)){
-                    // swap the two first elements of L1 when
-                    // L0 and L1 are identical
-                    Picture temp= frame_list[1][0];
-                    frame_list[1][0] = frame_list[1][1];
-                    frame_list[1][1] = temp;
-                }
+        // find the largest poc
+        for(list=0; list<2; list++){
+            int index = 0;
+            int j= -99;
+            int step= list ? -1 : 1;
 
-            }
-
-            for(list=0; list<2; list++){
-                if (FIELD_PICTURE)
-                    len[list] = split_field_ref_list(h->default_ref_list[list],
-                                                     h->ref_count[list],
-                                                     frame_list[list],
-                                                     len[list],
-                                                     s->picture_structure,
-                                                     short_len[list]);
-
-                if(len[list] < h->ref_count[ list ])
-                    memset(&h->default_ref_list[list][len[list]], 0, sizeof(Picture)*(h->ref_count[ list ] - len[list]));
-            }
-
-
-        }else{
-            int index=0;
-            int short_len;
-            for(i=0; i<h->short_ref_count; i++){
+            for(i=0; i<h->short_ref_count && index < h->ref_count[list]; i++, j+=step) {
                 int sel;
-                sel = h->short_ref[i]->reference | structure_sel;
+                while(j<0 || j>= h->short_ref_count){
+                    if(j != -99 && step == (list ? -1 : 1))
+                        return -1;
+                    step = -step;
+                    j= smallest_poc_greater_than_current + (step>>1);
+                }
+                sel = sorted_short_ref[j].reference | structure_sel;
                 if(sel != PICT_FRAME) continue;
-                frame_list[0][index  ]= *h->short_ref[i];
-                frame_list[0][index++].pic_id= h->short_ref[i]->frame_num;
+                frame_list[list][index  ]= sorted_short_ref[j];
+                frame_list[list][index++].pic_id= sorted_short_ref[j].frame_num;
             }
-            short_len = index;
-            for(i = 0; i < 16; i++){
+            short_len[list] = index;
+
+            for(i = 0; i < 16 && index < h->ref_count[ list ]; i++){
                 int sel;
                 if(h->long_ref[i] == NULL) continue;
                 sel = h->long_ref[i]->reference | structure_sel;
                 if(sel != PICT_FRAME) continue;
-                frame_list[0][index  ]= *h->long_ref[i];
-                frame_list[0][index++].pic_id= i;;
+
+                frame_list[ list ][index  ]= *h->long_ref[i];
+                frame_list[ list ][index++].pic_id= i;;
+            }
+            len[list] = index;
+
+            if(list && (smallest_poc_greater_than_current<=0 || smallest_poc_greater_than_current>=h->short_ref_count) && (1 < index)){
+                // swap the two first elements of L1 when
+                // L0 and L1 are identical
+                Picture temp= frame_list[1][0];
+                frame_list[1][0] = frame_list[1][1];
+                frame_list[1][1] = temp;
             }
 
+        }
+
+        for(list=0; list<2; list++){
             if (FIELD_PICTURE)
-                index = split_field_ref_list(h->default_ref_list[0],
-                                             h->ref_count[0], frame_list[0],
-                                             index, s->picture_structure,
-                                             short_len);
+                len[list] = split_field_ref_list(h->default_ref_list[list],
+                                                 h->ref_count[list],
+                                                 frame_list[list],
+                                                 len[list],
+                                                 s->picture_structure,
+                                                 short_len[list]);
+
+            if(len[list] < h->ref_count[ list ])
+                memset(&h->default_ref_list[list][len[list]], 0, sizeof(Picture)*(h->ref_count[ list ] - len[list]));
+        }
+
+
+    }else{
+        int index=0;
+        int short_len;
+        for(i=0; i<h->short_ref_count; i++){
+            int sel;
+            sel = h->short_ref[i]->reference | structure_sel;
+            if(sel != PICT_FRAME) continue;
+            frame_list[0][index  ]= *h->short_ref[i];
+            frame_list[0][index++].pic_id= h->short_ref[i]->frame_num;
+        }
+        short_len = index;
+        for(i = 0; i < 16; i++){
+            int sel;
+            if(h->long_ref[i] == NULL) continue;
+            sel = h->long_ref[i]->reference | structure_sel;
+            if(sel != PICT_FRAME) continue;
+            frame_list[0][index  ]= *h->long_ref[i];
+            frame_list[0][index++].pic_id= i;;
+        }
+
+        if (FIELD_PICTURE)
+            index = split_field_ref_list(h->default_ref_list[0],
+                                         h->ref_count[0], frame_list[0],
+                                         index, s->picture_structure,
+                                         short_len);
 
-            if(index < h->ref_count[0])
-                memset(&h->default_ref_list[0][index], 0, sizeof(Picture)*(h->ref_count[0] - index));
+        if(index < h->ref_count[0])
+            memset(&h->default_ref_list[0][index], 0, sizeof(Picture)*(h->ref_count[0] - index));
     }
 #ifdef TRACE
     for (i=0; i<h->ref_count[0]; i++) {
-------------- next part --------------
--- ../ffmpeg-defreflist-indent/libavcodec/h264.c	2007-10-03 16:38:32.000000000 -0400
+++ libavcodec/h264.c	2007-10-03 16:41:12.000000000 -0400
@@ -3018,9 +3018,37 @@
 static void print_short_term(H264Context *h);
 static void print_long_term(H264Context *h);
 
+/**
+ * Extract structure information about the picture described by pic_num in
+ * the current decoding context (frame or field). Note that pic_num is
+ * picture number without wrapping (so, 0<=pic_num<max_pic_num).
+ * @param pic_num picture number for which to extract structure information
+ * @param structure one of PICT_XXX describing structure of picture
+ *                      with pic_num
+ * @return frame number (short term) or long term index of picture
+ *         described by pic_num
+ */
+static int pic_num_extract(H264Context *h, int pic_num, int *structure){
+    MpegEncContext * const s = &h->s;
+    int ret;
+
+    if (FIELD_PICTURE) {
+        ret = pic_num >> 1;
+        if (pic_num & 1) {
+            *structure = s->picture_structure;
+        } else {
+            *structure = PICT_FRAME - s->picture_structure;
+        }
+    } else {
+        ret = pic_num;
+        *structure = PICT_FRAME;
+    }
+    return ret;
+}
+
 static int decode_ref_pic_list_reordering(H264Context *h){
     MpegEncContext * const s = &h->s;
-    int list, index;
+    int list, index, pic_structure;
 
     print_short_term(h);
     print_long_term(h);
@@ -3049,6 +3077,7 @@
                 if(reordering_of_pic_nums_idc<3){
                     if(reordering_of_pic_nums_idc<2){
                         const unsigned int abs_diff_pic_num= get_ue_golomb(&s->gb) + 1;
+                        int frame_num;
 
                         if(abs_diff_pic_num >= h->max_pic_num){
                             av_log(h->s.avctx, AV_LOG_ERROR, "abs_diff_pic_num overflow\n");
@@ -3059,25 +3088,34 @@
                         else                                pred+= abs_diff_pic_num;
                         pred &= h->max_pic_num - 1;
 
+                        frame_num = pic_num_extract(h, pred, &pic_structure);
+
                         for(i= h->short_ref_count-1; i>=0; i--){
                             ref = h->short_ref[i];
-                            assert(ref->reference == 3);
+                            assert(ref->reference);
                             assert(!ref->long_ref);
-                            if(ref->data[0] != NULL && ref->frame_num == pred && ref->long_ref == 0) // ignore non existing pictures by testing data[0] pointer
+                            if(ref->data[0] != NULL &&
+                                   ref->frame_num == frame_num &&
+                                   (ref->reference & pic_structure) &&
+                                   ref->long_ref == 0) // ignore non existing pictures by testing data[0] pointer
                                 break;
                         }
                         if(i>=0)
-                            ref->pic_id= ref->frame_num;
+                            ref->pic_id= pred;
                     }else{
+                        int long_idx;
                         pic_id= get_ue_golomb(&s->gb); //long_term_pic_idx
-                        if(pic_id>31){
+
+                        long_idx= pic_num_extract(h, pic_id, &pic_structure);
+
+                        if(long_idx>31){
                             av_log(h->s.avctx, AV_LOG_ERROR, "long_term_pic_idx overflow\n");
                             return -1;
                         }
-                        ref = h->long_ref[pic_id];
-                        if(ref){
+                        ref = h->long_ref[long_idx];
+                        assert(!(ref && !ref->reference));
+                        if(ref && (ref->reference & pic_structure)){
                             ref->pic_id= pic_id;
-                            assert(ref->reference == 3);
                             assert(ref->long_ref);
                             i=0;
                         }else{
@@ -3097,6 +3135,10 @@
                             h->ref_list[list][i]= h->ref_list[list][i-1];
                         }
                         h->ref_list[list][index]= *ref;
+                        if (FIELD_PICTURE){
+                            int bot = pic_structure == PICT_BOTTOM_FIELD;
+                            pic_as_field(&h->ref_list[list][index], bot);
+                        }
                     }
                 }else{
                     av_log(h->s.avctx, AV_LOG_ERROR, "illegal reordering_of_pic_nums_idc\n");
-------------- next part --------------
--- ../tmp/ffmpeg-reordering/libavcodec/h264.c	2007-10-03 16:43:23.000000000 -0400
+++ libavcodec/h264.c	2007-10-03 16:25:15.000000000 -0400
@@ -177,7 +177,7 @@
 
     //wow what a mess, why didn't they simplify the interlacing&intra stuff, i can't imagine that these complex rules are worth it
 
-    top_xy     = mb_xy  - s->mb_stride;
+    top_xy     = mb_xy  - (s->mb_stride << FIELD_PICTURE);
     topleft_xy = top_xy - 1;
     topright_xy= top_xy + 1;
     left_xy[1] = left_xy[0] = mb_xy-1;
@@ -3342,6 +3342,7 @@
     idr(h);
     if(h->s.current_picture_ptr)
         h->s.current_picture_ptr->reference= 0;
+    h->s.first_field= 0;
 }
 
 /**
@@ -3461,7 +3462,7 @@
 static int execute_ref_pic_marking(H264Context *h, MMCO *mmco, int mmco_count){
     MpegEncContext * const s = &h->s;
     int i, j;
-    int current_is_long=0;
+    int current_ref_assigned=0;
     Picture *pic;
 
     if((s->avctx->debug&FF_DEBUG_MMCO) && mmco_count==0)
@@ -3473,38 +3474,78 @@
 
         switch(mmco[i].opcode){
         case MMCO_SHORT2UNUSED:
-            pic= remove_short(h, mmco[i].short_pic_num);
-            if(pic)
-                unreference_pic(h, pic, 0);
-            else if(s->avctx->debug&FF_DEBUG_MMCO)
-                av_log(h->s.avctx, AV_LOG_DEBUG, "mmco: remove_short() failure\n");
+            if(s->avctx->debug&FF_DEBUG_MMCO)
+                av_log(h->s.avctx, AV_LOG_DEBUG, "mmco: unref short %d count %d\n", h->mmco[i].short_pic_num, h->short_ref_count);
+            pic = find_short(h, mmco[i].short_pic_num >> FIELD_PICTURE, &j);
+            if (pic) {
+                int mask = (!FIELD_PICTURE || mmco[i].short_pic_num & 1) ?
+                         ~s->picture_structure : s->picture_structure;
+                if (unreference_pic(h, pic, mask))
+                    remove_short_at_index(h, j);
+            } else if(s->avctx->debug&FF_DEBUG_MMCO)
+                av_log(h->s.avctx, AV_LOG_DEBUG, "mmco: unref short failure\n");
             break;
         case MMCO_SHORT2LONG:
-            pic= remove_long(h, mmco[i].long_arg);
-            if(pic) unreference_pic(h, pic, 0);
+            if (FIELD_PICTURE && mmco[i].long_arg < h->long_ref_count &&
+                    h->long_ref[mmco[i].long_arg]->frame_num ==
+                                              mmco[i].short_pic_num / 2) {
+                /* do nothing, we've already moved this field pair. */
+            } else {
+                int frame_num = mmco[i].short_pic_num >> FIELD_PICTURE;
 
-            h->long_ref[ mmco[i].long_arg ]= remove_short(h, mmco[i].short_pic_num);
+                pic= remove_long(h, mmco[i].long_arg);
+                if(pic) unreference_pic(h, pic, 0);
+
+                h->long_ref[ mmco[i].long_arg ]= remove_short(h, frame_num);
             if (h->long_ref[ mmco[i].long_arg ]){
                 h->long_ref[ mmco[i].long_arg ]->long_ref=1;
                 h->long_ref_count++;
             }
+            }
             break;
         case MMCO_LONG2UNUSED:
-            pic= remove_long(h, mmco[i].long_arg);
-            if(pic)
-                unreference_pic(h, pic, 0);
-            else if(s->avctx->debug&FF_DEBUG_MMCO)
-                av_log(h->s.avctx, AV_LOG_DEBUG, "mmco: remove_long() failure\n");
+            j = mmco[i].long_arg >> FIELD_PICTURE;
+            pic = h->long_ref[j];
+            if (pic) {
+                int mask = (!FIELD_PICTURE || mmco[i].long_arg & 1) ?
+                         ~s->picture_structure : s->picture_structure;
+                if (unreference_pic(h, pic, mask))
+                    remove_long_at_index(h, j);
+            } else if(s->avctx->debug&FF_DEBUG_MMCO)
+                av_log(h->s.avctx, AV_LOG_DEBUG, "mmco: unref long failure\n");
             break;
         case MMCO_LONG:
+            j = 1;
+            if (FIELD_PICTURE && !s->first_field) {
+                if (h->long_ref[mmco[i].long_arg] == s->current_picture_ptr) {
+                    /* Just mark second field as referenced */
+                    j = 0;
+                } else if (s->current_picture_ptr->reference) {
+                    /* First field in pair is in short term list or
+                     * at a different long term index.
+                     * This is not allowed; see 7.4.3, notes 2 and 3.
+                     * Report the problem and keep the pair where it is,
+                     * and mark this field valid.
+                     */
+                    av_log(h->s.avctx, AV_LOG_ERROR,
+                        "illegal long term reference assignment for second "
+                        "field in complementary field pair (first field is "
+                        "short term or has non-matching long index)\n");
+                    j = 0;
+                }
+            }
+
+            if (j) {
             pic= remove_long(h, mmco[i].long_arg);
             if(pic) unreference_pic(h, pic, 0);
 
             h->long_ref[ mmco[i].long_arg ]= s->current_picture_ptr;
             h->long_ref[ mmco[i].long_arg ]->long_ref=1;
             h->long_ref_count++;
+            }
 
-            current_is_long=1;
+            s->current_picture_ptr->reference |= s->picture_structure;
+            current_ref_assigned=1;
             break;
         case MMCO_SET_MAX_LONG:
             assert(mmco[i].long_arg <= 16);
@@ -3528,7 +3569,36 @@
         }
     }
 
-    if(!current_is_long){
+    if (!current_ref_assigned && FIELD_PICTURE &&
+            !s->first_field && s->current_picture_ptr->reference) {
+
+        /* Second field of complementary field pair; the first field of
+         * which is already referenced. If short referenced, it
+         * should be first entry in short_ref. If not, it must exist
+         * in long_ref; trying to put it on the short list here is an
+         * error in the encoded bit stream (ref: 7.4.3, NOTE 2 and 3).
+         * If on neither list, we have a problem elsewhere
+         */
+        if (h->short_ref_count && h->short_ref[0] == s->current_picture_ptr) {
+            /* Just mark the second field valid */
+            s->current_picture_ptr->reference = PICT_FRAME;
+        } else if (s->current_picture_ptr->long_ref) {
+            av_log(h->s.avctx, AV_LOG_ERROR, "illegal short term reference "
+                                             "assignment for second field "
+                                             "in complementary field pair "
+                                             "(first field is long term)\n");
+        } else {
+            av_log(h->s.avctx, AV_LOG_ERROR, "problem in internal reference "
+                                             "list handling; marking second "
+                                             "field in pair finds first field "
+                                             "in reference, but not in any "
+                                             "ref list\n");
+            s->current_picture_ptr->reference = 0;
+        }
+        current_ref_assigned = 1;
+    }
+
+    if(!current_ref_assigned){
         pic= remove_short(h, s->current_picture_ptr->frame_num);
         if(pic){
             unreference_pic(h, pic, 0);
@@ -3541,6 +3611,7 @@
         h->short_ref[0]= s->current_picture_ptr;
         h->short_ref[0]->long_ref=0;
         h->short_ref_count++;
+        s->current_picture_ptr->reference |= s->picture_structure;
     }
 
     print_short_term(h);
@@ -3568,7 +3639,7 @@
 
                 h->mmco[i].opcode= opcode;
                 if(opcode==MMCO_SHORT2UNUSED || opcode==MMCO_SHORT2LONG){
-                    h->mmco[i].short_pic_num= (h->frame_num - get_ue_golomb(gb) - 1) & ((1<<h->sps.log2_max_frame_num)-1); //FIXME fields
+                    h->mmco[i].short_pic_num= (h->curr_pic_num - get_ue_golomb(gb) - 1) & (h->max_pic_num - 1);
 /*                    if(h->mmco[i].short_pic_num >= h->short_ref_count || h->short_ref[ h->mmco[i].short_pic_num ] == NULL){
                         av_log(s->avctx, AV_LOG_ERROR, "illegal short ref in memory management control operation %d\n", mmco);
                         return -1;
@@ -3576,7 +3647,7 @@
                 }
                 if(opcode==MMCO_SHORT2LONG || opcode==MMCO_LONG2UNUSED || opcode==MMCO_LONG || opcode==MMCO_SET_MAX_LONG){
                     unsigned int long_arg= get_ue_golomb(gb);
-                    if(/*h->mmco[i].long_arg >= h->long_ref_count || h->long_ref[ h->mmco[i].long_arg ] == NULL*/ long_arg >= 16){
+                    if(long_arg >= 32 || (long_arg >= 16 && !(opcode == MMCO_LONG2UNUSED && FIELD_PICTURE))){
                         av_log(h->s.avctx, AV_LOG_ERROR, "illegal long ref in memory management control operation %d\n", opcode);
                         return -1;
                     }
@@ -3594,10 +3665,17 @@
         }else{
             assert(h->long_ref_count + h->short_ref_count <= h->sps.ref_frame_count);
 
-            if(h->long_ref_count + h->short_ref_count == h->sps.ref_frame_count){ //FIXME fields
+            if(h->long_ref_count + h->short_ref_count == h->sps.ref_frame_count &&
+                    !(FIELD_PICTURE && !s->first_field && s->current_picture_ptr->reference)) {
                 h->mmco[0].opcode= MMCO_SHORT2UNUSED;
                 h->mmco[0].short_pic_num= h->short_ref[ h->short_ref_count - 1 ]->frame_num;
                 h->mmco_index= 1;
+                if (FIELD_PICTURE) {
+                    h->mmco[0].short_pic_num *= 2;
+                    h->mmco[1].opcode= MMCO_SHORT2UNUSED;
+                    h->mmco[1].short_pic_num= h->mmco[0].short_pic_num + 1;
+                    h->mmco_index= 2;
+                }
             }else
                 h->mmco_index= 0;
         }
@@ -3685,11 +3763,15 @@
         field_poc[1]= poc;
     }
 
-    if(s->picture_structure != PICT_BOTTOM_FIELD)
+    if(s->picture_structure != PICT_BOTTOM_FIELD) {
         s->current_picture_ptr->field_poc[0]= field_poc[0];
-    if(s->picture_structure != PICT_TOP_FIELD)
+        s->current_picture_ptr->poc = field_poc[0];
+    }
+    if(s->picture_structure != PICT_TOP_FIELD) {
         s->current_picture_ptr->field_poc[1]= field_poc[1];
-    if(s->picture_structure == PICT_FRAME) // FIXME field pix?
+        s->current_picture_ptr->poc = field_poc[1];
+    }
+    if(!FIELD_PICTURE || !s->first_field)
         s->current_picture_ptr->poc= FFMIN(field_poc[0], field_poc[1]);
 
     return 0;
@@ -3755,6 +3837,7 @@
     dst->s.current_picture      = src->s.current_picture;
     dst->s.linesize             = src->s.linesize;
     dst->s.uvlinesize           = src->s.uvlinesize;
+    dst->s.first_field          = src->s.first_field;
 
     dst->prev_poc_msb           = src->prev_poc_msb;
     dst->prev_poc_lsb           = src->prev_poc_lsb;
@@ -3782,12 +3865,14 @@
  */
 static int decode_slice_header(H264Context *h, H264Context *h0){
     MpegEncContext * const s = &h->s;
+    MpegEncContext * const s0 = &h0->s;
     unsigned int first_mb_in_slice;
     unsigned int pps_id;
     int num_ref_idx_active_override_flag;
     static const uint8_t slice_type_map[5]= {P_TYPE, B_TYPE, I_TYPE, SP_TYPE, SI_TYPE};
     unsigned int slice_type, tmp, i;
     int default_ref_list_done = 0;
+    int last_pic_structure;
 
     s->dropable= h->nal_ref_idc == 0;
 
@@ -3795,7 +3880,8 @@
 
     if((s->flags2 & CODEC_FLAG2_CHUNKS) && first_mb_in_slice == 0){
         h0->current_slice = 0;
-        s->current_picture_ptr= NULL;
+        if (!s0->first_field)
+            s->current_picture_ptr= NULL;
     }
 
     slice_type= get_ue_golomb(&s->gb);
@@ -3864,6 +3950,7 @@
             return -1;  // we cant (re-)initialize context during parallel decoding
         if (MPV_common_init(s) < 0)
             return -1;
+        s->first_field = 0;
 
         init_scan_tables(h);
         alloc_tables(h);
@@ -3902,12 +3989,12 @@
 
     h->mb_mbaff = 0;
     h->mb_aff_frame = 0;
+    last_pic_structure = s0->picture_structure;
     if(h->sps.frame_mbs_only_flag){
         s->picture_structure= PICT_FRAME;
     }else{
         if(get_bits1(&s->gb)) { //field_pic_flag
             s->picture_structure= PICT_TOP_FIELD + get_bits1(&s->gb); //bottom_field_flag
-            av_log(h->s.avctx, AV_LOG_ERROR, "PAFF interlacing is not implemented\n");
         } else {
             s->picture_structure= PICT_FRAME;
             h->mb_aff_frame = h->sps.mb_aff;
@@ -3915,8 +4002,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;
+
+            } else {
+                if (h->nal_ref_idc &&
+                        s0->current_picture_ptr->reference &&
+                        s0->current_picture_ptr->frame_num != h->frame_num) {
+                    /*
+                     * This and previous field were reference, but had
+                     * different frame_nums. Consider this field first in
+                     * pair. Throw away previous field except for reference
+                     * purposes.
+                     */
+                    s0->first_field = 1;
+                    s0->current_picture_ptr = NULL;
+
+                } else {
+                    /* Second field in complementary pair */
+                    s0->first_field = 0;
+                }
+            }
+
+        } else {
+            /* Frame or first field in a potentially complementary pair */
+            assert(!s0->current_picture_ptr);
+            s0->first_field = FIELD_PICTURE;
+        }
+
+        if((!FIELD_PICTURE || s0->first_field) && frame_start(h) < 0) {
+            s0->first_field = 0;
             return -1;
+        }
     }
     if(h != h0)
         clone_slice(h, h0);
@@ -3931,6 +4060,11 @@
     }
     s->resync_mb_x = s->mb_x = first_mb_in_slice % s->mb_width;
     s->resync_mb_y = s->mb_y = (first_mb_in_slice / s->mb_width) << h->mb_aff_frame;
+    if (FIELD_PICTURE) {
+        s->resync_mb_y = s->mb_y = s->mb_y * 2;
+        if (s->picture_structure == PICT_BOTTOM_FIELD)
+            s->resync_mb_y = s->mb_y = s->mb_y + 1;
+    }
     assert(s->mb_y < s->mb_height);
 
     if(s->picture_structure==PICT_FRAME){
@@ -3973,7 +4107,7 @@
     if(h->slice_type == P_TYPE || h->slice_type == SP_TYPE || h->slice_type == B_TYPE){
         if(h->slice_type == B_TYPE){
             h->direct_spatial_mv_pred= get_bits1(&s->gb);
-            if(h->sps.mb_aff && h->direct_spatial_mv_pred)
+            if(h->mb_aff_frame && h->direct_spatial_mv_pred)
                 av_log(h->s.avctx, AV_LOG_ERROR, "MBAFF + spatial direct mode is not implemented\n");
         }
         num_ref_idx_active_override_flag= get_bits1(&s->gb);
@@ -4093,7 +4227,7 @@
     h->slice_num = ++h0->current_slice;
 
     h->emu_edge_width= (s->flags&CODEC_FLAG_EMU_EDGE) ? 0 : 16;
-    h->emu_edge_height= FRAME_MBAFF ? 0 : h->emu_edge_width;
+    h->emu_edge_height= (FRAME_MBAFF || FIELD_PICTURE) ? 0 : h->emu_edge_width;
 
     if(s->avctx->debug&FF_DEBUG_PICT_INFO){
         av_log(h->s.avctx, AV_LOG_DEBUG, "slice:%d %s mb:%d %c pps:%u frame:%d poc:%d/%d ref:%d/%d qp:%d loop:%d:%d:%d weight:%d%s\n",
@@ -4991,6 +5125,8 @@
         int mb_xy = mb_x + mb_y*s->mb_stride;
         mba_xy = mb_xy - 1;
         mbb_xy = mb_xy - s->mb_stride;
+        if (FIELD_PICTURE)
+            mbb_xy -= s->mb_stride;
     }
 
     if( h->slice_table[mba_xy] == h->slice_num && !IS_SKIP( s->current_picture.mb_type[mba_xy] ))
@@ -5434,6 +5570,8 @@
         if (left_mb_frame_flag != curr_mb_frame_flag) {
             h->left_mb_xy[0] = pair_xy - 1;
         }
+    } else if (FIELD_PICTURE) {
+        h->top_mb_xy -= s->mb_stride;
     }
     return;
 }
@@ -6669,7 +6807,7 @@
                 s->mb_x = 0;
                 ff_draw_horiz_band(s, 16*s->mb_y, 16);
                 ++s->mb_y;
-                if(FRAME_MBAFF) {
+                if(FRAME_MBAFF || FIELD_PICTURE) {
                     ++s->mb_y;
                 }
             }
@@ -6706,7 +6844,7 @@
                 s->mb_x=0;
                 ff_draw_horiz_band(s, 16*s->mb_y, 16);
                 ++s->mb_y;
-                if(FRAME_MBAFF) {
+                if(FRAME_MBAFF || FIELD_PICTURE) {
                     ++s->mb_y;
                 }
                 if(s->mb_y >= s->mb_height){
@@ -7283,6 +7421,8 @@
         hx = h->thread_context[context_count - 1];
         s->mb_x = hx->s.mb_x;
         s->mb_y = hx->s.mb_y;
+        s->dropable = hx->s.dropable;
+        s->picture_structure = hx->s.picture_structure;
         for(i = 1; i < context_count; i++)
             h->s.error_count += h->thread_context[i]->s.error_count;
     }
@@ -7305,7 +7445,8 @@
 #endif
     if(!(s->flags2 & CODEC_FLAG2_CHUNKS)){
         h->current_slice = 0;
-        s->current_picture_ptr= NULL;
+        if (!s->first_field)
+            s->current_picture_ptr= NULL;
     }
 
     for(;;){
@@ -7602,16 +7743,34 @@
 
         h->prev_frame_num_offset= h->frame_num_offset;
         h->prev_frame_num= h->frame_num;
-        if(s->current_picture_ptr->reference & s->picture_structure){
+        if(!s->dropable) {
             h->prev_poc_msb= h->poc_msb;
             h->prev_poc_lsb= h->poc_lsb;
             execute_ref_pic_marking(h, h->mmco, h->mmco_index);
         }
 
-        ff_er_frame_end(s);
+        /*
+         * FIXME: Error handling code does not seem to support interlaced
+         * when slices span multiple rows
+         * The ff_er_add_slice calls don't work right for bottom
+         * fields; they cause massive erroneous error concealing
+         * Error marking covers both fields (top and bottom).
+         * This causes a mismatched s->error_count
+         * and a bad error table. Further, the error count goes to
+         * INT_MAX when called for bottom field, because mb_y is
+         * past end by one (callers fault) and resync_mb_y != 0
+         * causes problems for the first MB line, too.
+         */
+        if (!FIELD_PICTURE)
+            ff_er_frame_end(s);
 
         MPV_frame_end(s);
 
+        if (s->first_field) {
+            /* Wait for second field. */
+            *data_size = 0;
+
+        } else {
     //FIXME do something with unavailable reference frames
 
 #if 0 //decode order
@@ -7682,6 +7841,7 @@
             *pict= *(AVFrame*)out;
         else
             av_log(avctx, AV_LOG_DEBUG, "no picture\n");
+        }
     }
 
     assert(pict->data[0] || !*data_size);
--- ../tmp/ffmpeg-reordering/libavcodec/h264.h	2007-10-03 16:43:23.000000000 -0400
+++ libavcodec/h264.h	2007-10-03 14:52:50.000000000 -0400
@@ -59,7 +59,7 @@
 #define MB_MBAFF h->mb_mbaff
 #define MB_FIELD h->mb_field_decoding_flag
 #define FRAME_MBAFF h->mb_aff_frame
-#define FIELD_PICTURE 0
+#define FIELD_PICTURE (s->picture_structure != PICT_FRAME)
 #else
 #define MB_MBAFF 0
 #define MB_FIELD 0
--- ../tmp/ffmpeg-reordering/libavcodec/mpegvideo.c	2007-10-03 16:43:23.000000000 -0400
+++ libavcodec/mpegvideo.c	2007-10-03 14:50:38.000000000 -0400
@@ -954,7 +954,7 @@
 
     assert(s->pict_type == I_TYPE || (s->last_picture_ptr && s->last_picture_ptr->data[0]));
 
-    if(s->picture_structure!=PICT_FRAME){
+    if(s->picture_structure!=PICT_FRAME && s->out_format != FMT_H264){
         int i;
         for(i=0; i<4; i++){
             if(s->picture_structure == PICT_BOTTOM_FIELD){



More information about the ffmpeg-devel mailing list