[FFmpeg-devel] [RFC]Nvidia VDPAU patches

[Kostya]

On Sun, Nov 16, 2008 at 01:14:43AM +0100, Carl Eugen Hoyos wrote:
> Hi!
> 
> In case Michael wants to review them, here are Nvidias patches for ffmpeg 
> for hardware accelerated decoding.
> 
> There seem to be one or two unrelated "fixes" (?) for vc1 in this patch, 
> perhaps Kostya can comment.
> 
> Carl Eugen

Nobody wants to use a shovel to dig out codec-specific changes but just in case
here are my comments on it.

> Index: libavcodec/vc1.c
> ===================================================================
> --- libavcodec/vc1.c	(revision 14529)
> +++ libavcodec/vc1.c	(working copy)
> @@ -1005,8 +1042,24 @@
>      if(get_bits1(gb)) { //Display Info - decoding is not affected by it
>          int w, h, ar = 0;
>          av_log(v->s.avctx, AV_LOG_DEBUG, "Display extended info:\n");
> -        v->s.avctx->width  = v->s.width  = w = get_bits(gb, 14) + 1;
> -        v->s.avctx->height = v->s.height = h = get_bits(gb, 14) + 1;
> +        // FIXME: The w/h parsed here are the *display* width/height, not the
> +        // coded width/height. Ideally, we should make the commented
> +        // assignments below, but that causes problems:
> +        // * The SW decoder in this file experiences errors, because it
> +        //   assumes these assigned values are the coded size:
> +        //   [vc1 @ 0x86f2130]concealing 150 DC, 150 AC, 150 MV errors
> +        // * VDPAU also assumes these are the coded size, since this is the
> +        //   only size passed to vo_vdpau.c:config(). This causes errors
> +        //   during the decode process.
> +        // However, simply removing these assignments is not the complete fix,
> +        // because without them, the stream is displayed at its coded size,
> +        // not this requested display size. Ideally, setting:
> +        // sample_aspect_ratio = (AVRational){w, h}
> +        // in the case when ar is not present/set would persuade other modules
> +        // to scale to this requested size. However, sample_aspect_ratio
> +        // appears to be completely ignored elsewhere.
> +        /*v->s.avctx->width  = v->s.width  =*/ w = get_bits(gb, 14) + 1;
> +        /*v->s.avctx->height = v->s.height =*/ h = get_bits(gb, 14) + 1;
>          av_log(v->s.avctx, AV_LOG_DEBUG, "Display dimensions: %ix%i\n", w, h);
>          if(get_bits1(gb))
>              ar = get_bits(gb, 4);

That's at least partially wrong. Sample is welcome.

> @@ -1057,13 +1110,13 @@
>  static int decode_entry_point(AVCodecContext *avctx, GetBitContext *gb)
>  {
>      VC1Context *v = avctx->priv_data;
> -    int i, blink, clentry, refdist;
> +    int i, blink, clentry;
>  
>      av_log(avctx, AV_LOG_DEBUG, "Entry point: %08X\n", show_bits_long(gb, 32));
>      blink = get_bits1(gb); // broken link
>      clentry = get_bits1(gb); // closed entry
>      v->panscanflag = get_bits1(gb);
> -    refdist = get_bits1(gb); // refdist flag
> +    v->refdist_flag = get_bits1(gb);
>      v->s.loop_filter = get_bits1(gb);
>      v->fastuvmc = get_bits1(gb);
>      v->extended_mv = get_bits1(gb);
> @@ -1084,20 +1137,22 @@
>      }
>      if(v->extended_mv)
>          v->extended_dmv = get_bits1(gb);
> -    if(get_bits1(gb)) {
> +    v->range_mapy_flag = get_bits1(gb);
> +    if(v->range_mapy_flag) {
>          av_log(avctx, AV_LOG_ERROR, "Luma scaling is not supported, expect wrong picture\n");
> -        skip_bits(gb, 3); // Y range, ignored for now
> +        v->range_mapy = get_bits(gb, 3);
>      }
> -    if(get_bits1(gb)) {
> +    v->range_mapuv_flag = get_bits1(gb);
> +    if(v->range_mapuv_flag) {
>          av_log(avctx, AV_LOG_ERROR, "Chroma scaling is not supported, expect wrong picture\n");
> -        skip_bits(gb, 3); // UV range, ignored for now
> +        v->range_mapuv = get_bits(gb, 3);
>      }
>  
>      av_log(avctx, AV_LOG_DEBUG, "Entry point info:\n"
>          "BrokenLink=%i, ClosedEntry=%i, PanscanFlag=%i\n"
>          "RefDist=%i, Postproc=%i, FastUVMC=%i, ExtMV=%i\n"
>          "DQuant=%i, VSTransform=%i, Overlap=%i, Qmode=%i\n",
> -        blink, clentry, v->panscanflag, refdist, v->s.loop_filter,
> +        blink, clentry, v->panscanflag, v->refdist_flag, v->s.loop_filter,
>          v->fastuvmc, v->extended_mv, v->dquant, v->vstransform, v->overlap, v->quantizer_mode);

Those are pretty harmless and useless changes
  
>      return 0;
> @@ -1395,6 +1450,9 @@
>  
>      if(v->s.pict_type == FF_I_TYPE || v->s.pict_type == FF_P_TYPE) v->use_ic = 0;
>  
> +    if(v->postprocflag)
> +        v->postproc = get_bits(gb, 2);
> +
>      switch(v->s.pict_type) {
>      case FF_I_TYPE:
>      case FF_BI_TYPE:
> @@ -1414,8 +1472,6 @@
>          }
>          break;
>      case FF_P_TYPE:
> -        if(v->postprocflag)
> -            v->postproc = get_bits1(gb);
>          if (v->extended_mv) v->mvrange = get_unary(gb, 0, 3);
>          else v->mvrange = 0;
>          v->k_x = v->mvrange + 9 + (v->mvrange >> 1); //k_x can be 9 10 12 13
> @@ -1505,8 +1561,6 @@
>          }
>          break;
>      case FF_B_TYPE:
> -        if(v->postprocflag)
> -            v->postproc = get_bits1(gb);
>          if (v->extended_mv) v->mvrange = get_unary(gb, 0, 3);
>          else v->mvrange = 0;
>          v->k_x = v->mvrange + 9 + (v->mvrange >> 1); //k_x can be 9 10 12 13

Hmm, somehow I've managed to miss postproc flag for I/BI-frames. Will be applied.

[...]

The rest is acceleration specific.