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

[Ronald S. Bultje]

Hi,

On Fri, Feb 19, 2010 at 6:05 PM, Martin Storsj? <martin at martin.st> wrote:
> On Fri, 19 Feb 2010, Ronald S. Bultje wrote:
>> On Fri, Feb 19, 2010 at 4:50 PM, Martin Storsj? <martin at martin.st> wrote:
>> > On Fri, 19 Feb 2010, Ronald S. Bultje wrote:
>> >> On Fri, Feb 19, 2010 at 11:27 AM, Ronald S. Bultje <rsbultje at gmail.com> wrote:
>> >>
>> >> I'm wondering if we discussed this before. Can you use
>> >> AVFormatContext->oformat != NULL instead of is_output? Or something
>> >> along those lines?
>> >
>> > Ah, that would work, yes. However, at some of the places where it is used
>> > (rtsp_close_streams), there's no AVFormatContext pointer available, only
>> > RTSPState, but that can be fixed of course. Do you want me to try that
>> > approach?
>>
>> If possible, yes. Having a AVFormatContext around would also be useful
>> if we want to add calls to av_log() or so.
>
> Updated patch series with this change.

1, 3-6 applied.

For 2:

+                    av_write_trailer(rtpctx);
+                    url_fclose(rtpctx->pb);
+                    av_free(rtpctx->streams[0]);
+                    av_free(rtpctx);
[..]
+    /* Remove the local codec, link to the original codec
+     * context instead, to give the rtp muxer access to
+     * codec parameters. */
+    av_free(rtpctx->streams[0]->codec);
+    rtpctx->streams[0]->codec = st->codec;

Is that right? I think this could lead to memleaks of other things
potentially allocated in the AVCodecContext. In both cases, I'd
recommend not calling av_free() directly on the AVStream, but having
the default function take care of that, and just setting
rtpctx->streams[0]->codec to NULL before free'ing the inside? Also, is
it possible that there's extradata and should you free that in the
second part of the quoted code?

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

Ronald