| Message ID | 20200809051107.86008-1-linjie.justin.fu@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] lavu/hwcontext_qsv: add 10-bit RGB support for QSV | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
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@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). - Mark
On Mon, Aug 10, 2020 at 4:31 AM Mark Thompson <sw@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@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). - Linjie
On Mon, Aug 10, 2020 at 6:07 AM Linjie Fu <linjie.justin.fu@gmail.com> wrote: > > On Mon, Aug 10, 2020 at 4:31 AM Mark Thompson <sw@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@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
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; }
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@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(+)