diff mbox series

[FFmpeg-devel,4/4] vaapi_encode_h265: Enable 4:2:2 support

Message ID 20200305002528.11418-4-sw@jkqxz.net
State New
Headers show
Series [FFmpeg-devel,1/4] vaapi_encode: Move block size calculation after entrypoint selection
Related show

Checks

Context Check Description
andriy/ffmpeg-patchwork pending
andriy/ffmpeg-patchwork success Applied patch
andriy/ffmpeg-patchwork success Configure finished
andriy/ffmpeg-patchwork success Make finished
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Mark Thompson March 5, 2020, 12:25 a.m. UTC
---
With <https://github.com/intel/media-driver/pull/866>, intra-only 4:2:2 encode works on Ice Lake with something like:

 -vf 'format=yuyv422,hwupload' -c:v hevc_vaapi -g 1 out.mp4

There is still something wrong with inter frames.

 libavcodec/vaapi_encode_h265.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Fu, Linjie March 5, 2020, 2:49 a.m. UTC | #1
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Mark Thompson
> Sent: Thursday, March 5, 2020 08:25
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH 4/4] vaapi_encode_h265: Enable 4:2:2
> support
> 
> ---
> With <https://github.com/intel/media-driver/pull/866>, intra-only 4:2:2
> encode works on Ice Lake with something like:
> 
>  -vf 'format=yuyv422,hwupload' -c:v hevc_vaapi -g 1 out.mp4
> 
> There is still something wrong with inter frames.

The possible reason is media-driver for gen11 (ice lake) lacks the capability
query support for VAProfileHEVCMain422_10. With [1] applied in driver,
the encoding for yuyv422 works well for me, for both intra and inter frames.

Ps.
In case you may be interested, I prepared several patches [2] for 4:2:2/4:4: 8/10 bit encoding.

They works for me with correct outputs. And the reason for not upstreaming is:
1. 444 pixel format not ready;
2. recon surface of Y210 or 444 (AYUV and Y410 in media-driver) depends on the surface hint [3] in
libva and corresponding code in media-driver to resize the recon surface which is not upstreamed
yet.

[1] https://github.com/intel/media-driver/pull/855/files
[2] https://github.com/fulinjie/ffmpeg/commit/dfb153fa68029f2a4060bcfd8bf500fd8ee44254
[3] https://github.com/intel/libva/pull/344

- Linjie
Mark Thompson March 7, 2020, 4:14 p.m. UTC | #2
On 05/03/2020 02:49, Fu, Linjie wrote:
> 2. recon surface of Y210 or 444 (AYUV and Y410 in media-driver) depends on the surface hint [3] in
> libva and corresponding code in media-driver to resize the recon surface which is not upstreamed
> yet.

What is the reasoning for forcing the user to pass new extra attributes with this rather than handling it transparently so that it works like all other encoders?

In some places in Mesa surfaces are reallocated with different properties when they are used for a purpose they don't currently support, which avoids weird constraints being exported to the user (e.g. see <https://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/state_trackers/va/surface.c#n1000>).  Since reconstructed picture surfaces are pretty unlikely to be used for anything else (just being copied out for debugging maybe?), it seems like an answer like that would be simpler in this case too.  (Though perhaps I'm missing something weird about the Intel hardware which makes this case uglier.)

- Mark
Fu, Linjie May 15, 2020, 7:21 a.m. UTC | #3
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Mark Thompson
> Sent: Sunday, March 8, 2020 00:15
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH 4/4] vaapi_encode_h265: Enable 4:2:2
> support
> 
> On 05/03/2020 02:49, Fu, Linjie wrote:
> > 2. recon surface of Y210 or 444 (AYUV and Y410 in media-driver) depends
> on the surface hint [3] in
> > libva and corresponding code in media-driver to resize the recon surface
> which is not upstreamed
> > yet.
> 
> What is the reasoning for forcing the user to pass new extra attributes with
> this rather than handling it transparently so that it works like all other
> encoders?
> 
> In some places in Mesa surfaces are reallocated with different properties
> when they are used for a purpose they don't currently support, which avoids
> weird constraints being exported to the user (e.g. see
> <https://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/state_trackers/v
> a/surface.c#n1000>).  Since reconstructed picture surfaces are pretty unlikely
> to be used for anything else (just being copied out for debugging maybe?), it
> seems like an answer like that would be simpler in this case too.  (Though
> perhaps I'm missing something weird about the Intel hardware which makes
> this case uglier.)
> 

Implemented the surface reallocation inside media driver in [1], merged the query
support in [2],  verified that it works for both AYUV(or XYUV)/Y410, yuyv422.

And for Y210, it seems to be better to implement render target support in
vaapi_encoder in this patch as well:
{ "YUV422_10", VA_RT_FORMAT_YUV422_10,    10, 3, 1, 0 },

Hence patch LGTM with or without above modifications, thx.

- Linjie

[1] < https://github.com/intel/media-driver/pull/915>
[2] < https://github.com/intel/media-driver/pull/855>
Linjie Fu July 20, 2020, 2:32 p.m. UTC | #4
On Fri, May 15, 2020 at 3:21 PM Fu, Linjie <linjie.fu@intel.com> wrote:
>
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > Mark Thompson
> > Sent: Sunday, March 8, 2020 00:15
> > To: ffmpeg-devel@ffmpeg.org
> > Subject: Re: [FFmpeg-devel] [PATCH 4/4] vaapi_encode_h265: Enable 4:2:2
> > support
> >
> > On 05/03/2020 02:49, Fu, Linjie wrote:
> > > 2. recon surface of Y210 or 444 (AYUV and Y410 in media-driver) depends
> > on the surface hint [3] in
> > > libva and corresponding code in media-driver to resize the recon surface
> > which is not upstreamed
> > > yet.
> >
> > What is the reasoning for forcing the user to pass new extra attributes with
> > this rather than handling it transparently so that it works like all other
> > encoders?
> >
> > In some places in Mesa surfaces are reallocated with different properties
> > when they are used for a purpose they don't currently support, which avoids
> > weird constraints being exported to the user (e.g. see
> > <https://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/state_trackers/v
> > a/surface.c#n1000>).  Since reconstructed picture surfaces are pretty unlikely
> > to be used for anything else (just being copied out for debugging maybe?), it
> > seems like an answer like that would be simpler in this case too.  (Though
> > perhaps I'm missing something weird about the Intel hardware which makes
> > this case uglier.)
> >
>
> Implemented the surface reallocation inside media driver in [1], merged the query
> support in [2],  verified that it works for both AYUV(or XYUV)/Y410, yuyv422.
>
> And for Y210, it seems to be better to implement render target support in
> vaapi_encoder in this patch as well:
> { "YUV422_10", VA_RT_FORMAT_YUV422_10,    10, 3, 1, 0 },
>
> Hence patch LGTM with or without above modifications, thx.
>
> [1] < https://github.com/intel/media-driver/pull/915>
> [2] < https://github.com/intel/media-driver/pull/855>

Since it's well supported for now, prefer to apply this soon
together with the patch for 422 10-bit encoding :

https://patchwork.ffmpeg.org/project/ffmpeg/patch/1595254554-12809-1-git-send-email-linjie.fu@intel.com/
Linjie Fu July 24, 2020, 6:46 a.m. UTC | #5
On Mon, Jul 20, 2020 at 10:32 PM Linjie Fu <linjie.justin.fu@gmail.com> wrote:
>
> On Fri, May 15, 2020 at 3:21 PM Fu, Linjie <linjie.fu@intel.com> wrote:
> >
> > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > > Mark Thompson
> > > Sent: Sunday, March 8, 2020 00:15
> > > To: ffmpeg-devel@ffmpeg.org
> > > Subject: Re: [FFmpeg-devel] [PATCH 4/4] vaapi_encode_h265: Enable 4:2:2
> > > support
> > >
> > > On 05/03/2020 02:49, Fu, Linjie wrote:
> > > > 2. recon surface of Y210 or 444 (AYUV and Y410 in media-driver) depends
> > > on the surface hint [3] in
> > > > libva and corresponding code in media-driver to resize the recon surface
> > > which is not upstreamed
> > > > yet.
> > >
> > > What is the reasoning for forcing the user to pass new extra attributes with
> > > this rather than handling it transparently so that it works like all other
> > > encoders?
> > >
> > > In some places in Mesa surfaces are reallocated with different properties
> > > when they are used for a purpose they don't currently support, which avoids
> > > weird constraints being exported to the user (e.g. see
> > > <https://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/state_trackers/v
> > > a/surface.c#n1000>).  Since reconstructed picture surfaces are pretty unlikely
> > > to be used for anything else (just being copied out for debugging maybe?), it
> > > seems like an answer like that would be simpler in this case too.  (Though
> > > perhaps I'm missing something weird about the Intel hardware which makes
> > > this case uglier.)
> > >
> >
> > Implemented the surface reallocation inside media driver in [1], merged the query
> > support in [2],  verified that it works for both AYUV(or XYUV)/Y410, yuyv422.
> >
> > And for Y210, it seems to be better to implement render target support in
> > vaapi_encoder in this patch as well:
> > { "YUV422_10", VA_RT_FORMAT_YUV422_10,    10, 3, 1, 0 },
> >
> > Hence patch LGTM with or without above modifications, thx.
> >
> > [1] < https://github.com/intel/media-driver/pull/915>
> > [2] < https://github.com/intel/media-driver/pull/855>
>
> Since it's well supported for now, prefer to apply this soon
> together with the patch for 422 10-bit encoding :
>
> https://patchwork.ffmpeg.org/project/ffmpeg/patch/1595254554-12809-1-git-send-email-linjie.fu@intel.com/

Applied, thx.

- Linjie
diff mbox series

Patch

diff --git a/libavcodec/vaapi_encode_h265.c b/libavcodec/vaapi_encode_h265.c
index 59d150f503..79e3c70602 100644
--- a/libavcodec/vaapi_encode_h265.c
+++ b/libavcodec/vaapi_encode_h265.c
@@ -1197,6 +1197,10 @@  static const VAAPIEncodeProfile vaapi_encode_h265_profiles[] = {
 #if VA_CHECK_VERSION(0, 37, 0)
     { FF_PROFILE_HEVC_MAIN_10, 10, 3, 1, 1, VAProfileHEVCMain10     },
     { FF_PROFILE_HEVC_REXT,    10, 3, 1, 1, VAProfileHEVCMain10     },
+#endif
+#if VA_CHECK_VERSION(1, 2, 0)
+    { FF_PROFILE_HEVC_REXT,     8, 3, 1, 0, VAProfileHEVCMain422_10 },
+    { FF_PROFILE_HEVC_REXT,    10, 3, 1, 0, VAProfileHEVCMain422_10 },
 #endif
     { FF_PROFILE_UNKNOWN }
 };