Nvv4l2h264enc NACK / PLI / FIR event handling

[razvanphp]

Hello

We noticed that the orin hardware encoder does not react when Chrome sends NACK via webrtc to force sending back key unit events. When digging further we discussed with gstreamer maintainers and they told us that:

GStreamer’s RTP stack (rtpsession/rtpbin) will handle PLI (Picture Loss Indication) and FIR (Full Intra Refresh) requests from the receiver (Chrome browser) and convert them into GstForceKeyUnit events (Forcing keyframes). It is these force key unit events that an encoder is expected to handle. However, the nvv4l2*enc family of elements have never supported this interface and instead have a dedicated action signal ‘force-IDR’ for this purpose. webrtcsink (and many applications) contain a workaround for this: net/webrtc/src/webrtcsink/imp.rs · main · GStreamer / gst-plugins-rs · GitLab.

Can you please implement also handling this event name in the encoders closed source so we can only send those key frames only when needed (on lost packets)?

Thank you in advance!
R

[DaneLLL]

Hi,
We will check this but it may take some time. Since gst-v4l2 is public, would be great if you can check the source code and try to implement it. The source code is in

Jetson Linux | NVIDIA Developer
Driver Package (BSP) Sources

[razvanphp]

But… is there any nvv4l2h264enc source code in there? Did not know it’s open-source… right?

I found only some other v4l2 plugins, but not sure in which one you mean we should check.

PS: if we provide a diff with this fix, can you include it upstream in the next JP? Thanks!

[DaneLLL]

Hi,
After extracting gst-nvvideo4linux2_src.tbz2, please check the files for encoder:

source/gst-v4l2/gstv4l2videoenc.c
source/gst-v4l2/gstv4l2h264enc.c

And if you are willing to share the patch, please attach it and our teams will review it, and check whether it can be included into future releases.

[razvanphp]

Ok, I’ve implemented the native event type, tested it and it works, most of the time. Is there any limitation of how many of those keyframes can we generate in a timeframe? I noticed that at first 2-4 events, the encoder does not respond, but then it works as expected.

Any concerns about this implementation or are you willing to include it upstream?

Thanks!

diff --git a/gst-v4l2/gstv4l2videoenc.c b/gst-v4l2/gstv4l2videoenc.c
index bbf2c23..658d94a 100644
--- a/gst-v4l2/gstv4l2videoenc.c
+++ b/gst-v4l2/gstv4l2videoenc.c
@@ -44,6 +44,7 @@
 
 #include <string.h>
 #include <gst/gst-i18n-plugin.h>
+#include <gst/video/video-event.h>
 
 GST_DEBUG_CATEGORY_STATIC (gst_v4l2_video_enc_debug);
 #define GST_CAT_DEFAULT gst_v4l2_video_enc_debug
@@ -2011,6 +2012,14 @@ gst_v4l2_video_enc_sink_event (GstVideoEncoder * encoder, GstEvent * event)
     }
   }
 
+  if (GST_EVENT_TYPE (event) == GST_EVENT_CUSTOM_UPSTREAM ||
+      GST_EVENT_TYPE (event) == GST_EVENT_CUSTOM_DOWNSTREAM) {
+      if (gst_video_event_is_force_key_unit (event)) {
+        GST_WARNING_OBJECT (self, "received ForceKeyUnit %s event, forcing IDR", GST_EVENT_TYPE_NAME (event));
+        gst_v4l2_video_encoder_forceIDR (self);
+      }
+  }
+
   if ((GstNvCustomEventType)GST_EVENT_TYPE (event) == GST_NVEVENT_ENC_FORCE_IDR) {
     gchar* stream_id = NULL;
     gst_nvevent_parse_enc_force_idr (event, &stream_id, &self->force_idr);

[DaneLLL]

Hi,
Thanks for the patch. We will review it.

@razvanphp

I noticed that at first 2-4 events, the encoder does not respond, but then it works as expected.

The force-IDR call will have a few frames delay before taking effect. You may ensure it has taken effect and then do next force-IDR call.

[razvanphp]

Thank you, looking forward for your feedback on this.

I have came with a new diff after more tests, it seems this is needed for automatic handling in Chrome-webrtc-gstreamer combo.

diff --git a/gst-v4l2/gstv4l2videoenc.c b/gst-v4l2/gstv4l2videoenc.c
index bbf2c23..69ba63f 100644
--- a/gst-v4l2/gstv4l2videoenc.c
+++ b/gst-v4l2/gstv4l2videoenc.c
@@ -44,6 +44,7 @@
 
 #include <string.h>
 #include <gst/gst-i18n-plugin.h>
+#include <gst/video/video-event.h>
 
 GST_DEBUG_CATEGORY_STATIC (gst_v4l2_video_enc_debug);
 #define GST_CAT_DEFAULT gst_v4l2_video_enc_debug
@@ -1713,6 +1714,13 @@ gst_v4l2_video_enc_handle_frame (GstVideoEncoder * encoder,
   }
 
   if (frame->input_buffer) {
+
+    #ifdef USE_V4L2_TARGET_NV
+    if (GST_VIDEO_CODEC_FRAME_IS_FORCE_KEYFRAME(frame)) {
+        GST_WARNING_OBJECT (self, "received ForceKeyFrame, forcing IDR");
+        gst_v4l2_video_encoder_forceIDR (self);
+    }
+    #endif
     
     GstVideoSEIMeta *meta =
         (GstVideoSEIMeta *) gst_buffer_get_meta (frame->input_buffer,
@@ -2011,6 +2019,14 @@ gst_v4l2_video_enc_sink_event (GstVideoEncoder * encoder, GstEvent * event)
     }
   }
 
+  if (GST_EVENT_TYPE (event) == GST_EVENT_CUSTOM_UPSTREAM ||
+      GST_EVENT_TYPE (event) == GST_EVENT_CUSTOM_DOWNSTREAM) {
+      if (gst_video_event_is_force_key_unit (event)) {
+        GST_WARNING_OBJECT (self, "received ForceKeyUnit %s event, forcing IDR", GST_EVENT_TYPE_NAME (event));
+        gst_v4l2_video_encoder_forceIDR (self);
+      }
+  }
+
   if ((GstNvCustomEventType)GST_EVENT_TYPE (event) == GST_NVEVENT_ENC_FORCE_IDR) {
     gchar* stream_id = NULL;
     gst_nvevent_parse_enc_force_idr (event, &stream_id, &self->force_idr);

PS: do you think in sink_event makes sense to handle both upstream & downstream events or only downstream is ever possible there? Or we don’t need either implementation as GstVideoEncoder will already take care of handling a GstForceKeyUnit in either direction (both upstream and downstream)?

[DaneLLL]

Hi,
Could you share why the check is added into if (frame->input_buffer)? Wonder if this may make multiple gst_v4l2_video_encoder_forceIDR() call for a single GstForceKeyUnit event.

[DavidDDD]

Hi,

We are using gst-launch-1.0 to verify that the gst_v4l2_video_enc_handle_frame function is taking effect.
However, we do not observe the gst_v4l2_video_enc_sink_event patch being triggered.
How do you ensure that the gst_v4l2_video_enc_sink_event patch is taking effect?

Thanks

[razvanphp]

I also suspected that gst_v4l2_video_enc_sink_event part is not needed, but wanted your confirmation as well. My idea was that it would serve if user app land creates this custom event type manually, but you can remove this part if you consider fit.

the gst_v4l2_video_enc_handle_frame part is needed for the Chrome-webrtc Auto PLI event to work correctly out of the box without custom code, so that is what we would love to see integrated into the next release.

I am also aware that sending a force_IDR before streaming has started may result in a crash of the nvidia encoder to which not implementing the src_event handler would be much cleaner to implement.

The GstForceKeyUnit event can also contain an optional running time to queue up key frames in the future e.g. for HLS segmentation. Using the methods provided by GstVideoEncoder (through GstVideoCodecFrame) and not using a src_event handler make that scenario also become supported for free.

Thank you!
R

[DavidDDD]

Hi,

Thanks for your detail explanation.
We confirm the gst_v4l2_video_enc_sink_event part is not need, and we modify patch for proper condition.

Could you test below patch with your test case and check whether work correctly?

diff --git a/gst-v4l2/gstv4l2videoenc.c b/gst-v4l2/gstv4l2videoenc.c
index bbf2c23..69ba63f 100644
--- a/gst-v4l2/gstv4l2videoenc.c
+++ b/gst-v4l2/gstv4l2videoenc.c
@@ -44,6 +44,7 @@
 
 #include <string.h>
 #include <gst/gst-i18n-plugin.h>
+#include <gst/video/video-event.h>
 
 GST_DEBUG_CATEGORY_STATIC (gst_v4l2_video_enc_debug);
 #define GST_CAT_DEFAULT gst_v4l2_video_enc_debug
@@ -1713,6 +1714,13 @@ gst_v4l2_video_enc_handle_frame (GstVideoEncoder * encoder,
   }
 
   if (frame->input_buffer) {
+
+    #ifdef USE_V4L2_TARGET_NV
+    if (is_cuvid == FALSE && GST_VIDEO_CODEC_FRAME_IS_FORCE_KEYFRAME(frame)) {
+        GST_WARNING_OBJECT (self, "received ForceKeyFrame, forcing IDR");
+        gst_v4l2_video_encoder_forceIDR (self);
+    }
+    #endif
     
     GstVideoSEIMeta *meta =
         (GstVideoSEIMeta *) gst_buffer_get_meta (frame->input_buffer,

Thanks

[razvanphp]

I can confirm our application still reacts to the automatic PLI signals sent from Chrome/webrtc/rptpay with this code change.

Feb 07 13:41:23 orin60 python3[6575]: 0:00:34.552447655  6575 0xffff90002300 DEBUG           v4l2videoenc gstv4l2videoenc.c:1640:gst_v4l2_video_enc_handle_frame:<nvv4l2h264enc0> Handling frame 63
Feb 07 13:41:23 orin60 python3[6575]: 0:00:34.552457159  6575 0xffff90002300 WARN            v4l2videoenc gstv4l2videoenc.c:1708:gst_v4l2_video_enc_handle_frame:<nvv4l2h264enc0> received ForceKeyFrame, forcing IDR
Feb 07 13:41:23 orin60 python3[6575]: 0:00:34.552467975  6575 0xffff90002300 DEBUG           v4l2videoenc gstv4l2videoenc.c:1718:gst_v4l2_video_enc_handle_frame: NO META RETRIEVED BY ENCODER

So will you include this patch in the future releases from now on? Thank you in advance!

[DavidDDD]

@razvanphp

will you include this patch in the future releases from now on?

Hi
The patch will include the future release.
Thank you for your contribution.

Thanks