[FFmpeg-devel] ftp protocol support
Lukasz M
lukasz.m.luki at gmail.com
Thu May 16 15:58:53 CEST 2013
2013/5/15 Michael Niedermayer <michaelni at gmx.at>:
> 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
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
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.
Best Regards
Lukasz Marek
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-FTP-protocol-support.patch
Type: application/octet-stream
Size: 24808 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130516/0cb20e25/attachment.obj>
More information about the ffmpeg-devel
mailing list