[FFmpeg-devel] [PATCH] urlconcat protocol

Stefano Sabatini stefano.sabatini-lala
Sat Feb 6 01:39:35 CET 2010


On date Friday 2010-02-05 23:22:57 +0100, Michele Orr? encoded:
> fixed.
> 
> 2010/2/5 Michael Niedermayer <michaelni at gmx.at>
> 
> >
> >
> > also the whole error handling looks a little obfuscated
> > maybe it could be made less obfuscated by using a few lines
> > more code. I personally dont really mind either way but we might
> > overlook bugs in this ...
> >
> no good ideas about other ways to handle errors, there are so many little
> changes between each one.

> Index: libavformat/concat.c
> ===================================================================
> --- libavformat/concat.c	(revisione 0)
> +++ libavformat/concat.c	(revisione 0)
> @@ -0,0 +1,193 @@
> +/*
> + * Concat URL protocol
> + * Copyright (c) 2006 Steve Lhomme
> + * Copyright (c) 2007 Wolfram Gloger
> + * Copyright (c) 2010 Michele Orr??
> + *
> + * 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
> + */
> +
> +//#include <fcntl.h>
> +
> +#include "avformat.h"
> +#include "libavutil/avstring.h"
> +#include "libavutil/mem.h"
> +
> +#define AV_CAT_SEPARATOR "|"
> +
> +struct urlconcat_nodes {
> +    URLContext *uc;                   ///< node's URLContext
> +    int64_t     size;                 ///< url filesize
> +};
> +
> +struct urlconcat_data {
> +    struct urlconcat_nodes *nodes;    ///< list of nodes to concat
> +    size_t                  length;   ///< number of cat'ed nodes
> +    size_t                  current;  ///< index of currently read node
> +};

> +
> +

Nit, avoid double empty lines.

> +static int urlconcat_close(URLContext *h)
> +{
> +    int err = 0;
> +    size_t i;
> +    struct urlconcat_data *udata = h->priv_data;
> +    struct urlconcat_nodes *unodes = udata->nodes;
> +
> +    for (i=0; i != udata->length; i++)
> +       err |= url_close(unodes[i].uc) < 0 ? -1 : 0;
> +    av_freep(&udata->nodes);
> +    av_freep(&h->priv_data);
> +    return err;
> +}
> +
> +static int urlconcat_open(URLContext *h, const char *uri, int flags)
> +{

> +    char *urinod = NULL, *tmpunod;

Again, please use sane names, urinod is a weird abbreviation, logical
choice would be "node_uri" and "tmp_uri", also tmp_unod is used only
in the for (...) { } block, in general is a good idea to define
variables only in the block where they're used.

> +    int err = 0;
> +    int64_t size;
> +    size_t  len, i;
> +    URLContext *uc;
> +    struct urlconcat_data *udata;
> +    struct urlconcat_nodes *unodes;
> +
> +    av_strstart(uri, "cat:", &uri);
> +
> +    /* creating udata */
> +    if (!(udata = av_mallocz(sizeof(*udata))))
> +        return AVERROR(ENOMEM);
> +    h->priv_data = udata;

An empty line here may help readability.

> +    /* creating udata->nodes */
> +    for (i=0, len = 1; uri[i]; i++)  /* cat:[url]|[url] -> nodes = sep+1 */

nit i = 0, more K&R, also the comment is confusing, better to remove it.

[...]

Regards.
-- 
FFmpeg = Funny & Free Merciless Philosofic Earthshaking Glue



More information about the ffmpeg-devel mailing list