rtpjpegpay/depay: Replace framerate caps field with fraction

The previous implementation had the formatting of SDP attributes happen
in each RTP payloader, now instead the constituent values are propagated
as caps fields. This allows for applications to do SDP offer/answer
based on caps negotiation.

Fixes https://bugzilla.gnome.org/show_bug.cgi?id=700748

diff --git a/gst/rtp/gstrtpjpegdepay.c b/gst/rtp/gstrtpjpegdepay.c
index 0dbc736..0ba9c40 100644 (file)
--- a/gst/rtp/gstrtpjpegdepay.c
+++ b/gst/rtp/gstrtpjpegdepay.c
@@ -41,6 +41,8 @@ GST_STATIC_PAD_TEMPLATE ("src",
         /*
          * "width = (int) 0, "
          * "height = (int) 0, "
+         * "framerate = (fraction) 0/1, "
+         * "x-dimensions = (string) "0\,0", "
          */
     );
 
@@ -56,6 +58,7 @@ static GstStaticPadTemplate gst_rtp_jpeg_depay_sink_template =
         /*
          * "width = (int) 0, "
          * "height = (int) 0, "
+         * "framerate = (fraction) 0/1, "
          * "a-framerate = (string) 0.00, "
          * "x-framerate = (string) 0.00, "
          * "x-dimensions = (string) "0\,0", "
@@ -68,6 +71,7 @@ static GstStaticPadTemplate gst_rtp_jpeg_depay_sink_template =
         /*
          * "width = (int) 0, "
          * "height = (int) 0, "
+         * "framerate = (fraction) 0/1, "
          * "a-framerate = (string) 0.00, "
          * "x-framerate = (string) 0.00, "
          * "x-dimensions = (string) "0\,0", "
@@ -439,6 +443,7 @@ gst_rtp_jpeg_depay_setcaps (GstRTPBaseDepayload * depayload, GstCaps * caps)
   gint clock_rate;
   const gchar *media_attr;
   gint width = 0, height = 0;
+  gint num = 0, denom = 1;
 
   rtpjpegdepay = GST_RTP_JPEG_DEPAY (depayload);
 
@@ -449,14 +454,6 @@ gst_rtp_jpeg_depay_setcaps (GstRTPBaseDepayload * depayload, GstCaps * caps)
     clock_rate = 90000;
   depayload->clock_rate = clock_rate;
 
-  /* reset defaults */
-  rtpjpegdepay->width = 0;
-  rtpjpegdepay->height = 0;
-  rtpjpegdepay->media_width = 0;
-  rtpjpegdepay->media_height = 0;
-  rtpjpegdepay->frate_num = 0;
-  rtpjpegdepay->frate_denom = 1;
-
   /* check for optional SDP attributes */
   if ((media_attr = gst_structure_get_string (structure, "x-dimensions"))) {
     if (sscanf (media_attr, "%d,%d", &width, &height) != 2 || width <= 0 ||
@@ -478,9 +475,8 @@ gst_rtp_jpeg_depay_setcaps (GstRTPBaseDepayload * depayload, GstCaps * caps)
     media_attr = gst_structure_get_string (structure, "x-framerate");
 
   if (media_attr) {
-    GValue src = { 0 };
-    GValue dest = { 0 };
     gchar *s;
+    gdouble rate;
 
     /* canonicalise floating point string so we can handle framerate strings
      * in the form "24.930" or "24,930" irrespective of the current locale */
@@ -488,19 +484,25 @@ gst_rtp_jpeg_depay_setcaps (GstRTPBaseDepayload * depayload, GstCaps * caps)
     g_strdelimit (s, ",", '.');
 
     /* convert the float to a fraction */
-    g_value_init (&src, G_TYPE_DOUBLE);
-    g_value_set_double (&src, g_ascii_strtod (s, NULL));
-    g_value_init (&dest, GST_TYPE_FRACTION);
-    g_value_transform (&src, &dest);
-
-    rtpjpegdepay->frate_num = gst_value_get_fraction_numerator (&dest);
-    rtpjpegdepay->frate_denom = gst_value_get_fraction_denominator (&dest);
-
+    rate = g_ascii_strtod (s, NULL);
+    gst_util_double_to_fraction (rate, &num, &denom);
     g_free (s);
+    if (num < 0 || denom <= 0) {
+      goto invalid_framerate;
+    }
+  }
+
+  if (gst_structure_get_fraction (structure, "framerate", &num, &denom) &&
+      (num < 0 || denom <= 0)) {
+    goto invalid_framerate;
   }
 
+  rtpjpegdepay->width = 0;
+  rtpjpegdepay->height = 0;
   rtpjpegdepay->media_width = width;
   rtpjpegdepay->media_height = height;
+  rtpjpegdepay->frate_num = num;
+  rtpjpegdepay->frate_denom = denom;
 
   return TRUE;
 
@@ -509,6 +511,11 @@ invalid_dimension:
     GST_ERROR_OBJECT (rtpjpegdepay, "invalid width/height from caps");
     return FALSE;
   }
+invalid_framerate:
+  {
+    GST_ERROR_OBJECT (rtpjpegdepay, "invalid framerate from caps");
+    return FALSE;
+  }
 }
 
 static GstBuffer *
@@ -643,15 +650,18 @@ gst_rtp_jpeg_depay_process (GstRTPBaseDepayload * depayload, GstBuffer * buf)
     if (rtpjpegdepay->width != width || rtpjpegdepay->height != height) {
       GstCaps *outcaps;
 
-      outcaps =
-          gst_caps_new_simple ("image/jpeg", "framerate", GST_TYPE_FRACTION,
-          rtpjpegdepay->frate_num, rtpjpegdepay->frate_denom, NULL);
+      outcaps = gst_caps_new_empty_simple ("image/jpeg");
 
       if (width > 0 && height > 0) {
         gst_caps_set_simple (outcaps, "width", G_TYPE_INT, width, "height",
             G_TYPE_INT, height, NULL);
       }
 
+      if (rtpjpegdepay->frate_num > 0) {
+        gst_caps_set_simple (outcaps, "framerate", GST_TYPE_FRACTION,
+            rtpjpegdepay->frate_num, rtpjpegdepay->frate_denom, NULL);
+      }
+
       gst_pad_set_caps (depayload->srcpad, outcaps);
       gst_caps_unref (outcaps);
 

diff --git a/gst/rtp/gstrtpjpegpay.c b/gst/rtp/gstrtpjpegpay.c
index bea26e6..0ff684b 100644 (file)
--- a/gst/rtp/gstrtpjpegpay.c
+++ b/gst/rtp/gstrtpjpegpay.c
@@ -48,9 +48,18 @@ static GstStaticPadTemplate gst_rtp_jpeg_pay_sink_template =
     GST_STATIC_CAPS ("image/jpeg, "
         "  width = (int) [ 1, MAX ], "
         "  height = (int) [ 1, MAX ]; "
+        /* optional SDP attributes */
+        /*
+         * "framerate = (fraction) [ 0/1, MAX/1 ], "
+         */
         "  video/x-jpeg, "
         "  width = (int) [ 1, MAX ], "
-        "  height = (int) [ 1, MAX ]")
+        "  height = (int) [ 1, MAX ]"
+        /* optional SDP attributes */
+        /*
+         * "framerate = (fraction) [ 0/1, MAX/1 ] "
+         */
+      )
     );
 
 static GstStaticPadTemplate gst_rtp_jpeg_pay_src_template =
@@ -299,8 +308,7 @@ gst_rtp_jpeg_pay_setcaps (GstRTPBasePayload * basepayload, GstCaps * caps)
   GstRtpJPEGPay *pay;
   gboolean res;
   gint width, height;
-  gint num = 0, denom;
-  gchar *rate = NULL;
+  gint num = 0, denom = 1;
 
   pay = GST_RTP_JPEG_PAY (basepayload);
 
@@ -331,25 +339,15 @@ gst_rtp_jpeg_pay_setcaps (GstRTPBasePayload * basepayload, GstCaps * caps)
 
   gst_rtp_base_payload_set_options (basepayload, "video", TRUE, "JPEG", 90000);
 
-  if (num > 0)
-  {
-    gdouble framerate;
-    gst_util_fraction_to_double (num, denom, &framerate);
-    rate = g_strdup_printf("%f", framerate);
-  }
-
-  if (rate != NULL) {
+  if (num > 0) {
     res = gst_rtp_base_payload_set_outcaps (basepayload, "width", G_TYPE_INT,
-        width, "height", G_TYPE_INT, height, "a-framerate", G_TYPE_STRING,
-        rate, NULL);
-  } else if (rate == NULL) {
+        width, "height", G_TYPE_INT, height, "framerate", GST_TYPE_FRACTION,
+        num, denom, NULL);
+  } else {
     res = gst_rtp_base_payload_set_outcaps (basepayload, "width",
         G_TYPE_INT, width, "height", G_TYPE_INT, height, NULL);
   }
 
-  if (rate != NULL)
-    g_free (rate);
-
   return res;
 
   /* ERRORS */

diff --git a/tests/check/elements/rtp-payloading.c b/tests/check/elements/rtp-payloading.c
index 3de1f91..b1dcc2c 100644 (file)
--- a/tests/check/elements/rtp-payloading.c
+++ b/tests/check/elements/rtp-payloading.c
@@ -781,8 +781,8 @@ static int rtp_jpeg_frame_count = 1;
 GST_START_TEST (rtp_jpeg)
 {
   rtp_pipeline_test (rtp_jpeg_frame_data, rtp_jpeg_frame_data_size,
-      rtp_jpeg_frame_count, "video/x-jpeg,height=640,width=480", "rtpjpegpay",
-      "rtpjpegdepay", 0, 0, FALSE);
+      rtp_jpeg_frame_count, "video/x-jpeg,height=640,width=480,framerate=30/1",
+      "rtpjpegpay", "rtpjpegdepay", 0, 0, FALSE);
 }
 
 GST_END_TEST;