[issue2270] RTP parser fails on RFC 3550 header extensions
Martin Storsjö mar...@martin.st added the comment: I looked at the packet dump, and the patch looks quite ok to me. I touched it up a little, here's another version of it. I'll apply it in a few days if nobody objects. -- nosy: +mstorsjo FFmpeg issue tracker iss...@roundup.ffmpeg.org https://roundup.ffmpeg.org/issue2270 0001-rtpdec-Handle-RTP-header-extension.patch Description: Binary data
[issue2270] RTP parser fails on RFC 3550 header extensions
Ronald S. Bultje rsbul...@gmail.com added the comment:
+/* RFC 3550 Section 5.3.1 RTP Header Extension handling */
+if (ext) {
+if (len 4)
+return -1;
+// retrieve header extension length (number of 32-bit words)
+ext = AV_RB16(buf + 2);
+
+// add header extension itself to extension length
+ext++;
+
+// convert header extension length to bytes
+ext = 2;
ext = (AV_RB16(..) + 1) 2;, this isn't barbie-code.
+// abort if extension length exceeds remaining buffer length
+if (len ext)
+return -1;
+
+// skip past RTP header extension
+len -= ext;
+buf += ext;
+}
Rest OK.
FFmpeg issue tracker iss...@roundup.ffmpeg.org
https://roundup.ffmpeg.org/issue2270
[issue2270] RTP parser fails on RFC 3550 header extensions
Martin Storsjö mar...@martin.st added the comment: Applied in SVN rev 25372. -- status: open - closed substatus: needs_changes - implemented FFmpeg issue tracker iss...@roundup.ffmpeg.org https://roundup.ffmpeg.org/issue2270
[issue2270] RTP parser fails on RFC 3550 header extensions
Carl Eugen Hoyos ceho...@rainbow.studorg.tuwien.ac.at added the comment: Please produce your patch with svn diff, use tools/patcheck to test and attach it to this issue. -- status: new - open substatus: analyzed - needs_changes FFmpeg issue tracker iss...@roundup.ffmpeg.org https://roundup.ffmpeg.org/issue2270
[issue2270] RTP parser fails on RFC 3550 header extensions
Robert Schlabbach rober...@gmx.net added the comment:
Here is the SVN diff patch, tested with patcheck,
which only complained about a missing changelog entry.
But since the source file had no changelog entries at
all, I suppose that's ok?
FFmpeg issue tracker iss...@roundup.ffmpeg.org
https://roundup.ffmpeg.org/issue2270
Index: ffmpeg/libavformat/rtpdec.c
===
--- ffmpeg/libavformat/rtpdec.c (.../svn://svn.ffmpeg.org/ffmpeg/trunk)
(revision 25334)
+++ ffmpeg/libavformat/rtpdec.c (.../ffmpeg)(working copy)
@@ -416,6 +416,7 @@
{
unsigned int ssrc, h;
int payload_type, seq, ret, flags = 0;
+int ext;
AVStream *st;
uint32_t timestamp;
int rv= 0;
@@ -443,9 +444,36 @@
}
s-seq = seq;
+
+/* RFC 3550 Section 5.3.1 RTP Header Extension handling */
+
+// store RTP header extension bit
+ext = buf[0] 0x10;
+
+// skip past RTP header
len -= 12;
buf += 12;
+// check for RTP header extension
+if (ext) {
+// retrieve header extension length (number of 32-bit words)
+ext = AV_RB16(buf + 2);
+
+// add header extension itself to extension length
+ext++;
+
+// convert header extension length to bytes
+ext = 2;
+
+// abort if extension length exceeds remaining buffer length
+if (len ext)
+return -1;
+
+// skip past RTP header extension
+len -= ext;
+buf += ext;
+};
+
if (!st) {
/* specific MPEG2TS demux support */
ret = ff_mpegts_parse_packet(s-ts, pkt, buf, len);
[issue2270] RTP parser fails on RFC 3550 header extensions
Luca Barbato lu_z...@gentoo.org added the comment: hi, the patch looks more or less ok (just a }; that should be } as Martin noted on irc), do you have test samples for it? what's the producer? FFmpeg issue tracker iss...@roundup.ffmpeg.org https://roundup.ffmpeg.org/issue2270
[issue2270] RTP parser fails on RFC 3550 header extensions
Robert Schlabbach rober...@gmx.net added the comment: The RTP/UDP multicast source which exposed this problem is Deutsche Telekom's IPTV service (T-Home Entertain) in Germany. If you happen to have a developer amongst you who has this service, you can reproduces this problem e.g. with: ffplay rtp://@239.35.129.11:1 or ffplay rtp://@239.35.10.1:1 These two commands play the German TV station Das Erste and Das Erste HD, respectively, and exhibit frequent video and audio glitches with the current ffmpeg code. If you don't have anyone with DT's IPTV service, how can I submit a sample? Submitting a sample of the contained MPEG-2 Transport Stream would be pointless, as the issue is with the RTP encapsulation ;) Would a Wireshark capture of the RTP multicast packages be sufficient? I don't know how you'd be able to play that back, though... FFmpeg issue tracker iss...@roundup.ffmpeg.org https://roundup.ffmpeg.org/issue2270
[issue2270] RTP parser fails on RFC 3550 header extensions
Robert Schlabbach rober...@gmx.net added the comment: Ok, here is a single RTP/UDP multicast packet showing a RTP header extension of 8 32-bit words. Plus the 4 bytes of the header extension itself yields 36 bytes to be skipped by the RTP decoder. Wireshark does not autodetect it as RTP, so you have to right-click the packet, select Decode As... and then RTP as the protocol. FFmpeg issue tracker iss...@roundup.ffmpeg.org https://roundup.ffmpeg.org/issue2270 RTP_Header_Extension_Sample.cap Description: Binary data
[issue2270] RTP parser fails on RFC 3550 header extensions
New submission from Robert Schlabbach rober...@gmx.net:
The RTP parser in libavformat/rtpdec.c, in the
function rtp_parse_packet_internal() does not check
the RTP header extension bit and neglects to skip
header extensions according to RFC 3550 chapter 5.3.1.
This results e.g. in MPEG-2 Transport Stream parsing
errors if the header extension should contain the MPEG-
2 synchronization byte (0x47).
Here is a suggested (untested) patch for this
shortcoming, to be inserted at this point:
445 s-seq = seq;
446 len -= 12;
447 buf += 12;
Change this to:
s-seq = seq;
{
// declare extension flag/length
variable
int ext;
// store RTP header extension bit
ext = buf[0] 0x10;
// skip past standard RTP header
len -= 12;
buf += 12;
// handle RTP header extensions (RFC
3550 chapter 5.3.1)
if (ext)
{
// retrieve header extension
length (in 32-bit words)
ext = AV_RB16(buf + 2);
// add header extension itself
to extension length
ext++;
// convert header extension
length to bytes
ext = 2;
// abort if extension length
exceeds remaining buffer length
if (len ext)
return -1;
// skip header extension
len -= ext;
buf += ext;
};
}
The declaration of the ext variable can actually be
moved to the other variable declarations of this
function, allowing the extra bracket level to be
removed.
--
messages: 12124
priority: normal
status: new
substatus: analyzed
title: RTP parser fails on RFC 3550 header extensions
type: patch
FFmpeg issue tracker iss...@roundup.ffmpeg.org
https://roundup.ffmpeg.org/issue2270
