On Fri, 2010-01-15 at 13:27 +0100, Daniel Stenberg wrote:
> On Tue, 12 Jan 2010, Chris Conroy wrote:
>
> > Updated patch is ready for review.
>
> I've read the code a bit more now and I have a few follow-up comments:
>
> * All the *_getsock() functions in the struct Curl_handler are set to
> ZERO_NULL in rtsp.c. This will effectively prevent RTSP from being a nice
> multi interface citizen. I suggest you try writing a multi interface using
> test app and make some adaptions to get that working too!
Ah yes. This was something I meant to write a TODO for since it was
unclear what I needed to do regarding the _getsock functions. Looking at
the code some more I should probably be able to reuse the HTTP ones.

>
> * /* Increment the CSeq on success */
>
> The comment says so at line 467 in lib/rtsp.c, but the incrementation is
> done disregarding if the previous function were successful or not! Is the
> comment or the code wrong?
The code is wrong. Sorry for that silly error.

>
> * 'data->state.rtsp_next_server_CSeq' seems not to be really used anywhere
> but yet you provide a function to extract its value. It looks like the
> implementation isn't complete?
I made a few comments in my previous emails regarding Server->Client
requests. It's the first thing listed on the TODO list. So, yes, the
implementation is not complete. I have yet to find a server in the wild
that actually makes use of this feature, but presumably it *should* be
implemented because it's in the spec. However, at this time, I think the
ugliness introduced by it might be more trouble than it's worth, at
least while we iron out the kinks in the other areas of the protocol.

--Chris
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2010-01-18