[FFmpeg-devel] [PATCH] urlconcat protocol
Stefano Sabatini
stefano.sabatini-lala
Wed Jan 27 02:18:16 CET 2010
On date Tuesday 2010-01-26 18:55:30 +0100, Michele Orr? encoded:
> 2010/1/26 Michele Orr? <maker.py at gmail.com>
> >
> >
> > 2010/1/26 Benoit Fouet <benoit.fouet at free.fr>
> >
> >> Hi,
> >>
> >> [..]
> >
> > ok, thanks, I forgot checking for memory leak ( :O ). Now it should be ok.
> >
> > Regards,
> > Michele Orr?.
> >
>
> Oh, sorry, please use this patch insted.
> Index: libavformat/concat.c
> ===================================================================
> --- libavformat/concat.c (revisione 0)
> +++ libavformat/concat.c (revisione 0)
> @@ -0,0 +1,194 @@
> +/*
> + * Concat URL protocol
> + * Copyright (c) 2006 Steve Lhomme
> + * Copyright (c) 2007 Wolfram Gloger
> + * Copyright (c) 2010 Michele Orru'
Are not we using UTF-8 in files? In that case Orr? is fine.
First of all, won't this feature be superseded by the concat muxer/demuxer
(that implemented in the GSOC concat repo)? (This is more intended as a
question to the other devs.)
> + * 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_info {
> + URLContext * uc;
Nit: * ... uc -> *uc;
> + int64_t size;
> +};
> +
> +struct urlconcat_data {
> + struct urlconcat_info *urls;
> + size_t length;
> + size_t current;
> +};
Please doxify.
Also I'm not very happy with the urlconcat_info name, "info" is easily
confused with "data", maybe something like urlconcat_entry -> entries
may be better.
> +static int urlconcat_open(URLContext *h, const char *filename, int flags)
> +{
> + char *fn = NULL;
> + int err = 0;
> + int64_t size;
> + size_t len, i;
> + URLContext *uc;
> + struct urlconcat_data *udata;
> + struct urlconcat_info *uinfo;
> +
> + av_strstart(filename, "cat:", &filename);
> +
> + /* creating udata */
> + udata = av_mallocz(sizeof(*udata));
> + if (!udata)
> + return AVERROR(ENOMEM);
> + h->priv_data = udata;
> + /* creating udata->urls */
> + for (i=0, len = 1; filename[i]; i++) // cat:[url]|[url]->urls = sep+1
> + if (filename[i] == AV_CAT_SEPARATOR) len++;
> + uinfo = av_malloc(sizeof(*uinfo) * len);
> + if (!uinfo) {
> + av_free(udata);
> + h->priv_data = NULL;
> + return AVERROR(ENOMEM);
> + } else
> + udata->urls = uinfo;
> +
> + /* handle input */
> + if (!*filename) err = AVERROR(ENOENT);
> + for (i = 0; *filename; i++) {
> + /* parsing filename */
> + for (len = 0; filename[len] && filename[len] != AV_CAT_SEPARATOR; ++len);
> + fn = av_realloc(fn, len);
> + if (!fn) {
> + err = AVERROR(ENOMEM);
> + break;
> + }
> + av_strlcpy(fn, filename, len+1);
> + filename += len;
> + if (*filename == AV_CAT_SEPARATOR) ++filename;
> + /* creating URLContext */
> + if (url_open(&uc, fn, flags) < 0) {
> + err = AVERROR(ENOENT);
> + break;
> + }
Suggestion: maybe it's just simpler to use goto in case of failure,
and deinit stuff just in a single point:
do_all_fine();
return 0;
fail:
free_stuff();
return ret;
> + /* creating size */
> + size = url_filesize(uc);
> + if (size < 0) {
> + err = AVERROR(ENOSYS);
> + break;
> + }
> + /* assembling */
> + uinfo[i].uc = uc;
> + uinfo[i].size = size;
> + }
> + av_free(fn);
> + if (err < 0) {
> + av_free(uinfo);
> + av_free(udata);
> + h->priv_data = NULL;
> + return err;
> + }
> +
> + udata->length = i;
> + //av_realloc(uinfo, udata->length+1);
> + return 0;
> +}
> +
> +static int urlconcat_read(URLContext *h, unsigned char *buf, int size)
> +{
> + int result, total = 0;
> + struct urlconcat_data *udata = h->priv_data;
> + struct urlconcat_info *uinfo = udata->urls;
> + size_t i = udata->current;
> +
> + while (size > 0) {
> + result = url_read(uinfo[i].uc, buf, size);
> + if (result < 0)
> + return result;
> + else if (result == 0)
> + if (i + 1 == udata->length ||
> + url_seek(uinfo[++i].uc, 0, SEEK_SET) < 0)
> + break;
> + total += result;
> + buf += result;
> + size -= result;
> + }
> + udata->current = i;
> + return total;
> +}
> +
> +static int64_t urlconcat_seek(URLContext *h, int64_t pos, int whence)
> +{
> + int64_t result;
> + struct urlconcat_data *udata = h->priv_data;
> + struct urlconcat_info *uinfo = udata->urls;
Since uinfo is used to reference an array, maybe it's better to use a
variable name which is a plural form.
> + size_t i;
> +
> + switch (whence) {
> + case SEEK_END:
> + for (i = udata->length - 1;
> + i != 0 && pos < -uinfo[i-1].size;
> + i--)
> + pos += uinfo[i-1].size;
> + break;
> + case SEEK_CUR:
> + /* get the absolute position */
> + for (i = 0; i != udata->current; i++)
> + pos += uinfo[i].size;
> + pos += url_seek(uinfo[i].uc, 0, SEEK_CUR);
> + whence = SEEK_SET;
> + /* fall through with the absolute position */
> + case SEEK_SET:
> + for (i = 0; i != udata->length - 1 && pos >= uinfo[i].size; i++)
> + pos -= uinfo->size;
> + break;
> + default:
> + return AVERROR(EINVAL);
> + }
> + result = url_seek(uinfo[i].uc, pos, whence);
> + if (result >= 0) {
> + udata->current = i;
> + while (i != 0)
> + result += uinfo[i--].size;
> + }
> + return result;
> +}
> +
> +static int urlconcat_close(URLContext *h)
> +{
> + int err = 0;
> + size_t i;
> + struct urlconcat_data *udata = h->priv_data;
> + struct urlconcat_info *uinfo = udata->urls;
> +
> + for (i=0; i != udata->length; i++)
> + err |= url_close(uinfo[i].uc);
Why '|=' ?
> + av_free(uinfo);
> + av_free(udata);
> + h->priv_data = NULL;
> + return err < 0 ? AVERROR(EIO) : 0;
Why not simply err?
Regards, and thanks for the patch!
--
FFmpeg = Furious & Faboulous Mind-dumbing Plastic Elastic God
More information about the ffmpeg-devel
mailing list