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 783449 - libs: encoder: refactor buffer parameters assignation
libs: encoder: refactor buffer parameters assignation
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer-vaapi
git master
Other Linux
: Normal normal
: 1.13.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks: 766832 778732
 
 
Reported: 2017-06-05 18:57 UTC by Víctor Manuel Jáquez Leal
Modified: 2017-06-07 15:42 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
libs: encoder: h264,vp8,mpeg2: refactor control rate (8.67 KB, patch)
2017-06-05 18:58 UTC, Víctor Manuel Jáquez Leal
committed Details | Review
libs: encoder: h264,h265,mpeg2,vp8: refactor HDR (6.92 KB, patch)
2017-06-05 18:58 UTC, Víctor Manuel Jáquez Leal
committed Details | Review
libs: encoder: vp8: refactor FrameRate parameter (2.40 KB, patch)
2017-06-05 18:58 UTC, Víctor Manuel Jáquez Leal
committed Details | Review
libs: encoder: vp8: fix frame rate calculation (1.68 KB, patch)
2017-06-05 18:58 UTC, Víctor Manuel Jáquez Leal
committed Details | Review
libs: encoder: vp8,h264,h265,mpeg2: refactor misc parameters (13.10 KB, patch)
2017-06-05 18:58 UTC, Víctor Manuel Jáquez Leal
committed Details | Review
libs: encoder: vp8,h264,h265,mpeg2: set misc param once (6.53 KB, patch)
2017-06-05 18:58 UTC, Víctor Manuel Jáquez Leal
committed Details | Review
libs: encoder: h264,h265,mpeg2: add framerate parameter (2.62 KB, patch)
2017-06-05 18:58 UTC, Víctor Manuel Jáquez Leal
committed Details | Review
libs: encoder: h265: add rate control parameter (1.24 KB, patch)
2017-06-05 18:58 UTC, Víctor Manuel Jáquez Leal
committed Details | Review

Description Víctor Manuel Jáquez Leal 2017-06-05 18:57:12 UTC
Buffer's VA parameters assignation can be improved, avoiding the same operations per buffer, but do then when the encoder is configured and just copy the structures to the buffer's parameters.
Comment 1 Víctor Manuel Jáquez Leal 2017-06-05 18:58:05 UTC
Created attachment 353219 [details] [review]
libs: encoder: h264,vp8,mpeg2: refactor control rate

Instead of filling the control rate param in ensure_misc_params(),
this patch refactor it out, as a first step to merge the same code
for all the encoders.
Comment 2 Víctor Manuel Jáquez Leal 2017-06-05 18:58:10 UTC
Created attachment 353220 [details] [review]
libs: encoder: h264,h265,mpeg2,vp8: refactor HDR

Move the Hypothetical Reference Decoder (HRD) parameter, from
ensure_misc_params() to ensure_control_rate_params(), since it
only shall be defined when the control rate is either VBR or CBR.
Comment 3 Víctor Manuel Jáquez Leal 2017-06-05 18:58:15 UTC
Created attachment 353221 [details] [review]
libs: encoder: vp8: refactor FrameRate parameter

Move frame-rate parameter from ensure_misc_params() to
ensure_contro_rate_param() since it only has meaning when the
control rate is either VBR or CBR.
Comment 4 Víctor Manuel Jáquez Leal 2017-06-05 18:58:20 UTC
Created attachment 353222 [details] [review]
libs: encoder: vp8: fix frame rate calculation

According to the VA documentation:

     The framerate is specified as a number of frames per second,
     as a fraction.  The denominator of the fraction is given in
     the top half (the high two bytes) of the framerate field, and
     the numerator is given in the bottom half (the low two bytes).

     For example, if framerate is set to (100 << 16 | 750), this is
     750 / 100, hence 7.5fps.

     If the denominator is zero (the high two bytes are both zero)
     then it takes the value one instead, so the framerate is just
     the integer in the low 2 bytes.

This patch fixes the the framerate calculation in vp8 encoder
according to this.
Comment 5 Víctor Manuel Jáquez Leal 2017-06-05 18:58:25 UTC
Created attachment 353223 [details] [review]
libs: encoder: vp8,h264,h265,mpeg2: refactor misc parameters

This is patch pretends to decouple the assignation of the values
in the parameter structures and the VA buffer's parameters setting.

It may lead to some issues since HRD, framerate or controlrate may
not be handled by the specific encoder, but they are set in
the VA buffer's parameters.

I leave as it because this patch is just a transitional patch.
Comment 6 Víctor Manuel Jáquez Leal 2017-06-05 18:58:31 UTC
Created attachment 353224 [details] [review]
libs: encoder: vp8,h264,h265,mpeg2: set misc param once

Instead of recalculating the miscellaneous buffer parameters for
every buffer, it is only done once, when the encoder is configured.
And for every buffer, the same structures are just copied.
Comment 7 Víctor Manuel Jáquez Leal 2017-06-05 18:58:36 UTC
Created attachment 353225 [details] [review]
libs: encoder: h264,h265,mpeg2: add framerate parameter
Comment 8 Víctor Manuel Jáquez Leal 2017-06-05 18:58:41 UTC
Created attachment 353226 [details] [review]
libs: encoder: h265: add rate control parameter
Comment 9 Hyunjun Ko 2017-06-07 06:42:46 UTC
To test these patches, I tried to merge to my branch, but it failed.
Probably, there's another patch for you, which is removing g_assert.

Anyway, I tested these myself finally and found no problem.
I like this refactoring, which was in my head while working on VBR support.
Comment 10 Víctor Manuel Jáquez Leal 2017-06-07 15:37:42 UTC
Attachment 353219 [details] pushed as b538a86 - libs: encoder: h264,vp8,mpeg2: refactor control rate
Attachment 353220 [details] pushed as fc5106b - libs: encoder: h264,h265,mpeg2,vp8: refactor HDR
Attachment 353221 [details] pushed as baac8dc - libs: encoder: vp8: refactor FrameRate parameter
Attachment 353222 [details] pushed as daff4e9 - libs: encoder: vp8: fix frame rate calculation
Attachment 353223 [details] pushed as acf106e - libs: encoder: vp8,h264,h265,mpeg2: refactor misc parameters
Attachment 353224 [details] pushed as 846c276 - libs: encoder: vp8,h264,h265,mpeg2: set misc param once
Attachment 353225 [details] pushed as d733714 - libs: encoder: h264,h265,mpeg2: add framerate parameter
Attachment 353226 [details] pushed as 8188668 - libs: encoder: h265: add rate control parameter