[FFmpeg-devel] [patch]add mmsh protocol and extract common code for mmst.c

[Ronald S. Bultje]

Hi,

On Sat, Aug 21, 2010 at 4:16 AM, Stefano Sabatini
<stefano.sabatini-lala at poste.it> wrote:
> On date Saturday 2010-08-21 11:04:01 +0800, zhentan feng encoded:
>> On Fri, Aug 20, 2010 at 11:33 PM, Stefano Sabatini <
>> stefano.sabatini-lala at poste.it> wrote:
>>
>> > On date Friday 2010-08-20 23:10:56 +0800, zhentan feng encoded:
>> > [...]
>> > > Index: libavformat/mmsh.c
>> > > ===================================================================
>> > > --- libavformat/mmsh.c ? ? ? ?(revision 0)
>> > > +++ libavformat/mmsh.c ? ? ? ?(revision 0)
>> > > @@ -0,0 +1,367 @@
>> > > +/*
>> > > + * MMS protocol over HTTP
>> > > + * Copyright (c) 2010 Zhentan Feng <spyfeng at gmail dot com>
>> > > + *
>> > > + * This file is part of FFmpeg.
>> > > + *
>> > > + * FFmpeg is free software; you can redistribute it and/or
>> > > + * modify it under the terms of the GNU Lesser General Public
>> > > + * License as published by the Free Software Foundation; either
>> > > + * version 2.1 of the License, or (at your option) any later version.
>> > > + *
>> > > + * FFmpeg is distributed in the hope that it will be useful,
>> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. ?See the GNU
>> > > + * Lesser General Public License for more details.
>> > > + *
>> > > + * You should have received a copy of the GNU Lesser General Public
>> > > + * License along with FFmpeg; if not, write to the Free Software
>> > > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>> > 02110-1301 USA
>> > > + */
>> > > +
>> >
>> > > +/*
>> > > + * Reference
>> > > + * Windows Media HTTP Streaming Protocol.
>> > > + * http://msdn.microsoft.com/en-us/library/cc251059(PROT.10).aspx
>> > > + */
>> > > +#include <string.h>
>> >
>> > Add an empty line between comments and first #include line for
>> > enhanced readability.
>> >
>> >
>>
>> added.
>>
>> > [...]
>> > > +static int read_data_packet(MMSHContext *mmsh, const int len)
>> > > +{
>> > > + ? ?MMSContext *mms ? = &mmsh->mms;
>> > > + ? ?int res;
>> > > + ? ?if (len > sizeof(mms->in_buffer)) {
>> >
>> > > + ? ? ? ?av_log(NULL, AV_LOG_ERROR,
>> > > + ? ? ? ? ? ? ?"Data packet length %d exceeds the in_buffer size %d\n",
>> > > + ? ? ? ? ? ? ? len, sizeof(mms->in_buffer));
>> >
>> > Indent one char more the "foo" lines, here and below, I mean:
>> >
>> > ? ? ? ?av_log(NULL, AV_LOG_ERROR,
>> > ? ? ? ? ? ? ? "Data packet length %d exceeds the in_buffer size %d\n",
>> > ? ? ? ? ? ? ? len, sizeof(mms->in_buffer));
>> >
>> > [...]
>> >
>> >
>>
>> modified.
>>
>> > > +static int handle_chunk_type(MMSHContext *mmsh)
>> > > +{
>> > > + ? ?MMSContext *mms = &mmsh->mms;
>> > > + ? ?int res, len = 0;
>> > > + ? ?ChunkType chunk_type;
>> > > + ? ?chunk_type = get_chunk_header(mmsh, &len);
>> > > +
>> > > + ? ?switch (chunk_type) {
>> > > + ? ?case CHUNK_TYPE_END:
>> > > + ? ? ? ?mmsh->chunk_seq = 0;
>> > > + ? ? ? ?av_log(NULL, AV_LOG_ERROR, "Stream ended!\n");
>> > > + ? ? ? ?return AVERROR(EIO);
>> > > + ? ?case CHUNK_TYPE_STREAM_CHANGE:
>> > > + ? ? ? ?mms->header_parsed = 0;
>> >
>> > > + ? ? ? ?if ((res = get_http_header_data(mmsh)) != 0) {
>> >
>> > This can also be:
>> > ? ? ? ? ? if ((res = get_http_header_data(mmsh))) {
>> >
>> > or maybe it's better check for < 0, more future-proof.
>> >
>> >
>>
>> modified.
>>
>> > > + ? ? ? ? ? ?av_log(NULL, AV_LOG_ERROR,"Stream changed! get new header
>> > failed!\n");
>> >
>> > Maybe better:
>> > av_log(NULL, AV_LOG_ERROR, "Stream changed! Failed to get new header!\n");
>> >
>>
>>
>>
>> yes, ?modified.
>
> No other objections from me, regards.

Awesome, applied then. Finally we have proper MMS support. \o/. Thanks
Zhentan, great work!

Ronald