[V2,2/3] lavu/hwcontext_qsv: Add support for pix_fmt RGB32.

Message ID 1521729713-22225-2-git-send-email-zhong.li@intel.com
State New
Headers show
Series
  • [V2,1/3] lavc/qsvdec: set complete_frame flags for progressive picture
Related show

Commit Message

Li, Zhong March 22, 2018, 2:41 p.m.
RGB32 format may be used as overlay with alpha blending.
So add RGB32 format support.

Signed-off-by: ChaoX A Liu <chaox.a.liu@intel.com>
Signed-off-by: Zhong Li <zhong.li@intel.com>
---
 libavutil/hwcontext_qsv.c | 43 +++++++++++++++++++++++++++++++++----------
 1 file changed, 33 insertions(+), 10 deletions(-)

Comments

Li, Zhong March 29, 2018, 2:05 p.m. | #1
Ping?

> -----Original Message-----
> From: Li, Zhong
> Sent: Thursday, March 22, 2018 10:42 PM
> To: libav-devel@libav.org
> Cc: Li, Zhong <zhong.li@intel.com>; Liu, ChaoX A <chaox.a.liu@intel.com>
> Subject: [PATCH V2 2/3] lavu/hwcontext_qsv: Add support for pix_fmt
> RGB32.
> 
> RGB32 format may be used as overlay with alpha blending.
> So add RGB32 format support.
> 
> Signed-off-by: ChaoX A Liu <chaox.a.liu@intel.com>
> Signed-off-by: Zhong Li <zhong.li@intel.com>
> ---
>  libavutil/hwcontext_qsv.c | 43
> +++++++++++++++++++++++++++++++++----------
>  1 file changed, 33 insertions(+), 10 deletions(-)
> 
> diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c index
> 5018a05..0db446b 100644
> --- a/libavutil/hwcontext_qsv.c
> +++ b/libavutil/hwcontext_qsv.c
> @@ -90,6 +90,7 @@ static const struct {
>      uint32_t           fourcc;
>  } supported_pixel_formats[] = {
>      { AV_PIX_FMT_NV12, MFX_FOURCC_NV12 },
> +    { AV_PIX_FMT_RGB32,MFX_FOURCC_RGB4 },
>      { AV_PIX_FMT_P010, MFX_FOURCC_P010 },
>      { AV_PIX_FMT_PAL8, MFX_FOURCC_P8   },
>  };
> @@ -730,6 +731,36 @@ static int
> qsv_transfer_data_child(AVHWFramesContext *ctx, AVFrame *dst,
>      return ret;
>  }
> 
> +static int map_frame_to_surface(const AVFrame *frame,
> mfxFrameSurface1
> +*surface) {
> +    switch (frame->format) {
> +        case AV_PIX_FMT_NV12:
> +            surface->Data.Y  = frame->data[0];
> +            surface->Data.UV = frame->data[1];
> +            break;
> +
> +        case AV_PIX_FMT_YUV420P:
> +            surface->Data.Y = frame->data[0];
> +            surface->Data.U = frame->data[1];
> +            surface->Data.V = frame->data[2];
> +            break;
> +
> +        case AV_PIX_FMT_RGB32:
> +            surface->Data.B = frame->data[0];
> +            surface->Data.G = frame->data[0] + 1;
> +            surface->Data.R = frame->data[0] + 2;
> +            surface->Data.A = frame->data[0] + 3;
> +            break;
> +
> +        default:
> +            return MFX_ERR_UNSUPPORTED;
> +    }
> +    surface->Data.Pitch     = frame->linesize[0];
> +    surface->Data.TimeStamp = frame->pts;
> +
> +    return 0;
> +}
> +
>  static int qsv_transfer_data_from(AVHWFramesContext *ctx, AVFrame
> *dst,
>                                    const AVFrame *src)  { @@
> -749,11 +780,7 @@ static int qsv_transfer_data_from(AVHWFramesContext
> *ctx, AVFrame *dst,
>      }
> 
>      out.Info = in->Info;
> -    out.Data.PitchLow = dst->linesize[0];
> -    out.Data.Y        = dst->data[0];
> -    out.Data.U        = dst->data[1];
> -    out.Data.V        = dst->data[2];
> -    out.Data.A        = dst->data[3];
> +    map_frame_to_surface(dst, &out);
> 
>      do {
>          err = MFXVideoVPP_RunFrameVPPAsync(s->session_download, in,
> &out, NULL, &sync); @@ -796,11 +823,7 @@ static int
> qsv_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
>      }
> 
>      in.Info = out->Info;
> -    in.Data.PitchLow = src->linesize[0];
> -    in.Data.Y        = src->data[0];
> -    in.Data.U        = src->data[1];
> -    in.Data.V        = src->data[2];
> -    in.Data.A        = src->data[3];
> +    map_frame_to_surface(src, &in);
> 
>      do {
>          err = MFXVideoVPP_RunFrameVPPAsync(s->session_upload, &in,
> out, NULL, &sync);
> --
> 1.8.3.1
Maxym Dmytrychenko March 29, 2018, 6:44 p.m. | #2
should be ok,

will take care of it shotly

On Thu, Mar 29, 2018 at 4:05 PM, Li, Zhong <zhong.li@intel.com> wrote:

> Ping?
>
> > -----Original Message-----
> > From: Li, Zhong
> > Sent: Thursday, March 22, 2018 10:42 PM
> > To: libav-devel@libav.org
> > Cc: Li, Zhong <zhong.li@intel.com>; Liu, ChaoX A <chaox.a.liu@intel.com>
> > Subject: [PATCH V2 2/3] lavu/hwcontext_qsv: Add support for pix_fmt
> > RGB32.
> >
> > RGB32 format may be used as overlay with alpha blending.
> > So add RGB32 format support.
> >
> > Signed-off-by: ChaoX A Liu <chaox.a.liu@intel.com>
> > Signed-off-by: Zhong Li <zhong.li@intel.com>
> > ---
> >  libavutil/hwcontext_qsv.c | 43
> > +++++++++++++++++++++++++++++++++----------
> >  1 file changed, 33 insertions(+), 10 deletions(-)
> >
> > diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c index
> > 5018a05..0db446b 100644
> > --- a/libavutil/hwcontext_qsv.c
> > +++ b/libavutil/hwcontext_qsv.c
> > @@ -90,6 +90,7 @@ static const struct {
> >      uint32_t           fourcc;
> >  } supported_pixel_formats[] = {
> >      { AV_PIX_FMT_NV12, MFX_FOURCC_NV12 },
> > +    { AV_PIX_FMT_RGB32,MFX_FOURCC_RGB4 },
> >      { AV_PIX_FMT_P010, MFX_FOURCC_P010 },
> >      { AV_PIX_FMT_PAL8, MFX_FOURCC_P8   },
> >  };
> > @@ -730,6 +731,36 @@ static int
> > qsv_transfer_data_child(AVHWFramesContext *ctx, AVFrame *dst,
> >      return ret;
> >  }
> >
> > +static int map_frame_to_surface(const AVFrame *frame,
> > mfxFrameSurface1
> > +*surface) {
> > +    switch (frame->format) {
> > +        case AV_PIX_FMT_NV12:
> > +            surface->Data.Y  = frame->data[0];
> > +            surface->Data.UV = frame->data[1];
> > +            break;
> > +
> > +        case AV_PIX_FMT_YUV420P:
> > +            surface->Data.Y = frame->data[0];
> > +            surface->Data.U = frame->data[1];
> > +            surface->Data.V = frame->data[2];
> > +            break;
> > +
> > +        case AV_PIX_FMT_RGB32:
> > +            surface->Data.B = frame->data[0];
> > +            surface->Data.G = frame->data[0] + 1;
> > +            surface->Data.R = frame->data[0] + 2;
> > +            surface->Data.A = frame->data[0] + 3;
> > +            break;
> > +
> > +        default:
> > +            return MFX_ERR_UNSUPPORTED;
> > +    }
> > +    surface->Data.Pitch     = frame->linesize[0];
> > +    surface->Data.TimeStamp = frame->pts;
> > +
> > +    return 0;
> > +}
> > +
> >  static int qsv_transfer_data_from(AVHWFramesContext *ctx, AVFrame
> > *dst,
> >                                    const AVFrame *src)  { @@
> > -749,11 +780,7 @@ static int qsv_transfer_data_from(AVHWFramesContext
> > *ctx, AVFrame *dst,
> >      }
> >
> >      out.Info = in->Info;
> > -    out.Data.PitchLow = dst->linesize[0];
> > -    out.Data.Y        = dst->data[0];
> > -    out.Data.U        = dst->data[1];
> > -    out.Data.V        = dst->data[2];
> > -    out.Data.A        = dst->data[3];
> > +    map_frame_to_surface(dst, &out);
> >
> >      do {
> >          err = MFXVideoVPP_RunFrameVPPAsync(s->session_download, in,
> > &out, NULL, &sync); @@ -796,11 +823,7 @@ static int
> > qsv_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
> >      }
> >
> >      in.Info = out->Info;
> > -    in.Data.PitchLow = src->linesize[0];
> > -    in.Data.Y        = src->data[0];
> > -    in.Data.U        = src->data[1];
> > -    in.Data.V        = src->data[2];
> > -    in.Data.A        = src->data[3];
> > +    map_frame_to_surface(src, &in);
> >
> >      do {
> >          err = MFXVideoVPP_RunFrameVPPAsync(s->session_upload, &in,
> > out, NULL, &sync);
> > --
> > 1.8.3.1
>
> _______________________________________________
> libav-devel mailing list
> libav-devel@libav.org
> https://lists.libav.org/mailman/listinfo/libav-devel
Mark Thompson March 29, 2018, 7:20 p.m. | #3
On 22/03/18 14:41, Zhong Li wrote:
> RGB32 format may be used as overlay with alpha blending.
> So add RGB32 format support.
> 
> Signed-off-by: ChaoX A Liu <chaox.a.liu@intel.com>
> Signed-off-by: Zhong Li <zhong.li@intel.com>
> ---
>  libavutil/hwcontext_qsv.c | 43 +++++++++++++++++++++++++++++++++----------
>  1 file changed, 33 insertions(+), 10 deletions(-)

Please write BGRA rather than RGB32.  I doubt this will ever run on a big-endian machine, but it would be clearer.

> diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
> index 5018a05..0db446b 100644
> --- a/libavutil/hwcontext_qsv.c
> +++ b/libavutil/hwcontext_qsv.c
> @@ -90,6 +90,7 @@ static const struct {
>      uint32_t           fourcc;
>  } supported_pixel_formats[] = {
>      { AV_PIX_FMT_NV12, MFX_FOURCC_NV12 },
> +    { AV_PIX_FMT_RGB32,MFX_FOURCC_RGB4 },
>      { AV_PIX_FMT_P010, MFX_FOURCC_P010 },
>      { AV_PIX_FMT_PAL8, MFX_FOURCC_P8   },
>  };
> @@ -730,6 +731,36 @@ static int qsv_transfer_data_child(AVHWFramesContext *ctx, AVFrame *dst,
>      return ret;
>  }
>  
> +static int map_frame_to_surface(const AVFrame *frame, mfxFrameSurface1 *surface)
> +{
> +    switch (frame->format) {
> +        case AV_PIX_FMT_NV12:

Indentation - case labels should be at the same level as switch.

> +            surface->Data.Y  = frame->data[0];
> +            surface->Data.UV = frame->data[1];
> +            break;
> +
> +        case AV_PIX_FMT_YUV420P:
> +            surface->Data.Y = frame->data[0];
> +            surface->Data.U = frame->data[1];
> +            surface->Data.V = frame->data[2];
> +            break;
> +
> +        case AV_PIX_FMT_RGB32:
> +            surface->Data.B = frame->data[0];
> +            surface->Data.G = frame->data[0] + 1;
> +            surface->Data.R = frame->data[0] + 2;
> +            surface->Data.A = frame->data[0] + 3;
> +            break;
> +
> +        default:
> +            return MFX_ERR_UNSUPPORTED;
> +    }
> +    surface->Data.Pitch     = frame->linesize[0];

What happens if linesize[0] != linesize[1]?  (You aren't introducing that problem, but I hadn't seen it before.)

> +    surface->Data.TimeStamp = frame->pts;
> +
> +    return 0;
> +}
> +
>  static int qsv_transfer_data_from(AVHWFramesContext *ctx, AVFrame *dst,
>                                    const AVFrame *src)
>  {
> @@ -749,11 +780,7 @@ static int qsv_transfer_data_from(AVHWFramesContext *ctx, AVFrame *dst,
>      }
>  
>      out.Info = in->Info;
> -    out.Data.PitchLow = dst->linesize[0];
> -    out.Data.Y        = dst->data[0];
> -    out.Data.U        = dst->data[1];
> -    out.Data.V        = dst->data[2];
> -    out.Data.A        = dst->data[3];
> +    map_frame_to_surface(dst, &out);
>  
>      do {
>          err = MFXVideoVPP_RunFrameVPPAsync(s->session_download, in, &out, NULL, &sync);
> @@ -796,11 +823,7 @@ static int qsv_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
>      }
>  
>      in.Info = out->Info;
> -    in.Data.PitchLow = src->linesize[0];
> -    in.Data.Y        = src->data[0];
> -    in.Data.U        = src->data[1];
> -    in.Data.V        = src->data[2];
> -    in.Data.A        = src->data[3];
> +    map_frame_to_surface(src, &in);
>  
>      do {
>          err = MFXVideoVPP_RunFrameVPPAsync(s->session_upload, &in, out, NULL, &sync);
> 

This has slightly changed what gets passed for YUV420P and NV12 - it no longer sets the unused pointers to NULL.  Presumably that's always ok, even with old versions?

Thanks,

- Mark
Li, Zhong April 2, 2018, 9:32 a.m. | #4
> Please write BGRA rather than RGB32.  I doubt this will ever run on a
> big-endian machine, but it would be clearer.

Agree, will update.

> > diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
> > index 5018a05..0db446b 100644
> > --- a/libavutil/hwcontext_qsv.c
> > +++ b/libavutil/hwcontext_qsv.c
> > @@ -90,6 +90,7 @@ static const struct {
> >      uint32_t           fourcc;
> >  } supported_pixel_formats[] = {
> >      { AV_PIX_FMT_NV12, MFX_FOURCC_NV12 },
> > +    { AV_PIX_FMT_RGB32,MFX_FOURCC_RGB4 },
> >      { AV_PIX_FMT_P010, MFX_FOURCC_P010 },
> >      { AV_PIX_FMT_PAL8, MFX_FOURCC_P8   },
> >  };
> > @@ -730,6 +731,36 @@ static int
> qsv_transfer_data_child(AVHWFramesContext *ctx, AVFrame *dst,
> >      return ret;
> >  }
> >
> > +static int map_frame_to_surface(const AVFrame *frame,
> > +mfxFrameSurface1 *surface) {
> > +    switch (frame->format) {
> > +        case AV_PIX_FMT_NV12:
> 
> Indentation - case labels should be at the same level as switch.

Sure, thanks for remind. Will update.
> 
> > +            surface->Data.Y  = frame->data[0];
> > +            surface->Data.UV = frame->data[1];
> > +            break;
> > +
> > +        case AV_PIX_FMT_YUV420P:
> > +            surface->Data.Y = frame->data[0];
> > +            surface->Data.U = frame->data[1];
> > +            surface->Data.V = frame->data[2];
> > +            break;
> > +
> > +        case AV_PIX_FMT_RGB32:
> > +            surface->Data.B = frame->data[0];
> > +            surface->Data.G = frame->data[0] + 1;
> > +            surface->Data.R = frame->data[0] + 2;
> > +            surface->Data.A = frame->data[0] + 3;
> > +            break;
> > +
> > +        default:
> > +            return MFX_ERR_UNSUPPORTED;
> > +    }
> > +    surface->Data.Pitch     = frame->linesize[0];
> 
> What happens if linesize[0] != linesize[1]?  (You aren't introducing that
> problem, but I hadn't seen it before.)

I don't think MSDK can handle this case perfectly since there is only one pitch.
Take YUV420p as example, IMHO it is required linesize of Y must be twice of U and V.

> 
> > +    surface->Data.TimeStamp = frame->pts;
> > +
> > +    return 0;
> > +}
> > +
> >  static int qsv_transfer_data_from(AVHWFramesContext *ctx, AVFrame
> *dst,
> >                                    const AVFrame *src)  { @@
> -749,11
> > +780,7 @@ static int qsv_transfer_data_from(AVHWFramesContext *ctx,
> AVFrame *dst,
> >      }
> >
> >      out.Info = in->Info;
> > -    out.Data.PitchLow = dst->linesize[0];
> > -    out.Data.Y        = dst->data[0];
> > -    out.Data.U        = dst->data[1];
> > -    out.Data.V        = dst->data[2];
> > -    out.Data.A        = dst->data[3];
> > +    map_frame_to_surface(dst, &out);
> >
> >      do {
> >          err = MFXVideoVPP_RunFrameVPPAsync(s->session_download,
> in,
> > &out, NULL, &sync); @@ -796,11 +823,7 @@ static int
> qsv_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
> >      }
> >
> >      in.Info = out->Info;
> > -    in.Data.PitchLow = src->linesize[0];
> > -    in.Data.Y        = src->data[0];
> > -    in.Data.U        = src->data[1];
> > -    in.Data.V        = src->data[2];
> > -    in.Data.A        = src->data[3];
> > +    map_frame_to_surface(src, &in);
> >
> >      do {
> >          err = MFXVideoVPP_RunFrameVPPAsync(s->session_upload,
> &in,
> > out, NULL, &sync);
> >
> 
> This has slightly changed what gets passed for YUV420P and NV12 - it no
> longer sets the unused pointers to NULL.  Presumably that's always ok,
> even with old versions?

Yes, it is. But as you can see there were still other unused pointers hadn't been set to NULL before this patch. 
Since it is UNUSED, IMHO it is not necessary to memset them.

> Thanks,
> 
> - Mark
Mark Thompson April 2, 2018, 12:45 p.m. | #5
On 02/04/18 10:32, Li, Zhong wrote:
>>> diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
>>> index 5018a05..0db446b 100644
>>> --- a/libavutil/hwcontext_qsv.c
>>> +++ b/libavutil/hwcontext_qsv.c
>>> ...
>>> +            surface->Data.Y  = frame->data[0];
>>> +            surface->Data.UV = frame->data[1];
>>> +            break;
>>> +
>>> +        case AV_PIX_FMT_YUV420P:
>>> +            surface->Data.Y = frame->data[0];
>>> +            surface->Data.U = frame->data[1];
>>> +            surface->Data.V = frame->data[2];
>>> +            break;
>>> +
>>> +        case AV_PIX_FMT_RGB32:
>>> +            surface->Data.B = frame->data[0];
>>> +            surface->Data.G = frame->data[0] + 1;
>>> +            surface->Data.R = frame->data[0] + 2;
>>> +            surface->Data.A = frame->data[0] + 3;
>>> +            break;
>>> +
>>> +        default:
>>> +            return MFX_ERR_UNSUPPORTED;
>>> +    }
>>> +    surface->Data.Pitch     = frame->linesize[0];
>>
>> What happens if linesize[0] != linesize[1]?  (You aren't introducing that
>> problem, but I hadn't seen it before.)
> 
> I don't think MSDK can handle this case perfectly since there is only one pitch.
> Take YUV420p as example, IMHO it is required linesize of Y must be twice of U and V.

That isn't going to be true for a general frame in libav - the pitches for each plane are independent.  Since they are usually created by taking the width of the plane and rounding up to the appropriate alignment (usually 32, I think) it will work for widths which are multiples of large powers of two - e.g. 1920 width will work because both 1920 and 960 are already aligned to a 32-byte boundary.  It won't work for less aligned widths (e.g. 720 width from NTSC or PAL will give luma pitch = align(720, 32) = 736 but chroma pitch = align(360, 32) = 384), nor will it work for other ways of laying out the frame such as line-interleaving.

This problem was preexisting, though, so I guess it isn't necessary to deal with it in this patch.  Not sure what the right answer is - maybe it could just reject non-matching pitches and return an error?  Or it could make a temporary frame with the stricter alignment and copy to that before uploading?  (Though that might be slow and defeat the point of this upload path.)

- Mark
Maxym Dmytrychenko April 2, 2018, 1:06 p.m. | #6
how about second option :  temporary frame with the stricter alignment and
copy to that before uploading
but with log/INFO message included ?

On Mon, Apr 2, 2018 at 2:45 PM, Mark Thompson <sw@jkqxz.net> wrote:

> On 02/04/18 10:32, Li, Zhong wrote:
> >>> diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
> >>> index 5018a05..0db446b 100644
> >>> --- a/libavutil/hwcontext_qsv.c
> >>> +++ b/libavutil/hwcontext_qsv.c
> >>> ...
> >>> +            surface->Data.Y  = frame->data[0];
> >>> +            surface->Data.UV = frame->data[1];
> >>> +            break;
> >>> +
> >>> +        case AV_PIX_FMT_YUV420P:
> >>> +            surface->Data.Y = frame->data[0];
> >>> +            surface->Data.U = frame->data[1];
> >>> +            surface->Data.V = frame->data[2];
> >>> +            break;
> >>> +
> >>> +        case AV_PIX_FMT_RGB32:
> >>> +            surface->Data.B = frame->data[0];
> >>> +            surface->Data.G = frame->data[0] + 1;
> >>> +            surface->Data.R = frame->data[0] + 2;
> >>> +            surface->Data.A = frame->data[0] + 3;
> >>> +            break;
> >>> +
> >>> +        default:
> >>> +            return MFX_ERR_UNSUPPORTED;
> >>> +    }
> >>> +    surface->Data.Pitch     = frame->linesize[0];
> >>
> >> What happens if linesize[0] != linesize[1]?  (You aren't introducing
> that
> >> problem, but I hadn't seen it before.)
> >
> > I don't think MSDK can handle this case perfectly since there is only
> one pitch.
> > Take YUV420p as example, IMHO it is required linesize of Y must be twice
> of U and V.
>
> That isn't going to be true for a general frame in libav - the pitches for
> each plane are independent.  Since they are usually created by taking the
> width of the plane and rounding up to the appropriate alignment (usually
> 32, I think) it will work for widths which are multiples of large powers of
> two - e.g. 1920 width will work because both 1920 and 960 are already
> aligned to a 32-byte boundary.  It won't work for less aligned widths (e.g.
> 720 width from NTSC or PAL will give luma pitch = align(720, 32) = 736 but
> chroma pitch = align(360, 32) = 384), nor will it work for other ways of
> laying out the frame such as line-interleaving.
>
> This problem was preexisting, though, so I guess it isn't necessary to
> deal with it in this patch.  Not sure what the right answer is - maybe it
> could just reject non-matching pitches and return an error?  Or it could
> make a temporary frame with the stricter alignment and copy to that before
> uploading?  (Though that might be slow and defeat the point of this upload
> path.)
>
> - Mark
> _______________________________________________
> libav-devel mailing list
> libav-devel@libav.org
> https://lists.libav.org/mailman/listinfo/libav-devel
Li, Zhong April 3, 2018, 7:51 a.m. | #7
> From: libav-devel [mailto:libav-devel-bounces@libav.org] On Behalf Of Mark
> Thompson
> Sent: Monday, April 2, 2018 8:45 PM
> To: libav-devel@libav.org
> Subject: Re: [libav-devel] [PATCH V2 2/3] lavu/hwcontext_qsv: Add support
> for pix_fmt RGB32.
> 
> On 02/04/18 10:32, Li, Zhong wrote:
> >>> diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
> >>> index 5018a05..0db446b 100644
> >>> --- a/libavutil/hwcontext_qsv.c
> >>> +++ b/libavutil/hwcontext_qsv.c
> >>> ...
> >>> +            surface->Data.Y  = frame->data[0];
> >>> +            surface->Data.UV = frame->data[1];
> >>> +            break;
> >>> +
> >>> +        case AV_PIX_FMT_YUV420P:
> >>> +            surface->Data.Y = frame->data[0];
> >>> +            surface->Data.U = frame->data[1];
> >>> +            surface->Data.V = frame->data[2];
> >>> +            break;
> >>> +
> >>> +        case AV_PIX_FMT_RGB32:
> >>> +            surface->Data.B = frame->data[0];
> >>> +            surface->Data.G = frame->data[0] + 1;
> >>> +            surface->Data.R = frame->data[0] + 2;
> >>> +            surface->Data.A = frame->data[0] + 3;
> >>> +            break;
> >>> +
> >>> +        default:
> >>> +            return MFX_ERR_UNSUPPORTED;
> >>> +    }
> >>> +    surface->Data.Pitch     = frame->linesize[0];
> >>
> >> What happens if linesize[0] != linesize[1]?  (You aren't introducing
> >> that problem, but I hadn't seen it before.)
> >
> > I don't think MSDK can handle this case perfectly since there is only one
> pitch.
> > Take YUV420p as example, IMHO it is required linesize of Y must be twice
> of U and V.
> 
> That isn't going to be true for a general frame in libav - the pitches for each
> plane are independent.  Since they are usually created by taking the width
> of the plane and rounding up to the appropriate alignment (usually 32, I think)
> it will work for widths which are multiples of large powers of two - e.g. 1920
> width will work because both 1920 and 960 are already aligned to a 32-byte
> boundary.  It won't work for less aligned widths (e.g. 720 width from NTSC
> or PAL will give luma pitch = align(720, 32) = 736 but chroma pitch = align(360,
> 32) = 384), nor will it work for other ways of laying out the frame such as
> line-interleaving.

Perfectly correct. Actually I've seen a bug when transcoding a 720x480 clips.
In that case, must 64 bytes aligned to make sure luma_pitch = 2 x chroma_pitch.

> This problem was preexisting, though, so I guess it isn't necessary to deal
> with it in this patch.  Not sure what the right answer is - maybe it could just
> reject non-matching pitches and return an error?  Or it could make a
> temporary frame with the stricter alignment and copy to that before
> uploading?  (Though that might be slow and defeat the point of this upload
> path.)

"Return an error" is not user-friendly because they can do nothing after receive such an error message.
Agree with Maxym, I prefer the second option too and there are existing piece of code to handle the cases haven't aligned as libmfx's requirement (though haven't handle the 720x480 yuv420p issue).
But I'm not sure why it will "defeat the point of this upload path". Could you help to give some detail info?

Patch

diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
index 5018a05..0db446b 100644
--- a/libavutil/hwcontext_qsv.c
+++ b/libavutil/hwcontext_qsv.c
@@ -90,6 +90,7 @@  static const struct {
     uint32_t           fourcc;
 } supported_pixel_formats[] = {
     { AV_PIX_FMT_NV12, MFX_FOURCC_NV12 },
+    { AV_PIX_FMT_RGB32,MFX_FOURCC_RGB4 },
     { AV_PIX_FMT_P010, MFX_FOURCC_P010 },
     { AV_PIX_FMT_PAL8, MFX_FOURCC_P8   },
 };
@@ -730,6 +731,36 @@  static int qsv_transfer_data_child(AVHWFramesContext *ctx, AVFrame *dst,
     return ret;
 }
 
+static int map_frame_to_surface(const AVFrame *frame, mfxFrameSurface1 *surface)
+{
+    switch (frame->format) {
+        case AV_PIX_FMT_NV12:
+            surface->Data.Y  = frame->data[0];
+            surface->Data.UV = frame->data[1];
+            break;
+
+        case AV_PIX_FMT_YUV420P:
+            surface->Data.Y = frame->data[0];
+            surface->Data.U = frame->data[1];
+            surface->Data.V = frame->data[2];
+            break;
+
+        case AV_PIX_FMT_RGB32:
+            surface->Data.B = frame->data[0];
+            surface->Data.G = frame->data[0] + 1;
+            surface->Data.R = frame->data[0] + 2;
+            surface->Data.A = frame->data[0] + 3;
+            break;
+
+        default:
+            return MFX_ERR_UNSUPPORTED;
+    }
+    surface->Data.Pitch     = frame->linesize[0];
+    surface->Data.TimeStamp = frame->pts;
+
+    return 0;
+}
+
 static int qsv_transfer_data_from(AVHWFramesContext *ctx, AVFrame *dst,
                                   const AVFrame *src)
 {
@@ -749,11 +780,7 @@  static int qsv_transfer_data_from(AVHWFramesContext *ctx, AVFrame *dst,
     }
 
     out.Info = in->Info;
-    out.Data.PitchLow = dst->linesize[0];
-    out.Data.Y        = dst->data[0];
-    out.Data.U        = dst->data[1];
-    out.Data.V        = dst->data[2];
-    out.Data.A        = dst->data[3];
+    map_frame_to_surface(dst, &out);
 
     do {
         err = MFXVideoVPP_RunFrameVPPAsync(s->session_download, in, &out, NULL, &sync);
@@ -796,11 +823,7 @@  static int qsv_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
     }
 
     in.Info = out->Info;
-    in.Data.PitchLow = src->linesize[0];
-    in.Data.Y        = src->data[0];
-    in.Data.U        = src->data[1];
-    in.Data.V        = src->data[2];
-    in.Data.A        = src->data[3];
+    map_frame_to_surface(src, &in);
 
     do {
         err = MFXVideoVPP_RunFrameVPPAsync(s->session_upload, &in, out, NULL, &sync);