[FFmpeg-soc] [soc]: r4382 - in rtmp: TODO checkout.sh rtmp.h rtmp.patch rtmpdec.c rtmppkt.c rtmppkt.h rtmpproto.c

Diego Biurrun diego at biurrun.de
Thu Jun 4 15:45:16 CEST 2009


On Thu, Jun 04, 2009 at 01:19:27PM +0300, Kostya wrote:
> On Thu, Jun 04, 2009 at 11:56:34AM +0200, Diego Biurrun wrote:
> > On Thu, Jun 04, 2009 at 08:57:37AM +0200, kostya wrote:
> > > 
> > > Log:
> > > Some initial work on RTMP client.
> > > 
> > > --- /dev/null	00:00:00 1970	(empty, because file is newly added)
> > > +++ rtmp/checkout.sh	Thu Jun  4 08:57:37 2009	(r4382)
> > > @@ -0,0 +1,14 @@
> > > +#!/bin/sh
> > > +
> > > +svn co svn://svn.ffmpeg.org/ffmpeg/trunk ffmpeg && \
> > > +cd ffmpeg
> > > +
> > > +patch -p1 < ../rtmp.patch && \
> > > +cd libavformat
> > 
> > The \ are redundant.
> 
> copied from soc/aacenc, blame Robert Swain 

It's not important, I just wanted to share some shell-fu.

> > > --- /dev/null	00:00:00 1970	(empty, because file is newly added)
> > > +++ rtmp/rtmp.patch	Thu Jun  4 08:57:37 2009	(r4382)
> > 
> > Documentation and changelog update missing.
> 
> Too early.

Hardly, especially the documentation should not conflict.  Also, give a
good example for the other students so they don't forget about it ;)

> > > --- /dev/null	00:00:00 1970	(empty, because file is newly added)
> > > +++ rtmp/rtmpdec.c	Thu Jun  4 08:57:37 2009	(r4382)
> > > @@ -0,0 +1,350 @@
> > > +
> > > +/* needed for gethostname() */
> > > +#define _XOPEN_SOURCE 600
> > 
> > Hmmm, I think it may be time to add this globally.
> 
> You know official FFmpeg motto. libavformat/network.h awaits.

?

> > > +static void flv_set_audio_codec(AVFormatContext *s, AVStream *astream, int flv_codecid) {
> > 
> > This is not K&R, which you use in other places of this file.
> 
> This was copypasted from libavformat/flvdec.c

Fixed locally.

> > > --- /dev/null	00:00:00 1970	(empty, because file is newly added)
> > > +++ rtmp/rtmpproto.c	Thu Jun  4 08:57:37 2009	(r4382)
> > > @@ -0,0 +1,117 @@
> > > +
> > > +/**
> > > + * url syntax: rtp://host:port[?option=val...]
> > > + * option: 'ttl=n'       : set the ttl value (for multicast only)
> > > + *         'localport=n' : set the local port to n
> > > + *
> > > + */
> > > +
> > > +static int rtmp_open(URLContext *h, const char *uri, int flags)
> > 
> > nit: stray empty line
> 
> copypasted from libavformat/rtpproto.c, all doxy comments there have an
> additional empty line

Do you intend to keep the c+p code?

Diego


More information about the FFmpeg-soc mailing list