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

[Michael Niedermayer]

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 ...


> 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

> Index: libavcodec/h263.c
> ===================================================================
> --- libavcodec/h263.c	(revision 16921)
> +++ libavcodec/h263.c	(working copy)
> @@ -5047,10 +5111,7 @@
>          s->obmc= get_bits1(&s->gb); /* Advanced prediction mode */
>          s->unrestricted_mv = s->h263_long_vectors || s->obmc;
>  
> -        if (get_bits1(&s->gb) != 0) {
> -            av_log(s->avctx, AV_LOG_ERROR, "H263 PB frame not supported\n");
> -            return -1; /* not PB frame */
> -        }
> +        s->pb_frame = get_bits1(&s->gb);
>          s->chroma_qscale= s->qscale = get_bits(&s->gb, 5);
>          skip_bits1(&s->gb); /* Continuous Presence Multipoint mode: off */
>  
> @@ -5105,6 +5166,7 @@
>          switch(s->pict_type){
>          case 0: s->pict_type= FF_I_TYPE;break;
>          case 1: s->pict_type= FF_P_TYPE;break;
> +        case 2: s->pict_type= FF_P_TYPE;s->pb_frame = 3;break;
>          case 3: s->pict_type= FF_B_TYPE;break;
>          case 7: s->pict_type= FF_I_TYPE;break; //ZYGO
>          default:
> @@ -5194,6 +5256,16 @@
>      s->mb_height = (s->height  + 15) / 16;
>      s->mb_num = s->mb_width * s->mb_height;
>  
> +    if (s->pb_frame) {
> +        skip_bits(&s->gb, 3); /* Temporal reference for B-pictures */
> +        if (s->custom_pcf)
> +            skip_bits(&s->gb, 2); //extended Temporal reference
> +        skip_bits(&s->gb, 2); /* Quantization information for B-pictures */
> +
> +        av_log(s->avctx, AV_LOG_ERROR, "PB frame mode no supported\n");
> +        return -1;      /* PB frame mode */
> +    }
> +
>      /* PEI */
>      while (get_bits1(&s->gb) != 0) {
>          skip_bits(&s->gb, 8);

above (and only above) is ok


[...]
> +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!

[...]

> @@ -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


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

The educated differ from the uneducated as much as the living from the
dead. -- Aristotle 
-------------- 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/20090220/31804176/attachment.pgp>