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

[Michael Niedermayer]

On Wed, Jul 22, 2009 at 07:58:05AM +0300, Kostya wrote:
> On Tue, Jul 21, 2009 at 11:30:26PM +0200, Michael Niedermayer wrote:
> > On Tue, Jul 21, 2009 at 11:04:09AM +0300, Kostya wrote:
> > > 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:
[...]
> > [...]
> > > +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;
> > 
> > uint8_t digest[32]={ ...}
>  
> Ahem, no need to initialize digest[] since hash function will write to
> it anyway. And if you meant tosend[],

minor mixop ...


> I think current initialization
> for it is clearer. 

hmm, do you think :

uint8_t tosend    [RTMP_HANDSHAKE_PACKET_SIZE+1]={
    3,
    0,0,0,0,
    RTMP_CLIENT_VER1,
    RTMP_CLIENT_VER2,
    RTMP_CLIENT_VER3,
    RTMP_CLIENT_VER4,
};

is worse? to me it looks clearer ...


[...]
> > [...]
> > > +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);
> > > +        if (rt->chunk_size <= 0) {
> > > +            //av_log(s, AV_LOG_ERROR, "Incorrect chunk size %d\n", rt->chunk_size);
> > > +            return -1;
> > > +        }
> > 
> > this leaves an invalid chunk_size in the context, is it guranteed that its 
> > not used?
> 
> Yes, it will return EIO in packet_read(). 

and we hope that no future calls happen after that?


[...]
> > [...]
> > > +URLProtocol rtmp_protocol = {
> > > +    "rtmp",
> > > +    rtmp_open,
> > > +    rtmp_read,
> > > +    rtmp_write,
> > > +    NULL, /* seek */
> > 
> > speaking of seeking, doesnt rtmp support seeking of some kind?
> 
> It does, the question is how to invoke it since it requires sending a
> command to server "seek to Nth second" and FLV demuxer does not know
> about that.
> 
> Can we introduce protocol-aware seeking, i.e. seek() call from demuxer
> will try to call such seek and if it fails - generic one.

we possibly will have to add something somewhere ...
i think this or a similar discussion occured already in the context of
some other network stuff, maybe someone remembers what/if we decided
something there?

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

I hate to see young programmers poisoned by the kind of thinking
Ulrich Drepper puts forward since it is simply too narrow -- Roman Shaposhnik
-------------- 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/20090722/b83e4b02/attachment.pgp>