[FFmpeg-soc] shrink rtsp sdp buffer
Ronald S. Bultje
rsbultje at gmail.com
Tue Jun 8 15:24:08 CEST 2010
Hi,
On Tue, Jun 8, 2010 at 8:51 AM, Martin Storsjö <martin at martin.st> wrote:
> On Thu, 27 May 2010, Martin Storsjö wrote:
>> On Wed, 26 May 2010, Josh Allmann wrote:
>> > The following reverts changes made in r18508 and shrinks the sdp
>> > buffer in rtsp.c. I only do this because 16k on the stack is a lot.
>> >
>> > This cleans up the last remnants of Vorbis in rtsp.c -- its sdp/fmtp
>> > processing is now done in rtpdec_xiph.c, so this codepath is not
>> > touched by Vorbis over RTP anymore except in the case of an error in
>> > parsing sdp/fmtp. Even then, it should still fall-through rather
>> > cleanly.
>>
>> Did you test it?
>>
>> > Index: libavformat/rtsp.c
>> > ===================================================================
>> > --- libavformat/rtsp.c (revision 23344)
>> > +++ libavformat/rtsp.c (working copy)
>> > @@ -271,9 +271,7 @@
>> > static void sdp_parse_fmtp(AVStream *st, const char *p)
>> > {
>> > char attr[256];
>> > - /* Vorbis setup headers can be up to 12KB and are sent base64
>> > - * encoded, giving a 12KB * (4/3) = 16KB FMTP line. */
>> > - char value[16384];
>> > + char value[4096];
>> > int i;
>> > RTSPStream *rtsp_st = st->priv_data;
>> > AVCodecContext *codec = st->codec;
>>
>> This should be ok.
>>
>> > @@ -537,10 +535,8 @@
>> > * contain long SDP lines containing complete ASF Headers (several
>> > * kB) or arrays of MDPR (RM stream descriptor) headers plus
>> > * "rulebooks" describing their properties. Therefore, the SDP line
>> > - * buffer is large.
>> > - *
>> > - * The Vorbis FMTP line can be up to 16KB - see sdp_parse_fmtp. */
>> > - char buf[16384], *q;
>> > + * buffer is large. */
>> > + char buf[8192], *q;
>> > SDPParseState sdp_parse_state, *s1 = &sdp_parse_state;
>> >
>> > memset(s1, 0, sizeof(SDPParseState));
>>
>> You can't shrink this one, since this codepath still is run for all cases,
>> even if the RTP depacketizer handles the parsing of fmtp lines itself.
>> sdp_parse() calls sdp_parse_line(), which then either calls
>> rtsp_st->dynamic_handler->parse_sdp_a_line() or sdp_parse_fmtp(). But the
>> comment referring to sdp_parse_fmtp for vorbis should perhaps be updated.
>
> Ping?
Leave the buffer size as is, and apply the rest of the patch, I'd say...
Ronald
More information about the FFmpeg-soc
mailing list