[FFmpeg-soc] shrink rtsp sdp buffer

[Martin Storsjö]

On Tue, 8 Jun 2010, Ronald S. Bultje wrote:

> 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...

Applied the parts I was ok with.

Josh, please get back to patches that you've sent when reviewers have 
questions and ping them.

// Martin