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

Kostya kostya.shishkov
Sat Feb 21 13:29:09 CET 2009


On Sat, Feb 21, 2009 at 11:41:37AM +0100, Michael Niedermayer wrote:
> On Sat, Feb 21, 2009 at 10:34:05AM +0200, Kostya wrote:
> > 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) 
> 
> code that is under if(not true in current working cases) is not going to break
> the release and thus there is
> no sense to delay it also code that handles cases that just returned -1
> before is also not going to break the release.
> Compared to this many bug fixes which are needed have a very high potential
> to break the release.
> 
> also iam considering to forbid any future freeze of head if development is
> stoped like that. If the release manager considers a freeze needed that can
> be done after forking and limited to the fork.
> Its enough that development of core parts is stoped, stopping addition of
> new features that cannot break existing feaures is just lame.
> 
> A freeze is NOT about only doing bug fixes (that may or may not break the
> whole ffmpeg) BUT
> its about NOT doing changes that could break the release unless they are
> essential bug fixes.

I'll wait until we have formal release rules instead of flamebait. 
 
[...]
> 
> > +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);
> 
> please dont use swaps they do 50% redundant operations.
> this code is reasonable speed critial

ok, throwed out those redundant 50%

> [...]
> > +    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);
> > +    }
> 
> ehh? !s->mb_intra && s->pict_type == FF_P_TYPE
> redundant ...

It is not - P-frames can contain intra-coded blocks.
 
> [...]
> -- 
> 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,50 @@
     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;
+
+    /* 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)
+     */
+    mbi = s->mb_intra;
+    s->mb_intra = 0;
+    for (i = 0; i < 6; i++) {
+        if (h263_decode_block(s, dblock, i, cbp&32) < 0)
+            return -1;
+        cbp+=cbp;
+    }
+    s->mb_intra = mbi;
+    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 +3963,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 +4028,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 +4120,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 +4145,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 +4157,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