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

[Kostya]

On Thu, Jul 30, 2009 at 03:10:23AM +0200, Michael Niedermayer wrote:
> On Wed, Jul 29, 2009 at 09:26:05AM +0300, Kostya wrote:
> 
> [...]
> 
> > +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;
> > +    uint32_t extra = 0;
> > +    uint8_t type;
> > +
> > +    if (url_read(h, &hdr, 1) != 1) {
> > +        return AVERROR(EIO);
> > +    }
> > +    channel_id = hdr & 0x3F;
> > +
> > +    hdr >>= 6;
> > +    if (hdr == RTMP_PS_ONEBYTE) {
> > +        //todo
> > +        return -1;
> > +    } else {
> > +        if (url_read_complete(h, buf, 3) != 3)
> > +            return AVERROR(EIO);
> > +        timestamp = AV_RB24(buf);
> > +        if (hdr != RTMP_PS_FOURBYTES) {
> > +            if (url_read_complete(h, buf, 3) != 3)
> > +                return AVERROR(EIO);
> > +            data_size = AV_RB24(buf);
> > +            if (url_read_complete(h, &type, 1) != 1)
> > +                return AVERROR(EIO);
> > +            if (hdr == RTMP_PS_TWELVEBYTES) {
> > +                if (url_read_complete(h, buf, 4) != 4)
> > +                    return AVERROR(EIO);
> > +                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;
> > +        }
> 
> data_size = prev_pkt[channel_id].data_size;
> type      = prev_pkt[channel_id].type;
> extra     = prev_pkt[channel_id].extra;
> 
> if()
>       data_size = AV_RB24(buf);
> ...
 
changed
 
> > +    }
> > +    if (ff_rtmp_packet_create(p, channel_id, type, timestamp, data_size))
> > +        return -1;
> > +    p->extra = extra;
> > +    // save history
> > +    prev_pkt[channel_id].channel_id = channel_id;
> > +    prev_pkt[channel_id].type       = type;
> > +    prev_pkt[channel_id].data_size  = data_size;
> > +    prev_pkt[channel_id].timestamp  = timestamp;
> > +    prev_pkt[channel_id].extra      = extra;
> > +    while (data_size > 0) {
> > +        int toread = FFMIN(data_size, chunk_size);
> > +        if (url_read_complete(h, p->data + offset, toread) != toread) {
> > +            ff_rtmp_packet_destroy(p);
> > +            return AVERROR(EIO);
> > +        }
> > +        data_size -= chunk_size;
> > +        offset    += chunk_size;
> > +        if (data_size > 0) {
> > +            url_read_complete(h, &t, 1); //marker
> > +            if (t != (0xC0 + channel_id))
> > +                return -1;
> > +        }
> > +    }
> > +    return 0;
> > +}
> > +
> 
> [...]
> 
> > +int ff_amf_get_field_value(const uint8_t *data, const uint8_t *data_end,
> > +                           const uint8_t *name, uint8_t *dst, int dst_size)
> > +{
> > +    int namelen = strlen(name);
> > +    int len;
> > +
> > +    if (data >= data_end - 3)
> 
> i assume you meant
> 
> 3 >= data_end - data
> ?
> 
> because as written if data_end is NULL+2, this wouldnt work
 
yes 
 
> > +        return -1;
> > +    if (*data++ != AMF_DATA_TYPE_OBJECT)
> > +        return -1;
> > +    for (;;) {
> > +        int size = bytestream_get_be16(&data);
> > +        if (!size)
> > +            break;
> 
> > +        data += size;
> 
> this can overflow

added overflow checks here and in other places
 
> > +        if (data >= data_end)
> > +            return -1;
> > +        if (size == namelen && !memcmp(data-size, name, namelen)) {
> > +            switch (*data++) {
> > +            case AMF_DATA_TYPE_NUMBER:
> > +                snprintf(dst, dst_size, "%g", av_int2dbl(AV_RB64(data)));
> > +                return 0;
> > +            case AMF_DATA_TYPE_BOOL:
> > +                snprintf(dst, dst_size, "%s", *data ? "true" : "false");
> > +                return 0;
> > +            case AMF_DATA_TYPE_STRING:
> > +                len = bytestream_get_be16(&data);
> > +                av_strlcpy(dst, data, FFMIN(len+1, dst_size));
> > +                return 0;
> > +            default:
> > +                return -1;
> > +            }
> 
> the return 0 can be factored out

factored out 
 
> > +        }
> > +        len = ff_amf_tag_size(data, data_end);
> > +        if (len < 0 || data + len >= data_end)
> > +            return -1;
> 
> again overflow
> and there are more ...
> 
> > +        data += len;
> > +    }
> > +    return -1;
> > +}
> [...]
> 
> > +/**
> > + * structure for holding RTMP packets
> > + */
> > +typedef struct RTMPPacket {
> > +    uint8_t        channel_id; ///< RTMP channel ID (nothing to do with audio/video channels though)
> > +    RTMPPacketType type;       ///< packet payload type
> > +    uint32_t       timestamp;  ///< packet timestamp increment to the previous one (in milliseconds)
> 
> id call it timestamp_delta or _diff if its not a absolute timestamp

err, the funny thing it serves as both depending on packet type 
 
> [...]
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
-------------- next part --------------
A non-text attachment was scrubbed...
Name: rtmp.patch
Type: text/x-diff
Size: 43105 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090730/a9210c2e/attachment.patch>