[FFmpeg-devel] [PATCH] rtsp: Store some parsed values straight to RTSPState

[Ronald S. Bultje]

Hi Martin,

On Mon, Dec 27, 2010 at 4:04 AM, Martin Storsj? <martin at martin.st> wrote:
> Hi,
>
> In the review of the patch for adding support for the Content-Base header
> some weeks ago, Ronald pointed out that storing large strings (a URL) in
> the RTSPMessageHeader struct isn't really optimal. (This struct is stored
> on the stack in many places.)
>
> Attached is a patchset that changes ff_rtsp_parse_line to receive a
> pointer to the RTSPState, together with the method name (since we might
> want to handle some headers differently depending on which method it was a
> reply to), and lastly changes the parsing of Content-Base to store the
> result directly in RTSPState.
>
> This is a slight change in the architecture of the RTSP header parsing -
> initially it never took any action, it simply parsed out the reply headers
> into a struct that the caller inspected, who chose what to do with it.
> With this in place, the parser itself can change state, too. (The http
> auth parsing was an exception before, which also updated state directly.)
>
> With this in place, parsing of the RTP-Info header is much more
> straightforward - if done without this, RTSPMessageHeader would grow a
> lot, if storing all the urls of all streams.

It looks good to me. I'm not a huge lover of free strings, can we
somehow change the const char *method and make it an enum? That way,
we're not calling str(case)cmp(), but simply comparing integers, which
may be slightly faster and also needs no NULL check (simply pass
RTSP_METHOD_DONTCARE or so).

That's a minor thing though, if others disagree this is fine with me
also. Other than that, no comments, thanks for this, will save a nice
few bytes of memory!

Ronald