[FFmpeg-devel] [PATCH] MMS protocol support patch 1

[Björn Axelsson]

On Wed, 2007-10-10 at 04:09 +0200, Michael Niedermayer wrote:
> Hi
> 
> On Tue, Oct 09, 2007 at 02:00:25PM +0200, Bj?rn Axelsson wrote:
> [...]
> > I have also fixed the issues mentioned by Diego earlier in this thread
> > (some trailing whitespace and many long lines).
> > 
> > Attached patches:
> > 1. aviobuf_resetbuf.diff: add functionality to set the direction of a
> > ByteIOContext buffer.
> > 2. mms_protocol_base.diff: basic mmsh protocol support
> > 3. mms_seek_hack.diff: Add MMS seek and pause support to the ASF
> > demuxer.
> > 
> > Thanks to Neil Brown for suggesting using quilt for patch management,
> > and to Michael for taking his time for the review.
> [...]
> 
> (1. aviobuf_resetbuf.diff)
> 
> patch ok

Yay!

> [...]
> > @@ -105,6 +107,16 @@
> >  }
> >  #endif
> >  
> > +#if (defined CONFIG_MMSH_PROTOCOL)
> 
> #ifdef

Changed

> [...]
> > +    /** @name Outgoing Control Buffer
> > +     * Buffer for building outgoing MMST packets. */
> > +    /*@{*/
> > +    ByteIOContext outgoing_packet_data;                      ///< packet stream
> > +    uint8_t outgoing_packet_buffer[MMS_MAXIMUM_PACKET_LENGTH]; ///< packet data
> > +    /*@}*/
> 
> unused

Removed. Will be back in some form with the MMST protocol.

> > +
> > +    /** @name Incoming Control Buffer
> > +     * Buffer for incoming control packets. */
> > +    /*@{*/
> > +    uint8_t incoming_buffer[8*1024];///< Incoming buffer location.
> > +    int incoming_buffer_length;     ///< Incoming buffer length.
> > +    /*@}*/
> 
> unused

Removed and not missed.

> [...]
> 
> > +extern MMSProtocol mmst_mmsprotocol, mmsh_mmsprotocol;
> 
> first is unused
> 
> 
> [...]
> > +extern URLProtocol mmst_protocol;
> 
> unused

Both removed for now.

> [...]
> > +    int done = 0;
> > +
> > +    while(!done) {
> > +        int ch = get_byte(context);
> > +        if(url_feof(context) || ch == '\n') {
> > +            done = 1;
> > +        } else if(ch == '\r') {
> > +            /* eat it. */
> > +        } else if(dst-buffer < max_size-1) {
> > +            *dst++ = ch;
> > +        }
> > +    }
> 
> for(;;){
>     ...
>     if(...)
>         break;
>     ...
> }

Fixed.

> [...]
> > +    *chunk_type = get_le16(&mms->incoming_io_buffer);
> > +    *chunk_length = get_le16(&mms->incoming_io_buffer);
> 
> nitpick:
> *chunk_type   = get_le16(&mms->incoming_io_buffer);
> *chunk_length = get_le16(&mms->incoming_io_buffer);
> 
> 
> [...]
> > +        } else if(chunk_type == CHUNK_TYPE_HTTP_HEADER_START &&
> > +                  chunk_length == CHUNK_TYPE_HTTP_HEADER_SECOND) {
> 
> } else if(chunk_type   == CHUNK_TYPE_HTTP_HEADER_START &&
>           chunk_length == CHUNK_TYPE_HTTP_HEADER_SECOND   ) {
> 

Both fixed.

> [...]
> > +                if(http_code>=200 && http_code<300) {
> > +                    if(mms->state == AWAITING_PAUSE_ACKNOWLEDGE) {
> > +                        packet_type = SC_PACKET_STREAM_STOPPED_TYPE;
> > +                    }
> > +                } else if(http_code>=300 && http_code<400) {
> > +                    av_log(mms, AV_LOG_ERROR,
> > +                            "3xx redirection not implemented: %d %s\n",
> > +                            http_code, http_status);
> > +                    return SC_PACKET_TYPE_ERROR;
> > +                } else if(http_code != 200) {
> 
> how can it be 200 here? if it cant, why check ...

Removed extraneous check. Also removed one level of nesting.

> > +                    av_log(mms, AV_LOG_ERROR, "%d %s\n", http_code, http_status);
> > +                    return SC_PACKET_TYPE_ERROR;
> > +                }
> > +            } else {
> > +                av_log(mms, AV_LOG_ERROR, "Bad HTTP response: %s\n", line);
> > +                return SC_PACKET_TYPE_ERROR;
> > +            }
> > +        } else if((match = matches(line, "Content-Type: ")) != 0) {
> 
> > +            strncpy(content_type, match, sizeof(content_type));
> 
> av_strlcpy() is safer i think ...

True. Code changed.

> [...]
> 
> > +            if((features = matches_anywhere(match, "client-id=")) != 0) {
> > +                char id[64];
> > +                const char *src = features;
> > +                char *dst = id;
> > +
> > +                while(*src && *src !=',' && (src-features<sizeof(id)-1))
> > +                    *dst++ = *src++;
> > +                *dst = '\0';
> > +
> > +                mms->http_client_id = atoi(id);
> 
> why that copy before atoi() ?

I honestly hadn't even looked at that code. Simplified a lot.

> [...]
> > +    exact_length = strlen(outgoing_buffer);
> > +    write_result = url_write(mms->mms_hd, outgoing_buffer, exact_length);
> > +    av_log(mms, AV_LOG_DEBUG, "Sent header request:\n%s", outgoing_buffer);
> > +    if(write_result != exact_length) {
> > +        av_log(mms, AV_LOG_ERROR, "write failed, %d != %d\n",
> > +                write_result, exact_length);
> > +        ff_mms_set_state(mms, STATE_ERROR);
> > +        if(write_result >= 0)
> > +            write_result = AVERROR(EIO);
> > +    } else
> > +        ff_mms_set_state(mms, AWAITING_ASF_HEADER);
> > +    return write_result;
> > +}
> [...]
> > +    exact_length = strlen(outgoing_buffer);
> > +    write_result = url_write(mms->mms_hd, outgoing_buffer, exact_length);
> > +    if(write_result != exact_length) {
> > +        av_log(mms, AV_LOG_ERROR, "write failed, %d != %d\n",
> > +                write_result, exact_length);
> > +        ff_mms_set_state(mms, STATE_ERROR);
> > +        if(write_result >= 0)
> > +            write_result = AVERROR(EIO);
> > +    } else
> > +        ff_mms_set_state(mms, AWAITING_HTTP_PAUSE_CONTROL_ACKNOWLEDGE);
> > +
> > +    return write_result;
> > +}
> 
> duplicate

Duplicate code extracted from the two mentioned functions and also from
request_streaming_from().

> [...]
> > +/* Context stuff for logging */
> > +static const char *mmscontext_to_name(void *ptr)
> 
> not doxygen compatible

Changed to a doxygen group declaration for the context stuff.

-- 
Bj?rn Axelsson                    Phone: +46-(0)90-18 98 97
Intinor AB                          Fax: +46-(0)920-757 10
www.intinor.se
Interactive Television & Digital Media Distribution
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mms_protocol_base.diff
Type: text/x-patch
Size: 56023 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20071011/6228c104/attachment.bin>