[FFmpeg-devel] ftp protocol support

Michael Niedermayer michaelni at gmx.at
Wed May 15 23:22:22 CEST 2013


On Wed, May 15, 2013 at 06:11:35PM +0200, Lukasz M wrote:
> Hello,
> 
> I prepared patch for FTP protocol support. (Fixes #1672)
> Test with following FTP servers (Ubuntu 12.04)
> proftpd
> pure-ftpd
> wu-ftpd
> 
> Read operation works fine with all of them.
> Write operation is more complicated.
> It is working also fine when seek is not required, but usually does.
> 
> proftpd required following line in config to enable seek in write mode.
> AllowStoreRestart on
> 
> pure-ftpd truncutes file at current position after write so it is useless.
> 
> wu-ftpd is OK.
> 
> Because of that I added an option to enable seeking in write mode
> which is turned of by default.
> 
> I tested with following scenarios (few different media files):
> Write operation:
> ./ffmpeg -i 1.avi -vcodec copy -strict experimental
> -ftp-write-seekable 1 -acodec copy
> ftp://user:pass@localhost/output.avi
> binary diff to result of the same command with localfile as an output
> 
> Read operation
> ffplay ftp://user:pass@localhost/1.avi
> 
> Looking forward any remarks.

> 
> Best Regards,
> Lukasz Marek

>  configure                |    1 
>  libavformat/Makefile     |    1 
>  libavformat/allformats.c |    1 
>  libavformat/ftp.c        |  683 +++++++++++++++++++++++++++++++++++++++++++++++

Changelog entry & documentation update missing

[...]
> +/*
> + * This routine returns ftp server response code.
> + * Server may send more than one response for a certain command, following priorites are used:
> + *   - 5xx code is returned if occured. (means error)
> + *   - When pref_code is set then pref_code is return if occured. (expected result)
> + *   - The lowest code is returned. (means success)
> + */
> +static int ftp_status(FTPContext *s, int *major, int *minor, int *extra, char **line, int pref_code)
> +{
> +    int err, result = -1, pref_code_found = 0;
> +    char buf[CONTROL_BUFFER_SIZE];
> +    unsigned char d_major, d_minor, d_extra;
> +
> +    /* Set blocking mode */
> +    s->conn_control_block_flag = 0;
> +    for(;;) {
> +        if ((err = ftp_get_line(s, buf, CONTROL_BUFFER_SIZE)) < 0) {
> +            if (err == AVERROR_EXIT)
> +                return result;
> +            return err;
> +        }
> +
> +        if (strlen(buf) < 3)
> +            continue;
> +        d_major = buf[0];
> +        if (d_major < '1' || d_major > '6' || d_major == '4')
> +            continue;
> +        d_minor = buf[1];
> +        if (d_minor < '0' || d_minor > '9')
> +            continue;
> +        d_extra = buf[2];
> +        if (d_extra < '0' || d_extra > '9')
> +            continue;
> +

> +        av_log(NULL, AV_LOG_DEBUG, "%s\n", buf);

should use s or another context instead of NULL

[...]
> +
> +static int ftp_open(URLContext *h, const char *url, int flags)
> +{
> +    char proto[10], auth[1024], path[MAX_URL_SIZE], buf[CONTROL_BUFFER_SIZE], opts_format[20];
> +    AVDictionary *opts = NULL;
> +    int port, err;
> +    FTPContext *s = h->priv_data;
> +

> +#ifdef FTP_DEVEL_DEBUG
> +    av_log(h, AV_LOG_DEBUG, "open\n");
> +#endif

maybe av_dlog() or something equivalent could be used, avoiding a
#if on every use 

[...]
> +
> +static int64_t ftp_seek(URLContext *h, int64_t pos, int whence)
> +{
> +    FTPContext *s = h->priv_data;
> +    char buf[CONTROL_BUFFER_SIZE];
> +    int err;
> +    int64_t new_pos;
> +
> +#ifdef FTP_DEVEL_DEBUG
> +    av_log(h, AV_LOG_DEBUG, "seek %lld %d\n", pos, whence);
> +#endif
> +
> +    switch(whence) {
> +    case AVSEEK_SIZE:
> +        return s->filesize;
> +    case SEEK_SET:
> +        new_pos = pos;
> +        break;
> +    case SEEK_CUR:
> +        new_pos = s->position + pos;
> +        break;
> +    case SEEK_END:
> +        if (s->filesize < 0)
> +            return AVERROR(EIO);
> +        new_pos = s->filesize + pos;
> +        break;
> +    default:
> +        return AVERROR(EINVAL);
> +    }
> +
> +    if  (h->is_streamed)
> +        return AVERROR(EIO);
> +
> +    if (new_pos < 0 || (s->filesize >= 0 && new_pos > s->filesize))
> +        return AVERROR(EINVAL);
> +
> +    if (new_pos != s->position) {
> +
> +        if (s->state != READY) {
> +            if (s->conn_data) {
> +                /* abort existing transfer */
> +                if (s->state == DOWNLOADING) {
> +                    snprintf(buf, sizeof(buf), "ABOR\r\n");
> +                    if ((err = ffurl_write(s->conn_control, buf, strlen(buf))) < 0)
> +                        return err;
> +                }
> +                ffurl_closep(&s->conn_data);
> +                s->state = DISCONNECTED;
> +                /* Servers return 225 or 226 */
> +                ftp_status(s, &err, NULL, NULL, NULL, -1);
> +                if (err != 2)
> +                    return AVERROR(EIO);
> +            }
> +
> +            /* set passive */
> +            if ((err = ftp_passive_mode(s)) < 0)
> +                return err;
> +
> +            /* open new data connection */
> +            if ((err = ftp_reconnect_data_connection(h)) < 0)
> +                return err;
> +        }
> +

> +        /* resume from pos position */
> +        snprintf(buf, sizeof(buf), "REST %lld\r\n", pos);

should use PRId64


> +        if ((err = ffurl_write(s->conn_control, buf, strlen(buf))) < 0)
> +            return err;
> +        if (350 != ftp_status(s, NULL, NULL, NULL, NULL, 350))
> +            return AVERROR(EIO);
> +
> +        s->position = pos;
> +    }
> +    return new_pos;
> +}
> +
> +static int ftp_close(URLContext *h)
> +{
> +    FTPContext *s = h->priv_data;
> +
> +#ifdef FTP_DEVEL_DEBUG
> +    av_log(h, AV_LOG_DEBUG, "close\n");
> +#endif
> +

> +    if (s->conn_control)
> +        ffurl_closep(&s->conn_control);
> +    if (s->conn_data)
> +        ffurl_closep(&s->conn_data);

The null checks should not be needed


> +
> +    return 0;
> +}
> +
[...]


> +#if CONFIG_FTP_PROTOCOL
> +URLProtocol ff_ftp_protocol = {
> +    .name                = "ftp",
> +    .url_open            = ftp_open,
> +    .url_read            = ftp_read,
> +    .url_write           = ftp_write,
> +    .url_seek            = ftp_seek,
> +    .url_close           = ftp_close,
> +    .url_get_file_handle = ftp_get_file_handle,
> +    .url_shutdown        = ftp_shutdown,
> +    .priv_data_size      = sizeof(FTPContext),
> +    .priv_data_class     = &ftp_context_class,
> +    .flags               = URL_PROTOCOL_FLAG_NETWORK,
> +};
> +#endif

the #if should not be needed

you also might want to add yourself to MAINTAINERS

[...]

Thanks

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130515/808b3c77/attachment.asc>


More information about the ffmpeg-devel mailing list