[FFmpeg-devel] [PATCH] SDP muxer

[Luca Abeni]

Hi,

Stefano Sabatini wrote:
[...]
>> Depending on the answers, maybe a patch like the attached one (completely
>> untested, and maybe needs some fixes/cleanup) would make sense. This patch
>> would greatly simplify the SDP muxer.
> 
> I see the point of your patch, but I cannot see how it could simplify
> the SDP muxer.

The idea is that with this patch the SDP muxer could directly call 
avf_sdp_create() passing the input AVFormatContext to it.

But looking at Aurelien's email it seems that AVStream->filename cannot 
be used in this way... So the patch is not good (and I think the SDP 
muxer should be designed to work in a different way, since I suspect it 
cannot use AVStream->filename).

[...]
> Also in the code we're assuming that an RTP AVOutputStream may contain
> more than one AVStream, does this make sense?

I do not know... In the definition of AVOutputStream in ffmpeg.c I see 
that it contains a pointer to a single AVStream: where is this "more 
than one AVStream" assumption?

[...]
>> - sdp-rename.patch: NACK. Some of the renames might make sense (I am
>>   still undecided, but I do not like the ones like
>>
>>   "sdp_write_header ---> write_sdp_header".
> 
> What about something like:
> sdp_write_header ---> sdp_write_sdp_header

Sorry, no. sdp_write_header is ok. If you really hate this name, or you 
want to use it for different things, this can be changed in something 
like sdp_write_session_level_description.


> All I want is to avoid semantic conflicts with the names of the
> function, now that sdp.c contains now both the code for avf_create_sdp
> and the SDP muxer (for which a write_header() function which writes
> the whole SDP is needed).

I am beginning to think that the SDP muxer should be in a different file 
(this would avoid #ifdefs when a user dos not want to compile it).


>> - implement-avf-sdp-create2.patch: I think this is not needed (and we
>>   do not need a new function).
>> - implement-sdp-muxer-2.patch: This must be modified to use the attached
>>   patch (if it makes sense - it depends on what AVStream->filename is, and
>>   on how it is used), or to forge a proper array of AVFormatContexts and
>>   then call avf_sdp_create()
> 
> The SDP muxer will work taking an array of streams as found in the
> AVOutputStream, and issuing the SDP from them.
Well, it will receive a pointer to AVFormatContext, like all the other 
muxers. What it has to do is to get the relevant information from it, 
and fill an array of AVOutputStream to pass to avf_sdp_create().

[...]
> Then I thought that having avf_create_sdp2() in the public API may be
> useful, but we could simply avoid to make it public.

No, I do not think we need another function in the public API, unless 
you show that there is some situation in which avf_create_sdp() is not 
enough.


				Luca