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

[Michael Niedermayer]

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


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

#ifdef


[...]
> +    /** @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


> +
> +    /** @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


[...]

> +extern MMSProtocol mmst_mmsprotocol, mmsh_mmsprotocol;

first is unused


[...]
> +extern URLProtocol mmst_protocol;

unused


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


[...]
> +    *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   ) {


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


> +                    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 ...


[...]

> +            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() ?


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


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

not doxygen compatible

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

Opposition brings concord. Out of discord comes the fairest harmony.
-- Heraclitus
-------------- 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/20071010/99eb0f0b/attachment.pgp>