Discussion:
[Bug 768248] New: vaapiencode: Add Encoding region-of-interest (ROI) support
Add Reply
"GStreamer" (GNOME Bugzilla)
2016-06-30 14:42:14 UTC
Reply
Permalink
Raw Message
https://bugzilla.gnome.org/show_bug.cgi?id=768248

Bug ID: 768248
Summary: vaapiencode: Add Encoding region-of-interest (ROI)
support
Classification: Platform
Product: GStreamer
Version: unspecified
OS: Linux
Status: NEW
Severity: enhancement
Priority: Normal
Component: gstreamer-vaapi
Assignee: gstreamer-***@lists.freedesktop.org
Reporter: ***@gmail.com
QA Contact: gstreamer-***@lists.freedesktop.org
CC: ***@gmail.com, ***@igalia.com
GNOME version: ---

Add support for QP adjustments in specific(user provided) areas of the frames
when encoding.

ROIs can be set through the VAENCMiscParameterBufferROIs. The ROI set through
this structure is applicable only to the current frame/field, so must be sent
every frame/field to be applied. The number of supported ROIs can be queried
through the VAConfigAttribEncROI. The encoder will use the ROI information to
adjust the QP values of the MB's that fall within the ROIs.

This feature is for giving more control to the user in order to tune the encode
quality.
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2016-10-11 02:15:26 UTC
Reply
Permalink
Raw Message
https://bugzilla.gnome.org/show_bug.cgi?id=768248

Lim Siew Hoon <***@intel.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@intel.com
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-01-12 08:07:12 UTC
Reply
Permalink
Raw Message
https://bugzilla.gnome.org/show_bug.cgi?id=768248

Hyunjun Ko <***@igalia.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@igalia.com
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-01-17 07:15:14 UTC
Reply
Permalink
Raw Message
https://bugzilla.gnome.org/show_bug.cgi?id=768248

--- Comment #1 from Hyunjun Ko <***@igalia.com> ---
I've been working for this issue and I found some issues to discuss.

1. Handling region of interest meta?
There's already video meta for this, called "GstVideoRegionOfInterestMeta"
But, afaik, there's no element to use this except for opencv element.(face
detection)
Does vaapi encoder need to handle this or just create property like roi-x,
roi-y, roi-w, roi-h? Or both?

2. Other property to be created in vaapi encoder.
Maybe these 2 properties are necessary to provide.
- enable-roi : Enable/Disable roi encoding.
- roi-value : a kind of level of roi region
http://01org.github.io/libva_staging_doxygen/struct__VAEncMiscParameterBufferRoi_1_1VAEncROI.html#ab37eca45463295659565a46430b0b925

And not sure if these attributes are necessary or not as a property.
- max/min delta qp : works only if CQP mode
- num_of_roi : if we supports this, we should support for setting of the number
of {x,y,w,h}, corresponding to num_of_roi. but how?

Refer to
http://01org.github.io/libva_staging_doxygen/struct__VAEncMiscParameterBufferRoi.html


I think I'm going to start implementation of kinda version 0.1, but need
opinions about this :)
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-01-17 17:11:16 UTC
Reply
Permalink
Raw Message
https://bugzilla.gnome.org/show_bug.cgi?id=768248

--- Comment #2 from Víctor Manuel Jáquez Leal <***@igalia.com> ---
From my point of view, the "gst" way of implementing this, is creating a new
GST_EVENT_CUSTOM_DOWNSTREAM (perhaps also upstream) that the application could
inject into the pipeline with gst_element_send_event().

The event will have an GstStructure named, for example
GstVaapiEncoderRegionOfInterest, with roi-x, roi-y, roi-width, roi-height, etc.

The vaapi encoders will catch this event and will configure themselves
accordingly.

That's it, from the top of my head.
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-01-17 21:46:03 UTC
Reply
Permalink
Raw Message
https://bugzilla.gnome.org/show_bug.cgi?id=768248

Julien Isorce <***@gmail.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@gmail.com
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-01-31 08:02:43 UTC
Reply
Permalink
Raw Message
https://bugzilla.gnome.org/show_bug.cgi?id=768248

--- Comment #3 from Hyunjun Ko <***@igalia.com> ---
Created attachment 344625
--> https://bugzilla.gnome.org/attachment.cgi?id=344625&action=edit
libs: encoder/context: query region of interest support

Query Region of Interest support during config creation.
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-01-31 08:03:25 UTC
Reply
Permalink
Raw Message
https://bugzilla.gnome.org/show_bug.cgi?id=768248

--- Comment #4 from Hyunjun Ko <***@igalia.com> ---
Created attachment 344626
--> https://bugzilla.gnome.org/attachment.cgi?id=344626&action=edit
libs: encoder: add api gst_vaapi_encoder_set_roi

Implements and exposes new api gst_vaapi_encoder_set_roi to set ROI regions.
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-01-31 08:04:01 UTC
Reply
Permalink
Raw Message
https://bugzilla.gnome.org/show_bug.cgi?id=768248

--- Comment #5 from Hyunjun Ko <***@igalia.com> ---
Created attachment 344627
--> https://bugzilla.gnome.org/attachment.cgi?id=344627&action=edit
libs: encoder: h264: set ROI params during encoding

Set ROI params during encoding each frame,
which is set via gst_vaapi_encoder_set_roi
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-01-31 08:04:42 UTC
Reply
Permalink
Raw Message
https://bugzilla.gnome.org/show_bug.cgi?id=768248

--- Comment #6 from Hyunjun Ko <***@igalia.com> ---
Created attachment 344628
--> https://bugzilla.gnome.org/attachment.cgi?id=344628&action=edit
tests: simple-encoder: add an option to set ROI

Adds an option "roi" to test roi support of encoder.
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-01-31 08:09:23 UTC
Reply
Permalink
Raw Message
https://bugzilla.gnome.org/show_bug.cgi?id=768248

--- Comment #7 from Hyunjun Ko <***@igalia.com> ---
Internally, query/set ROI params are implemented in these patches.
Once these can be acceptable, then we can discuss how to provide the way to set
ROI params to users. (GstEvent, or anytihng else)

And we can go further to support for other codecs such as vp8.
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-02-17 01:47:51 UTC
Reply
Permalink
Raw Message
https://bugzilla.gnome.org/show_bug.cgi?id=768248

--- Comment #8 from Hyunjun Ko <***@igalia.com> ---
Ping.
Victor, Sree? :)
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-02-17 15:41:20 UTC
Reply
Permalink
Raw Message
https://bugzilla.gnome.org/show_bug.cgi?id=768248

Víctor Manuel Jáquez Leal <***@igalia.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #344625|none |needs-work
status| |

--- Comment #9 from Víctor Manuel Jáquez Leal <***@igalia.com> ---
Review of attachment 344625:
--> (https://bugzilla.gnome.org/review?bug=768248&attachment=344625)

::: gst-libs/gst/vaapi/gstvaapicontext.c
@@ +314,3 @@
+ GST_ERROR ("ROI unsupported - number of regions supported: %d"
+ " ROI delta QP: %d", roi_config->bits.num_roi_regions,
+ roi_config->bits.roi_rc_qp_delat_support);

I guess you should improve the error message, imo it is misleading because ROI
can be supported but not the number of requested regions. Also, I would keep
this as a warning and do not break the encoding.

::: gst-libs/gst/vaapi/gstvaapiencoder.c
@@ +573,3 @@
+ GST_INFO ("ROI unsupported - number of regions supported: %d"
+ " ROI delta QP: %d", roi_config->bits.num_roi_regions,
+ roi_config->bits.roi_rc_qp_delat_support);

If ROI is unsupported, there is no reason to print the number of regions or if
delta QP is supported, since any of they are zero.

By the way, I guess we should open a bug in libva because of the typo delat ==
delta, but that will break the API.
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-02-17 15:48:25 UTC
Reply
Permalink
Raw Message
https://bugzilla.gnome.org/show_bug.cgi?id=768248

Víctor Manuel Jáquez Leal <***@igalia.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #344626|none |needs-work
status| |

--- Comment #10 from Víctor Manuel Jáquez Leal <***@igalia.com> ---
Review of attachment 344626:
--> (https://bugzilla.gnome.org/review?bug=768248&attachment=344626)

::: gst-libs/gst/vaapi/gstvaapiencoder.c
@@ +1230,3 @@
+
+ if (!config->roi_capability)
+ return;

Turn the function into boolean to report back if it's possible to execute the
operation.

@@ +1240,3 @@
+ g_array_free (encoder->roi_regions, FALSE);
+
+ encoder->roi_regions = g_array_new (FALSE, FALSE, sizeof (GstVaapiROI));

As we know the size, we should use g_array_sized_new ()

::: gst-libs/gst/vaapi/gstvaapiencoder.h
@@ +129,3 @@
+ guint w;
+ guint h;
+} GstVaapiROI;

I would reuse the GstVaapiRectangle nested in GstVaapiROI

@@ +187,3 @@

+void
+gst_vaapi_encoder_set_roi (GstVaapiEncoder * encoder, GArray * roi_regions);

What about an API to add and delete single ROIs??

gboolean gst_vaapi_encoder_add_roi (GstVaapiEncoder * encoder, GstVaapiROI *
roi);
gboolean gst_vaapi_encoder_del_roi (GstVaapiEncoder * encoder, GstVaapiROI *
roi);
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-02-17 15:53:20 UTC
Reply
Permalink
Raw Message
https://bugzilla.gnome.org/show_bug.cgi?id=768248

Víctor Manuel Jáquez Leal <***@igalia.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #344627|none |reviewed
status| |

--- Comment #11 from Víctor Manuel Jáquez Leal <***@igalia.com> ---
Review of attachment 344627:
--> (https://bugzilla.gnome.org/review?bug=768248&attachment=344627)

lgtm
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-02-17 16:02:49 UTC
Reply
Permalink
Raw Message
https://bugzilla.gnome.org/show_bug.cgi?id=768248

Víctor Manuel Jáquez Leal <***@igalia.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #344628|none |reviewed
status| |

--- Comment #12 from Víctor Manuel Jáquez Leal <***@igalia.com> ---
Review of attachment 344628:
--> (https://bugzilla.gnome.org/review?bug=768248&attachment=344628)

lgtm

::: tests/simple-encoder.c
@@ +197,3 @@
+ }
+
+ gst_vaapi_encoder_set_roi (encoder, roi_regions);

Looking at this, I guess that the approach of {add,del}_roi is the way to go.
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-02-17 16:05:01 UTC
Reply
Permalink
Raw Message
https://bugzilla.gnome.org/show_bug.cgi?id=768248

--- Comment #13 from Víctor Manuel Jáquez Leal <***@igalia.com> ---
Review of attachment 344626:
--> (https://bugzilla.gnome.org/review?bug=768248&attachment=344626)

::: gst-libs/gst/vaapi/gstvaapiencoder.c
@@ +1227,3 @@
+ guint roi_num, i;
+
+ g_return_if_fail (roi_regions != NULL);

Is is possible to add/delete ROIs when the encoder is already processing a
stream?

If not, we should also rely on the queued_codedbuf_num
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-02-21 04:45:46 UTC
Reply
Permalink
Raw Message
https://bugzilla.gnome.org/show_bug.cgi?id=768248

--- Comment #14 from Hyunjun Ko <***@igalia.com> ---
(In reply to Víctor Manuel Jáquez Leal from comment #9)
Post by "GStreamer" (GNOME Bugzilla)
::: gst-libs/gst/vaapi/gstvaapicontext.c
@@ +314,3 @@
+ GST_ERROR ("ROI unsupported - number of regions supported: %d"
+ " ROI delta QP: %d", roi_config->bits.num_roi_regions,
+ roi_config->bits.roi_rc_qp_delat_support);
I guess you should improve the error message, imo it is misleading because
ROI can be supported but not the number of requested regions. Also, I would
keep this as a warning and do not break the encoding.
I think this should be error since the supported roi thing is already queried
and set to config(cip->config.encoder). That's why roi supported_num should be
same. If we want to update, we should turn off "static" for
cip->config.encoder, but current logic isn't aimed at it, I guess.
Post by "GStreamer" (GNOME Bugzilla)
::: gst-libs/gst/vaapi/gstvaapiencoder.c
@@ +573,3 @@
+ GST_INFO ("ROI unsupported - number of regions supported: %d"
+ " ROI delta QP: %d", roi_config->bits.num_roi_regions,
+ roi_config->bits.roi_rc_qp_delat_support);
If ROI is unsupported, there is no reason to print the number of regions or
if delta QP is supported, since any of they are zero.
Agree, I'll change it.
Post by "GStreamer" (GNOME Bugzilla)
By the way, I guess we should open a bug in libva because of the typo delat
== delta, but that will break the API.
Wow, nice catch. I'm going to raise this issue.
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-02-21 04:48:20 UTC
Reply
Permalink
Raw Message
https://bugzilla.gnome.org/show_bug.cgi?id=768248

--- Comment #15 from Hyunjun Ko <***@igalia.com> ---
(In reply to Víctor Manuel Jáquez Leal from comment #10)
Post by "GStreamer" (GNOME Bugzilla)
::: gst-libs/gst/vaapi/gstvaapiencoder.c
@@ +1230,3 @@
+
+ if (!config->roi_capability)
+ return;
Turn the function into boolean to report back if it's possible to execute
the operation.
@@ +1240,3 @@
+ g_array_free (encoder->roi_regions, FALSE);
+
+ encoder->roi_regions = g_array_new (FALSE, FALSE, sizeof (GstVaapiROI));
As we know the size, we should use g_array_sized_new ()
::: gst-libs/gst/vaapi/gstvaapiencoder.h
@@ +129,3 @@
+ guint w;
+ guint h;
+} GstVaapiROI;
I would reuse the GstVaapiRectangle nested in GstVaapiROI
@@ +187,3 @@
+void
+gst_vaapi_encoder_set_roi (GstVaapiEncoder * encoder, GArray * roi_regions);
What about an API to add and delete single ROIs??
gboolean gst_vaapi_encoder_add_roi (GstVaapiEncoder * encoder, GstVaapiROI *
roi);
gboolean gst_vaapi_encoder_del_roi (GstVaapiEncoder * encoder, GstVaapiROI *
roi);
Well, I like your approach better than mine.
Let me finish this work according to your suggestion.
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-02-21 04:50:14 UTC
Reply
Permalink
Raw Message
https://bugzilla.gnome.org/show_bug.cgi?id=768248

--- Comment #16 from Hyunjun Ko <***@igalia.com> ---
(In reply to Víctor Manuel Jáquez Leal from comment #13)
Post by "GStreamer" (GNOME Bugzilla)
::: gst-libs/gst/vaapi/gstvaapiencoder.c
@@ +1227,3 @@
+ guint roi_num, i;
+
+ g_return_if_fail (roi_regions != NULL);
Is is possible to add/delete ROIs when the encoder is already processing a
stream?
If not, we should also rely on the queued_codedbuf_num
Yes, it's possible theoretically though I haven't tested it.
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-02-23 09:57:24 UTC
Reply
Permalink
Raw Message
https://bugzilla.gnome.org/show_bug.cgi?id=768248

Hyunjun Ko <***@igalia.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #344625|needs-work |none
status| |
Attachment #344625|0 |1
is obsolete| |
Attachment #344626|needs-work |none
status| |
Attachment #344626|0 |1
is obsolete| |
Attachment #344627|reviewed |none
status| |
Attachment #344627|0 |1
is obsolete| |
Attachment #344628|reviewed |none
status| |
Attachment #344628|0 |1
is obsolete| |

--- Comment #17 from Hyunjun Ko <***@igalia.com> ---
Created attachment 346550
--> https://bugzilla.gnome.org/attachment.cgi?id=346550&action=edit
libs: encoder/context: query region of interest support

Query Region of Interest support during config creation.
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-02-23 09:57:59 UTC
Reply
Permalink
Raw Message
https://bugzilla.gnome.org/show_bug.cgi?id=768248

--- Comment #18 from Hyunjun Ko <***@igalia.com> ---
Created attachment 346551
--> https://bugzilla.gnome.org/attachment.cgi?id=346551&action=edit
libs: encoder: add api gst_vaapi_encoder_add/del_roi

Implements and exposes new api gst_vaapi_encoder_add/del_roi to set ROI
regions.
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-02-23 09:58:41 UTC
Reply
Permalink
Raw Message
https://bugzilla.gnome.org/show_bug.cgi?id=768248

--- Comment #19 from Hyunjun Ko <***@igalia.com> ---
Created attachment 346552
--> https://bugzilla.gnome.org/attachment.cgi?id=346552&action=edit
libs: encoder: h264: set ROI params during encoding

Set ROI params during encoding each frame, which is set via
gst_vaapi_encoder_add_roi
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-02-23 09:59:10 UTC
Reply
Permalink
Raw Message
https://bugzilla.gnome.org/show_bug.cgi?id=768248

--- Comment #20 from Hyunjun Ko <***@igalia.com> ---
Created attachment 346553
--> https://bugzilla.gnome.org/attachment.cgi?id=346553&action=edit
tests: simple-encoder: add an option to set ROI
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
Loading...