[FFmpeg-devel] [PATCH] Implement url_check()

[Michael Niedermayer]

On Thu, Sep 30, 2010 at 05:45:48PM +0200, Stefano Sabatini wrote:
> On date Tuesday 2010-09-14 16:49:17 +0200, Michael Niedermayer encoded:
> > On Tue, Sep 14, 2010 at 03:07:09PM +0200, Stefano Sabatini wrote:
> > > On date Thursday 2010-01-21 12:29:36 +0100, Stefano Sabatini encoded:
> > > > On date Wednesday 2010-01-20 21:24:46 +0100, Maker aka Michele encoded:
> > > > > > if ((ret = url_check(file, w) != AVERROR(ENOENT))  // the file exists
> > > > > >    ask_for_overwrite_confirmation();
> > > > > >
> > > > > > uhm, yeah, may a function like "url_check" isn't really like url_exists,
> > > > > but should be more useful.
> > > > > But my opinion is that a file shouldn't be opened if we just want to know if
> > > > > it is writable/readable. So, what about extending URLProtocol with a pointer
> > > > > to function,  which checks if the url can be opened in a certain mode
> > > > > (specific for each protocol)?
> > > > 
> > > > (Quick reply as I'm pretty busy now)
> > > > Options:
> > > > 
> > > > 1) implement url_check()
> > > > 
> > > > 2) extend url_exist() with a simple if (url == "file") then stat(...);,
> > > > not elegant but it should work.
> > > > 
> > > > 3) extend URLProtocol and make it implement an url_exist method() for
> > > > each protocol, this looks a little overkill to me.
> > > > 
> > > > I have no strong preference on these, so I'll let the decision to the
> > > > maintainers, in the meaningwhile you may provide a patch implementing
> > > > your favourite approach.
> > > 
> > > Ping. Another solution would be implementing a per protocol
> > > url_check() function, which checks if a "protocol stream" can be
> > > opened with a certain mode, and can be used for the testing if a file
> > > exists.
> > > 
> > > In the case of the "file" protocol it would use stat(), and should
> > > possibly fix the problem at hand.
> > 
> > you will need something per protocol yes, and there needs to be a fallback
> > that uses open if the protocol has no specific implementation of url_check
> 
> Check attached.
> 
> BTW should we use the av_ prefix, and so have av_check_url? (In this case
> the prefix should be attached to the other url* functions).

do you plan to use it from outside lavf?
if not ff_ is definitly more correct than av_


> 
> Regards.
> -- 
> FFmpeg = Fantastic Fanciful Multimedia Patchable Elaborated God

>  avio.c |   16 ++++++++++++++++
>  avio.h |    9 +++++++++
>  2 files changed, 25 insertions(+)
> f4d904b664fbdc2980f7fcb8d444b1f8bb5f58a0  0001-Add-url_check-function.patch
> From 06950c55a35123a91366aa3073ca87dadef86797 Mon Sep 17 00:00:00 2001
> From: Stefano Sabatini <stefano.sabatini-lala at poste.it>
> Date: Thu, 30 Sep 2010 12:45:32 +0200
> Subject: [PATCH 1/3] Add url_check() function.
> 
> ---
>  libavformat/avio.c |   16 ++++++++++++++++
>  libavformat/avio.h |    9 +++++++++
>  2 files changed, 25 insertions(+), 0 deletions(-)
> 
> diff --git a/libavformat/avio.c b/libavformat/avio.c
> index 4399878..ac23d59 100644
> --- a/libavformat/avio.c
> +++ b/libavformat/avio.c
> @@ -285,6 +285,22 @@ int url_exist(const char *filename)
>      return 1;
>  }
>  
> +int url_check(const char *url, int flags)
> +{
> +    URLContext *h;
> +    int ret = url_alloc(&h, url, flags);
> +    if (ret)
> +        return ret;
> +
> +    if (h->prot->url_check)
> +        ret = h->prot->url_check(h, flags);
> +    else
> +        ret = (ret = url_connect(h)) < 0 ? ret : 1;
> +
> +    url_close(h);
> +    return ret;
> +}

having to create a URLContext for this looks wrong


> +
>  int64_t url_filesize(URLContext *h)
>  {
>      int64_t pos, size;
> diff --git a/libavformat/avio.h b/libavformat/avio.h
> index 1e97357..db5f081 100644
> --- a/libavformat/avio.h
> +++ b/libavformat/avio.h
> @@ -166,6 +166,14 @@ int url_close(URLContext *h);
>  int url_exist(const char *url);
>  
>  /**
> + * Return a positive value if the resource indicated by url can be
> + * accessed with the mode indicated by flags, 0 if the resource cannot
> + * be accessed in that way, a negative value corresponding to an
> + * AVERROR code in case of failure.
> + */
> +int url_check(const char *url, int flags);

requireing one call to url_check() for testing each flag is bad
a better api wpould be

flags are a bit mask of which properties we want
and the return value provides these through similar flags

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

He who knows, does not speak. He who speaks, does not know. -- Lao Tsu
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101001/69e998da/attachment.pgp>