[Libva] [PATCH 2/4] add rgbx->nv12 conversion in post-processing

[Xiang, Haihao]

On Fri, 2012-07-13 at 08:20 +0200, Gwenole Beauchesne wrote: 
> Hi,
> 
> 2012/7/13 Zhao Halley <halley.zhao at intel.com>:
> 
> > +    case VA_FOURCC('A','Y','U','V'):
> 
> Drop all references to AYUV, that's not is this patchset. :)
> 
> + Add RGBX/BGRX image formats.

Currently only subpicture supports RGBA/BGRA formats. If you want to add
new image formats RGBX/BGRX, could you also provide shaders for
RGBX/BGRX<-->RGBX/BGRX so that vaGetPutImage()/vaPutImage() can work for
the same formats ? BTW it would be better to add support for new image
formats and new surface formats in another patch.



> > +        case VA_FOURCC('R', 'G', 'B', 'A'):
> > +        case VA_FOURCC('R', 'G', 'B', 'X'):
> > +        case VA_FOURCC('B', 'G', 'R', 'A'):
> > +        case VA_FOURCC('B', 'G', 'R', 'X'):
> > +            obj_surface->y_cb_offset = 0;
> > +            obj_surface->y_cr_offset = 0;
> > +            obj_surface->cb_cr_width = obj_surface->orig_width ;
> > +            obj_surface->cb_cr_height = obj_surface->orig_height;
> > +            obj_surface->cb_cr_pitch = obj_surface->width * 4;
> 
> This doesn't look right.
> 
> > @@ -2542,6 +2591,13 @@ get_sampling_from_fourcc(unsigned int fourcc)
> >      case VA_FOURCC('Y', 'U', 'Y', '2'):
> >          surface_sampling = SUBSAMPLE_YUV422H;
> >          break;
> > +    case VA_FOURCC('A','Y','U','V'):
> > +    case VA_FOURCC('R','G','B','A'):
> > +    case VA_FOURCC('R','G','B','X'):
> > +    case VA_FOURCC('B','G','R','A'):
> > +    case VA_FOURCC('B','G','R','X'):
> > +    surface_sampling = SUBSAMPLE_YUV444;
> > +        break;
> >      default:
> >          break;
> >      }
> 
> Same here.
> 
> > @@ -3642,7 +3698,12 @@ i965_GetSurfaceAttributes(
> >                          if (attrib_list[i].value.value.i != VA_FOURCC('N', 'V', '1', '2') &&
> >                              attrib_list[i].value.value.i != VA_FOURCC('I', '4', '2', '0') &&
> >                              attrib_list[i].value.value.i != VA_FOURCC('Y', 'V', '1', '2') &&
> > -                            attrib_list[i].value.value.i != VA_FOURCC('Y', 'U', 'Y', '2')) {
> > +                            attrib_list[i].value.value.i != VA_FOURCC('Y', 'U', 'Y', '2') &&
> > +                            attrib_list[i].value.value.i != VA_FOURCC('A', 'Y', 'U', 'V') &&
> > +                            attrib_list[i].value.value.i != VA_FOURCC('B', 'G', 'R', 'A') &&
> > +                            attrib_list[i].value.value.i != VA_FOURCC('B', 'G', 'R', 'X') &&
> > +                            attrib_list[i].value.value.i != VA_FOURCC('R', 'G', 'B', 'X') &&
> > +                            attrib_list[i].value.value.i != VA_FOURCC('R', 'G', 'B', 'A')) {
> >                              attrib_list[i].value.value.i = 0;
> >                              attrib_list[i].flags &= ~VA_SURFACE_ATTRIB_SETTABLE;
> >                          }
> 
> You now see why I recommended to use a switch/case in the first place...
> 
> > diff --git a/src/i965_post_processing.h b/src/i965_post_processing.h
> > index 21e525c..64229e8 100755
> > --- a/src/i965_post_processing.h
> > +++ b/src/i965_post_processing.h
> > @@ -51,9 +51,10 @@ enum
> >      PP_PL3_LOAD_SAVE_PA,
> >      PP_PA_LOAD_SAVE_NV12,
> >      PP_PA_LOAD_SAVE_PL3,
> > +    PP_RGBX_LOAD_SAVE_NV12,
> >  };
> >
> > -#define NUM_PP_MODULES                  13
> > +#define NUM_PP_MODULES                  14
> 
> I wonder why NUM_PP_MODULES is not in the enum. That'd prevent you
> from increasing that value all the time. Please produce another patch
> with this change.
> 
> Thanks,
> Gwenole.
> _______________________________________________
> Libva mailing list
> Libva at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/libva