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

[Michael Niedermayer]

On Sun, Apr 25, 2010 at 04:17:37PM +0800, zhentan feng wrote:
> Hi
> 
> On Fri, Apr 23, 2010 at 10:22 PM, Michael Niedermayer <michaelni at gmx.at>wrote:
[...]



> +/** Pad media packets smaller than max_packet_size and/or adjust read position
> +  * after a seek. */
> +static void pad_media_packet(MMSContext *mms)
> +{
> +    if(mms->remaining_in_len<mms->asf_packet_len) {
> +        int padding_size = mms->asf_packet_len - mms->remaining_in_len;
> +        memset(mms->in_buffer + mms->remaining_in_len, 0, padding_size);
> +        mms->remaining_in_len += padding_size;
> +    }
> +}
> +
> +/** Read incoming MMST media, header or command packet. */
> +static MMSSCPacketType get_tcp_server_response(MMSContext *mms)
> +{
> +    int read_result;
> +    MMSSCPacketType packet_type= -1;
> +
> +    for(;;) {
> +        if((read_result= url_read_complete(mms->mms_hd, mms->in_buffer, 8))==8) {


> +            // handle command packet.
> +            if(AV_RL32(mms->in_buffer + 4)==0xb00bface) {
> +                mms->incoming_flags= mms->in_buffer[3];
> +                read_result= url_read_complete(mms->mms_hd, mms->in_buffer+8, 4);
> +                if(read_result == 4) {
> +                    int length_remaining= AV_RL32(mms->in_buffer+8) + 4;
> +
> +                    dprintf(NULL, "Length remaining is %d\n", length_remaining);
> +                    // read the rest of the packet.
> +                    if (length_remaining < 0
> +                        || length_remaining > sizeof(mms->in_buffer) - 12) {
> +                        dprintf("Incoming message len %d exceeds buffer len %d\n",
> +                            length_remaining, sizeof(mms->in_buffer) - 12);
> +                        break;
> +                    }
> +                    read_result = url_read_complete(mms->mms_hd, mms->in_buffer + 12,
> +                                                  length_remaining) ;
> +                    if (read_result == length_remaining) {
> +                        packet_type= AV_RL16(mms->in_buffer+36);

this is still missing the check, the comment above says
"handle command packet" but this does not enforce it it can be a data packet
and the later code will treat it like it is one because its too late by that
time to detect. but its just packet_type that says its a data packet the
variables for data packets simply havnt been initialized

correct code is either
if(AV_RL16(mms->in_buffer+36) >= 0x40)
    break
packet_type= AV_RL16(mms->in_buffer+36);

or to change the values of the data packets in the enum to values that
are outside 0..0xFFFF


[...]
> +static int mms_safe_send_recv(MMSContext *mms, int (*send_fun)(), const MMSSCPacketType expect_type)
> +{
> +    MMSSCPacketType type;
> +    if(send_fun) {
> +        if (send_fun(mms) < 0) {
> +            dprintf(NULL, "Send Packet error before expecting recv packet %d\n", expect_type);
> +            return -1;

you should not loose the error code but return it


> +        }
> +    }
> +    if((type = get_tcp_server_response(mms)) != expect_type) {
> +        dprintf(NULL,"Unhandled packet type %d\n", type);
> +        return -1;
> +    }

this looks fragile as it assumes packets are send in order and without
buffering.

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

I wish the Xiph folks would stop pretending they've got something they
do not.  Somehow I fear this will remain a wish. -- M?ns Rullg?rd
-------------- 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/20100505/f772cc43/attachment.pgp>