[FFmpeg-devel] [PATCH] Add the frame size attribute to SDP descriptions

[Martin Storsjö]

On Fri, 8 Jan 2010, Luca Abeni wrote:

> Martin Storsj? wrote:
> [...]
> > > 2) Not using c->{width,height}, but using the AVMetadata
> > >    infrastructure... This might be a little bit more complex, but
> > >    I believe it is more flexible, allowing the user to add more "a="
> > >    lines if needed (without needing to modify sdp.c every time).
> > >    My basic idea is that if the AVFormatContext contains a metadata
> > >    tag with key "key" and value "value", then the "a=key:value" line
> > >    is added to the SDP (the "title" metadata should be excluded
> > >    because it is already used for the "s=" line).
> > 
> > That would at least be the most general thing, leaving the choice to the
> > libavformat user. That'd also be good for adding the other 3gpp SDP headers
> > mentioned in the same document.
> 
> Yes, this is why I had this idea.
> 
> 
> > However, if e.g. transcoding a stream with ffmpeg, where the input file has
> > some normal metadata, wouldn't these be copied into the SDP, resulting in
> > completely bogus SDP entries?
> 
> Right... I did not think about this problem...
> 
> > Should the AVMetadata entries to be added to the SDP be tagged in some way,
> > e.g. key = sdp:keyname, adding a a=keyname:value in that case?
> 
> This looks a good solution to the problem, thanks!

Ok, I'll perhaps try to put a patch together with this functionality.

> > As for setting this particular attribute - it includes the payload type
> > number, which is set internally within the rtp muxer and sdp writer and not
> > available to the libavformat user setting the AVMetadata entries, so setting
> > it externally isn't too easy, if we don't make assumptions about in which
> > way libavformat assigns the payload type numbers.
> 
> Yes... I thought about this problem. Right now, libavformat is using
> dynamic payload type 96 for video and 97 for audio (so, externally setting
> this attribute should not be a problem), but this might change in the
> future.
> I think the best solution would be to support some kind of "special keyword"
> in the attribute value, which the SDP generator replaces with the payload
> type. For example, something like "%PT 352-288" will generate "96 352-288"...

That's probably also a good idea, but makes the whole thing a lot more 
complex.

> Anyway, this is becoming complex. If there is need for the "framesize"
> attribute now, I can commit your patch with a comment... (hoping that
> someone can provide the "correct patch" in a second time).
> Let me know.

I don't have a concrete need for it (yet at least), I just happened to 
have the patch locally, so until the real need arises, we can leave it 
out.

> > Some other formats (such as AAC, H.264) add extradata to the SDP, so in
> > those cases, you can't change parameters later on in the stream anyway.
> 
> I do not know about AAC, but H.264 does not require "out-of-band config
> informations". In other words "sprop-parameters-set" is not mandatory, and
> you can encode H.264 video without using CODEC_FLAG_GLOBAL_HEADER. This
> will place SPS and PPS inline in the video stream and will allow changing
> the video resolution. I've been told that some DVB-H channels do this (for
> commercials, or similar reasons).

Ah, that's true. IIRC, QuickTime didn't like this version of H.264/RTP 
though, it required having the parameters in SDP, but that's off topic.

So, I'd say we leave this patch be for now, until a concrete need arises. 
Until then, we might consider the methods for adding user-supplied 
attributes through AVMetadata, as described above.

// Martin