[FFmpeg-soc] shrink rtsp sdp buffer

[Ronald S. Bultje]

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