WunderRadio changes
WunderRadio uses FFmpeg and even provides source: http://www.wunderradio.com/code.html I just had a 5 minute look, so I don't know any details, but there are some changes in there that we might wish to pick up: RTSP stuff and some ARM libswscale improvements. Diego
On Mon, 23 Aug 2010, Diego Biurrun wrote:
I just had a 5 minute look, so I don't know any details, but there are some changes in there that we might wish to pick up: RTSP stuff and some ARM libswscale improvements.
A quick check of their RTSP changes: - Hardcoded to use TCP as lower transport - Send keep-alive OPTIONS regularly (which we added in trunk a few weeks ago) - Require TCP-interleaved RTP packets to be at least 11 bytes, instead of 12. (A minimal RTP packet is 12 bytes, but a minimal RTCP packet can be much smaller, at least as small as 8 bytes.) This could be adjusted with something like this: diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c index 36fe753..8c4c29b 100644 --- a/libavformat/rtsp.c +++ b/libavformat/rtsp.c @@ -1735,7 +1735,7 @@ redo: #ifdef DEBUG_RTP_TCP dprintf(s, "id=%d len=%d\n", id, len); #endif - if (len > buf_size || len < 12) + if (len > buf_size || len < 8) goto redo; /* get the data */ ret = url_read_complete(rt->rtsp_hd, buf, len); Luca B, Ronald, any opinions on adjusting this? // Martin
Hi, On Mon, Aug 23, 2010 at 9:50 AM, Martin Storsj? <martin at martin.st> wrote:
On Mon, 23 Aug 2010, Diego Biurrun wrote:
I just had a 5 minute look, so I don't know any details, but there are some changes in there that we might wish to pick up: RTSP stuff and some ARM libswscale improvements.
A quick check of their RTSP changes: - Hardcoded to use TCP as lower transport - Send keep-alive OPTIONS regularly (which we added in trunk a few weeks ago) - Require TCP-interleaved RTP packets to be at least 11 bytes, instead of 12. (A minimal RTP packet is 12 bytes, but a minimal RTCP packet can be much smaller, at least as small as 8 bytes.) This could be adjusted with something like this:
diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c index 36fe753..8c4c29b 100644 --- a/libavformat/rtsp.c +++ b/libavformat/rtsp.c @@ -1735,7 +1735,7 @@ redo: ?#ifdef DEBUG_RTP_TCP ? ? dprintf(s, "id=%d len=%d\n", id, len); ?#endif - ? ?if (len > buf_size || len < 12) + ? ?if (len > buf_size || len < 8) ? ? ? ? goto redo; ? ? /* get the data */ ? ? ret = url_read_complete(rt->rtsp_hd, buf, len);
Luca B, Ronald, any opinions on adjusting this?
Only if we actually do something with the RTCP packet. Otherwise ignoring is better. Ronald
On Mon, 23 Aug 2010, Ronald S. Bultje wrote:
On Mon, Aug 23, 2010 at 9:50 AM, Martin Storsj? <martin at martin.st> wrote:
On Mon, 23 Aug 2010, Diego Biurrun wrote:
I just had a 5 minute look, so I don't know any details, but there are some changes in there that we might wish to pick up: RTSP stuff and some ARM libswscale improvements.
A quick check of their RTSP changes: - Hardcoded to use TCP as lower transport - Send keep-alive OPTIONS regularly (which we added in trunk a few weeks ago) - Require TCP-interleaved RTP packets to be at least 11 bytes, instead of 12. (A minimal RTP packet is 12 bytes, but a minimal RTCP packet can be much smaller, at least as small as 8 bytes.) This could be adjusted with something like this:
diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c index 36fe753..8c4c29b 100644 --- a/libavformat/rtsp.c +++ b/libavformat/rtsp.c @@ -1735,7 +1735,7 @@ redo: ?#ifdef DEBUG_RTP_TCP ? ? dprintf(s, "id=%d len=%d\n", id, len); ?#endif - ? ?if (len > buf_size || len < 12) + ? ?if (len > buf_size || len < 8) ? ? ? ? goto redo; ? ? /* get the data */ ? ? ret = url_read_complete(rt->rtsp_hd, buf, len);
Luca B, Ronald, any opinions on adjusting this?
Only if we actually do something with the RTCP packet. Otherwise ignoring is better.
We do pass the packet on to the rtpdec code, that may or may not react to it (e.g. RTCP BYE support). If we "ignore" small packets this way, the payload data of the packets are left unread and will show up as garbage before receiving the next packet, I'm not sure how tolerant the code currently is against such cases. Adjusting this limit down would avoid such cases, at least for valid inputs. // Martin
Hi, On Mon, Aug 23, 2010 at 11:23 AM, Martin Storsj? <martin at martin.st> wrote:
On Mon, 23 Aug 2010, Ronald S. Bultje wrote:
On Mon, Aug 23, 2010 at 9:50 AM, Martin Storsj? <martin at martin.st> wrote:
On Mon, 23 Aug 2010, Diego Biurrun wrote:
I just had a 5 minute look, so I don't know any details, but there are some changes in there that we might wish to pick up: RTSP stuff and some ARM libswscale improvements.
A quick check of their RTSP changes: - Hardcoded to use TCP as lower transport - Send keep-alive OPTIONS regularly (which we added in trunk a few weeks ago) - Require TCP-interleaved RTP packets to be at least 11 bytes, instead of 12. (A minimal RTP packet is 12 bytes, but a minimal RTCP packet can be much smaller, at least as small as 8 bytes.) This could be adjusted with something like this:
diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c index 36fe753..8c4c29b 100644 --- a/libavformat/rtsp.c +++ b/libavformat/rtsp.c @@ -1735,7 +1735,7 @@ redo: ?#ifdef DEBUG_RTP_TCP ? ? dprintf(s, "id=%d len=%d\n", id, len); ?#endif - ? ?if (len > buf_size || len < 12) + ? ?if (len > buf_size || len < 8) ? ? ? ? goto redo; ? ? /* get the data */ ? ? ret = url_read_complete(rt->rtsp_hd, buf, len);
Luca B, Ronald, any opinions on adjusting this?
Only if we actually do something with the RTCP packet. Otherwise ignoring is better.
We do pass the packet on to the rtpdec code, that may or may not react to it (e.g. RTCP BYE support). If we "ignore" small packets this way, the payload data of the packets are left unread and will show up as garbage before receiving the next packet, I'm not sure how tolerant the code currently is against such cases. Adjusting this limit down would avoid such cases, at least for valid inputs.
Well, valid only for RTCP, not for RTP. It'd be nice to distinguish if we can. Ronald
Hi martin the patch below (on ffmpeg-dev from you) still applies to rtspdec.c) is this patch still correct (in which case id like to apply it to ffmpeg) ? On Mon, Aug 23, 2010 at 04:50:27PM +0300, Martin Storsjö wrote:
On Mon, 23 Aug 2010, Diego Biurrun wrote:
I just had a 5 minute look, so I don't know any details, but there are some changes in there that we might wish to pick up: RTSP stuff and some ARM libswscale improvements.
A quick check of their RTSP changes: - Hardcoded to use TCP as lower transport - Send keep-alive OPTIONS regularly (which we added in trunk a few weeks ago) - Require TCP-interleaved RTP packets to be at least 11 bytes, instead of 12. (A minimal RTP packet is 12 bytes, but a minimal RTCP packet can be much smaller, at least as small as 8 bytes.) This could be adjusted with something like this:
diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c index 36fe753..8c4c29b 100644 --- a/libavformat/rtsp.c +++ b/libavformat/rtsp.c @@ -1735,7 +1735,7 @@ redo: #ifdef DEBUG_RTP_TCP dprintf(s, "id=%d len=%d\n", id, len); #endif - if (len > buf_size || len < 12) + if (len > buf_size || len < 8) goto redo; /* get the data */ ret = url_read_complete(rt->rtsp_hd, buf, len);
Luca B, Ronald, any opinions on adjusting this?
// Martin _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@mplayerhq.hu https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
-- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Asymptotically faster algorithms should always be preferred if you have asymptotical amounts of data
On Tue, 20 Sep 2011, Michael Niedermayer wrote:
the patch below (on ffmpeg-dev from you) still applies to rtspdec.c) is this patch still correct (in which case id like to apply it to ffmpeg) ?
On Mon, Aug 23, 2010 at 04:50:27PM +0300, Martin Storsjö wrote:
On Mon, 23 Aug 2010, Diego Biurrun wrote:
I just had a 5 minute look, so I don't know any details, but there are some changes in there that we might wish to pick up: RTSP stuff and some ARM libswscale improvements.
A quick check of their RTSP changes: - Hardcoded to use TCP as lower transport - Send keep-alive OPTIONS regularly (which we added in trunk a few weeks ago) - Require TCP-interleaved RTP packets to be at least 11 bytes, instead of 12. (A minimal RTP packet is 12 bytes, but a minimal RTCP packet can be much smaller, at least as small as 8 bytes.) This could be adjusted with something like this:
diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c index 36fe753..8c4c29b 100644 --- a/libavformat/rtsp.c +++ b/libavformat/rtsp.c @@ -1735,7 +1735,7 @@ redo: #ifdef DEBUG_RTP_TCP dprintf(s, "id=%d len=%d\n", id, len); #endif - if (len > buf_size || len < 12) + if (len > buf_size || len < 8) goto redo; /* get the data */ ret = url_read_complete(rt->rtsp_hd, buf, len);
It's as usable now as it was then - that is, it might fix some odd corner case - I haven't heard about any such concrete case, but I don't think it will hurt anything in practice to do this change. // Martin
On Tue, Sep 20, 2011 at 08:12:24PM +0300, Martin Storsjö wrote:
On Tue, 20 Sep 2011, Michael Niedermayer wrote:
the patch below (on ffmpeg-dev from you) still applies to rtspdec.c) is this patch still correct (in which case id like to apply it to ffmpeg) ?
On Mon, Aug 23, 2010 at 04:50:27PM +0300, Martin Storsjö wrote:
On Mon, 23 Aug 2010, Diego Biurrun wrote:
I just had a 5 minute look, so I don't know any details, but there are some changes in there that we might wish to pick up: RTSP stuff and some ARM libswscale improvements.
A quick check of their RTSP changes: - Hardcoded to use TCP as lower transport - Send keep-alive OPTIONS regularly (which we added in trunk a few weeks ago) - Require TCP-interleaved RTP packets to be at least 11 bytes, instead of 12. (A minimal RTP packet is 12 bytes, but a minimal RTCP packet can be much smaller, at least as small as 8 bytes.) This could be adjusted with something like this:
diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c index 36fe753..8c4c29b 100644 --- a/libavformat/rtsp.c +++ b/libavformat/rtsp.c @@ -1735,7 +1735,7 @@ redo: #ifdef DEBUG_RTP_TCP dprintf(s, "id=%d len=%d\n", id, len); #endif - if (len > buf_size || len < 12) + if (len > buf_size || len < 8) goto redo; /* get the data */ ret = url_read_complete(rt->rtsp_hd, buf, len);
It's as usable now as it was then - that is, it might fix some odd corner case - I haven't heard about any such concrete case, but I don't think it will hurt anything in practice to do this change.
applied [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Many things microsoft did are stupid, but not doing something just because microsoft did it is even more stupid. If everything ms did were stupid they would be bankrupt already.
Diego Biurrun <diego at biurrun.de> writes:
WunderRadio uses FFmpeg and even provides source:
http://www.wunderradio.com/code.html
I just had a 5 minute look, so I don't know any details, but there are some changes in there that we might wish to pick up: RTSP stuff and some ARM libswscale improvements.
I refuse to touch libswscale until it has acquired a half-decent design. Right now RGB32 to YUV conversion crashes with an unaligned memory access on most architectures, and people are quite happily ignoring it. That is in addition to the beyond abominable "style" of it all. -- M?ns Rullg?rd mans at mansr.com
On Mon, Aug 23, 2010 at 03:19:27PM +0100, M?ns Rullg?rd wrote:
Diego Biurrun <diego at biurrun.de> writes:
WunderRadio uses FFmpeg and even provides source:
http://www.wunderradio.com/code.html
I just had a 5 minute look, so I don't know any details, but there are some changes in there that we might wish to pick up: RTSP stuff and some ARM libswscale improvements.
I refuse to touch libswscale until it has acquired a half-decent design.
any other volunteers for maintaining arm in swscale?
Right now RGB32 to YUV conversion crashes with an unaligned memory access on most architectures, and people are quite happily ignoring it.
i have something like 150 mails on ffmpeg-dev to look at think about, and in some cases review and reply to. Thats just so other people can continue to work and arent being held up waiting .... once iam done with that ill add the ifdefs and 5 lines of c code to fix the unaligned accesses. Anyway i must admit i thought people wherent that childish anymore to refuse to fix trivial bugs due to not liking the code style
That is in addition to the beyond abominable "style" of it all.
yes the world will end due to that abominable "style" [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB When you are offended at any man's fault, turn to yourself and study your own failings. Then you will forget your anger. -- Epictetus
Michael Niedermayer <michaelni at gmx.at> writes:
On Mon, Aug 23, 2010 at 03:19:27PM +0100, M?ns Rullg?rd wrote:
Diego Biurrun <diego at biurrun.de> writes:
WunderRadio uses FFmpeg and even provides source:
http://www.wunderradio.com/code.html
I just had a 5 minute look, so I don't know any details, but there are some changes in there that we might wish to pick up: RTSP stuff and some ARM libswscale improvements.
I refuse to touch libswscale until it has acquired a half-decent design.
any other volunteers for maintaining arm in swscale?
Right now RGB32 to YUV conversion crashes with an unaligned memory access on most architectures, and people are quite happily ignoring it.
i have something like 150 mails on ffmpeg-dev to look at think about, and in some cases review and reply to. Thats just so other people can continue to work and arent being held up waiting .... once iam done with that ill add the ifdefs and 5 lines of c code to fix the unaligned accesses. Anyway i must admit i thought people wherent that childish anymore to refuse to fix trivial bugs due to not liking the code style
It's not about "not liking the style", it's about not understanding what the fuck it's trying to do or what the author was smoking.
That is in addition to the beyond abominable "style" of it all.
yes the world will end due to that abominable "style"
Why do you think nobody wants to work on it? -- M?ns Rullg?rd mans at mansr.com
2010/8/23 M?ns Rullg?rd <mans at mansr.com>:
Michael Niedermayer <michaelni at gmx.at> writes:
On Mon, Aug 23, 2010 at 03:19:27PM +0100, M?ns Rullg?rd wrote: [...]
?That is in addition to the beyond abominable "style" of it all.
yes the world will end due to that abominable "style"
Why do you think nobody wants to work on it?
I'm curious, could you give some examples of the abominable style? (I'm not saying there aren't any, just want to know which ones are the most bothersome).
Ramiro Polla <ramiro.polla at gmail.com> writes:
2010/8/23 M?ns Rullg?rd <mans at mansr.com>:
Michael Niedermayer <michaelni at gmx.at> writes:
On Mon, Aug 23, 2010 at 03:19:27PM +0100, M?ns Rullg?rd wrote: [...]
?That is in addition to the beyond abominable "style" of it all.
yes the world will end due to that abominable "style"
Why do you think nobody wants to work on it?
I'm curious, could you give some examples of the abominable style? (I'm not saying there aren't any, just want to know which ones are the most bothersome).
All of it. Seriously, just opening the file and looking at it makes me want to puke. The hellish mix of ifdefs and inline asm isn't exactly helpful either. -- M?ns Rullg?rd mans at mansr.com
participants (8)
-
diego＠biurrun.de -
mans＠mansr.com -
Martin Storsjö -
martin＠martin.st
-
Michael Niedermayer -
michaelni＠gmx.at
-
ramiro.polla＠gmail.com -
rsbultje＠gmail.com