[Ffmpeg-devel] [PATCH] VC1 decoding of Advanced Profile's b-frames

[Kostya]

On Thu, Nov 23, 2006 at 07:37:20PM +0100, skal wrote:
>   Hi everybody,
> 
>   vc1.c was just few typo away from being able to decode
>   b-frames in Lets-Fix-The-Syntax-Again-Advanced Profile.
>   Patch attached, which should hopefully be ok.
> 
>   I also included some small unrelated typo fixes (namely:
>   " edges = (3 << v->dqsbedge) & 15" , "q1 = q1 * 2 + v->halfpq - 1",
>   "q2 = q2 * 2 + v->halfpq - 1". Don't bug me because it's in
>    the same patch, i just won't have time to work on that further
>    so take it, leave it or fix it :)
> 
>    Sample files to try this patch on:
> http://3dtv.at/Movies/Katana576p.zip
>    should do
> 
>    bye!
> 
> Skal
> 
> PS: Konstantin, i also found another problem in overlap smoothing:
> the rounding constant (4/3) should alternate on a per-pixel
> basis, not on a per-8x8block one. (cf. vc1smooth.c:118 e.g)
Right, I'll work with it tomorrow.

And here is patch review
> Index: vc1.c
> ===================================================================
> --- vc1.c	(revision 7159)
> +++ vc1.c	(working copy)
> @@ -1591,8 +1591,7 @@
>          break;
>      case 1:
>          v->s.pict_type = B_TYPE;
> -        return -1;
> -//      break;
> +        break;
>      case 2:
>          v->s.pict_type = I_TYPE;
>          break;
[...]

If you just added another case in switch it would be easier to read and
understand. At least it works.

> @@ -1801,7 +1826,7 @@
>      if(v->dqprofile == DQPROFILE_SINGLE_EDGE)                  \
>          edges = 1 << v->dqsbedge;                              \
>      else if(v->dqprofile == DQPROFILE_DOUBLE_EDGES)            \
> -        edges = (3 << v->dqsbedge) % 15;                       \
> +        edges = (3 << v->dqsbedge) & 15;                       \
>      else if(v->dqprofile == DQPROFILE_FOUR_EDGES)              \
>          edges = 15;                                            \
>      if((edges&1) && !s->mb_x)                                  \

This is wrong. The purpose of this code is cyclic rotation of
0011b, so modulo division is needed.

> @@ -2843,8 +2868,8 @@
>          if(use_pred) {
>              /* scale predictors if needed*/
>              if(q2 && q1!=q2) {
> -                q1 = q1 * 2 - 1;
> -                q2 = q2 * 2 - 1;
> +                q1 = q1 * 2 + v->halfpq - 1;
> +                q2 = q2 * 2 + v->halfpq - 1;
>  
>                  if(dc_pred_dir) { //left
>                      for(k = 1; k < 8; k++)

It should be in all places but that's my work after all.