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
> > --- /dev/null 00:00:00 1970 (empty, because file is newly added)
> > +++ rtmp/rtmp.h Thu Jun 4 08:57:37 2009 (r4382)
> > @@ -0,0 +1,39 @@
> > +
> > +#ifndef AVFORMAT_RTMP_H
> > +#define AVFORMAT_RTMP_H
> > +#include "avformat.h"
>
> nit: an empty line would help readability here.
will be added
> > --- /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.
> > --- /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.
> > + url_write(rt->rtmp_hd, buf, RTMP_HANDSHAKE_PACKET_SIZE + 1);
> > + i = url_read_complete(rt->rtmp_hd, buf2, RTMP_HANDSHAKE_PACKET_SIZE*2
> > + 1);
> > + if (i != 1 + RTMP_HANDSHAKE_PACKET_SIZE*2 || memcmp(buf + 1, buf2 + 1
> > + RTMP_HANDSHAKE_PACKET_SIZE, RTMP_HANDSHAKE_PACKET_SIZE)) {
>
> nit: That's a loooong line, same in other places.
This place will be rewritten, same for other places.
> > +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
> > +static int flv_set_video_codec(AVFormatContext *s, AVStream *vstream, int
> > flv_codecid) {
>
> ditto
>
> > --- /dev/null 00:00:00 1970 (empty, because file is newly added)
> > +++ rtmp/rtmppkt.c Thu Jun 4 08:57:37 2009 (r4382)
> > @@ -0,0 +1,267 @@
> > +
> > +/* needed for gethostname() */
> > +#define _XOPEN_SOURCE 600
>
> see my comment above
>
> > --- /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
> Diego
_______________________________________________
FFmpeg-soc mailing list
[email protected]
https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-soc
