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 783567 - Encode: Fix the hardcoded range of quality level
Encode: Fix the hardcoded range of quality level
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer-vaapi
unspecified
Other Linux
: Normal normal
: 1.13.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-06-08 18:32 UTC by sreerenj
Modified: 2017-06-13 14:56 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
vaapiencode: Fix the hardcoded range of quality level (1.65 KB, patch)
2017-06-08 18:34 UTC, sreerenj
none Details | Review
libs: encoder: Fix the quality level clamping (2.00 KB, patch)
2017-06-09 21:40 UTC, sreerenj
committed Details | Review

Description sreerenj 2017-06-08 18:32:49 UTC
Change the quality level hard-coded range from {1-8} to {1-7}, since it is the range Intel Open source driver supports.
Comment 1 sreerenj 2017-06-08 18:34:45 UTC
Created attachment 353425 [details] [review]
vaapiencode: Fix the hardcoded range of quality level
Comment 2 sreerenj 2017-06-08 23:46:43 UTC
Hm actually there is a bit of problem here.!
All 1.8.x series driver are returning a max val of 8 and https://github.com/01org/intel-vaapi-driver/pull/194/files patch going to change it to 7.

For gstreaemr-vaapi the below codeo will case issues
GST_VAAPI_ENCODER_QUALITY_LEVEL (encoder) =
        gst_util_uint64_scale_int_ceil (GST_VAAPI_ENCODER_QUALITY_LEVEL
        (encoder), quality_level_max, 7);

May be it is more better to use 8 in the gst_util_uint64_scale_int_ceil(),
but encoder property should advertise the max val as 7.

Otherwise,

Users having driver-1.8.x are going end up in trouble by setting 8 as quality level:
8*8/8 = 8

On the other hand, if we keep 7 as hard-coded value as in patch,
Users having driver-1.8.x are going to end up in trouble by setting 7 as quality level:
7*8/7 = 8
Comment 3 sreerenj 2017-06-09 21:40:19 UTC
Created attachment 353494 [details] [review]
libs: encoder: Fix the quality level clamping

Change the hard-coded range of quality-level from {1-8} to {1-7},
since it is the range Intel Open source driver supports.
Also perform the range clamping only if the user provided
quality-level is greater than the max-range suppored by the driver,
because there could be non-intel drivers giving lower value than
he hard-coded max value 7.

Anyway user won't be able to request for a value more than 7 since we set the property range to a hard-coded magic number 7.

eg: let's say driver query returns 2 as max range and user requested for a quality level 3.
The previous code(with out patch) will set the quality as ceil (3 * 2/8) = 1 which is wrong.

I think we just need to set the quality-level to max-range returned by driver(if user requested quality-level is greater than driver-max-range).
Comment 4 Víctor Manuel Jáquez Leal 2017-06-13 14:50:13 UTC
Indeed, doing a clamp was my first option, but then I thought it was better to do a scaling value to the drivers' maximum.

I'm going to change a bit the code to reflect the clamping, by using CLAMP :)
Comment 5 Víctor Manuel Jáquez Leal 2017-06-13 14:54:52 UTC
Attachment 353494 [details] pushed as 595f8b7 - libs: encoder: Fix the quality level clamping