After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 797264 - vaapih265dec: Support 422 10bit decode.
vaapih265dec: Support 422 10bit decode.
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer-vaapi
git master
Other Linux
: Normal normal
: 1.15.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on: 797267
Blocks: 796627
 
 
Reported: 2018-10-09 07:59 UTC by Fei
Modified: 2018-11-02 15:04 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch (5.87 KB, patch)
2018-10-24 06:31 UTC, Fei
none Details | Review
patch (5.87 KB, patch)
2018-10-24 07:56 UTC, Fei
none Details | Review
patch (5.86 KB, patch)
2018-11-01 04:27 UTC, Fei
committed Details | Review
streams (2.92 MB, application/gzip)
2018-11-02 01:11 UTC, Fei
  Details

Description Fei 2018-10-09 07:59:43 UTC
Support hevc 422 10bit stream decode.
Comment 1 Fei 2018-10-09 08:02:01 UTC
Currently, libva only support Y210 pixel format(422,10bit), need support this format in gstreamer side. I am cooking a patch to support this.
Comment 2 Fei 2018-10-24 06:31:09 UTC
Created attachment 374021 [details] [review]
patch
Comment 3 Fei 2018-10-24 06:40:15 UTC
This patch depends on a patch on gst-plugin-base which add support for format Y210.
https://bugzilla.gnome.org/show_bug.cgi?id=797267

@Victor, could you help to review both of them if you have the authority? Thanks.

And here is still a know issue that will cause GPU hang when decode main-422 10bit steam. This should be the media-driver's issue, which also happen when decode main-444 streams. I will report it to media-driver after this patch merged.
Comment 4 Fei 2018-10-24 07:56:44 UTC
Created attachment 374024 [details] [review]
patch
Comment 5 Víctor Manuel Jáquez Leal 2018-10-25 15:41:59 UTC
@Fei,

I'll try to look at them this weekend, during the hackfest in Edinburgh.
Comment 6 Fei 2018-10-26 00:54:32 UTC
(In reply to Víctor Manuel Jáquez Leal from comment #5)
> @Fei,
> 
> I'll try to look at them this weekend, during the hackfest in Edinburgh.

No problem, thanks Victor.
Comment 7 Fei 2018-10-31 01:24:25 UTC
@Victor, any comments? :)
Comment 8 Víctor Manuel Jáquez Leal 2018-10-31 18:55:46 UTC
Review of attachment 374024 [details] [review]:

overall it looks good, just a couple library version guards.

Also, Nicolas already merged the color space definition patches.

::: gst-libs/gst/vaapi/gstvaapisurface.h
@@ +82,3 @@
   GST_VAAPI_CHROMA_TYPE_RGB16,
+  GST_VAAPI_CHROMA_TYPE_YUV420_10BPP,
+  GST_VAAPI_CHROMA_TYPE_YUV422_10BPP

nitpick: add a comma at the end, thus if a new chroma type is added, wouldn't be need to modify the previous line as in this case.

::: gst-libs/gst/vaapi/gstvaapiutils.c
@@ +403,3 @@
       break;
+    case VA_RT_FORMAT_YUV422_10:
+      chroma_type = GST_VAAPI_CHROMA_TYPE_YUV422_10BPP;

VA_RT_FORMAT_YUV422_10 was defined in VA version of 1.4 so you must guard this code around a 

#if VA_CHECK_VERSION(1,4,0)

@@ +454,3 @@
       break;
+    case GST_VAAPI_CHROMA_TYPE_YUV422_10BPP:
+      format = VA_RT_FORMAT_YUV422_10;

same here
Comment 9 Víctor Manuel Jáquez Leal 2018-10-31 19:03:47 UTC
@Fei,

I don't have hardware where to try this change so I'll have to trust in your testing :/

And thanks for the mug! :)
Comment 10 Fei 2018-11-01 04:27:59 UTC
Created attachment 374115 [details] [review]
patch
Comment 11 Fei 2018-11-01 04:32:46 UTC
(In reply to Víctor Manuel Jáquez Leal from comment #8)
> Review of attachment 374024 [details] [review] [review]:
> 
> overall it looks good, just a couple library version guards.
> 
> Also, Nicolas already merged the color space definition patches.
> 
> ::: gst-libs/gst/vaapi/gstvaapisurface.h
> @@ +82,3 @@
>    GST_VAAPI_CHROMA_TYPE_RGB16,
> +  GST_VAAPI_CHROMA_TYPE_YUV420_10BPP,
> +  GST_VAAPI_CHROMA_TYPE_YUV422_10BPP
> 
> nitpick: add a comma at the end, thus if a new chroma type is added,
> wouldn't be need to modify the previous line as in this case.
> 
> ::: gst-libs/gst/vaapi/gstvaapiutils.c
> @@ +403,3 @@
>        break;
> +    case VA_RT_FORMAT_YUV422_10:
> +      chroma_type = GST_VAAPI_CHROMA_TYPE_YUV422_10BPP;
> 
> VA_RT_FORMAT_YUV422_10 was defined in VA version of 1.4 so you must guard
> this code around a 
> 
> #if VA_CHECK_VERSION(1,4,0)
> 
> @@ +454,3 @@
>        break;
> +    case GST_VAAPI_CHROMA_TYPE_YUV422_10BPP:
> +      format = VA_RT_FORMAT_YUV422_10;
> 
> same here

Thanks Victor for your reviewing. I checked libva, VA_RT_FORMAT_YUV422_10 actually was defined in version 1.2.0. :)
Comment 12 Víctor Manuel Jáquez Leal 2018-11-01 13:11:46 UTC
@fei

Do you have a stream with this format?

Just to check the bailout if the hardware doesn't support it, also when libva is older than 1.2.0
Comment 13 Fei 2018-11-02 01:11:07 UTC
Created attachment 374130 [details]
streams
Comment 14 Fei 2018-11-02 01:15:16 UTC
(In reply to Víctor Manuel Jáquez Leal from comment #12)
> @fei
> 
> Do you have a stream with this format?
> 
> Just to check the bailout if the hardware doesn't support it, also when
> libva is older than 1.2.0

Hi Victor, upload to attachment. And the streams come from ITU:
https://www.itu.int/net/itu-t/sigdb/menu.aspx
Comment 15 Víctor Manuel Jáquez Leal 2018-11-02 15:03:11 UTC
pushe attachment 374115 [details] [review] as commit 63800487 - libs: dec: h265: support decode for main-10-422 10bit streams.