After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 700747 - rtsp-sdp: Parse framerate and optional framesize attribute from payloaded caps and put in SDP
rtsp-sdp: Parse framerate and optional framesize attribute from payloaded cap...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-rtsp-server
git master
Other All
: Normal enhancement
: 1.2.3
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-05-20 21:22 UTC by Sebastian Rasmussen
Modified: 2014-02-25 22:28 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed patch adding framerate to SDP. (1.87 KB, patch)
2013-05-20 21:23 UTC, Sebastian Rasmussen
none Details | Review
Proposed patch adding framesize to SDP. (1.74 KB, patch)
2013-05-20 21:23 UTC, Sebastian Rasmussen
none Details | Review
Proposed patch adding framerate to SDP. (1.87 KB, patch)
2013-05-20 21:25 UTC, Sebastian Rasmussen
none Details | Review
Proposed patch adding framesize to SDP. (1.74 KB, patch)
2013-05-20 21:25 UTC, Sebastian Rasmussen
none Details | Review
Proposed patch adding framesize and x-dimensions to SDP (2.06 KB, patch)
2013-05-22 01:45 UTC, Sebastian Rasmussen
committed Details | Review
Proposed patch adding framerate to SDP (1.94 KB, patch)
2013-05-22 01:45 UTC, Sebastian Rasmussen
committed Details | Review

Description Sebastian Rasmussen 2013-05-20 21:22:07 UTC
RFC4566 (http://tools.ietf.org/html/rfc4566) describes on page 30 in chapter 6 an SDP attribute that tells an RTSP-client what framerate to expect before the video stream is started.

RFC6064 (http://tools.ietf.org/html/rfc6064) describes on page 6 in chapter 4.2 an SDP attribute that describes to an RTSP-client the dimensions of the video stream before it is started.

The attached patches add code to parse SDP attribute values from the payloaded caps and add the relevant SDP attributes to the SDP itself. gstrtpjpegdepay already stored an SDP attribute value in a caps field named "a-framerate", so the proposed patches follows the same pattern.
Comment 1 Sebastian Rasmussen 2013-05-20 21:23:18 UTC
Created attachment 244864 [details] [review]
Proposed patch adding framerate to SDP.
Comment 2 Sebastian Rasmussen 2013-05-20 21:23:33 UTC
Created attachment 244865 [details] [review]
Proposed patch adding framesize to SDP.
Comment 3 Sebastian Rasmussen 2013-05-20 21:25:06 UTC
Created attachment 244866 [details] [review]
Proposed patch adding framerate to SDP.
Comment 4 Sebastian Rasmussen 2013-05-20 21:25:29 UTC
Created attachment 244867 [details] [review]
Proposed patch adding framesize to SDP.
Comment 5 Olivier Crête 2013-05-20 23:02:29 UTC
I'm a bit concerned about adding the framerate to the caps as a string because that makes it non-negotiable. It probably makes sense to make it a GstFraction type, since it doesn't go into rtpmap, then it can be made negotiable.

Probably the same can be said of framesize (maybe put it as width and height of type int).. Although this is a bit less problematic as it is defined as purely declarative in the RFC.
Comment 6 Sebastian Rasmussen 2013-05-21 07:58:52 UTC
> I'm a bit concerned about adding the framerate to the caps as a string because
> that makes it non-negotiable. It probably makes sense to make it a GstFraction
> type, since it doesn't go into rtpmap, then it can be made negotiable.

How about the x-dimensions already existing in rtpjpegpay/depay? I was trying to adhere to that way of doing it. Maybe this is viewed as non-negotiable which means that... 

> Probably the same can be said of framesize (maybe put it as width and height of
> type int).. Although this is a bit less problematic as it is defined as purely
> declarative in the RFC.

...one can follow this line of reasoning. I agree though that both attributes probably should be treated the same. Should I additionally do something about x-dimensions..? If I supply width/height in the caps then there is little reason to supply x-dimensions as a string..?

PS. Oh, and using strings doesn't _strictly_ mean making something non-negotiable, but it is certainly more difficult so I do get your point. ;)
Comment 7 Olivier Crête 2013-05-21 08:23:20 UTC
I guess the x-dimensions from the jpeg pay/depay ends up in the a=rtpmap, not in it's own thing?
Comment 8 Sebastian Rasmussen 2013-05-22 01:38:21 UTC
> I guess the x-dimensions from the jpeg pay/depay ends up in the a=rtpmap, not
> in it's own thing?

exactly, which is really ought to according to http://www.live555.com/openRTSP/. so I fix this on the fly.
Comment 9 Sebastian Rasmussen 2013-05-22 01:45:07 UTC
Created attachment 245008 [details] [review]
Proposed patch adding framesize and x-dimensions to SDP
Comment 10 Sebastian Rasmussen 2013-05-22 01:45:31 UTC
Created attachment 245009 [details] [review]
Proposed patch adding framerate to SDP
Comment 11 Sebastian Dröge (slomo) 2013-05-22 09:14:09 UTC
Looks both good to me, Olivier?
Comment 12 Olivier Crête 2013-05-22 09:36:15 UTC
++
Comment 13 Sebastian Dröge (slomo) 2013-05-23 19:03:22 UTC
commit d6a4dee03642a2d2c05fec4752dc3ccb60b19494
Author: Sebastian Rasmussen <sebrn@axis.com>
Date:   Wed May 22 03:29:38 2013 +0200

    rtsp-sdp: Parse framerate caps field and set SDP attribute
    
    The SDP attribute and its format is described in RFC4566.
    
    Fixes https://bugzilla.gnome.org/show_bug.cgi?id=700747

commit 5fd034ff1a517db7f629ffcc3ed16839c61f5c97
Author: Sebastian Rasmussen <sebrn@axis.com>
Date:   Wed May 22 03:29:30 2013 +0200

    rtsp-sdp: Parse width/height from caps and set SDP attribute
    
    The SDP attribute and its format is described in RFC6064.
    
    Fixes https://bugzilla.gnome.org/show_bug.cgi?id=700747
Comment 14 Wim Taymans 2013-05-31 13:45:38 UTC
Reverted but added this. This makes the framerate and framesize of the jpeg payloader appear in the SDP:

   attributes:
    attribute 'rtpmap' : '96 JPEG/90000'
    attribute 'control' : 'stream=0'
    attribute 'framerate' : '30.000000'
    attribute 'framesize' : '320-240'


commit cfdf2e6db53ece4456bd1594d66ca9f80c204c59
Author: Wim Taymans <wim.taymans@collabora.co.uk>
Date:   Fri May 31 15:41:55 2013 +0200

    rtsp: place a- and x- properties as attributes
    
    application/x-rtp has properties with a- and x- prefixes that should be
    placed as attributes in the SDP for the media instead of being added to the
    fmtp.