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

Michael Niedermayer michaelni
Sat Jul 18 11:29:34 CEST 2009


On Fri, Jul 17, 2009 at 06:38:46PM +0300, Kostya wrote:
> $subj
> 
> For those who have not read Adobe RTMP spec (like me):
>  RTMP works over TCP (HTTP tunneling and encrypted version are possible
> too), sending AMF requests and getting AMF responses. Video and audio
> data may be sent in a single packets or as a bunch of FLV packets at
> once.
> 
> I could implement it in several ways, but I prefer to make it as a
> protocol handler faking FLV file:
> * We need protocol handler anyway - either native one or hacked TCP one
>   which is unclean IMO;
> * Because of probing we should either return no data on first read in
>   protocol handler or fake some data. This makes either decoder or proto
>   handler almost unused.
> * RTMP demuxer must handle the same data as FLV demuxer does, so some
>   work should be passed to it. In case of demuxer we must use
>   ByteIOContext and somehow fake FLV demuxer context.
> 
> So I've tried dummy RTMP proto + RTMP demuxer + FLV demuxer hack and
> found it rather unclean, so current version fakes whole FLV in protocol
> handler.

[...]
> Index: libavformat/rtmp.h
> ===================================================================
> --- libavformat/rtmp.h	(revision 0)
> +++ libavformat/rtmp.h	(revision 0)
> @@ -0,0 +1,39 @@
> +/*
> + * RTMP definitions
> + * Copyright (c) 2009 Kostya Shishkov
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#ifndef AVFORMAT_RTMP_H
> +#define AVFORMAT_RTMP_H
> +
> +#include "avformat.h"
> +

> +/** RTMP default port */
> +#define RTMP_DEFAULT_PORT 1935

very usefull comment


> +
> +/** RTMP handshake data size */
> +#define RTMP_HANDSHAKE_PACKET_SIZE 1536

same


> +
> +#define RTMP_CLIENT_PLATFORM "LNX"

LooNiX ?


[...]
> +/** protocol handler context */
> +typedef struct RTMPContext {

> +    URLContext*   rtmp_hd;                    ///< context for TCP stream

wouldnt a variable name that was related to "context for TCP stream" be
clearer?



> +    RTMPPacket    prev_pkt[2][RTMP_CHANNELS]; ///< packet history used when reading and sending packets

> +    int           chunk_size;                 ///< chunk size

every comment that duplicates the variable name clutters ones view with
useless information
every comment that duplicates the variable name clutters ones view with
useless information

see? you also dont like it if i repeat a comment twice, and similarly a
reader trying to understand the code almost certainly prefers not to find
in a comment just the variable name


> +    char          playpath[256];              ///< RTMP playpath
> +    ClientState   state;                      ///< current state
> +    int           main_channel_id;            ///< an additional channel id which is used for some invokes
> +    uint8_t*      flv_data;                   ///< buffer with data for demuxer
> +    int           flv_size;                   ///< current buffer size
> +    int           flv_off;                    ///< number of bytes read from current buffer

> +    uint32_t      video_ts;                   ///< current video timestamp
> +    uint32_t      audio_ts;                   ///< current audio timestamp

when it comes to timestamps my first thought tends to be, in what timebase
are they? that should be in the comment IMHO


> +} RTMPContext;
> +
> +#define PLAYER_KEY_OPEN_PART_LEN 30   ///< length of partial key used for first client digest signing
> +/** Client key used for digest signing */
> +static const uint8_t rtmp_player_key[] =
> +{
> +    0x47, 0x65, 0x6E, 0x75, 0x69, 0x6E, 0x65, 0x20, 0x41, 0x64, 0x6F, 0x62, 0x65,
> +    0x20, 0x46, 0x6C, 0x61, 0x73, 0x68, 0x20, 0x50, 0x6C, 0x61, 0x79, 0x65, 0x72,
> +    0x20, 0x30, 0x30, 0x31, 0xF0, 0xEE, 0xC2, 0x4A, 0x80, 0x68, 0xBE, 0xE8, 0x2E,
> +    0x00, 0xD0, 0xD1, 0x02, 0x9E, 0x7E, 0x57, 0x6E, 0xEC, 0x5D, 0x2D, 0x29, 0x80,
> +    0x6F, 0xAB, 0x93, 0xB8, 0xE6, 0x36, 0xCF, 0xEB, 0x31, 0xAE
> +};
> +

> +#define SERVER_KEY_OPEN_PART_LEN 36   ///< length of partial key used for first server digest signing
> +/** Key used for RTMP server digest signing */
> +static const uint8_t rtmp_server_key[] =
> +{
> +    0x47, 0x65, 0x6E, 0x75, 0x69, 0x6E, 0x65, 0x20, 0x41, 0x64, 0x6F, 0x62, 0x65,
> +    0x20, 0x46, 0x6C, 0x61, 0x73, 0x68, 0x20, 0x4D, 0x65, 0x64, 0x69, 0x61, 0x20,
> +    0x53, 0x65, 0x72, 0x76, 0x65, 0x72, 0x20, 0x30, 0x30, 0x31, // Genuine Adobe Flash Media Server 001

64 6f 20 79 6f 75 20 63 6f 6e 73 69 64 65 72 20
68 65 78 20 6d 6f 72 65 20 72 65 61 64 61 62 6c
65 20 74 68 61 6e 20 70 6c 61 69 6e 20 74 65 78
74 3f 0a


[...]
> +
> +static void gen_connect(URLContext *s, RTMPContext *rt, const char *proto,
> +                        const char *host, int port, const char *app)
> +{
> +    RTMPPacket pkt;
> +    uint8_t ver[32], *p;
> +    char tcurl[512];
> +    double num = 1.0;
> +    uint8_t bool;
> +
> +    rtmp_packet_create(&pkt, RTMP_VIDEO_CHANNEL, RTMP_PT_INVOKE, 0, 4096);
> +    p = pkt.data;
> +
> +    snprintf(tcurl, sizeof(tcurl), "%s://%s:%d/%s", proto, host, port, app);
> +    rtmp_amf_write_tag(&p, AMF_STRING, "connect");
> +    rtmp_amf_write_tag(&p, AMF_NUMBER, &num);
> +    rtmp_amf_write_tag(&p, AMF_OBJECT, NULL);
> +    rtmp_amf_write_tag(&p, AMF_STRING_IN_OBJECT, "app");
> +    rtmp_amf_write_tag(&p, AMF_STRING, app);
> +
> +    snprintf(ver, sizeof(ver), "%s %d,%d,%d,%d", RTMP_CLIENT_PLATFORM, RTMP_CLIENT_VER1,
> +             RTMP_CLIENT_VER2, RTMP_CLIENT_VER3, RTMP_CLIENT_VER4);
> +    rtmp_amf_write_tag(&p, AMF_STRING_IN_OBJECT, "flashVer");
> +    rtmp_amf_write_tag(&p, AMF_STRING, ver);
> +    rtmp_amf_write_tag(&p, AMF_STRING_IN_OBJECT, "tcUrl");
> +    rtmp_amf_write_tag(&p, AMF_STRING, tcurl);
> +    bool = 0;
> +    rtmp_amf_write_tag(&p, AMF_STRING_IN_OBJECT, "fpad");
> +    rtmp_amf_write_tag(&p, AMF_NUMBER, &bool);
> +    num = 15.0;
> +    rtmp_amf_write_tag(&p, AMF_STRING_IN_OBJECT, "capabilities");
> +    rtmp_amf_write_tag(&p, AMF_NUMBER, &num);
> +    num = 1639.0;
> +    rtmp_amf_write_tag(&p, AMF_STRING_IN_OBJECT, "audioCodecs");
> +    rtmp_amf_write_tag(&p, AMF_NUMBER, &num);
> +    num = 252.0;
> +    rtmp_amf_write_tag(&p, AMF_STRING_IN_OBJECT, "videoCodecs");
> +    rtmp_amf_write_tag(&p, AMF_NUMBER, &num);
> +    num = 1.0;
> +    rtmp_amf_write_tag(&p, AMF_STRING_IN_OBJECT, "videoFunction");
> +    rtmp_amf_write_tag(&p, AMF_NUMBER, &num);
> +    rtmp_amf_write_tag(&p, AMF_OBJECT_END, NULL);

it feels a little ugly to do things like
num=1
write(&num)
instead of
write(1);


[...]
> +//TODO: Move HMAC code somewhere. Eventually.

good idea!
also it should be a seperate patch


[...]
> +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 ...


[...]
> +    case RTMP_PT_INVOKE:
> +        if (!memcmp(pkt->data, "\002\000\006_error", 9)) {//TODO: search data for error description

yes i agree with the TODO


[...]
> +int rtmp_packet_write(URLContext *h, RTMPPacket *pkt,
> +                      int chunk_size, RTMPPacket *prev_pkt)
> +{
> +    uint8_t pkt_hdr[16], *p = pkt_hdr;
> +    int mode = RTMP_PS_TWELVEBYTES;
> +    int off = 0;
> +
> +//    if (pkt->type != RTMP_PT_INVOKE)
> +//        mode = RTMP_PS_EIGHTBYTES;
> +    bytestream_put_byte(&p, pkt->channel_id | (mode << 6));

that is debuging left over? or unfinished?
either way random outcommented code with no comment is not ok IMHO


[...]
> +/* maximum possible number of different RTMP channels */
> +#define RTMP_CHANNELS 64

doxy


[...]
> +/**
> + * AMF types used in RTMP packets
> + */
> +typedef enum AMFType {
> +    AMF_NUMBER = 0,   ///< number (double precision)
> +    AMF_BOOLEAN,      ///< boolean value
> +    AMF_STRING,       ///< Pascal-style string with length < 65536
> +    AMF_OBJECT,       ///< AMF object, contains property names and values
> +    AMF_MOVIE,        ///< Flash object
> +    AMF_NULL,         ///< NULL value
> +    AMF_UNDEFINED,    ///< undefined (return?) value
> +    AMF_REFERENCE,    ///< reference
> +    AMF_ECMA_ARRAY,   ///< ECMA array, almost like AMF object but has number of entries
> +    AMF_OBJECT_END,   ///< marker for end of AMF object or ECMA array
> +    AMF_STRICT_ARRAY, ///< strict array
> +    AMF_DATE,         ///< date
> +    AMF_LONG_STRING,  ///< Pascal-style string with possible length up to 4GB
> +    AMF_UNSUPPORTED,  ///< unsipported feature indicator
> +    AMD_RECORD_SET,   ///< record set
> +    AMF_XML_OBJECT,   ///< XML object
> +    AMF_TYPED_OBJECT, ///< typed object
> +
> +    AMF_STRING_IN_OBJECT = 99, ///< internal type used for AMF object field names
> +} AMFType;

is this duplicated of flv.h ?


[...]
> +/**
> + * 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
> + */
> +int rtmp_packet_create(RTMPPacket *pkt, int channel_id, RTMPPacketType type,
> +                       int timestamp, int size);

that is missing a ff/av prefix or static keyword

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

Old school: Use the lowest level language in which you can solve the problem
            conveniently.
New school: Use the highest level language in which the latest supercomputer
            can solve the problem without the user falling asleep waiting.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090718/9ab7ae29/attachment.pgp>



More information about the ffmpeg-devel mailing list