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

[zhentan feng]

Hi

On Wed, May 5, 2010 at 8:13 AM, Michael Niedermayer <michaelni at gmx.at>wrote:

> 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
>
>
fixed it use the second solution.

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


>
> > +        }
> > +    }
> > +    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.
>
> according to the specification, the sequence of TCP packet is explicit
except for some situation.
I relax  the check condition to make it more robust.

please review the new patch.
thanks.
-- 
Best wishes~
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mmst_13.patch
Type: application/octet-stream
Size: 26922 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100512/0de9071b/attachment.obj>