GNOME Bugzilla – Bug 726416
rtph263pay/-depay: add framesize SDP attribute
Last modified: 2015-04-08 09:19:57 UTC
https://bugzilla.gnome.org/show_bug.cgi?id=726415 removes the framesize attribute from the JPEG RTP payloader because http://tools.ietf.org/html/rfc6064 defines the attribute for H.263 and not for JPEG. This proposed patch adds the framesize attribute in the H.263 payloader. Please pay attention to that the format of the attribute is defined as: a=framesize:<payload type number> <width>-<height> Which includes the payload type number. This likely means that a patch is necessary in gst-rtp-server to add the payload type. I'll whip up such a patch if the change in this bug is considered good.
Created attachment 272013 [details] [review] Proposed patch correctly(?) introducing the framesize SDP attribute for the H263 pay/depay.
Oh and an additional note -- the attached patch does _not_ include <payload type number> as of right now. As I understood the discussion payloaders should not do this and this is by design. I'm not sure I understand why though (why is <payload type number> treated any different than the dash between width and height)?
I think you should include the pt in this case, I don't think it applies to generic "a-.." parameters, only to specific ones like fmtp or rtcp-fb that the SDP parser/generator knows.
(In reply to comment #3) > I think you should include the pt in this case, if the SDP parser/generator doesn't know -- absolutely , but... > I don't think it applies to generic "a-.." parameters, only to specific > ones like fmtp or rtcp-fb that the SDP parser/generator knows. ...I guess the question is rather: should the parser/generator know about the framerate attribute too?
It's h.263 specific, so I'd vote no, but Wim may have an different opinion ?
(In reply to comment #5) > It's h.263 specific, so I'd vote no, but Wim may have an different opinion ? do I read you right if I assume that you'd vote no to that the rtsp-server should prepend pt..? how about the concept of adding the attribute to the H.263 payloader at all?
I was saying that if it's a h.263 specific attribute, then it should be handled in the h263 element ideally and the other code should just treat it generically.
Created attachment 280099 [details] [review] Proposed patch introducing H263-specific framesize SDP attribute for the H263 pay/depay. Update format after Oliviers comments. Also attempted to improve the parsing somewhat.
Created attachment 300803 [details] [review] Proposed patch introducing H263-specific framesize SDP attribute for the H263 pay/depay. Amended patch now that http://cgit.freedesktop.org/gstreamer/gst-rtsp-server/commit/?id=9dadaed2fd598a0abe146d042177086fdbeaed4a has been accepted.
(didn't realize the Fixes in from of HTTP breaks git-bz) commit cf54d4cc67e687605aba05e5d184cf381d768593 Author: Sebastian Rasmussen <sebras@hotmail.com> Date: Sat Mar 15 15:23:01 2014 +0100 rtph263pay/-depay: add framesize SDP attribute Fixes https://bugzilla.gnome.org/show_bug.cgi?id=726416
This breaks the elements/rtp-payloading unit test (not-negotiated).
Created attachment 301103 [details] [review] gstrtph263depay: fix to parse framesize SDP attribute and succeed in elements/rtp-payloading unit test
Pushed a saner fix commit 5e0329235e6e0fb8ab849b68ff583546b0d940a7 Author: Edward Hervey <bilboed@bilboed.com> Date: Wed Apr 8 11:17:31 2015 +0200 rtph263depay: Fix framesize parsing The string passed to the parsing function only contains a framesize, and not <pt> + <framesize> Fixes https://bugzilla.gnome.org/show_bug.cgi?id=726416