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 797202 - vaapi: libs: h264: Update H.264 level table with 6, 6.1 and 6.2.
vaapi: libs: h264: Update H.264 level table with 6, 6.1 and 6.2.
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer-vaapi
1.4.0
Other Linux
: Normal normal
: 1.15.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2018-09-25 21:44 UTC by Matteo Valdina
Modified: 2018-10-09 15:28 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch for updating H.264 levels (3.43 KB, patch)
2018-09-25 21:44 UTC, Matteo Valdina
committed Details | Review
Patch for replacing g_debug with GST_DEBUG. (6.74 KB, patch)
2018-09-26 19:59 UTC, Matteo Valdina
committed Details | Review

Description Matteo Valdina 2018-09-25 21:44:04 UTC
Created attachment 373759 [details] [review]
Patch for updating H.264 levels

Hi,
I don't know if it is intentional or makes any difference.

But I noticed that the VAAPI H.264 levels table is not updated to the latest specs.
It is missing the 6, 6.1 and 6.2 levels.

This patch will update the level with the latest values.

I also noticed that gstvaapiutils_h264.c uses g_debug instead of GST_DEBUG. Is it intentional?

Best
Matteo Valdina
Comment 1 Víctor Manuel Jáquez Leal 2018-09-26 06:21:35 UTC
(In reply to Matteo Valdina from comment #0)

> I also noticed that gstvaapiutils_h264.c uses g_debug instead of GST_DEBUG.
> Is it intentional?

In the past there were efforts to convert libgstvaapi into a non-gst library. So they are leftovers of that intention. Please, post a patch for that. Thanks!

Did you test some streams decoding/encoding with those levels?
Comment 2 Matteo Valdina 2018-09-26 19:24:14 UTC
I'll give a look. Thanks

No, I tested up to 4K@30 that it is level 5.1. As per my understanding the Intel HD 630 support up to 5.1 (and not 5.2). (https://en.wikichip.org/wiki/intel/hd_graphics_630).
A level like 6 and up are for a bigger resolution for example 8K@30.
Comment 3 Matteo Valdina 2018-09-26 19:59:27 UTC
Created attachment 373774 [details] [review]
Patch for replacing g_debug with GST_DEBUG.
Comment 4 Víctor Manuel Jáquez Leal 2018-09-27 10:20:15 UTC
Review of attachment 373774 [details] [review]:

I think all these g_debug should be turned in GST_WARNING since those logs are really warnings...
Comment 5 Víctor Manuel Jáquez Leal 2018-09-27 10:49:19 UTC
Comment on attachment 373759 [details] [review]
Patch for updating H.264 levels

pushed attachment 373759 [details] [review] as commit b1b36a44 - libs: h264: Update level table to "Recommendation H.264 (04/17)".
Comment 6 Matteo Valdina 2018-09-28 19:25:48 UTC
Hi Victor,
I'm not 100% sure. I saw that these functions are used without checking if the input parameters are valid.
And after all, they return a value that means the specific function fails.

For example, in the gst_vaapi_utils_h264_get_profile case:

This function is called for each vaapi profile. This includes VP8, VP9, H265 and etc.
So this function fails in the normal scenario and returns zero. The caller properly handles the failure and move on.
Comment 7 Víctor Manuel Jáquez Leal 2018-10-09 15:02:44 UTC
(In reply to Matteo Valdina from comment #6)
> Hi Victor,
> I'm not 100% sure. I saw that these functions are used without checking if
> the input parameters are valid.
> And after all, they return a value that means the specific function fails.
> 
> For example, in the gst_vaapi_utils_h264_get_profile case:
> 
> This function is called for each vaapi profile. This includes VP8, VP9, H265
> and etc.
> So this function fails in the normal scenario and returns zero. The caller
> properly handles the failure and move on.

Fair enough. (sorry for the late review!)
Comment 8 Víctor Manuel Jáquez Leal 2018-10-09 15:28:31 UTC
attachment 373774 [details] [review] pushed as commit 5567a3d2 - libs: Move from g_debug to GST_DEBUG.