[FFmpeg-devel] [PATCH] lavu/hwcontext_qsv: add 10-bit RGB support for QSV

[Hendrik Leppkes]

On Mon, Aug 10, 2020 at 6:07 AM Linjie Fu <linjie.justin.fu at gmail.com> wrote:
>
> On Mon, Aug 10, 2020 at 4:31 AM Mark Thompson <sw at jkqxz.net> wrote:
> >
> > On 09/08/2020 06:11, Linjie Fu wrote:
> > > After adding a copy pass through path inside MSDK, x2rgb10 is now
> > > available as an output of VPP.
> > >
> > > Command line for CSC:
> > > ffmpeg -hwaccel qsv -v verbose -c:v hevc_qsv -i
> > >      p010.h265 -vf scale_qsv=format=x2rgb10,hwdownload,format=x2rgb10
> > >      -vframes 1 out.yuv
> > >
> > > Signed-off-by: Linjie Fu <linjie.justin.fu at gmail.com>
> > > ---
> > > [1]Issue:
> > > https://github.com/Intel-Media-SDK/MediaSDK/issues/1654
> > > [2]Adding copy passthrou:
> > > https://github.com/AntonGrishin/MediaSDK/commit/20058c0e1dbd521e2571e14f6246c48fea996094
> > > [3]Still in need to be fixed:
> > > https://github.com/Intel-Media-SDK/MediaSDK/pull/2268
> > >
> > > This patch would be ready to merge after [3] is handled decently.
> > >
> > >   libavutil/hwcontext_qsv.c | 8 ++++++++
> > >   1 file changed, 8 insertions(+)
> > >
> > > diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
> > > index 35a944f8f8..64cb1663d8 100644
> > > --- a/libavutil/hwcontext_qsv.c
> > > +++ b/libavutil/hwcontext_qsv.c
> > > @@ -110,6 +110,10 @@ static const struct {
> > >   #if CONFIG_VAAPI
> > >       { AV_PIX_FMT_YUYV422,
> > >                          MFX_FOURCC_YUY2 },
> > > +#if QSV_VERSION_ATLEAST(1, 9)
> > > +    { AV_PIX_FMT_X2RGB10,
> > > +                    MFX_FOURCC_A2RGB10 },
> > > +#endif
> > >   #if QSV_VERSION_ATLEAST(1, 27)
> > >       { AV_PIX_FMT_Y210,
> > >                          MFX_FOURCC_Y210 },
> > > @@ -798,6 +802,10 @@ static int map_frame_to_surface(const AVFrame *frame, mfxFrameSurface1 *surface)
> > >           surface->Data.V16 = (mfxU16 *)frame->data[0] + 3;
> > >           break;
> > >   #endif
> > > +    case AV_PIX_FMT_X2RGB10:
> > > +        surface->Data.A2RGB10 = (mfxA2RGB10 *)frame->data[0];
> > > +        break;
> > > +
> > >       default:
> > >           return MFX_ERR_UNSUPPORTED;
> > >       }
> > >
> >
> > What happens to the alpha information here?  We don't specify what is in the unused bits of X2RGB10, so saying that it has an alpha component seems likely to do something ugly in cases where that is used (e.g. overlay).
>
> Yep, and as discussed previously, this patch is to add support for
> x2rgb10(instead of pixel format a2rgb10) in ffmpeg level, which didn't
> say it has the alpha component.
> So I think this would be fine, since we don't care about the unused 2
> bits for x2rgb10 (no matter whether it contains alpha information or
> not).

I think Marks point is that the QSV format is called
MFX_FOURCC_A2RGB10, eg. A2RGB10, not X2RGB10. It sounds like the QSV
format expects alpha to be valid.

It should be clarified what happens with the alpha, and made sure that
it doesnt produce files with an alpha channel full of nonsense.

- Hendrik