rtph26[45]pay: Change default aggregate-mode to "none" for backwards compatibility

We didn't aggregate at all in previous versions and there are apparently
various RTP implementations that don't handle aggregation well at all.

As part of this also document that for RTSP it is recommended to keep it
set to "none" while for WebRTC it should be set to "zero-latency".

Fixes https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/issues/749

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/merge_requests/692>

diff --git a/docs/gst_plugins_cache.json b/docs/gst_plugins_cache.json
index 9f6dcd1..8ab2456 100644 (file)
--- a/docs/gst_plugins_cache.json
+++ b/docs/gst_plugins_cache.json
@@ -14809,12 +14809,12 @@
                 },
                 "properties": {
                     "aggregate-mode": {
-                        "blurb": "Bundle suitable SPS/PPS NAL units into STAP-A aggregate packets. ",
+                        "blurb": "Bundle suitable SPS/PPS NAL units into STAP-A aggregate packets",
                         "conditionally-available": false,
                         "construct": false,
                         "construct-only": false,
                         "controllable": false,
-                        "default": "zero-latency (1)",
+                        "default": "none (0)",
                         "mutable": "null",
                         "readable": true,
                         "type": "GstRtpH264AggregateMode",
@@ -14909,7 +14909,7 @@
                         "construct": false,
                         "construct-only": false,
                         "controllable": false,
-                        "default": "zero-latency (1)",
+                        "default": "none (0)",
                         "mutable": "null",
                         "readable": true,
                         "type": "GstRtpH265AggregateMode",

diff --git a/gst/rtp/gstrtph264pay.c b/gst/rtp/gstrtph264pay.c
index 01a3659..6735328 100644 (file)
--- a/gst/rtp/gstrtph264pay.c
+++ b/gst/rtp/gstrtph264pay.c
@@ -99,7 +99,7 @@ GST_STATIC_PAD_TEMPLATE ("src",
 
 #define DEFAULT_SPROP_PARAMETER_SETS    NULL
 #define DEFAULT_CONFIG_INTERVAL         0
-#define DEFAULT_AGGREGATE_MODE          GST_RTP_H264_AGGREGATE_ZERO_LATENCY
+#define DEFAULT_AGGREGATE_MODE          GST_RTP_H264_AGGREGATE_NONE
 
 enum
 {
@@ -167,12 +167,25 @@ gst_rtp_h264_pay_class_init (GstRtpH264PayClass * klass)
           G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS)
       );
 
+  /**
+   * GstRtpH264Pay:aggregate-mode
+   *
+   * Bundle suitable SPS/PPS NAL units into STAP-A aggregate packets.
+   *
+   * This can potentially reduce RTP packetization overhead but not all
+   * RTP implementations handle it correctly.
+   *
+   * For best compatibility, it is recommended to set this to "none" (the
+   * default) for RTSP and for WebRTC to "zero-latency".
+   *
+   * Since: 1.18
+   */
   g_object_class_install_property (G_OBJECT_CLASS (klass),
       PROP_AGGREGATE_MODE,
       g_param_spec_enum ("aggregate-mode",
           "Attempt to use aggregate packets",
           "Bundle suitable SPS/PPS NAL units into STAP-A "
-          "aggregate packets. ",
+          "aggregate packets",
           GST_TYPE_RTP_H264_AGGREGATE_MODE,
           DEFAULT_AGGREGATE_MODE, G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS)
       );

diff --git a/gst/rtp/gstrtph265pay.c b/gst/rtp/gstrtph265pay.c
index fc11e60..3793ad6 100644 (file)
--- a/gst/rtp/gstrtph265pay.c
+++ b/gst/rtp/gstrtph265pay.c
@@ -133,7 +133,7 @@ GST_STATIC_PAD_TEMPLATE ("src",
     );
 
 #define DEFAULT_CONFIG_INTERVAL         0
-#define DEFAULT_AGGREGATE_MODE          GST_RTP_H265_AGGREGATE_ZERO_LATENCY
+#define DEFAULT_AGGREGATE_MODE          GST_RTP_H265_AGGREGATE_NONE
 
 enum
 {
@@ -192,6 +192,19 @@ gst_rtp_h265_pay_class_init (GstRtpH265PayClass * klass)
           G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS)
       );
 
+  /**
+   * GstRtpH265Pay:aggregate-mode
+   *
+   * Bundle suitable SPS/PPS NAL units into STAP-A aggregate packets.
+   *
+   * This can potentially reduce RTP packetization overhead but not all
+   * RTP implementations handle it correctly.
+   *
+   * For best compatibility, it is recommended to set this to "none" (the
+   * default) for RTSP and for WebRTC to "zero-latency".
+   *
+   * Since: 1.18
+   */
   g_object_class_install_property (G_OBJECT_CLASS (klass),
       PROP_AGGREGATE_MODE,
       g_param_spec_enum ("aggregate-mode",

diff --git a/tests/check/elements/rtp-payloading.c b/tests/check/elements/rtp-payloading.c
index 2285e71..c7f4977 100644 (file)
--- a/tests/check/elements/rtp-payloading.c
+++ b/tests/check/elements/rtp-payloading.c
@@ -180,8 +180,12 @@ rtp_pipeline_create (const guint8 * frame_data, int frame_data_size,
   p->pipeline = gst_pipeline_new (pipeline_name);
   g_free (pipeline_name);
   p->appsrc = gst_element_factory_make ("appsrc", NULL);
-  p->rtppay = gst_element_factory_make (pay, NULL);
-  p->rtpdepay = gst_element_factory_make (depay, NULL);
+  p->rtppay =
+      gst_parse_bin_from_description_full (pay, TRUE, NULL,
+      GST_PARSE_FLAG_NO_SINGLE_ELEMENT_BINS, NULL);
+  p->rtpdepay =
+      gst_parse_bin_from_description_full (depay, TRUE, NULL,
+      GST_PARSE_FLAG_NO_SINGLE_ELEMENT_BINS, NULL);
   p->fakesink = gst_element_factory_make ("fakesink", NULL);
 
   /* One or more elements are not created successfully or failed to create p? */
@@ -867,7 +871,7 @@ GST_START_TEST (rtp_h264_list_lt_mtu)
   rtp_pipeline_test (rtp_h264_list_lt_mtu_frame_data,
       rtp_h264_list_lt_mtu_frame_data_size, rtp_h264_list_lt_mtu_frame_count,
       "video/x-h264,stream-format=(string)byte-stream,alignment=(string)nal",
-      "rtph264pay", "rtph264depay",
+      "rtph264pay aggregate-mode=zero-latency", "rtph264depay",
       rtp_h264_list_lt_mtu_bytes_sent, rtp_h264_list_lt_mtu_mtu_size, TRUE);
 }
 
@@ -893,7 +897,7 @@ GST_START_TEST (rtp_h264_list_lt_mtu_avc)
       rtp_h264_list_lt_mtu_frame_data_size, rtp_h264_list_lt_mtu_frame_count,
       "video/x-h264,stream-format=(string)avc,alignment=(string)au,"
       "codec_data=(buffer)01640014ffe1001867640014acd94141fb0110000003001773594000f142996001000568ebecb22c",
-      "rtph264pay", "rtph264depay",
+      "rtph264pay aggregate-mode=zero-latency", "rtph264depay",
       rtp_h264_list_lt_mtu_bytes_sent_avc, rtp_h264_list_lt_mtu_mtu_size, TRUE);
 }
 
@@ -1045,8 +1049,9 @@ GST_START_TEST (rtp_h265_list_lt_mtu_hvc1)
       "01840010c01ffff01c000000300800000030000030099ac0900a10001003042010101c00"
       "0000300800000030000030099a00a080f1fe36bbb5377725d602dc040404100000300010"
       "00003000a0800a2000100074401c172b02240",
-      "rtph265pay", "rtph265depay", rtp_h265_list_lt_mtu_bytes_sent_hvc1,
-      rtp_h265_list_lt_mtu_mtu_size, TRUE);
+      "rtph265pay aggregate-mode=zero-latency", "rtph265depay",
+      rtp_h265_list_lt_mtu_bytes_sent_hvc1, rtp_h265_list_lt_mtu_mtu_size,
+      TRUE);
 }
 
 GST_END_TEST;

diff --git a/tests/check/elements/rtph264.c b/tests/check/elements/rtph264.c
index 2d7e877..10c350c 100644 (file)
--- a/tests/check/elements/rtph264.c
+++ b/tests/check/elements/rtph264.c
@@ -675,7 +675,8 @@ GST_END_TEST;
 
 GST_START_TEST (test_rtph264pay_two_slices_timestamp)
 {
-  GstHarness *h = gst_harness_new_parse ("rtph264pay timestamp-offset=123");
+  GstHarness *h = gst_harness_new_parse ("rtph264pay timestamp-offset=123"
+      " aggregate-mode=zero-latency");
   GstFlowReturn ret;
   GstBuffer *buffer;
   GstRTPBuffer rtp = GST_RTP_BUFFER_INIT;
@@ -736,7 +737,8 @@ GST_END_TEST;
 
 GST_START_TEST (test_rtph264pay_marker_for_flag)
 {
-  GstHarness *h = gst_harness_new_parse ("rtph264pay timestamp-offset=123");
+  GstHarness *h = gst_harness_new_parse ("rtph264pay timestamp-offset=123"
+      " aggregate-mode=zero-latency");
   GstFlowReturn ret;
   GstBuffer *buffer;
   GstRTPBuffer rtp = GST_RTP_BUFFER_INIT;
@@ -815,7 +817,8 @@ GST_END_TEST;
 GST_START_TEST (test_rtph264pay_marker_for_fragmented_au)
 {
   GstHarness *h =
-      gst_harness_new_parse ("rtph264pay timestamp-offset=123 mtu=40");
+      gst_harness_new_parse ("rtph264pay timestamp-offset=123 mtu=40"
+      " aggregate-mode=zero-latency");
   GstFlowReturn ret;
   GstBuffer *slice1, *slice2, *buffer;
   GstRTPBuffer rtp = GST_RTP_BUFFER_INIT;
@@ -865,8 +868,7 @@ GST_START_TEST (test_rtph264pay_aggregate_two_slices_per_buffer)
   gst_harness_set_src_caps_str (h,
       "video/x-h264,alignment=nal,stream-format=byte-stream");
 
-  /* No aggregation latency mode */
-
+  /* No aggregation mode */
   g_object_set (e, "aggregate-mode", 0, NULL);
 
   buffer = wrap_static_buffer_with_pts (h264_idr_slice_1,
@@ -979,7 +981,8 @@ GST_END_TEST;
 
 GST_START_TEST (test_rtph264pay_aggregate_with_aud)
 {
-  GstHarness *h = gst_harness_new_parse ("rtph264pay timestamp-offset=123");
+  GstHarness *h = gst_harness_new_parse ("rtph264pay timestamp-offset=123"
+      " aggregate-mode=zero-latency");
   GstFlowReturn ret;
   GstBuffer *buffer;
   GstRTPBuffer rtp = GST_RTP_BUFFER_INIT;
@@ -1090,7 +1093,8 @@ GST_END_TEST;
 
 GST_START_TEST (test_rtph264pay_aggregate_with_discont)
 {
-  GstHarness *h = gst_harness_new_parse ("rtph264pay timestamp-offset=123");
+  GstHarness *h = gst_harness_new_parse ("rtph264pay timestamp-offset=123"
+      " aggregate-mode=zero-latency");
   GstFlowReturn ret;
   GstBuffer *buffer;
   GstRTPBuffer rtp = GST_RTP_BUFFER_INIT;
@@ -1148,7 +1152,7 @@ GST_END_TEST;
 GST_START_TEST (test_rtph264pay_aggregate_until_vcl)
 {
   GstHarness *h = gst_harness_new_parse ("rtph264pay timestamp-offset=123"
-      " name=p");
+      " name=p aggregate-mode=zero-latency");
   GstFlowReturn ret;
   GstBuffer *buffer;
   GstRTPBuffer rtp = GST_RTP_BUFFER_INIT;
@@ -1190,7 +1194,8 @@ GST_END_TEST;
 
 GST_START_TEST (test_rtph264pay_avc)
 {
-  GstHarness *h = gst_harness_new_parse ("rtph264pay timestamp-offset=123");
+  GstHarness *h = gst_harness_new_parse ("rtph264pay timestamp-offset=123"
+      " aggregate-mode=zero-latency");
   GstFlowReturn ret;
   GstBuffer *buffer;
   GstRTPBuffer rtp = GST_RTP_BUFFER_INIT;
@@ -1251,7 +1256,8 @@ GST_END_TEST;
 static void
 test_rtph264pay_avc_two_slices (gsize memory1_len, guint num_slices)
 {
-  GstHarness *h = gst_harness_new_parse ("rtph264pay timestamp-offset=123");
+  GstHarness *h = gst_harness_new_parse ("rtph264pay timestamp-offset=123"
+      " aggregate-mode=zero-latency");
   GstFlowReturn ret;
   GstBuffer *slice1;
   GstBuffer *slice2;

diff --git a/tests/check/elements/rtph265.c b/tests/check/elements/rtph265.c
index 2c2ff3c..2381808 100644 (file)
--- a/tests/check/elements/rtph265.c
+++ b/tests/check/elements/rtph265.c
@@ -463,7 +463,8 @@ static guint8 h265_idr_slice_2[] = {
 
 GST_START_TEST (test_rtph265pay_two_slices_timestamp)
 {
-  GstHarness *h = gst_harness_new_parse ("rtph265pay timestamp-offset=123");
+  GstHarness *h = gst_harness_new_parse ("rtph265pay timestamp-offset=123"
+      " aggregate-mode=zero-latency");
   GstFlowReturn ret;
   GstBuffer *buffer;
   GstRTPBuffer rtp = GST_RTP_BUFFER_INIT;
@@ -524,7 +525,8 @@ GST_END_TEST;
 
 GST_START_TEST (test_rtph265pay_marker_for_flag)
 {
-  GstHarness *h = gst_harness_new_parse ("rtph265pay timestamp-offset=123");
+  GstHarness *h = gst_harness_new_parse ("rtph265pay timestamp-offset=123"
+      " aggregate-mode=zero-latency");
   GstFlowReturn ret;
   GstBuffer *buffer;
   GstRTPBuffer rtp = GST_RTP_BUFFER_INIT;
@@ -602,7 +604,8 @@ GST_END_TEST;
 GST_START_TEST (test_rtph265pay_marker_for_fragmented_au)
 {
   GstHarness *h =
-      gst_harness_new_parse ("rtph265pay timestamp-offset=123 mtu=40");
+      gst_harness_new_parse ("rtph265pay timestamp-offset=123 mtu=40"
+      " aggregate-mode=zero-latency");
   GstFlowReturn ret;
   GstBuffer *slice1, *slice2, *buffer;
   GstRTPBuffer rtp = GST_RTP_BUFFER_INIT;
@@ -770,7 +773,8 @@ static guint8 h265_aud[] = {
 
 GST_START_TEST (test_rtph265pay_aggregate_with_aud)
 {
-  GstHarness *h = gst_harness_new_parse ("rtph265pay timestamp-offset=123");
+  GstHarness *h = gst_harness_new_parse ("rtph265pay timestamp-offset=123"
+      " aggregate-mode=zero-latency");
   GstFlowReturn ret;
   GstBuffer *buffer;
   GstRTPBuffer rtp = GST_RTP_BUFFER_INIT;
@@ -877,7 +881,8 @@ GST_END_TEST;
 
 GST_START_TEST (test_rtph265pay_aggregate_with_discont)
 {
-  GstHarness *h = gst_harness_new_parse ("rtph265pay timestamp-offset=123");
+  GstHarness *h = gst_harness_new_parse ("rtph265pay timestamp-offset=123"
+      " aggregate-mode=zero-latency");
   GstFlowReturn ret;
   GstBuffer *buffer;
   GstRTPBuffer rtp = GST_RTP_BUFFER_INIT;
@@ -939,7 +944,7 @@ static guint8 h265_eos[] = {
 GST_START_TEST (test_rtph265pay_aggregate_until_vcl)
 {
   GstHarness *h = gst_harness_new_parse ("rtph265pay timestamp-offset=123"
-      " name=p");
+      " name=p aggregate-mode=zero-latency");
   GstFlowReturn ret;
   GstBuffer *buffer;
   GstRTPBuffer rtp = GST_RTP_BUFFER_INIT;
@@ -1004,7 +1009,7 @@ GST_END_TEST;
 GST_START_TEST (test_rtph265pay_aggregate_verify_nalu_hdr)
 {
   GstHarness *h = gst_harness_new_parse ("rtph265pay timestamp-offset=123"
-      " name=p");
+      " name=p aggregate-mode=zero-latency");
   GstFlowReturn ret;
   GstBuffer *buffer;
   GstRTPBuffer rtp = GST_RTP_BUFFER_INIT;