Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support YUV444 on direct-backend #134

Merged
merged 6 commits into from
Dec 18, 2022

Conversation

thesword53
Copy link
Contributor

I added yuv444 decoding support and refactored color format into a static list formatsInfo to easily add a new one if needed. I also added yuv444p10, yuv444p12 and yuv444p16 but currently VAAPI doesn't expose theses formats.
To support YUV444 on any FFmpeg based program the commit FFmpeg/FFmpeg@d3f48e6 is needed otherwise it will fallback to software decoder even if vainfo lists VAProfileHEVCMain444.

YUV444 is only supported on HEVC codec and Turing architecture (RTX 2000/GTX 1600) or newer is needed.

@elFarto
Copy link
Owner

elFarto commented Dec 8, 2022

That looks great. I have a question about the Q410, Q412 and Q416 formats, is that something you've made up as I don't see any reference to them anywhere else.

@thesword53
Copy link
Contributor Author

I found DRM_FORMAT_Q410 here https://github.com/torvalds/linux/blob/master/include/uapi/drm/drm_fourcc.h#L356 for yuv444p10 and I guessed DRM_FORMAT_Q412 is yuv444p12le and DRM_FORMAT_Q416 is yuv444p16le and same for VA_FOURCC_Q410, VA_FOURCC_Q412 and VA_FOURCC_Q416.

@philipl
Copy link
Contributor

philipl commented Dec 8, 2022

Note that these formats are not the same as ffmpeg's yuv44p10 and yuv44p12 because the padding is at the opposite end (LSB vs MSB). For yuv44p16, because there is no padding, it is the same.

src/vabackend.c Outdated Show resolved Hide resolved
src/vabackend.c Outdated Show resolved Hide resolved
@thesword53
Copy link
Contributor Author

I added RGB planar format. It's the same as YUV444 but red, green and blue planes are stored instead of Y, Cb, Cr planes.

@elFarto
Copy link
Owner

elFarto commented Dec 9, 2022

We can't support RGBP without a lot of work, as NVDEC only outputs YUV encoded images. To convert them to RGB we'd need to do a colour space conversion which a, means knowing the colour space of the video and b, requires writing a CUDA program to do the conversion.

@thesword53
Copy link
Contributor Author

We can't support RGBP without a lot of work, as NVDEC only outputs YUV encoded images. To convert them to RGB we'd need to do a colour space conversion which a, means knowing the colour space of the video and b, requires writing a CUDA program to do the conversion.

I tested with a patched version of FFmpeg and a video with RGBP pixel format and it's working. Color conversion isn't done in decoder, it's done with FFmpeg or video player. When a video is stored as RGBP, it directly contains RGB data so no color space conversion is done.

@elFarto
Copy link
Owner

elFarto commented Dec 14, 2022

Do you have a link to the FFMPEG patch for the RGBP support? I don't see much usage of RGBP in FFMPEG at the moment, especially related to VA-API, so I'm a little hesitant to add in support for it.

@thesword53
Copy link
Contributor Author

I wrote patch of FFmpeg@8ad988ac37d4d92dbb60796e26c3ad558a3eebeb by myself. I just added RGBP support to FFmpeg.

diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
index 567e8d81d4..b09d1225b5 100644
--- a/libavcodec/hevcdec.c
+++ b/libavcodec/hevcdec.c
@@ -453,6 +453,7 @@ static enum AVPixelFormat get_format(HEVCContext *s, const HEVCSPS *sps)
 #endif
         break;
     case AV_PIX_FMT_YUV444P:
+    case AV_PIX_FMT_GBRP:
 #if CONFIG_HEVC_VAAPI_HWACCEL
         *fmt++ = AV_PIX_FMT_VAAPI;
 #endif
diff --git a/libavcodec/vaapi_decode.c b/libavcodec/vaapi_decode.c
index 134f10eca5..c0f34adad9 100644
--- a/libavcodec/vaapi_decode.c
+++ b/libavcodec/vaapi_decode.c
@@ -289,6 +289,8 @@ static const struct {
 #ifdef VA_FOURCC_I010
     MAP(I010, YUV420P10),
 #endif
+
+    MAP(RGBP, GBRP),
 #undef MAP
 };
 
diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c
index 938bd5447d..0a76668158 100644
--- a/libavutil/hwcontext_vaapi.c
+++ b/libavutil/hwcontext_vaapi.c
@@ -161,6 +161,8 @@ static const VAAPIFormatDescriptor vaapi_format_map[] = {
     // support for Y412, so we must fudge the mapping here.
     MAP(Y412, YUV444_12,  XV36, 0),
 #endif
+
+    MAP(RGBP, RGBP,  GBRP, 0),
 };
 #undef MAP
 

I don't see much usage of RGBP in FFMPEG at the moment, especially related to VA-API

It might be useful for professional usage such as lossless HEVC video. If you do YUV444->RGB and RGB->YUV444 conversion, you lose color precision due to rounding error.
FFmpeg is also able to encode RGBP video with NVENC.

@thesword53
Copy link
Contributor Author

I am using a GeForce RTX 2080 SUPER (desktop) to fo my tests so you can change from ❓ to ✅ on "Direct Backend"/Turing (20XX/16XX)/Desktop README.md section.

@elFarto
Copy link
Owner

elFarto commented Dec 17, 2022

I can't say I'm sold on the RGBP support. I'd not seen any mention of that format until now, and I don't think there would be much demand for it in this library. The other concern I have is the order of the planes, VA says RGB, FFMPEG says GBR which is a very odd order indeed.

Would it be possible for you to remove RGBP, or at least comment out the support for it?

@elFarto elFarto merged commit 7113c20 into elFarto:master Dec 18, 2022
@elFarto
Copy link
Owner

elFarto commented Dec 18, 2022

Thanks for your work on this!

@crimist
Copy link
Contributor

crimist commented Dec 18, 2022

Time to update the docs? As I understand it this YUV444 implementation requires:

  • >= Turing
  • HEVC
  • Direct backend

@thesword53
Copy link
Contributor Author

Yes! I will probably try to implement this with EGL backend if Nvidia fixes their drivers.

@elFarto
Copy link
Owner

elFarto commented Dec 19, 2022

I've been thinking on the EGL backend, and it may not actually be necessary for NVIDIA to fix their drivers for us to implement YUV444 support. The current way we export images is to use both CUarrays with the correct mapping (CU_EGL_COLOR_FORMAT_YUV420_SEMIPLANAR). However, this still just gives us back 2 fd's, so it's not like exporting them together is actually doing anything for us.

I think it should be possible to modify the egl_allocateBackingImage function to export each plane individually, as just a R/RG plane, with 8 or 16bits as required. You'd probably want to move the main exporting logic out of that function. exportBackingImage will likely have to be changed aswell as being called twice. There is a slight issue of what to do with the outputs from the eglExportDMABUFImageQueryMESA call (specifically the fourcc, but we should know anyway), but I think we really only need the modifiers from that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants