[FFmpeg-devel] ftp protocol support

Clément Bœsch ubitux at gmail.com
Thu May 16 16:17:15 CEST 2013


On Thu, May 16, 2013 at 03:58:53PM +0200, Lukasz M wrote:
[...]
> Hello,
> 
> Updated patch in attachment.
> 
> I'm not sure I supposed to add av_cold as patcheck tool suggested. The
> function it suggested to mark cold is called once in open callback. I
> think it would require open callback also make cold to make sense. I
> decided to ignore it.
> 

> From 753ccf7572060f94ddf1e7bff6f4f56dad25d866 Mon Sep 17 00:00:00 2001
> From: Lukasz Marek <lukasz.m.luki at gmail.com>
> Date: Wed, 15 May 2013 16:08:11 +0200
> Subject: [PATCH] FTP protocol support
> 
> Implementation of ftp protocol.
> 
> Fixes #1672
> 
> Signed-off-by: Lukasz Marek <lukasz.m.luki at gmail.com>
> ---
>  Changelog                |    1 +
>  MAINTAINERS              |    1 +
>  configure                |    1 +
>  doc/protocols.texi       |   34 +++
>  libavformat/Makefile     |    1 +
>  libavformat/allformats.c |    1 +
>  libavformat/ftp.c        |  661 ++++++++++++++++++++++++++++++++++++++++++++++
>  7 files changed, 700 insertions(+)
>  create mode 100644 libavformat/ftp.c
> 
> diff --git a/Changelog b/Changelog
> index c9b92d3..eee71c2 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -48,6 +48,7 @@ version <next>:
>  - zmq filters
>  - DCT denoiser filter (dctdnoiz)
>  - Wavelet denoiser filter ported from libmpcodecs as owdenoise (formerly "ow")
> +- FTP protocol support
>  
>  
>  version 1.2:
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 372f2a8..84fcbce 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -411,6 +411,7 @@ Muxers/Demuxers:
>  
>  Protocols:
>    bluray.c                              Petri Hintukainen
> +  ftp.c                                 Lukasz Marek
>    http.c                                Ronald S. Bultje
>    mms*.c                                Ronald S. Bultje
>    udp.c                                 Luca Abeni
> diff --git a/configure b/configure
> index 015b1bf..9d9cbe2 100755
> --- a/configure
> +++ b/configure
> @@ -2089,6 +2089,7 @@ ffrtmpcrypt_protocol_deps_any="gcrypt nettle openssl"
>  ffrtmpcrypt_protocol_select="tcp_protocol"
>  ffrtmphttp_protocol_deps="!librtmp_protocol"
>  ffrtmphttp_protocol_select="http_protocol"
> +ftp_protocol_select="tcp_protocol"
>  gopher_protocol_select="network"
>  httpproxy_protocol_select="tcp_protocol"
>  http_protocol_select="tcp_protocol"
> diff --git a/doc/protocols.texi b/doc/protocols.texi
> index 0c56b8b..70da4f5 100644
> --- a/doc/protocols.texi
> +++ b/doc/protocols.texi
> @@ -100,6 +100,40 @@ The ff* tools default to the file protocol, that is a resource
>  specified with the name "FILE.mpeg" is interpreted as the URL
>  "file:FILE.mpeg".
>  
> + at section ftp
> +
> +FTP (File Transfer Protocol)
> +
> +Allow to read from or write to remote resources using FTP protocol.
> +
> +Following syntax is required.
> + at example
> +ftp://[user[:password]@@]server[:port]/path/to/remote/resource.mpeg
> + at end example
> +
> +This protocol accepts the following options.
> +
> + at table @option
> + at item timeout
> +Set timeout of socket I/O operations used by the underlying low level
> +operation. By default it is set to -1, which means that the timeout is
> +not specified.
> +
> + at item ftp-anonymous-password
> +Password used when login as anonymous user. Typically an e-mail adress

address

> +should be used.
> +
> + at item ftp-write-seekable
> +Control seekability of connection during encoding. If set to 1 the
> +resource is supposed to be seekable, if set to 0 it is assumed not
> +to be seekable. Default value is 0.
> + at end table
> +
> +NOTE: Protocol can be used as output, but it is adviced to not do it,

advised, recommended

> +unless special care is taken (tests, customized server configuration etc.).
> +Different FTP servers behave in different way during seek operation.
> +ff* tools may produce incomplete content due to server limitations.
> +
>  @section gopher
>  
>  Gopher protocol.
> diff --git a/libavformat/Makefile b/libavformat/Makefile
> index 24dd3cd..abc2472 100644
> --- a/libavformat/Makefile
> +++ b/libavformat/Makefile
> @@ -424,6 +424,7 @@ OBJS-$(CONFIG_DATA_PROTOCOL)             += data_uri.o
>  OBJS-$(CONFIG_FFRTMPCRYPT_PROTOCOL)      += rtmpcrypt.o rtmpdh.o
>  OBJS-$(CONFIG_FFRTMPHTTP_PROTOCOL)       += rtmphttp.o
>  OBJS-$(CONFIG_FILE_PROTOCOL)             += file.o
> +OBJS-$(CONFIG_FTP_PROTOCOL)              += ftp.o
>  OBJS-$(CONFIG_GOPHER_PROTOCOL)           += gopher.o
>  OBJS-$(CONFIG_HLS_PROTOCOL)              += hlsproto.o
>  OBJS-$(CONFIG_HTTP_PROTOCOL)             += http.o httpauth.o urldecode.o
> diff --git a/libavformat/allformats.c b/libavformat/allformats.c
> index 2b2d46d..ab6a548 100644
> --- a/libavformat/allformats.c
> +++ b/libavformat/allformats.c
> @@ -313,6 +313,7 @@ void av_register_all(void)
>      REGISTER_PROTOCOL(FFRTMPCRYPT,      ffrtmpcrypt);
>      REGISTER_PROTOCOL(FFRTMPHTTP,       ffrtmphttp);
>      REGISTER_PROTOCOL(FILE,             file);
> +    REGISTER_PROTOCOL(FTP,              ftp);
>      REGISTER_PROTOCOL(GOPHER,           gopher);
>      REGISTER_PROTOCOL(HLS,              hls);
>      REGISTER_PROTOCOL(HTTP,             http);
> diff --git a/libavformat/ftp.c b/libavformat/ftp.c
> new file mode 100644
> index 0000000..b1dadb9
> --- /dev/null
> +++ b/libavformat/ftp.c
> @@ -0,0 +1,661 @@
> +/*
> + * FTP protocol for ffmpeg client

Looks pointless. If you insist in having it, it would belong below
copyright, with something like:

/**
 * @file
 * FTP protocol
 */

> + * Copyright (c) 2013 Lukasz Marek
> + *
> + * 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 <stdlib.h>
> +#include "libavutil/avstring.h"
> +#include "libavutil/avassert.h"
> +#include "avformat.h"
> +#include "internal.h"
> +#include "network.h"
> +#include "os_support.h"
> +#include "url.h"
> +#include "libavutil/opt.h"
> +
> +#define CONTROL_BUFFER_SIZE 1024
> +

Did you look at the AVBPrint API? It might be relevant to use it
eventually.

> +typedef enum {
> +    UNKNOWN,
> +    READY,
> +    DOWNLOADING,
> +    UPLOADING,
> +    DISCONNECTED
> +} FTPState;
> +
> +
> +typedef struct {
> +    const AVClass *class;
> +    URLContext *conn_control;        /**< Control connection */
> +    int conn_control_block_flag;     /**< Controls block/unblock mode of data connection */
> +    AVIOInterruptCB conn_control_interrupt_cb; /**< Controls block/unblock mode of data connection */
> +    URLContext *conn_data;           /**< Data connection, NULL when not connected */

> +    unsigned char control_buffer[CONTROL_BUFFER_SIZE], *control_buf_ptr, *control_buf_end; /**< Control connection buffer */

uint8_t

[...]
> +static int ftp_current_dir(FTPContext *s)
> +{
> +    char *res = NULL, *start = NULL, *end = NULL;
> +    int err, i = 0;
> +    const char *command = "PWD\r\n";
> +
> +    if ((err = ffurl_write(s->conn_control, command, strlen(command))) < 0)
> +        return err;
> +    if (257 != ftp_status(s, NULL, NULL, NULL, &res, 257))
> +        goto fail;
> +
> +    for (; i < strlen(res); ++i) {

for (i = 0; res[i]; i++)

> +        if (res[i] == '"') {
> +            start = res + i + 1;
> +            break;
> +        }
> +    }

> +    for (++i; i < strlen(res); ++i) {

ditto

Also, what if res[i]==0 at this point?

> +        if (res[i] == '"') {
> +            end = res + i;
> +            break;
> +        }
> +    }
> +    if (!start || !end)
> +        goto fail;
> +
> +    if (end[-1] == '/') {
> +        end[-1] = '\0';
> +    } else
> +        *end = '\0';
> +    av_strlcpy(s->path, start, sizeof(s->path));
> +
> +    av_free(res);
> +    return 0;
> +
> +  fail:
> +    av_free(res);
> +    return AVERROR(EIO);
> +}
> +
> +static int ftp_file_size(FTPContext *s)
> +{
> +    char buf[CONTROL_BUFFER_SIZE];
> +    int err;
> +    char *res = NULL;
> +
> +    snprintf(buf, sizeof(buf), "SIZE %s\r\n", s->path);
> +    if ((err = ffurl_write(s->conn_control, buf, strlen(buf))) < 0)
> +        return err;

> +    if (213 == ftp_status(s, NULL, NULL, NULL, &res, 213)) {

I have no opinion on the matter, but we tend to avoid yoda conditions in
the codebase.

[...]
> +static int ftp_reconnect_data_connection(URLContext *h)
> +{
> +    int err;
> +    char buf[CONTROL_BUFFER_SIZE], opts_format[20];
> +    AVDictionary *opts = NULL;
> +    FTPContext *s = h->priv_data;
> +
> +    if (!s->conn_data) {
> +        ff_url_join(buf, sizeof(buf), "tcp", NULL, s->hostname, s->server_data_port, NULL);
> +        if (s->rw_timeout != -1) {
> +            snprintf(opts_format, sizeof(opts_format), "%d", s->rw_timeout);
> +            av_dict_set(&opts, "timeout", opts_format, 0);
> +        } /* if option is not given, don't pass it and let tcp use its own default */
> +        err = ffurl_open(&s->conn_data, buf, AVIO_FLAG_READ_WRITE,
> +                            &h->interrupt_callback, &opts);

stylenit: vertical align

> +        av_dict_free(&opts);
> +        if (err < 0)
> +            return err;
> +    }
> +    s->state = READY;
> +    s->position = 0;
> +    return 0;
> +}
> +
[...]

Did you check with valgrind for any kind of memleaks?

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130516/436609ad/attachment.asc>


More information about the ffmpeg-devel mailing list