[FFmpeg-devel] Unix "fileno" libavformat module

[Benoit Fouet]

Hi,

William Ahern wrote:
> On Tue, Jul 10, 2007 at 11:44:30AM +0200, Diego Biurrun wrote:
>   
>> On Mon, Jul 09, 2007 at 06:13:20PM -0700, William Ahern wrote:
>>     
>>> On Mon, Jul 09, 2007 at 06:05:58PM -0700, William Ahern wrote:
>>> <snip> 
>>>       
>>>> Also, I put an LGPL 3 header on this file. I don't mean to start anything;
>>>> just let me know if I need to use an LGPL 2 header.
>>>>         
>>> Ouch. I'm just now reading the long thread, "GPL version matter". Please
>>> don't yell at me ;)
>>>       
>> I was about to.  Resubmit this with the default header we use please.
>>
>>     
>
> Attached is a proper patch against a checkout from earlier today (by GMT).
> I've added the requested license header and replaced my 8-column aligned
> tabs with 4-column aligned spaces (*shudder*). The altered build and header
> files are included, as well.
>
>   
> ------------------------------------------------------------------------
>
> Index: ffmpeg/libavformat/Makefile
> ===================================================================
> --- ffmpeg/libavformat/Makefile	(revision 1323)
> +++ ffmpeg/libavformat/Makefile	(revision 1326)
> @@ -169,6 +169,7 @@
>  OBJS-$(CONFIG_RTP_PROTOCOL)              += rtpproto.o
>  OBJS-$(CONFIG_TCP_PROTOCOL)              += tcp.o
>  OBJS-$(CONFIG_UDP_PROTOCOL)              += udp.o
> +OBJS-$(CONFIG_FILENO_PROTOCOL)           += fileno.o
>  
>  NAME=avformat
>  LIBVERSION=$(LAVFVERSION)
> Index: ffmpeg/libavformat/avio.h
> ===================================================================
> --- ffmpeg/libavformat/avio.h	(revision 1323)
> +++ ffmpeg/libavformat/avio.h	(revision 1326)
> @@ -270,5 +270,8 @@
>  /* http.c */
>  extern URLProtocol http_protocol;
>  
> +/* fileno.c */
> +extern URLProtocol fileno_protocol;
> +
>  #endif
>  
> Index: ffmpeg/libavformat/fileno.c
> ===================================================================
> --- ffmpeg/libavformat/fileno.c	(revision 0)
> +++ ffmpeg/libavformat/fileno.c	(revision 1326)
> @@ -0,0 +1,181 @@
> +/*
> + * Unix "fileno" I/O handler for libavformat.
> + *
> + * Copyright (c) 2007 RemoTV, Inc. (William Ahern <wahern at remotv.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
> + */
> +#if FILENO_DEBUG
>   

please break a line between header and your #if

> +#include <stdio.h>	/* stderr fputs(3) fprintf(3) */
>   

tabs are forbidden

> +#endif
> +
> +#include <stddef.h>	/* NULL */
> +#include <stdlib.h>	/* strtoul(3) */
> +
> +#include <limits.h>	/* INT_MAX ULONG_MAX */ 
>   

trailing whitespace (and tabs)

> +
> +#include <errno.h>	/* ENOMEM EINVAL ERANGE */
> +
>   

see AVERROR()

> +#include <string.h>	/* strncmp(3) */
> +
> +#include <sys/param.h>	/* MIN */
> +
>   

see FFMIN

> +#include <unistd.h>	/* read(2) write(2) close(2) */
> +
> +#include "avformat.h"
> +
> +
> +#if FILENO_DEBUG
> +#define MARK	fprintf(stderr, "@@ %d\n", __LINE__)
> +#endif
> +
> +
> +#define BUF_RLEN(b)	((b)->w.pos - (b)->r.pos)
> +#define BUF_WLEN(b)	((b)->end - (b)->w.pos)
> +
>   

more tabs

> +typedef struct {
> +    int fd;
> +
> +    struct {
> +        unsigned char base[65536];
> +
>   

can't this one be dynamically allocated ?

> +        struct {
> +            unsigned char *pos;
> +        } r, w;
> +
> +        unsigned char *end;
> +    } rbuf;
> +} FilenoContext;
> +
> +
> +static int fileno_open(URLContext *h, const char *uri, int flags) {
> +    FilenoContext *s;
> +    struct { const char *pos; char *end; } src;
> +    unsigned long lu;
> +
> +    if (0 == (s = av_malloc(sizeof *s)))
> +        return -ENOMEM;
> +
> +    s->rbuf.end = &s->rbuf.base[sizeof s->rbuf.base];
> +    s->rbuf.r.pos = s->rbuf.base;
> +    s->rbuf.w.pos = s->rbuf.base;
> +
> +    h->priv_data = s;
> +
> +    if (0 != strncmp(uri, "fileno://", sizeof "fileno://" - 1))
> +        return -EINVAL;
>   

mem leak

> +
> +    src.pos = uri + sizeof "fileno://" - 1;
> +
> +    lu = strtoul(src.pos, &src.end, 10);
> +
> +    if ((lu == ULONG_MAX && errno == ERANGE) || lu > INT_MAX)
> +        return -EINVAL;
> +
>   

mem leak

> +    s->fd = lu;
> +
> +    return 0;
> +} /* fileno_open() */
> +
> +
> +static int fileno_read(URLContext *h, uint8_t *buf, int size) {
> +    FilenoContext *s = h->priv_data;
> +    int n;
> +
> +retry:
> +    /* Read from buffer. */
> +    if (BUF_RLEN(&s->rbuf) > 0) {
> +        size = MIN(size, BUF_RLEN(&s->rbuf));
> +
>   

FFMIN

> +        (void)memcpy(buf, s->rbuf.r.pos, size);
>   

why this cast ?

> +        s->rbuf.r.pos += size;
> +
> +        return size;
> +    }
> +
> +    /* Fill buffer. */
> +    if  (BUF_WLEN(&s->rbuf) > 0) {
> +        n = read(s->fd, s->rbuf.w.pos, BUF_WLEN(&s->rbuf));
> +
> +        if (n <= 0)
> +            return n;
> +
> +        s->rbuf.w.pos += n;
> +
> +        goto retry;
> +    }
> +
> +    /* We've shot past our ability to buffer. Do raw I/O, now. */
> +    return read(s->fd, buf, size);
> +} /* fileno_read() */
> +
> +
> +static int fileno_write(URLContext *h, uint8_t *buf, int size) {
> +    FilenoContext *s = h->priv_data;
> +
> +    return write(s->fd, buf, size);
> +} /* fileno_write() */
> +
> +
> +static offset_t fileno_seek(URLContext *h, offset_t off, int whence) {
> +#if FILENO_DEBUG
> +    const char *how[] = {
> +        [SEEK_SET]	= "SEEK_SET",
> +        [SEEK_CUR]	= "SEEK_CUR",
> +        [SEEK_END]	= "SEEK_END",
> +        [AVSEEK_SIZE]	= "AVSEEK_SIZE",
> +    };
>   

tabs

> +#endif
> +
> +    FilenoContext *s = h->priv_data;
> +
> +    if (whence == AVSEEK_SIZE)
> +        return -1;
> +    if (whence == SEEK_CUR || whence == SEEK_END)
> +        return -1;
> +    if (BUF_WLEN(&s->rbuf) == 0 && BUF_RLEN(&s->rbuf) == 0)
> +        return -1;    /* We've already read too far. */
> +    if (off > sizeof s->rbuf.base || off > (s->rbuf.w.pos - s->rbuf.base))
> +        return -1;    /* We can't seek past the buffer (or can we?). */
> +    if (off < -1)
> +        return -1;    /* But they specified "SEEK_SET". */
> +
> +    s->rbuf.r.pos = s->rbuf.base + off;
> +
> +    return 0;
> +} /* fileno_seek() */
> +
> +
> +static int fileno_close(URLContext *h) {
> +    FilenoContext *s = h->priv_data;
> +
> +    (void)close(s->fd);
> +
>   

why this cast ?

> +    av_free(s);
> +
> +    return 0;
> +} /* fileno_close() */
> +
> +
> +URLProtocol fileno_protocol = {
> +    "fileno",
> +    fileno_open,
> +    fileno_read,
> +    fileno_write,
> +    fileno_seek,
> +    fileno_close,
> +}; /* fileno_protocol */
> Index: ffmpeg/libavformat/allformats.c
> ===================================================================
> --- ffmpeg/libavformat/allformats.c	(revision 1323)
> +++ ffmpeg/libavformat/allformats.c	(revision 1326)
> @@ -172,4 +172,5 @@
>      REGISTER_PROTOCOL(RTP, rtp);
>      REGISTER_PROTOCOL(TCP, tcp);
>      REGISTER_PROTOCOL(UDP, udp);
> +    REGISTER_PROTOCOL(FILENO, fileno);
>  }
>   
>   

-- 
Ben
Purple Labs S.A.
www.purplelabs.com