[FFmpeg-devel] [PATCH] SDP muxer

[Luca Abeni]

Hi Stefano,

Stefano Sabatini wrote:
[...]
>>> Yes, this solution won't break neither URLs syntax neither old API
>>> interface, while on the other hand I'm extending the interface, in
>>> order to create a representation of an SDP from a list of RTP files.
>> Ok, good. Note that avf_sdp_create() should already be able to do what
>> you need (you just need to prepare a proper "AVFormatContext *ac[]"
>> before calling it). So, I guess everything you need is a new muxer which
>> sets up the "ac" array and calls avf_sdp_create().
> 
> The problem with avf_sdp_create() is that it takes AVOutputFormat,

What is the exact problem with this?

> while I think the expected behaviour for the muxer would be to take a
> list of *AVStreams* and create from these the header.

Maybe. But then, is it still possible to have different destination
multicast groups for different streams? I mean: does an AVStream allow
to store the destination MC group (and port) somewhere?

> So I write a write_header() function which is very similar to
> avf_sdp_create(), but which reads from streams rather than from a list
> of files.

I suspect this will result in some code duplication; you should not do
it. What you should do is to write a write_header() function that prepares
an array of AVFormatContexts for avf_sdp_create(), and then calls it.


> I'm attaching my implementation, but I'm somehow not satisfied with it).
> 
> Patch consists of the part (I'll split it when we'll agree on some
> solution for the problem I'm going to discuss):
> 
> * cosmetics to sdp.c: struct sdp_session_level -> SDPContext and other

What's the point of this change? Note that it is wrong: sdp_session_level
is not a generic "SDP context", but it contains the session-level description
of an SDP.

> renames in order to make them more meaningful and consistent (e.g.:
> sdp_write_media -> write_sdp_media

Sorry, but again I do not see the point in this change.

> sdp_media_attributes -> write_sdp_media_attributes
> sdp_write_header -> write_sdp_header)

Idem.
Sorry, but I'll not approve this kind of changes.


> The use of av_dup_stream() is necessary for the "special" status of
> the SDP muxer, which takes streams which have been already defined.

I am not sure about it; let's see Michael's comments.


				Luca