[FFmpeg-devel] [RFC] SDP Generation

[Michael Niedermayer]

Hi

On Thu, Jun 07, 2007 at 11:45:21AM +0200, Luca Abeni wrote:
> Hi all,
> 
> here is a first attempt at SDP generation in libavformat. I put the new 
> function (av_sdp_create() - any ideas for a better name?) in a new file, 
> so that the size of applications not using it will not be increased.
> 
> av_sdp_create() currently handles static payload types and MPEG4 video 
> (assigning payload type 96, as in libavformat/rtp.c... We need to handle 
> this in a better way, but I think that the first important thing is to 
> have rtp.c and sdp.c consistent between them). So, it should be enough 
> for current libav*'s needs. I have a patch for converting ffserver to 
> use av_sdp_create(), and I've not been able to find any regression.
> (I also have a patch for enabling ffmpeg to output an SDP when streaming 
> to RTP, so that the stream can be easily received).
> 
> There still are some open issues (look at the FIXME's), but I'd like to 
> have some feedback to know if I am going in the right direction, and if 
> the interface is ok.

the interface does not look ok, it depends on a 1 stream per AVFormatContext
i thought this multi AVFormatContext was only needed for obscure multicast
cases noone would use in practice anyway, please elaborate on the
problem ...


[...]

>     p = buff;
>     res = snprintf(p, size, "v=%d\r\n", s->sdp_version);
>     if (res < 0) {
>         return NULL;
>     }
>     p += res;
>     size -= res;
> 
>     res = snprintf(p, size, "o=- %d %d IN IPV4 %s\r\n", s->id, s->version, s->src_addr);
>     if (res < 0) {
>         return NULL;
>     }
>     p += res;
>     size -= res;
> 
>     res = snprintf(p, size, "t=%d %d\r\n", s->start_time, s->end_time);
>     if (res < 0) {
>         return NULL;
>     }
>     p += res;
>     size -= res;
> 
>     res = snprintf(p, size, "s=%s\r\n", s->name);
>     if (res < 0) {
>         return NULL;
>     }
>     p += res;
>     size -= res;

code quadruplification, also the attribute_write() is pretty much the 5th
copy of this ...
(and no a macro is not a solution, it still would have the code duplicated
in the binary)


[...]
> static char *data_to_hex(char *buff, const uint8_t *src, int s)
> {
>     char *p;
>     int i;
> 
>     p = buff;
>     for(i = 0; i < s; i++) {
>         snprintf(p, 3, "%02x", src[i]);
>         p += 2;
>     }
> 
>     return p;
> }

p+buff is redundant
whats the sense in snprintf() with fixed 3 here? it cant be longer anyway


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have often repented speaking, but never of holding my tongue.
-- Xenocrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070608/a4eb6422/attachment.pgp>