[FFmpeg-devel] [PATCH] lavf: add SFTP protocol via libssh

[Lukasz M]

On 19 September 2013 18:58, James Almer <jamrial at gmail.com> wrote:

> On 19/09/13 1:39 PM, Derek Buitenhuis wrote:
> > On 9/18/2013 3:27 PM, Lukasz Marek wrote:
> >> +Example: play a file stored on remote server.
> >
> > s/play/Play/
> >
> >> +#define DEBUG 1
> >
> > This looks quite wrong
> >
> >> +#include "libavutil/avstring.h"
> >> +#include "avformat.h"
> >> +#include "internal.h"
> >> +#include "url.h"
> >> +#include "libavutil/opt.h"
> >
> > nit: Order.
> >
> >> +#include <libssh/sftp.h>
> >> +#include <fcntl.h>
> >
> > fcntl.h is not portable. Use the proper guards provided in config.h
>
> There's no configure check for fcntl.h, only for fcntl().
> Every file i could find that included that header did it without guards.


This header is included in file.c without guards, so I believe it is not a
problem.


> >> +#define D AV_OPT_FLAG_DECODING_PARAM
> >> +#define E AV_OPT_FLAG_ENCODING_PARAM
> >
> > D and E are far to short and potentially pollute and conflict
> > with the namespacing of system headers.
>
> options_table.h defines E and D the same way, so i don't think it will
> be a problem.
>

Many other files define it as well.