[FFmpeg-devel] [PATCH] MMS protocol support patch 1
Björn Axelsson
bjorn.axelsson
Thu Oct 11 14:59:00 CEST 2007
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>
More information about the ffmpeg-devel
mailing list