[FFmpeg-devel] [PATCH] RTSP muxer, round 5

[Martin Storsjö]

On Mon, 22 Feb 2010, Ronald S. Bultje wrote:

> Hi,
> 
> On Fri, Feb 19, 2010 at 6:42 PM, Martin Storsj? <martin at martin.st> wrote:
> > On Fri, 19 Feb 2010, Ronald S. Bultje wrote:
> >
> >> Also, is
> >> it possible that there's extradata and should you free that in the
> >> second part of the quoted code?
> >
> > Theoretically, perhaps, but the AVCodecContext was just allocated by the
> > av_new_stream above and never touched.
> 
> Hm, ok, I missed this. Allright then, patch OK, go test your SVN account. :-).

Applied. :-)

> >> For 7:
> >>
> >> ? fail:
> >> ? ? ?rtsp_close_streams(s);
> >> ? ? ?url_close(rt->rtsp_hd);
> >> - ? ?if (reply->status_code >=300 && reply->status_code < 400) {
> >> + ? ?if (reply->status_code >=300 && reply->status_code < 400 && s->iformat) {
> >> ? ? ? ? ?av_strlcpy(s->filename, reply->location, sizeof(s->filename));
> >> ? ? ? ? ?av_log(s, AV_LOG_INFO, "Status %d: Redirecting to %s\n",
> >> ? ? ? ? ? ? ? ? reply->status_code,
> >>
> >> (bottom) appears unrelated to the rest, could probably be a separate
> >> commit? (No need for a new patch, just yes/no is fine.)
> >
> > Yeah, could probably be a separate commit as well.
> 
> Let's go test your SVN account with this one also.

Ok, applied this in two separate commits.

> Patch 8 imo needs doxy comments for the functions since they are now
> non-static. Could you add these?

Yeah, sure. rtsp_read_reply already has a doxy comment in rtsp.c, I could  
move that one along to .h when making it non-static, and add something 
similar for the other ones. Do you prefer to have this as separate patches 
(first make non-static and move existing doxy, then add new doxy for the 
other ones)?

> I'll review the last patch (9) afterwards.

Ok :-)

// Martin