[FFmpeg-devel] [PATCH] PB-frames support for i263

Kostya kostya.shishkov
Sat Feb 21 09:34:05 CET 2009


On Fri, Feb 20, 2009 at 06:52:12PM +0100, Michael Niedermayer wrote:
> On Fri, Feb 20, 2009 at 08:17:24PM +0200, Kostya wrote:
> > On Fri, Feb 20, 2009 at 01:09:08PM +0100, Michael Niedermayer wrote:
> > > On Fri, Feb 20, 2009 at 11:37:40AM +0200, Kostya wrote:
> > [...]
> > > > 
> > > > This one should be right.
> > > 
> > > ok
> > 
> > I'll apply it after code thaw then (i.e. next week).
> 
> you dont want to fix pb frames in the release?
> well i dont care, as long as i dont have to redo the review ...

it's not fixing, it's rather new feature (i.e. not a thing for release) 
 
> > For now here are the patches for H.263 picture header parsing and actual
> > PB-frame decoding for all of them.
> >  
> > > [...]
> > > -- 
> > > Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
 
[h263 picture header parsing patch] 
> above (and only above) is ok

that was a separate patch anyway

> [...]
> > +static int h263_get_modb(GetBitContext *gb, int pb_frame, int *cbpb)
> > +{
> > +    int c, mv = 1;
> > +
> > +    switch(pb_frame){
> > +    case 1: // h.263 and i263 PB-frame
> > +        c = get_bits1(gb);
> > +        break;
> > +    case 2: // i263 improved PB-frame
> > +        c = get_bits1(gb);
> > +        if(c)
> > +            mv = !get_bits1(gb);        
> > +        break;
> > +    case 3: // h.263 improved PB-frame
> > +        mv = get_unary(gb, 0, 4) + 1;
> > +        c = mv & 1;
> > +        mv = !!(mv & 2);
> > +        break;
> 
> if(pb_frame<3){
>     c = get_bits1(gb);
>     if(pb_frame==2 && c)
>         mv = !get_bits1(gb);
> }else{
>     mv = get_unary(gb, 0, 4) + 1;
>     c = mv & 1;
>     mv = !!(mv & 2);
> }
> 
> 
> > +    default:
> > +        return -1;
> 
> not reachable, please do not add dead code!

ok
 
> [...]
> 
> > @@ -3987,12 +4030,19 @@
> >              }
> >          }
> >  
> > +        if(pb_mv_count){
> > +            h263_decode_motion(s, 0, 1);
> > +            h263_decode_motion(s, 0, 1);
> > +        }
> > +
> >          /* decode each block */
> >          for (i = 0; i < 6; i++) {
> >              if (h263_decode_block(s, block[i], i, cbp&32) < 0)
> >                  return -1;
> >              cbp+=cbp;
> >          }
> > +        if(s->pb_frame && h263_skip_b_part(s, cbpb))
> > +            return -1;
> >  
> >          if(s->obmc){
> >              if(s->pict_type == FF_P_TYPE && s->mb_x+1<s->mb_width && s->mb_num_left != 1)
> [...]
> > @@ -4128,12 +4180,24 @@
> >              h263_decode_dquant(s);
> >          }
> >  
> > +        pb_mv_count += !!s->pb_frame;
> > +        while(pb_mv_count--){
> > +            h263_decode_motion(s, 0, 1);
> > +            h263_decode_motion(s, 0, 1);
> > +        }
> > +
> >          /* decode each block */
> >          for (i = 0; i < 6; i++) {
> >              if (h263_decode_block(s, block[i], i, cbp&32) < 0)
> >                  return -1;
> >              cbp+=cbp;
> >          }
> > +        if(s->pb_frame){
> > +            s->mb_intra = 0;
> > +            if(h263_skip_b_part(s, cbpb))
> > +                return -1;
> > +            s->mb_intra = 1;
> > +        }
> 
> as ive said already this is duplicate and can be factored out

I can't factorize out pb_mv_count and cbpb reading because it's wrapped into
that logic:
 if(frame_type == FF_P_TYPE){
  ...
  if(s->mb_intra) goto intra;
  read pb_mv_count and cbpb
  ...
 }else if(frame_type == FF_I_TYPE){
  ...
intra:
  [read some bits]
  read pb_mv_count and cbpb
  ...
 }

at least B-part skipping was factored out (along with block reading though)
 
> [..]
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
-------------- next part --------------
Index: libavcodec/h263.c
===================================================================
--- libavcodec/h263.c	(revision 16921)
+++ libavcodec/h263.c	(working copy)
@@ -40,6 +40,7 @@
 #include "h263data.h"
 #include "mpeg4data.h"
 #include "mathops.h"
+#include "unary.h"
 
 //#undef NDEBUG
 //#include <assert.h>
@@ -3886,12 +3887,49 @@
     ff_set_qscale(s, s->qscale);
 }
 
+static int h263_skip_b_part(MpegEncContext *s, int cbp)
+{
+    DECLARE_ALIGNED(16, DCTELEM, dblock[64]);
+    int i, mbi = 0;
+
+    /* we have to set s->mb_intra to zero to decode B-part of PB-frame correctly
+     * but real value should be restored in order to be used later (in OBMC condition)
+     */
+    FFSWAP(int, mbi, s->mb_intra);
+    for (i = 0; i < 6; i++) {
+        if (h263_decode_block(s, dblock, i, cbp&32) < 0)
+            return -1;
+        cbp+=cbp;
+    }
+    FFSWAP(int, mbi, s->mb_intra);
+    return 0;
+}
+
+static int h263_get_modb(GetBitContext *gb, int pb_frame, int *cbpb)
+{
+    int c, mv = 1;
+
+    if (pb_frame < 3) { // h.263 Annex G and i263 PB-frame
+        c = get_bits1(gb);
+        if (pb_frame == 2 && c)
+            mv = !get_bits1(gb);
+    } else { // h.263 Annex M improved PB-frame
+        mv = get_unary(gb, 0, 4) + 1;
+        c = mv & 1;
+        mv = !!(mv & 2);
+    }
+    if(c)
+        *cbpb = get_bits(gb, 6);
+    return mv;
+}
+
 int ff_h263_decode_mb(MpegEncContext *s,
                       DCTELEM block[6][64])
 {
     int cbpc, cbpy, i, cbp, pred_x, pred_y, mx, my, dquant;
     int16_t *mot_val;
     const int xy= s->mb_x + s->mb_y * s->mb_stride;
+    int cbpb = 0, pb_mv_count = 0;
 
     assert(!s->h263_pred);
 
@@ -3924,6 +3962,8 @@
         s->mb_intra = ((cbpc & 4) != 0);
         if (s->mb_intra) goto intra;
 
+        if(s->pb_frame && get_bits1(&s->gb))
+            pb_mv_count = h263_get_modb(&s->gb, s->pb_frame, &cbpb);
         cbpy = get_vlc2(&s->gb, cbpy_vlc.table, CBPY_VLC_BITS, 1);
 
         if(s->alt_inter_vlc==0 || (cbpc & 3)!=3)
@@ -3987,17 +4027,10 @@
             }
         }
 
-        /* decode each block */
-        for (i = 0; i < 6; i++) {
-            if (h263_decode_block(s, block[i], i, cbp&32) < 0)
-                return -1;
-            cbp+=cbp;
+        if(pb_mv_count){
+            h263_decode_motion(s, 0, 1);
+            h263_decode_motion(s, 0, 1);
         }
-
-        if(s->obmc){
-            if(s->pict_type == FF_P_TYPE && s->mb_x+1<s->mb_width && s->mb_num_left != 1)
-                preview_obmc(s);
-        }
     } else if(s->pict_type==FF_B_TYPE) {
         int mb_type;
         const int stride= s->b8_stride;
@@ -4086,13 +4119,6 @@
         }
 
         s->current_picture.mb_type[xy]= mb_type;
-
-        /* decode each block */
-        for (i = 0; i < 6; i++) {
-            if (h263_decode_block(s, block[i], i, cbp&32) < 0)
-                return -1;
-            cbp+=cbp;
-        }
     } else { /* I-Frame */
         do{
             cbpc = get_vlc2(&s->gb, intra_MCBPC_vlc.table, INTRA_MCBPC_VLC_BITS, 2);
@@ -4118,6 +4144,8 @@
         }else
             s->ac_pred = 0;
 
+        if(s->pb_frame && get_bits1(&s->gb))
+            pb_mv_count = h263_get_modb(&s->gb, s->pb_frame, &cbpb);
         cbpy = get_vlc2(&s->gb, cbpy_vlc.table, CBPY_VLC_BITS, 1);
         if(cbpy<0){
             av_log(s->avctx, AV_LOG_ERROR, "I cbpy damaged at %d %d\n", s->mb_x, s->mb_y);
@@ -4128,13 +4156,25 @@
             h263_decode_dquant(s);
         }
 
-        /* decode each block */
-        for (i = 0; i < 6; i++) {
-            if (h263_decode_block(s, block[i], i, cbp&32) < 0)
-                return -1;
-            cbp+=cbp;
+        pb_mv_count += !!s->pb_frame;
+        while(pb_mv_count--){
+            h263_decode_motion(s, 0, 1);
+            h263_decode_motion(s, 0, 1);
         }
     }
+
+    /* decode each block */
+    for (i = 0; i < 6; i++) {
+        if (h263_decode_block(s, block[i], i, cbp&32) < 0)
+            return -1;
+        cbp+=cbp;
+    }
+    if(s->pb_frame && h263_skip_b_part(s, cbpb) < 0)
+        return -1;
+    if(s->obmc && !s->mb_intra){
+        if(s->pict_type == FF_P_TYPE && s->mb_x+1<s->mb_width && s->mb_num_left != 1)
+            preview_obmc(s);
+    }
 end:
 
         /* per-MB end of slice check */



More information about the ffmpeg-devel mailing list