[FFmpeg-devel] [PATCH] RTMP client support for lavf

Kostya kostya.shishkov
Tue Jul 21 10:04:09 CEST 2009


On Mon, Jul 20, 2009 at 05:05:41PM +0200, Michael Niedermayer wrote:
> On Sat, Jul 18, 2009 at 08:01:17PM +0300, Kostya wrote:
> > On Sat, Jul 18, 2009 at 11:29:34AM +0200, Michael Niedermayer wrote:
> > > On Fri, Jul 17, 2009 at 06:38:46PM +0300, Kostya wrote:
> > > > $subj
> > [...]
> > > > +/** RTMP default port */
> > > > +#define RTMP_DEFAULT_PORT 1935
> > > 
> > > very usefull comment
> >  
> > of course, it increases number of bytes and lines committed by me
> 
> then please seperare it into a seperate patch and file, no need to
> clutter this one
 
filed that patch to /dev/null
 
> >  
> > > > +
> > > > +/** RTMP handshake data size */
> > > > +#define RTMP_HANDSHAKE_PACKET_SIZE 1536
> > > 
> > > same
> > > 
> > > 
> > > > +
> > > > +#define RTMP_CLIENT_PLATFORM "LNX"
> > > 
> > > LooNiX ?
> > 
> > who cares? 
> 
> someone who tries to understand the code possibly

ok, added comment to that

[...]
> > > > +//TODO: Move HMAC code somewhere. Eventually.
> > > 
> > > good idea!
> > > also it should be a seperate patch
> > 
> > err, I've not found a place in libavcrypto to add it; also I suspect you
> > want generic version of HMAC performing on, say, MD5 as well.
> 
> as you mention it, yes i surely would want that
 
Please wait until I design API for it. Meanwhile, I'll leave this here
(since in most cases I need digest of two blocks and such calculation
saves me some memcpy()s).
 
> >  
> > > [...]
> > > > +static int rtmp_handshake(URLContext *s, RTMPContext *rt)
> > > > +{
> > > > +    AVLFG rnd;
> > > > +    uint8_t tosend    [RTMP_HANDSHAKE_PACKET_SIZE+1];
> > > > +    uint8_t clientdata[RTMP_HANDSHAKE_PACKET_SIZE];
> > > > +    uint8_t serverdata[RTMP_HANDSHAKE_PACKET_SIZE+1];
> > > > +    int i;
> > > > +    int server_pos, client_pos;
> > > > +    uint8_t digest[32];
> > > > +
> > > > +    //av_log(s, AV_LOG_DEBUG, "Handshaking...\n");
> > > > +
> > > > +    av_lfg_init(&rnd, 0xDEADC0DE);
> > > > +    // generate handshake packet - 1536 bytes of pseudorandom data
> > > 
> > > does it work to just send 0 ?
> > > because you are always sending the same anyway ...
> > 
> > it does, but as I understand it, it's also used to measure bandwidth so
> > why not give server what it expects?
> 
> 1536 zero bytes ?

Well, server seems to accept any random data including zeroes (imprinted with
digest, of course) but existing programs generate pseudorandom data
(including official player), so I'd rather make my code behave like
that. 
 
> [...]
> 
> > +/* needed for gethostname() */
> > +#define _XOPEN_SOURCE 600
> 
> doxy

no, it's not
 
> [...]
> > +static void rtmp_calc_digest(const uint8_t *src, int len, int gap,
> > +                             const uint8_t *key, int keylen, uint8_t *dst)
> > +{
> > +    struct AVSHA *sha;
> > +    uint8_t hmac_buf[64+32];
> > +    int i;
> > +
> > +    sha = av_mallocz(av_sha_size);
> > +
> > +    memset(hmac_buf, 0, 64);
> 
>  uint8_t hmac_buf[64+32]={0};

changed 

> [...]
> > +static int rtmp_handshake(URLContext *s, RTMPContext *rt)
> > +{
> > +    AVLFG rnd;
> > +    uint8_t tosend    [RTMP_HANDSHAKE_PACKET_SIZE+1];
> > +    uint8_t clientdata[RTMP_HANDSHAKE_PACKET_SIZE];
> > +    uint8_t serverdata[RTMP_HANDSHAKE_PACKET_SIZE+1];
> > +    int i;
> > +    int server_pos, client_pos;
> > +    uint8_t digest[32];
> > +
> > +    //av_log(s, AV_LOG_DEBUG, "Handshaking...\n");
> > +
> > +    av_lfg_init(&rnd, 0xDEADC0DE);
> > +    // generate handshake packet - 1536 bytes of pseudorandom data
> > +    tosend[0] = 3; //unencrypted data
> > +    memset(tosend+1, 0, 4);
> > +    //write client "version"
> > +    tosend[5] = RTMP_CLIENT_VER1;
> > +    tosend[6] = RTMP_CLIENT_VER2;
> > +    tosend[7] = RTMP_CLIENT_VER3;
> > +    tosend[8] = RTMP_CLIENT_VER4;
> > +    for (i = 9; i <= RTMP_HANDSHAKE_PACKET_SIZE; i++)
> > +        tosend[i] = av_lfg_get(&rnd) >> 24;
> > +    client_pos = rtmp_handshake_imprint_with_digest(tosend + 1);
> > +
> > +    url_write(rt->stream, tosend, RTMP_HANDSHAKE_PACKET_SIZE + 1);
> > +    i = url_read_complete(rt->stream, serverdata, RTMP_HANDSHAKE_PACKET_SIZE + 1);
> > +    if (i != RTMP_HANDSHAKE_PACKET_SIZE + 1) {
> > +        //av_log(s, AV_LOG_ERROR, "Cannot read RTMP handshake response\n");
> > +        return -1;
> > +    }
> > +    i = url_read_complete(rt->stream, clientdata, RTMP_HANDSHAKE_PACKET_SIZE);
> > +    if (i != RTMP_HANDSHAKE_PACKET_SIZE) {
> > +        //av_log(s, AV_LOG_ERROR, "Cannot read RTMP handshake response\n");
> > +        return -1;
> > +    }
> > +
> > +    //av_log(s, AV_LOG_DEBUG, "Server version %d.%d.%d.%d\n",
> > +    //       serverdata[5], serverdata[6], serverdata[7], serverdata[8]);
> > +
> 
> > +    server_pos = rtmp_validate_digest(serverdata + 1, 772);
> > +    if (!server_pos) {
> > +        server_pos = rtmp_validate_digest(serverdata + 1, 8);
> > +        if (!server_pos) {
> > +            //av_log(s, AV_LOG_ERROR, "Server response validating failed\n");
> > +            return -1;
> > +        }
> 
> why is the av_log commented out?

because until major version bump in libavformat/ one cannot use
URLContext in av_log()
 
> [...]
> > +static int rtmp_parse_result(URLContext *s, RTMPContext *rt, RTMPPacket *pkt)
> > +{
> > +    int i, t;
> > +
> > +    switch (pkt->type) {
> > +    case RTMP_PT_CHUNK_SIZE:
> > +        if (pkt->data_size != 4) {
> > +            //av_log(s, AV_LOG_ERROR, "Chunk size change packet is not 4 (%d)\n",
> > +            //       pkt->data_size);
> > +            return -1;
> > +        }
> > +        rt->chunk_size = AV_RB32(pkt->data);
> 
> chunk_size is signed, is it intended for this to be possibly negative?

check added
Theoretically speaking, it should be in some stricter range like 1-65536
but who knows?

> > +        //av_log(s, AV_LOG_DEBUG, "New chunk size = %d\n", rt->chunk_size);
> > +        break;
> > +    case RTMP_PT_PING:
> > +        t = AV_RB16(pkt->data);
> > +        if (t == 6)
> > +            gen_pong(s, rt, pkt);
> > +        break;
> > +    case RTMP_PT_INVOKE:
> > +        if (!memcmp(pkt->data, "\002\000\006_error", 9)) {
> > +            uint8_t tmpstr[256];
> > +
> > +            if (!ff_amf_find_field(pkt->data + 9, "description", tmpstr, sizeof(tmpstr)))
> > +                av_log(NULL/*s*/, AV_LOG_ERROR, "Server error: %s\n",tmpstr);
> > +            return -1;
> > +        }
> > +        if (!memcmp(pkt->data, "\002\000\007_result", 10)) {
> > +            switch (rt->state) {
> > +            case STATE_HANDSHAKED:
> > +                gen_create_stream(s, rt);
> > +                rt->state = STATE_CONNECTING;
> > +                break;
> > +            case STATE_CONNECTING:
> > +                //extract a number from result
> 
> > +                if (pkt->data[10] || pkt->data[19] != 5 || pkt->data[20])
> > +                    av_log(NULL, AV_LOG_WARNING, "Unexpected reply on connect()\n");
> > +                else
> 
> if{ }else

braced 
 
> [...]
> 
> > Index: libavformat/flv.h
> > ===================================================================
> > --- libavformat/flv.h	(revision 19461)
> > +++ libavformat/flv.h	(working copy)
> > @@ -105,8 +105,10 @@
> >      AMF_DATA_TYPE_UNDEFINED   = 0x06,
> >      AMF_DATA_TYPE_REFERENCE   = 0x07,
> >      AMF_DATA_TYPE_MIXEDARRAY  = 0x08,
> > +    AMF_DATA_TYPE_OBJECT_END  = 0x09,
> >      AMF_DATA_TYPE_ARRAY       = 0x0a,
> >      AMF_DATA_TYPE_DATE        = 0x0b,
> > +    AMF_DATA_TYPE_LONG_STRING = 0x0c,
> >      AMF_DATA_TYPE_UNSUPPORTED = 0x0d,
> >  } AMFDataType;
> 
> the changes to flv.h are ok

applied 

[...]
> > +
> > +void ff_amf_write_bool(uint8_t **dst, int val)
> > +{
> > +    bytestream_put_byte(dst, AMF_DATA_TYPE_BOOL);
> > +    bytestream_put_byte(dst, val);
> > +}
> > +
> > +void ff_amf_write_number(uint8_t **dst, double val)
> > +{
> > +    bytestream_put_byte(dst, AMF_DATA_TYPE_NUMBER);
> > +    bytestream_put_be64(dst, av_dbl2int(val));
> > +}
> > +
> > +void ff_amf_write_string(uint8_t **dst, const char *str)
> > +{
> > +    bytestream_put_byte(dst, AMF_DATA_TYPE_STRING);
> > +    bytestream_put_be16(dst, strlen(str));
> > +    bytestream_put_buffer(dst, str, strlen(str));
> > +}
> 
> these are duplicates of the flv code ...
> i see its possible not trivial to merge them due to API differences but
> it at least should be clearly documented that they are duplicates so that
> for example a bugfix done to one set is also applied to the 2nd
 
added some documentation to that
 
> > +
> > +void ff_amf_write_null(uint8_t **dst)
> > +{
> > +    bytestream_put_byte(dst, AMF_DATA_TYPE_NULL);
> > +}
> > +
> > +void ff_amf_write_object_start(uint8_t **dst)
> > +{
> > +    bytestream_put_byte(dst, AMF_DATA_TYPE_OBJECT);
> > +}
> > +
> > +void ff_amf_write_field_name(uint8_t **dst, const char *str)
> > +{
> > +    bytestream_put_be16(dst, strlen(str));
> > +    bytestream_put_buffer(dst, str, strlen(str));
> > +}
> > +
> > +void ff_amf_write_object_end(uint8_t **dst)
> > +{
> > +    // first two bytes are field name length = 0, AMF object should end with it and end marker
> > +    bytestream_put_be24(dst, AMF_DATA_TYPE_OBJECT_END);
> > +}
> > +
> > +int ff_rtmp_packet_read(URLContext *h, RTMPPacket *p,
> > +                     int chunk_size, RTMPPacket *prev_pkt)
> > +{
> > +    uint8_t hdr, t, buf[16];
> > +    int channel_id, timestamp, data_size, offset = 0, extra = 0;
> > +    uint8_t type;
> > +
> > +    if (url_read(h, &hdr, 1) != 1) {
> > +        return -1;
> > +    }
> > +    channel_id = hdr & 0x3F;
> > +
> > +    hdr >>= 6;
> > +    if (hdr == RTMP_PS_ONEBYTE) {
> > +        //todo
> > +        return -1;
> > +    } else {
> > +        if (url_read_complete(h, buf, 3) != 3) {
> > +            return -1;
> > +        }
> > +        timestamp = AV_RB24(buf);
> > +        if (hdr != RTMP_PS_FOURBYTES) {
> > +            if (url_read_complete(h, buf, 3) != 3) {
> > +                return -1;
> > +            }
> > +            data_size = AV_RB24(buf);
> > +            if (url_read_complete(h, &type, 1) != 1) {
> > +                return -1;
> > +            }
> > +            if (hdr == RTMP_PS_TWELVEBYTES) {
> > +                if (url_read(h, buf, 4) != 4) {
> > +                    return -1;
> > +                }
> > +                extra = AV_RL32(buf);
> > +            } else {
> > +                extra = prev_pkt[channel_id].extra;
> > +            }
> > +        } else {
> > +            data_size = prev_pkt[channel_id].data_size;
> > +            type      = prev_pkt[channel_id].type;
> > +            extra     = prev_pkt[channel_id].extra;
> > +        }
> > +    }
> > +    ff_rtmp_packet_create(p, channel_id, type, timestamp, data_size);
> > +    p->extra = extra;
> > +    // save history
> 
> > +    prev_pkt[channel_id].channel_id = type;
> > +    prev_pkt[channel_id].type       = channel_id;
> 
> is that exchange intended?

no 
 
> [...]
> > +int ff_rtmp_packet_create(RTMPPacket *pkt, int channel_id, RTMPPacketType type,
> > +                       int timestamp, int size)
> > +{
> > +    pkt->data = av_malloc(size);
> > +    if (!pkt->data)
> > +        return -1;
> 
> ENOMEM

added
 
> [...]
> > +/**
> > + * Creates new RTMP packet with given attributes.
> > + *
> > + * @param pkt        packet
> > + * @param channel_id packet channel ID
> > + * @param type       packet type
> > + * @param timestamp  packet timestamp
> > + * @param size       packet size
> 
> > + * @return zero on success, -1 otherwise
> 
> in ffmpeg <0 is error, please follow this convention, it simplifies returning
> error codes in the future (with above a review of any == -1 checks is needed)

changed documentation (and it was not checked for -1 anywhere)
 
> [...]
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
-------------- next part --------------
A non-text attachment was scrubbed...
Name: rtmp.patch
Type: text/x-diff
Size: 40275 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090721/5363596d/attachment.patch>



More information about the ffmpeg-devel mailing list