On Tue, Aug 11, 2015 at 3:00 AM, Michael Niedermayer <mich...@niedermayer.cc > wrote:
> On Tue, Aug 11, 2015 at 12:42:04AM +0200, Mariusz Szczepańczyk wrote: > > On Mon, Aug 10, 2015 at 6:41 PM, Michael Niedermayer > <mich...@niedermayer.cc > > > wrote: > > > > > On Sun, Aug 09, 2015 at 02:38:12AM +0200, Mariusz Szczepańczyk wrote: > > > > --- > > > > libavformat/ftp.c | 98 > > > +++++++++++++++++++++++++++++++++++++++++++++++-------- > > > > 1 file changed, 84 insertions(+), 14 deletions(-) > > > > > > > > diff --git a/libavformat/ftp.c b/libavformat/ftp.c > > > > index 542cf6a..5a312b9 100644 > > > > --- a/libavformat/ftp.c > > > > +++ b/libavformat/ftp.c > > > > @@ -38,6 +38,12 @@ typedef enum { > > > > DISCONNECTED > > > > } FTPState; > > > > > > > > +typedef enum { > > > > + UNKNOWN_METHOD, > > > > + NLST, > > > > + MLSD > > > > +} FTPListingMethod; > > > > + > > > > typedef struct { > > > > const AVClass *class; > > > > URLContext *conn_control; /**< Control > > > connection */ > > > > @@ -56,6 +62,8 @@ typedef struct { > > > > const char *anonymous_password; /**< Password to be > > > used for anonymous user. An email should be used. */ > > > > int write_seekable; /**< Control > > > seekability, 0 = disable, 1 = enable. */ > > > > FTPState state; /**< State of data > > > connection */ > > > > + FTPListingMethod listing_method; /**< Called listing > > > method */ > > > > + char *features; /**< List of > server's > > > features represented as raw response */ > > > > char *dir_buffer; > > > > size_t dir_buffer_size; > > > > size_t dir_buffer_offset; > > > > @@ -192,6 +200,8 @@ static int ftp_send_command(FTPContext *s, const > > > char *command, > > > > { > > > > int err; > > > > > > > > + av_dlog(s, "%s", command); > > > > + > > > > if (response) > > > > *response = NULL; > > > > > > > > @@ -449,32 +459,68 @@ static int ftp_set_dir(FTPContext *s) > > > > return 0; > > > > } > > > > > > > > -static int ftp_list(FTPContext *s) > > > > +static int ftp_list_mlsd(FTPContext *s) > > > > { > > > > static const char *command = "MLSD\r\n"; > > > > static const int mlsd_codes[] = {150, 500, 0}; /* 500 is > incorrect > > > code */ > > > > > > > > if (ftp_send_command(s, command, mlsd_codes, NULL) != 150) > > > > return AVERROR(ENOSYS); > > > > - s->state = LISTING_DIR; > > > > + s->listing_method = MLSD; > > > > + return 0; > > > > +} > > > > + > > > > +static int ftp_list_nlst(FTPContext *s) > > > > +{ > > > > + static const char *command = "NLST\r\n"; > > > > + static const int nlst_codes[] = {226, 425, 426, 451, 450, 550, > 0}; > > > > + > > > > + if (ftp_send_command(s, command, nlst_codes, NULL) != 226) > > > > + return AVERROR(ENOSYS); > > > > + s->listing_method = NLST; > > > > return 0; > > > > } > > > > > > > > +static int ftp_has_feature(FTPContext *s, const char *feature_name); > > > > + > > > > +static int ftp_list(FTPContext *s) > > > > +{ > > > > + s->state = LISTING_DIR; > > > > + > > > > + if (ftp_has_feature(s, "MLSD")) > > > > + return ftp_list_mlsd(s); > > > > + > > > > + return ftp_list_nlst(s); > > > > +} > > > > + > > > > +static int ftp_has_feature(FTPContext *s, const char *feature_name) > > > > +{ > > > > + if (!s->features) > > > > + return 0; > > > > + > > > > + return av_stristr(s->features, feature_name) != NULL; > > > > +} > > > > + > > > > static int ftp_features(FTPContext *s) > > > > { > > > > static const char *feat_command = "FEAT\r\n"; > > > > static const char *enable_utf8_command = "OPTS UTF8 ON\r\n"; > > > > static const int feat_codes[] = {211, 0}; > > > > static const int opts_codes[] = {200, 451, 0}; > > > > - char *feat = NULL; > > > > > > > + if (s->features) { > > > > + av_freep(&s->features); > > > > + s->features = NULL; > > > > + } > > > > > > the if and NULL setting is uneeded > > > > > > > > fixed > > > > > > > also please explain how each patch can be tested > > > > > > > > For now I'm using local ftp servers, and netcat to test "wrong" > responses. > > I'd like to write some blackbox tests (list of input commands + expected > > responses) but I'm still thinking how to fit them into the current > > architecture. > [...] > > > [...] > > > -- > > > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > > > > > Old school: Use the lowest level language in which you can solve the > > > problem > > > conveniently. > > > New school: Use the highest level language in which the latest > > > supercomputer > > > can solve the problem without the user falling asleep > waiting. > > > > > > _______________________________________________ > > > ffmpeg-devel mailing list > > > ffmpeg-devel@ffmpeg.org > > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > ftp.c | 94 > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------- > > 1 file changed, 80 insertions(+), 14 deletions(-) > > ae23918e29360a8a96c9884ed27d3aac4844e7bc > 0003-lavf-ftp-implement-NLST-method.patch > > From f682cc6da7fc21f8ae5d646ccc3ffd7fb6c3a993 Mon Sep 17 00:00:00 2001 > > From: =?UTF-8?q?Mariusz=20Szczepa=C5=84czyk?= <mszczepanc...@gmail.com> > > Date: Tue, 16 Jun 2015 20:41:31 +0200 > > Subject: [PATCH 3/3] lavf/ftp: implement NLST method > > this patch breaks > doc/examples/avio_list_dir ftp://ftp.idsoftware.com > > please test your code with some random servers out in the wild > > also fate tests or other would be highly welcome, but yes as you > say its not clear how to fit that in > > [...] > > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > I am the wisest man alive, for I know one thing, and that is that I know > nothing. -- Socrates > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > Attached new patch that calls MLSD first and then if it doesn't succeed calls NLST, instead of checking for MLSD support as some servers don't bother to get it announced.
From 7b15b78c7f78b22f18e00d30714ef4d3f104e905 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mariusz=20Szczepa=C5=84czyk?= <mszczepanc...@gmail.com> Date: Tue, 16 Jun 2015 20:41:31 +0200 Subject: [PATCH] lavf/ftp: implement NLST method --- libavformat/ftp.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 81 insertions(+), 14 deletions(-) diff --git a/libavformat/ftp.c b/libavformat/ftp.c index 5d00780..f603379 100644 --- a/libavformat/ftp.c +++ b/libavformat/ftp.c @@ -38,6 +38,12 @@ typedef enum { DISCONNECTED } FTPState; +typedef enum { + UNKNOWN_METHOD, + NLST, + MLSD +} FTPListingMethod; + typedef struct { const AVClass *class; URLContext *conn_control; /**< Control connection */ @@ -56,6 +62,8 @@ typedef struct { const char *anonymous_password; /**< Password to be used for anonymous user. An email should be used. */ int write_seekable; /**< Control seekability, 0 = disable, 1 = enable. */ FTPState state; /**< State of data connection */ + FTPListingMethod listing_method; /**< Called listing method */ + char *features; /**< List of server's features represented as raw response */ char *dir_buffer; size_t dir_buffer_size; size_t dir_buffer_offset; @@ -192,6 +200,8 @@ static int ftp_send_command(FTPContext *s, const char *command, { int err; + av_dlog(s, "%s", command); + if (response) *response = NULL; @@ -449,32 +459,65 @@ static int ftp_set_dir(FTPContext *s) return 0; } -static int ftp_list(FTPContext *s) +static int ftp_list_mlsd(FTPContext *s) { static const char *command = "MLSD\r\n"; static const int mlsd_codes[] = {150, 500, 0}; /* 500 is incorrect code */ if (ftp_send_command(s, command, mlsd_codes, NULL) != 150) return AVERROR(ENOSYS); - s->state = LISTING_DIR; + s->listing_method = MLSD; + return 0; +} + +static int ftp_list_nlst(FTPContext *s) +{ + static const char *command = "NLST\r\n"; + static const int nlst_codes[] = {226, 425, 426, 451, 450, 550, 0}; + + if (ftp_send_command(s, command, nlst_codes, NULL) != 226) + return AVERROR(ENOSYS); + s->listing_method = NLST; return 0; } +static int ftp_has_feature(FTPContext *s, const char *feature_name); + +static int ftp_list(FTPContext *s) +{ + int ret; + s->state = LISTING_DIR; + + if ((ret = ftp_list_mlsd(s)) < 0) + ret = ftp_list_nlst(s); + + return ret; +} + +static int ftp_has_feature(FTPContext *s, const char *feature_name) +{ + if (!s->features) + return 0; + + return av_stristr(s->features, feature_name) != NULL; +} + static int ftp_features(FTPContext *s) { static const char *feat_command = "FEAT\r\n"; static const char *enable_utf8_command = "OPTS UTF8 ON\r\n"; static const int feat_codes[] = {211, 0}; static const int opts_codes[] = {200, 451, 0}; - char *feat = NULL; - if (ftp_send_command(s, feat_command, feat_codes, &feat) == 211) { - if (av_stristr(feat, "UTF8")) { - if (ftp_send_command(s, enable_utf8_command, opts_codes, NULL) == 200) - s->utf8 = 1; - } + av_freep(&s->features); + if (ftp_send_command(s, feat_command, feat_codes, &s->features) != 211) { + av_freep(&s->features); + } + + if (ftp_has_feature(s, "UTF8")) { + if (ftp_send_command(s, enable_utf8_command, opts_codes, NULL) == 200) + s->utf8 = 1; } - av_freep(&feat); return 0; } @@ -607,8 +650,10 @@ static int ftp_connect(URLContext *h, const char *url) FTPContext *s = h->priv_data; s->state = DISCONNECTED; + s->listing_method = UNKNOWN; s->filesize = -1; s->position = 0; + s->features = NULL; av_url_split(proto, sizeof(proto), credencials, sizeof(credencials), @@ -815,6 +860,7 @@ static int ftp_close(URLContext *h) av_freep(&s->password); av_freep(&s->hostname); av_freep(&s->path); + av_freep(&s->features); return 0; } @@ -878,10 +924,13 @@ static int64_t ftp_parse_date(const char *date) return INT64_C(1000000) * av_timegm(&tv); } -/** - * @return 0 on success, negative on error, positive on entry to discard. - */ -static int ftp_parse_entry(char *mlsd, AVIODirEntry *next) +static int ftp_parse_entry_nlst(char *line, AVIODirEntry *next) +{ + next->name = av_strdup(line); + return 0; +} + +static int ftp_parse_entry_mlsd(char *mlsd, AVIODirEntry *next) { char *fact, *value; av_dlog(NULL, "%s\n", mlsd); @@ -914,6 +963,24 @@ static int ftp_parse_entry(char *mlsd, AVIODirEntry *next) return 0; } +/** + * @return 0 on success, negative on error, positive on entry to discard. + */ +static int ftp_parse_entry(URLContext *h, char *line, AVIODirEntry *next) +{ + FTPContext *s = h->priv_data; + + switch (s->listing_method) { + case MLSD: + return ftp_parse_entry_mlsd(line, next); + case NLST: + return ftp_parse_entry_nlst(line, next); + case UNKNOWN_METHOD: + default: + return -1; + } +} + static int ftp_read_dir(URLContext *h, AVIODirEntry **next) { FTPContext *s = h->priv_data; @@ -951,7 +1018,7 @@ static int ftp_read_dir(URLContext *h, AVIODirEntry **next) if (!*next) return AVERROR(ENOMEM); (*next)->utf8 = s->utf8; - ret = ftp_parse_entry(start, *next); + ret = ftp_parse_entry(h, start, *next); if (ret) { avio_free_directory_entry(next); if (ret < 0) -- 2.4.6
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel