[FFmpeg-devel] [PATCH 1/2] lavu/hwcontext_qsv: add support for AV_PIX_FMT_VUYX

[Xiang, Haihao]

On Sun, 2022-09-04 at 22:04 -0700, Philip Langdale wrote:
> On Mon, 5 Sep 2022 02:47:44 +0000
> "Xiang, Haihao" <haihao.xiang-at-intel.com at ffmpeg.org> wrote:
> 
> > On Mon, 2022-08-29 at 14:01 +0000, Xiang, Haihao wrote:
> > > On Mon, 2022-08-29 at 08:17 -0300, James Almer wrote:  
> > > > On 8/29/2022 4:27 AM, Xiang, Haihao wrote:  
> > > > > From: Haihao Xiang <haihao.xiang at intel.com>
> > > > > 
> > > > > AV_PIX_FMT_VUYX is used in FFmpeg for 8bit 4:4:4 content on
> > > > > Intel HW, and MFX_FOURCC_AYUV is used in the SDK  
> > > > 
> > > > Sounds like you want the VUYA pixfmt instead.  
> > > 
> > > AV_PIX_FMT_VUYX is used for 4:4:4 content in the VAAPI path,
> > > AV_PIX_FMT_VUYX is
> > > expected when creating vaapi urface. QSV is based on 
> > > VAAPI under Linux, so AV_PIX_FMT_VUYX is expected too in the QSV
> > > path when creating vaapi surface.
> > >   
> > > >   
> > > > > ---
> > > > >   libavutil/hwcontext_qsv.c | 8 ++++++++
> > > > >   1 file changed, 8 insertions(+)
> > > > > 
> > > > > diff --git a/libavutil/hwcontext_qsv.c
> > > > > b/libavutil/hwcontext_qsv.c index 510f422562..a3350eae0f 100644
> > > > > --- a/libavutil/hwcontext_qsv.c
> > > > > +++ b/libavutil/hwcontext_qsv.c
> > > > > @@ -119,6 +119,8 @@ static const struct {
> > > > >                          MFX_FOURCC_YUY2 },
> > > > >       { AV_PIX_FMT_Y210,
> > > > >                          MFX_FOURCC_Y210 },
> > > > > +    { AV_PIX_FMT_VUYX,
> > > > > +                       MFX_FOURCC_AYUV },
> > > > >   #endif
> > > > >   };
> > > > >   
> > > > > @@ -1502,6 +1504,12 @@ static int map_frame_to_surface(const
> > > > > AVFrame *frame,
> > > > > mfxFrameSurface1 *surface)
> > > > >           surface->Data.U16 = (mfxU16 *)frame->data[0] + 1;
> > > > >           surface->Data.V16 = (mfxU16 *)frame->data[0] + 3;
> > > > >           break;
> > > > > +    case AV_PIX_FMT_VUYX:
> > > > > +        surface->Data.V = frame->data[0];
> > > > > +        surface->Data.U = frame->data[0] + 1;
> > > > > +        surface->Data.Y = frame->data[0] + 2;
> > > > > +        surface->Data.A = frame->data[0] + 3;  
> > > > 
> > > > This will go wrong with VUYX. You need to use AV_PIX_FMT_VUYA.  
> > > 
> > > frame->data[0] + 3 is valid even if alpha channel is ignored in
> > > VUYX. Intel HW doesn't use the data in alpha channel actually, but
> > > the SDK uses Microsoft pixel
> > > format AYUV which is the alpha version, here set a valid address to
> > > alpha channel when mapping a VUYX AVFrame to a AYUV mfx surface. 
> > >   
> > 
> > Is there more comment about this patch ? 
> 
> Wouldn't it be more correct to fill the Data.A with 0xFF rather than
> using the undefined value from the frame? I assume that the encoder
> actually drops the value completely so it ultimately doesn't matter,
> but it makes it clear that there is no alpha and if the encoder was to
> start preserving it, it would preserve the desired value.

Yes, you're right that the encoder drops the value. I'll add some comment in the
patch. 

Thanks
Haihao