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

Michael Niedermayer michaelni
Tue Feb 17 18:48:57 CET 2009


On Tue, Feb 17, 2009 at 08:29:36PM +0200, Kostya wrote:
> On Tue, Feb 17, 2009 at 02:28:00PM +0100, Michael Niedermayer wrote:
> > On Tue, Feb 17, 2009 at 09:30:43AM +0200, Kostya wrote:
> > > $subj
> > > 
> > > For now it decodes only P part of PB frames and skips B blocks part
> > > (excellent idea to store data for two frames in one).
> > 
> > yes, PB frames where one of the most idiotic things in h263
> > the commitee apparently has never bother to ask the question how hard
> > some of these things may be to support in non academic implementations
> > (the complete lack of unambigous timestamps in mpeg-ps/ts and especially
> > h264 in them is another example, a single integer per packet would have
> > avoided hundreads of hours of work and lines of code)
> > 
> [...]
> > > @@ -6185,17 +6255,34 @@
> > >          return -1;      /* SAC: off */
> > >      }
> > >      s->obmc= get_bits1(&s->gb);
> > > -    if (get_bits1(&s->gb) != 0) {
> > > -        av_log(s->avctx, AV_LOG_ERROR, "PB frame mode no supported\n");
> > > -        return -1;      /* PB frame mode */
> > > +    s->pb_frame = get_bits1(&s->gb);
> > > +    s->impr_pb_frame = 0;
> > > +
> > > +    if(format == 7){
> > > +        format = get_bits(&s->gb, 3);
> > > +        if(format == 0 || format == 7){
> > > +            av_log(s->avctx, AV_LOG_ERROR, "Wrong Intel H263 format\n");
> > > +            return -1;        
> > > +        }
> > > +        skip_bits(&s->gb, 2);
> > > +        s->loop_filter = get_bits1(&s->gb);
> > > +        skip_bits(&s->gb, 1);
> > > +        s->impr_pb_frame = get_bits1(&s->gb);
> > > +        skip_bits(&s->gb, 10);
> > > +    }else{
> > > +        s->loop_filter = 0;
> > 
> > do you have a file that falls in this else category?
> > it at least has to be a seperate patch as its not related to pb
> > frames rather an extension of the fixed skip_bits 41
> 
> separated
> Since i263 are the only codec known (to me) to contain PB-frames, it's the
> only way to test code.

huh? ITU h263+ allows PB frames, and id be surprised if there are no reference
streams around ...


[...]
> Index: libavcodec/h263.c
> ===================================================================
> --- libavcodec/h263.c	(revision 16921)
> +++ libavcodec/h263.c	(working copy)
> @@ -3886,12 +3886,26 @@
>      ff_set_qscale(s, s->qscale);
>  }
>  
> +static int h263_skip_b_part(MpegEncContext *s, int cbp)
> +{
> +    DECLARE_ALIGNED(16, DCTELEM, dblock[64]);
> +    int i;
> +
> +    for (i = 0; i < 6; i++) {
> +        if (h263_decode_block(s, dblock, i, cbp&32) < 0)
> +            return -1;
> +        cbp+=cbp;
> +    }
> +    return 0;
> +}
> +
>  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 +3938,15 @@
>          s->mb_intra = ((cbpc & 4) != 0);
>          if (s->mb_intra) goto intra;
>  
> +        if(s->pb_frame){
> +            if(get_bits1(&s->gb)){
> +                cbpb = get_bits1(&s->gb);
> +                if(cbpb && s->pb_frame == 2) pb_mv_count = !get_bits1(&s->gb);
> +                else                         pb_mv_count = 1;
> +            }

> +            if(cbpb)
> +                cbpb = get_bits(&s->gb, 6);

that can be moved in the if() above


> +        }
>          cbpy = get_vlc2(&s->gb, cbpy_vlc.table, CBPY_VLC_BITS, 1);
>  
>          if(s->alt_inter_vlc==0 || (cbpc & 3)!=3)
> @@ -3987,12 +4010,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)
> @@ -4118,6 +4148,15 @@
>          }else
>              s->ac_pred = 0;
>  
> +        if(s->pb_frame){
> +            if(get_bits1(&s->gb)){
> +                cbpb = get_bits1(&s->gb);
> +                if(cbpb && s->pb_frame == 2) pb_mv_count = !get_bits1(&s->gb);
> +                else                         pb_mv_count = 1;
> +            }

> +            if(cbpb)
> +                cbpb = get_bits(&s->gb, 6);

same


> +        }
>          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,12 +4167,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;
> +        }

the code above that alraedy was there also can be factored out
also s->mb_intra = 0; can always be done before and the one afterwards
is just restoring the overwritten mode i suspect


>      }
>  end:
>  
> Index: libavcodec/mpegvideo.h
> ===================================================================
> --- libavcodec/mpegvideo.h	(revision 16921)
> +++ libavcodec/mpegvideo.h	(working copy)
> @@ -201,6 +201,7 @@
>      int bit_rate;     ///< wanted bit rate
>      enum OutputFormat out_format; ///< output format
>      int h263_pred;    ///< use mpeg4/h263 ac/dc predictions
> +    int pb_frame;     ///< PB frame mode (0 = none, 1 = base, 2 = improved)
>  
>  /* the following codec id fields are deprecated in favor of codec_id */
>      int h263_plus;    ///< h263 plus headers

> Index: libavcodec/h263.c
> ===================================================================
> --- libavcodec/h263.c	(revision 16921)
> +++ libavcodec/h263.c	(working copy)
> @@ -6185,17 +6236,32 @@
>          return -1;      /* SAC: off */
>      }
>      s->obmc= get_bits1(&s->gb);
> -    if (get_bits1(&s->gb) != 0) {
> -        av_log(s->avctx, AV_LOG_ERROR, "PB frame mode no supported\n");
> -        return -1;      /* PB frame mode */

> +    s->pb_frame = get_bits1(&s->gb);

i dont think that will compile without the other patch and the other
patch will not work without that
aka poorly split


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- Plato
-------------- 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/20090217/1b88d099/attachment.pgp>



More information about the ffmpeg-devel mailing list