On date Saturday 2011-04-09 02:03:19 +0200, Stefano Sabatini wrote:
> On date Friday 2011-04-08 23:33:33 +0200, Luca Barbato wrote:
> > On 4/8/11 8:10 PM, Anton Khirnov wrote:
> > >From: Stefano Sabatini<stefano.sabatini-l...@poste.it>
> > 
> > Seems fine in principle even if I won't use "check" and I'd merge it
> > using ffurl namespace.
> 
> Please wait because I have an updated patchset.

Attached updated proof of concept patchset, completely untested (it's
late and i need to sleep).

I was trying to address Michael's comments:
http://thread.gmane.org/gmane.comp.video.ffmpeg.devel/102825/focus=118515

|requiring one call to url_check() for testing each flag is bad
|a better api would be
|
|flags are a bit mask of which properties we want
|and the return value provides these through similar flags

I forgot why I didn't consider the patchset ready for posting after I
tried to address that comment, but I vaguely remember there was some
problematic corner case so it needs more testing.
-- 
Driver does not carry cash.
>From b563c12cb285f1e6eb8dc19d1a18323cd9280ea1 Mon Sep 17 00:00:00 2001
From: Stefano Sabatini <stefano.sabatini-l...@poste.it>
Date: Fri, 8 Apr 2011 18:32:25 +0200
Subject: [PATCH] avio: add avio_check()

The new function is more flexible than url_exist(), as it allows to
specify which access flags to check, and does not require an explicit
open of the checked resource.
---
 libavformat/avio.c |   19 +++++++++++++++++++
 libavformat/avio.h |   15 +++++++++++++++
 2 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/libavformat/avio.c b/libavformat/avio.c
index 7b066e3..cc57529 100644
--- a/libavformat/avio.c
+++ b/libavformat/avio.c
@@ -362,6 +362,25 @@ int url_exist(const char *filename)
     return 1;
 }
 
+int avio_check(const char *url, int flags)
+{
+    URLContext *h;
+    int ret = ffurl_alloc(&h, url, flags);
+    if (ret)
+        return ret;
+
+    if (h->prot->url_check) {
+        ret = h->prot->url_check(h, flags);
+    } else {
+        ret = ffurl_connect(h);
+        if (ret >= 0)
+            ret = flags;
+    }
+
+    ffurl_close(h);
+    return ret;
+}
+
 int64_t ffurl_size(URLContext *h)
 {
     int64_t pos, size;
diff --git a/libavformat/avio.h b/libavformat/avio.h
index 03b6f6f..051c06e 100644
--- a/libavformat/avio.h
+++ b/libavformat/avio.h
@@ -127,6 +127,20 @@ attribute_deprecated void url_set_interrupt_cb(URLInterruptCB *interrupt_cb);
 int url_exist(const char *url);
 
 /**
+ * Return AVIO_* access flags corresponding to the access permissions
+ * of the resource in url, or a negative value corresponding to an
+ * AVERROR code in case of failure. The returned access flags are
+ * masked by the value in flags.
+ *
+ * @note This function is intrinsecally unsafe, in the sense that the
+ * checked resource may change its existence or permission status from
+ * one call to another. Thus you should not trust the returned value,
+ * unless you are sure that no other processes are accessing the
+ * checked resource.
+ */
+int avio_check(const char *url, int flags);
+
+/**
  * The callback is called in blocking functions to test regulary if
  * asynchronous interruption is needed. AVERROR_EXIT is returned
  * in this case by the interrupted function. 'NULL' means no interrupt
@@ -157,6 +171,7 @@ typedef struct URLProtocol {
     int priv_data_size;
     const AVClass *priv_data_class;
     int flags;
+    int (*url_check)(URLContext *h, int mask);
 } URLProtocol;
 
 #if FF_API_REGISTER_PROTOCOL
-- 
1.7.2.3

>From 1dc3e93dc0ed88272dd427af81cf40410e1dad13 Mon Sep 17 00:00:00 2001
From: Stefano Sabatini <stefano.sabatini-l...@poste.it>
Date: Thu, 30 Sep 2010 13:21:42 +0200
Subject: [PATCH] file: implement url_check() callback in the file and pipe protocols

---
 libavformat/file.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/libavformat/file.c b/libavformat/file.c
index b8c7c4c..f939a8e 100644
--- a/libavformat/file.c
+++ b/libavformat/file.c
@@ -94,6 +94,20 @@ static int file_close(URLContext *h)
     return close(fd);
 }
 
+static int file_check(URLContext *h, int mask)
+{
+    struct stat st;
+    int ret = stat(h->filename, &st);
+    if (ret < 0)
+        return AVERROR(errno);
+
+    ret |= st.st_mode&S_IRUSR ? mask&AVIO_RDONLY : 0;
+    ret |= st.st_mode&S_IWUSR ? mask&AVIO_WRONLY : 0;
+    ret |= st.st_mode&S_IWUSR && st.st_mode&S_IRUSR ? mask&AVIO_RDWR : 0;
+
+    return ret;
+}
+
 URLProtocol ff_file_protocol = {
     "file",
     file_open,
@@ -102,6 +116,7 @@ URLProtocol ff_file_protocol = {
     file_seek,
     file_close,
     .url_get_file_handle = file_get_handle,
+    .url_check = file_check,
 };
 
 #endif /* CONFIG_FILE_PROTOCOL */
@@ -136,6 +151,7 @@ URLProtocol ff_pipe_protocol = {
     file_read,
     file_write,
     .url_get_file_handle = file_get_handle,
+    .url_check = file_check,
 };
 
 #endif /* CONFIG_PIPE_PROTOCOL */
-- 
1.7.2.3

>From 3337ee5fd1ca5772c293f76e7c15267077c8001f Mon Sep 17 00:00:00 2001
From: Stefano Sabatini <stefano.sabatini-l...@poste.it>
Date: Sat, 9 Apr 2011 01:32:37 +0200
Subject: [PATCH] prefer avio_check() over url_exist()

The problem with url_exist() is that it tries to open a resource in
RDONLY mode. If the file is a FIFO and there is already a reading
client, the open() call will hang.

By using avio_check() with access mode of 0, the second reading
process will check if the file exists without attempting to open it,
thus avoiding the lock.

Fix issue #1663.
---
 ffmpeg.c           |    2 +-
 ffserver.c         |    4 ++--
 libavformat/img2.c |    6 +++---
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/ffmpeg.c b/ffmpeg.c
index 40e5e67..9153a5b 100644
--- a/ffmpeg.c
+++ b/ffmpeg.c
@@ -3870,7 +3870,7 @@ static void opt_output_file(const char *filename)
             (strchr(filename, ':') == NULL ||
              filename[1] == ':' ||
              av_strstart(filename, "file:", NULL))) {
-            if (url_exist(filename)) {
+            if (avio_check(filename, 0) != AVERROR(ENOENT)) {
                 if (!using_stdin) {
                     fprintf(stderr,"File '%s' already exists. Overwrite ? [y/N] ", filename);
                     fflush(stderr);
diff --git a/ffserver.c b/ffserver.c
index 6e42979..5f165dd 100644
--- a/ffserver.c
+++ b/ffserver.c
@@ -3684,7 +3684,7 @@ static void build_feed_streams(void)
     for(feed = first_feed; feed != NULL; feed = feed->next_feed) {
         int fd;
 
-        if (url_exist(feed->feed_filename)) {
+        if (url_check(feed->feed_filename, URL_RDONLY)) {
             /* See if it matches */
             AVFormatContext *s;
             int matches = 0;
@@ -3757,7 +3757,7 @@ static void build_feed_streams(void)
                 unlink(feed->feed_filename);
             }
         }
-        if (!url_exist(feed->feed_filename)) {
+        if (!avio_check(feed->feed_filename, AVIO_RDONLY)) {
             AVFormatContext s1 = {0}, *s = &s1;
 
             if (feed->readonly) {
diff --git a/libavformat/img2.c b/libavformat/img2.c
index 4e55c22..0c7de97 100644
--- a/libavformat/img2.c
+++ b/libavformat/img2.c
@@ -132,11 +132,11 @@ static int find_image_range(int *pfirst_index, int *plast_index,
         if (av_get_frame_filename(buf, sizeof(buf), path, first_index) < 0){
             *pfirst_index =
             *plast_index = 1;
-            if(url_exist(buf))
+            if (avio_check(buf, AVIO_RDONLY) != AVERROR(ENOENT))
                 return 0;
             return -1;
         }
-        if (url_exist(buf))
+        if (avio_check(buf, AVIO_RDONLY) != AVERROR(ENOENT))
             break;
     }
     if (first_index == 5)
@@ -154,7 +154,7 @@ static int find_image_range(int *pfirst_index, int *plast_index,
             if (av_get_frame_filename(buf, sizeof(buf), path,
                                       last_index + range1) < 0)
                 goto fail;
-            if (!url_exist(buf))
+            if (avio_check(buf, AVIO_RDONLY) != AVERROR(ENOENT))
                 break;
             range = range1;
             /* just in case... */
-- 
1.7.2.3

_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to