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

[Stefano Sabatini]

On date Saturday 2010-08-21 11:04:01 +0800, zhentan feng encoded:
> Hi
> 
> 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.
-- 
FFmpeg = Furious and Fiendish Muttering Pitiless Ecumenical Gem