[FFmpeg-devel] [patch]MMS protocol over TCP

Michael Niedermayer michaelni
Tue Mar 30 00:42:01 CEST 2010


On Sun, Mar 28, 2010 at 12:13:24AM +0800, zhentan feng wrote:
[...]
> +
> +/** Read incoming MMST media, header or command packet. */
> +static MMSSCPacketType get_tcp_server_response(MMSContext *mms)
> +{
> +    int read_result;
> +    MMSSCPacketType packet_type= -1;
> +    int done;
> +
> +    do {
> +        done= 1;
> +        if((read_result= url_read_complete(mms->mms_hd, mms->incoming_buffer, 8))==8) {
> +            // handle command packet.
> +            if(AV_RL32(mms->incoming_buffer + 4)==0xb00bface) {
> +                mms->incoming_flags= mms->incoming_buffer[3];
> +                read_result= url_read_complete(mms->mms_hd, mms->incoming_buffer+8, 4);
> +                if(read_result == 4) {
> +                    int length_remaining= AV_RL32(mms->incoming_buffer+8) + 4;
> +
> +                    dprintf(NULL, "Length remaining is %d\n", length_remaining);
> +                    // read the rest of the packet.
> +                    read_result = url_read_complete(mms->mms_hd, mms->incoming_buffer + 12,
> +                                                  length_remaining) ;
> +                    if (read_result == length_remaining) {
> +                        mms->incoming_buffer_length= length_remaining+12;
> +                        packet_type= AV_RL16(mms->incoming_buffer+36);
> +
> +                    } else {
> +                        dprintf(NULL, "3 read returned %d!\n", read_result);
> +                    }
> +                } else {
> +                    dprintf(NULL, "2 read returned %d!\n", read_result);
> +                }

> +            } else {
> +                int length_remaining;
> +                int packet_id_type;
> +                int tmp;
> +
> +                assert(mms->pkt_buf_len==0);
> +
> +                //** VERIFY LENGTH REMAINING HAS SPACE
> +                // note we cache the first 8 bytes,
> +                // then fill up the buffer with the others
> +                tmp                       = AV_RL16(mms->incoming_buffer + 6);
> +                length_remaining          = (tmp - 8) & 0xffff;
> +                mms->incoming_packet_seq  = AV_RL32(mms->incoming_buffer);
> +                packet_id_type            = mms->incoming_buffer[4];
> +                mms->incoming_flags       = mms->incoming_buffer[5];
> +                mms->pkt_buf_len          = length_remaining;
> +                mms->pkt_read_ptr         = mms->incoming_buffer;
> +
> +                read_result= url_read_complete(mms->mms_hd, mms->incoming_buffer, length_remaining);

is there any check for not overwriting the array bounds?


> +                if(read_result != length_remaining) {
> +                    dprintf(NULL, "read_bytes result: %d asking for %d\n",
> +                            read_result, length_remaining);
> +                    break;
> +                } else {
> +                    // if we successfully read everything.
> +                    if(packet_id_type == mms->header_packet_id) {
> +                        packet_type = SC_PKT_ASF_HEADER;
> +                        // Store the asf header
> +                        if(!mms->header_parsed) {

> +                            mms->asf_header = av_realloc(mms->asf_header,
> +                                              mms->asf_header_size
> +                                              + mms->pkt_buf_len);

missing check for realloc failure
also can mms->asf_header_size + mms->pkt_buf_len overflow? if so it
must be checked


> +                            memcpy(mms->asf_header + mms->asf_header_size,
> +                                                 mms->pkt_read_ptr,
> +                                                 mms->pkt_buf_len);
> +                            mms->asf_header_size += mms->pkt_buf_len;
> +                        }
> +                    } else if(packet_id_type == mms->packet_id) {
> +                        packet_type = SC_PKT_ASF_MEDIA;
> +                    } else {
> +                        dprintf(NULL, "packet id type %d is old.", packet_id_type);
> +                        done= 0;
> +                    }
> +                }
> +            }
> +        } else {
> +            if(read_result<0) {
> +                dprintf(NULL, "Read error (or cancelled) returned %d!\n", read_result);
> +                packet_type = SC_PKT_CANCEL;
> +            } else {
> +                dprintf(NULL, "Read result of zero?!\n");
> +                packet_type = SC_PKT_NO_DATA;
> +            }

> +            done = 1;

can done be anything else than 1 here?


[...]
> +static int asf_header_parser(MMSContext *mms)
> +{
> +    uint8_t *p = mms->asf_header, *end = mms->asf_header + mms->asf_header_size;
> +    mms->stream_num = 0;
> +
> +    if (mms->asf_header_size < sizeof(ff_asf_guid) * 2 + 22 ||
> +        memcmp(p, ff_asf_header, sizeof(ff_asf_guid)))
> +        return -1;
> +
> +    p += sizeof(ff_asf_guid) + 14;
> +    do {
> +        uint64_t chunksize = AV_RL64(p + sizeof(ff_asf_guid));
> +        if (!memcmp(p, ff_asf_file_header, sizeof(ff_asf_guid))) {
> +            /* read packet size */
> +            if (end - p > sizeof(ff_asf_guid) * 2 + 68) {
> +                mms->asf_packet_len = AV_RL32(p + sizeof(ff_asf_guid) * 2 + 64);
> +            }
> +        } else if (!memcmp(p, ff_asf_stream_header, sizeof(ff_asf_guid))) {
> +            mms->stream_num++;

shouldnt the number be limited somehow? at some place in the code
you write data for each stream into a fixed size buffer


[...]
> +static int mms_open_cnx(URLContext *h, const char *url)

why is this a seperate function?


[...]
> +    mms = av_malloc(sizeof(MMSContext));
> +    if (!mms)
> +        return AVERROR(ENOMEM);
> +    memset(mms, 0, sizeof(MMSContext));

av_mallocz()


[...]
> +/** Read ASF data through the protocol. */
> +static int mms_read(URLContext *h, uint8_t *buf, int size)
> +{
> +    /* TODO: see tcp.c:tcp_read() about a possible timeout scheme */
> +    MMSContext *mms = h->priv_data;
> +    int result = 0;
> +
> +    /* Since we read the header at open(), this shouldn't be possible */
> +    assert(mms->header_parsed);
> +
> +    if (mms->asf_header_read_pos >= mms->asf_header_size
> +        && !mms->streaming_flag) {
> +        dprintf(NULL, "mms_read() before play().\n");
> +        clear_stream_buffers(mms);
> +        result = send_stream_selection_request(mms);
> +        if(result < 0)
> +            return result;
> +        if (get_tcp_server_response(mms) != SC_PKT_STREAM_ID_ACCEPTED) {
> +            dprintf(NULL, "Can't get stream id accepted packet.\n");
> +            return 0;
> +        }
> +
> +        // send media packet request
> +        send_media_packet_request(mms);
> +        if (get_tcp_server_response(mms) == SC_PKT_MEDIA_PKT_FOLLOWS) {
> +           mms->streaming_flag = 1;
> +        } else {

> +            dprintf(NULL, "Canot get media follows packet from server.\n");

using NULL as context for dprintf() is not ideal

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

Asymptotically faster algorithms should always be preferred if you have
asymptotical amounts of data
-------------- 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/20100330/065ec5e2/attachment.pgp>



More information about the ffmpeg-devel mailing list