| 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 | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | success | Make fate finished |
> -----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
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
> 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>
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/
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 --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 } };