[FFmpeg-devel] [PATCH] Use the actual RTSP peer IP for RTP sessions

[Luca Abeni]

Martin Storsj? wrote:
[...]
>>> Ok, here's the first attempt.
>>>
>>> If we update avf_sdp_create to take new parameters, what other flexibility
>>> may we need? These ones were the ones that I felt needed at the moment. As a
>>> bonus, this lets us avoid hardcoding the fact that the control uri in the
>>> SDP is exactly "streamid".
>> I am all for improving avf_sdp_create(), but:
>> 1) I am not convinced that the "url" parameter is a good idea: in my opinion,
>>    the AVFormatContexts passed to avf_sdp_create() should be the ones actually
>>    used for the RTP streams. So, having a destination address different from
>>    ac[]->filename does not make too much sense to me.
> 
> In the RTSP muxer case, we don't have the RTP muxers available at that 
> point yet. First we create an SDP that we announce, then we do the setup 
> of each individual stream, and create each RTP muxer at that point.

Ah, ok. Now I remember :)


>>    It seems to me that the right solution in this case would be to create
>>    a proper AVFormatContext (or array of AVFormatContexts) before calling
>>    avf_sdp_create() (you will have to do it anyway, for outputing the RTP
>>    streams... No?)
> 
> Yes, it is possible to change things that way, but it gets very messy IMO. 
> Currently, either we don't have an AVFormatContext for the RTP muxer at 
> all, or we have a fully initialized AVFormatContext, all handled within 
> rtsp_rtp_mux_open. Instead, we now may have an AVFormatContext that is 
> allocated but may or may not have av_write_header called (which changes 
> what should be done in the cleanup procedure).
> 
> Attached patch series that shows what this would look like - messy IMO.

I see. Yes, this way of doing things opens the possibility to have
RTP AVFormatContexts which are not fully initialised. And this makes
the code more complex, increasing the number of "if"s in the error paths.
But, on the other hand, I think this way of doing things is conceptually
more correct (we create the SDP based on the real RTP contexts).

Anyway, if you do not like it let's see if we can find an alternative
compromise. What about this: creating the SDP from an RTSP context (as
you are doing now), but instead of using the original AVFormatContext
you can create a copy (and modify the filename in the copy).
This looks like a good compromise to me, and should address the problem
in a clean way without opening the door to SDPs that have different
destination addresses respect to the RTP contexts (yes, I know this is
not what you are doing here, but if we modify the avf_sdp_create()
interface in this way we allow this kind of incorrect usage).


			Thanks,
				Luca