GNOME Bugzilla – Bug 726472
rtpbasepayload: Implement video SDP attributes
Last modified: 2015-10-02 14:45:05 UTC
This patch is an attempt to make the framerate SDP attribute defined for all video media in http://tools.ietf.org/html/rfc4566 available in GstRTPBasePayloader and therefore in all payloaders with video media. A test testing this newly introduced attribute has also been added. To complete this I believe that GstRTPBaseDepayloader must also be updated as well, but I'm waiting for opinions on this approach before making those modifications. :)
Created attachment 272085 [details] [review] Proposed patch adding the video media SDP attribute framerate to the GstRTPBasePayloader.
Review of attachment 272085 [details] [review]: ::: gst-libs/gst/rtp/gstrtpbasepayload.c @@ +416,3 @@ + g_value_init (&value, G_TYPE_DOUBLE); + g_value_set_double (&value, framerate); + gst_structure_set_value (payload->priv->video_attributes, "a-framerate", I think like all other SDP attributes, it should be string. @@ +455,3 @@ + s = gst_caps_get_structure (caps, 0); + if (g_str_has_prefix (gst_structure_get_name (s), "video")) { + parse_video_attributes (rtpbasepayload, s); Any reason to check for video here? I'd just push a-framerate for any time which has "framerate" or "max-framerate" in the caps. You can also just get it from the current caps in set_outcaps?
D'oh. That's pretty obvious. I'll fix that tonight.
Created attachment 272336 [details] [review] Proposed patch adding the video media SDP attribute framerate to the GstRTPBasePayloader. Ok, a bit late, but the patch is now updated.
Created attachment 272934 [details] [review] Proposed patch adding the video media SDP attribute framerate to the GstRTPBasePayloader. So the sinkpad need not have any caps, and if it doesn't there is no point in searching the missing caps for a media type or framerate attribute.
Created attachment 280097 [details] [review] Proposed patch adding the video media SDP attribute framerate to the GstRTPBasePayloader. Updated patch to apply on current git HEAD after implementation of basepayload caps renegotiation in May 2014.
Created attachment 280124 [details] [review] Proposed patch adding the video media SDP attribute framerate to the GstRTPBasePayloader. Fixed review comment by Olivier.
commit 042e71a117649a3559d16e790020c4e89e8a6bf9 Author: Sebastian Rasmussen <sebras@hotmail.com> Date: Sat Mar 15 17:35:56 2014 +0100 rtpbasepayload: Implement video SDP attributes Fixes https://bugzilla.gnome.org/show_bug.cgi?id=726472