[Bug 758169] rtph264depay should translate "a-framerate (string)" attribute to "framerate (fraction)" on certain cameras

[GStreamer (GNOME Bugzilla)]

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

Olivier Crête <olivier.crete at ocrete.ca> changed:

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

--- Comment #4 from Olivier Crête <olivier.crete at ocrete.ca> ---
Review of attachment 315731:
 --> (https://bugzilla.gnome.org/review?bug=758169&attachment=315731)

Generally looks good, just a few little things.

::: gst-libs/gst/rtp/gstrtpbasedepayload.c
@@ +279,3 @@
+      return;
+  } else
+    return;

if (media == NULL || g_strcmp0 (media, "video"))
  return;

@@ +282,3 @@
+
+  src_caps = gst_pad_get_current_caps (GST_RTP_BASE_DEPAYLOAD_SRCPAD
(filter));
+  if (!src_caps)

if (!src_caps || !gst_caps_is_fixed (src_caps))
  return;

Should probably also check if framerate or max-framerate is already there in
the src caps.

@@ +285,3 @@
+    return;
+
+  a_framerate = gst_structure_get_string (caps_struct, "a-framerate");

Can you also check for x-framerate, I've heard some people mistakenly use that.

@@ +301,3 @@
+
+    fps_num = gst_value_get_fraction_numerator (&dest);
+    fps_denom = gst_value_get_fraction_denominator (&dest);

You can avoid the annoying GValue using gst_util_double_to_fraction()

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.