Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH 1/2] avformat/file: Add a specialized url_check callback for pipe protocol
       [not found] <20220829144320.330685-1-bpeeluk.ref@yahoo.co.uk>
@ 2022-08-29 14:43 ` Neil Roberts
  2022-08-29 14:43   ` [FFmpeg-devel] [PATCH 2/2] avformat/tests: Add a test for avio_check with the " Neil Roberts
  2022-08-29 21:54   ` [FFmpeg-devel] [PATCH 1/2] avformat/file: Add a specialized url_check callback for " Michael Niedermayer
  0 siblings, 2 replies; 6+ messages in thread
From: Neil Roberts @ 2022-08-29 14:43 UTC (permalink / raw)
  To: ffmpeg-devel

Using file_check for the pipe protocol doesn't make sense. For example,
for a URL like “pipe:0” it would end up checking whether the “pipe:0”
file exists. This patch instead makes it check the access modes on the
corresponding file descriptor using fcntl.

Signed-off-by: Neil Roberts <bpeeluk@yahoo.co.uk>
---
 libavformat/file.c | 40 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 37 insertions(+), 3 deletions(-)

diff --git a/libavformat/file.c b/libavformat/file.c
index 98c9e81bcb..f17d219cb2 100644
--- a/libavformat/file.c
+++ b/libavformat/file.c
@@ -375,9 +375,8 @@ const URLProtocol ff_file_protocol = {
 
 #if CONFIG_PIPE_PROTOCOL
 
-static int pipe_open(URLContext *h, const char *filename, int flags)
+static int pipe_get_fd(const char *filename, int flags)
 {
-    FileContext *c = h->priv_data;
     int fd;
     char *final;
     av_strstart(filename, "pipe:", &filename);
@@ -390,6 +389,14 @@ static int pipe_open(URLContext *h, const char *filename, int flags)
             fd = 0;
         }
     }
+
+    return fd;
+}
+
+static int pipe_open(URLContext *h, const char *filename, int flags)
+{
+    FileContext *c = h->priv_data;
+    int fd = pipe_get_fd(filename, flags);
 #if HAVE_SETMODE
     setmode(fd, O_BINARY);
 #endif
@@ -398,13 +405,40 @@ static int pipe_open(URLContext *h, const char *filename, int flags)
     return 0;
 }
 
+static int pipe_check(URLContext *h, int mask)
+{
+    int fd = pipe_get_fd(h->filename, mask);
+    int status_flags = fcntl(fd, F_GETFL);
+    int access;
+
+    if (status_flags == -1)
+        return AVERROR(errno);
+
+    switch (status_flags & O_ACCMODE) {
+    case O_RDONLY:
+        access = AVIO_FLAG_READ;
+        break;
+    case O_WRONLY:
+        access = AVIO_FLAG_WRITE;
+        break;
+    case O_RDWR:
+        access = AVIO_FLAG_READ | AVIO_FLAG_WRITE;
+        break;
+    default:
+        access = 0;
+        break;
+    }
+
+    return access & mask;
+}
+
 const URLProtocol ff_pipe_protocol = {
     .name                = "pipe",
     .url_open            = pipe_open,
     .url_read            = file_read,
     .url_write           = file_write,
     .url_get_file_handle = file_get_handle,
-    .url_check           = file_check,
+    .url_check           = pipe_check,
     .priv_data_size      = sizeof(FileContext),
     .priv_data_class     = &pipe_class,
     .default_whitelist   = "crypto,data"
-- 
2.37.2

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

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [FFmpeg-devel] [PATCH 2/2] avformat/tests: Add a test for avio_check with the pipe protocol
  2022-08-29 14:43 ` [FFmpeg-devel] [PATCH 1/2] avformat/file: Add a specialized url_check callback for pipe protocol Neil Roberts
@ 2022-08-29 14:43   ` Neil Roberts
  2022-08-29 21:54   ` [FFmpeg-devel] [PATCH 1/2] avformat/file: Add a specialized url_check callback for " Michael Niedermayer
  1 sibling, 0 replies; 6+ messages in thread
From: Neil Roberts @ 2022-08-29 14:43 UTC (permalink / raw)
  To: ffmpeg-devel

Creates a UNIX pipe and then verifies that avio_check returns the right
access flags for the two ends of the pipe.

Signed-off-by: Neil Roberts <bpeeluk@yahoo.co.uk>
---
 libavformat/Makefile         |   1 +
 libavformat/tests/.gitignore |   1 +
 libavformat/tests/pipe.c     | 101 +++++++++++++++++++++++++++++++++++
 tests/fate/libavformat.mak   |   5 ++
 4 files changed, 108 insertions(+)
 create mode 100644 libavformat/tests/pipe.c

diff --git a/libavformat/Makefile b/libavformat/Makefile
index f67a99f839..9c681c58c5 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -732,6 +732,7 @@ TESTPROGS-$(CONFIG_MOV_MUXER)            += movenc
 TESTPROGS-$(CONFIG_NETWORK)              += noproxy
 TESTPROGS-$(CONFIG_SRTP)                 += srtp
 TESTPROGS-$(CONFIG_IMF_DEMUXER)          += imf
+TESTPROGS-$(CONFIG_PIPE_PROTOCOL)        += pipe
 
 TOOLS     = aviocat                                                     \
             ismindex                                                    \
diff --git a/libavformat/tests/.gitignore b/libavformat/tests/.gitignore
index cdd0cce061..567d6f9e40 100644
--- a/libavformat/tests/.gitignore
+++ b/libavformat/tests/.gitignore
@@ -7,3 +7,4 @@
 /srtp
 /url
 /seek_utils
+/pipe
diff --git a/libavformat/tests/pipe.c b/libavformat/tests/pipe.c
new file mode 100644
index 0000000000..68540c1a8c
--- /dev/null
+++ b/libavformat/tests/pipe.c
@@ -0,0 +1,101 @@
+/*
+ * Copyright (c) 2022 Neil Roberts
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include <string.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <unistd.h>
+#include "libavformat/avio.h"
+#include "libavutil/error.h"
+
+static int check_pipe(const char *url, int mask, int expected)
+{
+    int flags = avio_check(url, mask);
+
+    if (flags < 0) {
+        fprintf(stderr,
+                "avio_check for %s with mask 0x%x failed: %s\n",
+                url,
+                mask,
+                av_err2str(flags));
+        return 0;
+    }
+
+    if (flags != expected) {
+        fprintf(stderr,
+                "Wrong result returned from avio_check for %s with mask 0x%x. "
+                "Expected 0x%x but received 0x%x\n",
+                url,
+                mask,
+                flags,
+                expected);
+        return 0;
+    }
+
+    return 1;
+}
+
+int main(int argc, char **argv)
+{
+    int ret = 0;
+    int pipe_fds[2];
+    char read_url[20], write_url[20];
+    int check_invalid_ret;
+
+    if (pipe(pipe_fds) == -1) {
+        fprintf(stderr, "error creating pipe: %s\n", strerror(errno));
+        return 1;
+    }
+
+    snprintf(read_url, sizeof(read_url), "pipe:%d", pipe_fds[0]);
+    snprintf(write_url, sizeof(write_url), "pipe:%d", pipe_fds[1]);
+
+    if (!check_pipe(read_url,
+                    AVIO_FLAG_READ | AVIO_FLAG_WRITE,
+                    AVIO_FLAG_READ))
+        ret = 1;
+
+    if (!check_pipe(write_url,
+                    AVIO_FLAG_READ | AVIO_FLAG_WRITE,
+                    AVIO_FLAG_WRITE))
+        ret = 1;
+
+    /* Ensure that we don't get flags that we didn't ask for */
+    if (!check_pipe(read_url, AVIO_FLAG_WRITE, 0))
+        ret = 1;
+
+    close(pipe_fds[0]);
+    close(pipe_fds[1]);
+
+    /* An invalid fd should return EBADF */
+    check_invalid_ret = avio_check(read_url, AVIO_FLAG_READ);
+
+    if (check_invalid_ret != AVERROR(EBADF)) {
+        fprintf(stderr,
+                "avio_check on invalid FD expected to return %i "
+                "but %i was received\n",
+                AVERROR(EBADF),
+                check_invalid_ret);
+        ret = 1;
+    }
+
+    return ret;
+}
diff --git a/tests/fate/libavformat.mak b/tests/fate/libavformat.mak
index d2acb4c9e0..7a22f54c04 100644
--- a/tests/fate/libavformat.mak
+++ b/tests/fate/libavformat.mak
@@ -26,6 +26,11 @@ FATE_LIBAVFORMAT-$(CONFIG_IMF_DEMUXER) += fate-imf
 fate-imf: libavformat/tests/imf$(EXESUF)
 fate-imf: CMD = run libavformat/tests/imf$(EXESUF)
 
+FATE_LIBAVFORMAT-$(CONFIG_PIPE_PROTOCOL) += fate-pipe
+fate-pipe: libavformat/tests/pipe$(EXESUF)
+fate-pipe: CMD = run libavformat/tests/pipe$(EXESUF)
+fate-pipe: CMP = null
+
 FATE_LIBAVFORMAT += fate-seek_utils
 fate-seek_utils: libavformat/tests/seek_utils$(EXESUF)
 fate-seek_utils: CMD = run libavformat/tests/seek_utils$(EXESUF)
-- 
2.37.2

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

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/2] avformat/file: Add a specialized url_check callback for pipe protocol
  2022-08-29 14:43 ` [FFmpeg-devel] [PATCH 1/2] avformat/file: Add a specialized url_check callback for pipe protocol Neil Roberts
  2022-08-29 14:43   ` [FFmpeg-devel] [PATCH 2/2] avformat/tests: Add a test for avio_check with the " Neil Roberts
@ 2022-08-29 21:54   ` Michael Niedermayer
  2022-08-30 12:09     ` [FFmpeg-devel] [PATCH v2 " Neil Roberts
  1 sibling, 1 reply; 6+ messages in thread
From: Michael Niedermayer @ 2022-08-29 21:54 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 1373 bytes --]

On Mon, Aug 29, 2022 at 04:43:19PM +0200, Neil Roberts wrote:
> Using file_check for the pipe protocol doesn't make sense. For example,
> for a URL like “pipe:0” it would end up checking whether the “pipe:0”
> file exists. This patch instead makes it check the access modes on the
> corresponding file descriptor using fcntl.
> 
> Signed-off-by: Neil Roberts <bpeeluk@yahoo.co.uk>
> ---
>  libavformat/file.c | 40 +++++++++++++++++++++++++++++++++++++---
>  1 file changed, 37 insertions(+), 3 deletions(-)

breaks on mingw64

src/libavformat/file.c: In function ‘pipe_check’:
src/libavformat/file.c:411:24: error: implicit declaration of function ‘fcntl’; did you mean ‘rintl’? [-Werror=implicit-function-declaration]
     int status_flags = fcntl(fd, F_GETFL);
                        ^~~~~
                        rintl
src/libavformat/file.c:411:34: error: ‘F_GETFL’ undeclared (first use in this function)
     int status_flags = fcntl(fd, F_GETFL);
                                  ^~~~~~~
src/libavformat/file.c:411:34: note: each undeclared identifier is reported only once for each function it appears in
cc1: some warnings being treated as errors

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

There will always be a question for which you do not know the correct answer.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

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

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [FFmpeg-devel] [PATCH v2 1/2] avformat/file: Add a specialized url_check callback for pipe protocol
  2022-08-29 21:54   ` [FFmpeg-devel] [PATCH 1/2] avformat/file: Add a specialized url_check callback for " Michael Niedermayer
@ 2022-08-30 12:09     ` Neil Roberts
  2022-08-30 12:09       ` [FFmpeg-devel] [PATCH v2 2/2] avformat/tests: Add a test for avio_check with the " Neil Roberts
  2022-11-09 14:31       ` [FFmpeg-devel] [PATCH v2 1/2] avformat/file: Add a specialized url_check callback for " Neil Roberts
  0 siblings, 2 replies; 6+ messages in thread
From: Neil Roberts @ 2022-08-30 12:09 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Michael Niedermayer

Using file_check for the pipe protocol doesn't make sense. For example,
for a URL like “pipe:0” it would end up checking whether the “pipe:0”
file exists. This patch instead makes it check the access modes on the
corresponding file descriptor using fcntl on *nix and DuplicateHandle on
Windows.

v2: Use DuplicateHandle on Windows to check whether the duplicated
    handle can have the corresponding access flags.

Signed-off-by: Neil Roberts <bpeeluk@yahoo.co.uk>
---
 libavformat/file.c | 74 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 71 insertions(+), 3 deletions(-)

diff --git a/libavformat/file.c b/libavformat/file.c
index 98c9e81bcb..b3f7bc9282 100644
--- a/libavformat/file.c
+++ b/libavformat/file.c
@@ -375,9 +375,8 @@ const URLProtocol ff_file_protocol = {
 
 #if CONFIG_PIPE_PROTOCOL
 
-static int pipe_open(URLContext *h, const char *filename, int flags)
+static int pipe_get_fd(const char *filename, int flags)
 {
-    FileContext *c = h->priv_data;
     int fd;
     char *final;
     av_strstart(filename, "pipe:", &filename);
@@ -390,6 +389,14 @@ static int pipe_open(URLContext *h, const char *filename, int flags)
             fd = 0;
         }
     }
+
+    return fd;
+}
+
+static int pipe_open(URLContext *h, const char *filename, int flags)
+{
+    FileContext *c = h->priv_data;
+    int fd = pipe_get_fd(filename, flags);
 #if HAVE_SETMODE
     setmode(fd, O_BINARY);
 #endif
@@ -398,13 +405,74 @@ static int pipe_open(URLContext *h, const char *filename, int flags)
     return 0;
 }
 
+static int pipe_check(URLContext *h, int mask)
+{
+    int fd = pipe_get_fd(h->filename, mask);
+    int access = 0;
+
+#ifdef _WIN32
+
+    HANDLE pipe_handle = (HANDLE) _get_osfhandle(fd);
+    HANDLE tmp_handle;
+
+    if (pipe_handle == INVALID_HANDLE_VALUE)
+        return AVERROR(errno);
+
+    if (mask & AVIO_FLAG_READ &&
+        DuplicateHandle(GetCurrentProcess(), /* hSourceProcessHandle */
+                        pipe_handle,
+                        GetCurrentProcess(), /* hTargetProcessHandle */
+                        &tmp_handle,
+                        FILE_READ_DATA,
+                        FALSE, /* bInheritHandle */
+                        0 /* options */)) {
+        access |= AVIO_FLAG_READ;
+        CloseHandle(tmp_handle);
+    }
+
+    if (mask & AVIO_FLAG_WRITE &&
+        DuplicateHandle(GetCurrentProcess(), /* hSourceProcessHandle */
+                        pipe_handle,
+                        GetCurrentProcess(), /* hTargetProcessHandle */
+                        &tmp_handle,
+                        FILE_WRITE_DATA,
+                        FALSE, /* bInheritHandle */
+                        0 /* options */)) {
+        access |= AVIO_FLAG_WRITE;
+        CloseHandle(tmp_handle);
+    }
+
+#else /* _WIN32 */
+
+    int status_flags = fcntl(fd, F_GETFL);
+
+    if (status_flags == -1)
+        return AVERROR(errno);
+
+    switch (status_flags & O_ACCMODE) {
+    case O_RDONLY:
+        access = AVIO_FLAG_READ;
+        break;
+    case O_WRONLY:
+        access = AVIO_FLAG_WRITE;
+        break;
+    case O_RDWR:
+        access = AVIO_FLAG_READ | AVIO_FLAG_WRITE;
+        break;
+    }
+
+#endif /* _WIN32 */
+
+    return access & mask;
+}
+
 const URLProtocol ff_pipe_protocol = {
     .name                = "pipe",
     .url_open            = pipe_open,
     .url_read            = file_read,
     .url_write           = file_write,
     .url_get_file_handle = file_get_handle,
-    .url_check           = file_check,
+    .url_check           = pipe_check,
     .priv_data_size      = sizeof(FileContext),
     .priv_data_class     = &pipe_class,
     .default_whitelist   = "crypto,data"
-- 
2.37.2

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

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [FFmpeg-devel] [PATCH v2 2/2] avformat/tests: Add a test for avio_check with the pipe protocol
  2022-08-30 12:09     ` [FFmpeg-devel] [PATCH v2 " Neil Roberts
@ 2022-08-30 12:09       ` Neil Roberts
  2022-11-09 14:31       ` [FFmpeg-devel] [PATCH v2 1/2] avformat/file: Add a specialized url_check callback for " Neil Roberts
  1 sibling, 0 replies; 6+ messages in thread
From: Neil Roberts @ 2022-08-30 12:09 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Michael Niedermayer

Creates a UNIX pipe and then verifies that avio_check returns the right
access flags for the two ends of the pipe.

v2: Add support for the Windows version of _pipe

Signed-off-by: Neil Roberts <bpeeluk@yahoo.co.uk>
---
 libavformat/Makefile         |   1 +
 libavformat/tests/.gitignore |   1 +
 libavformat/tests/pipe.c     | 108 +++++++++++++++++++++++++++++++++++
 tests/fate/libavformat.mak   |   5 ++
 4 files changed, 115 insertions(+)
 create mode 100644 libavformat/tests/pipe.c

diff --git a/libavformat/Makefile b/libavformat/Makefile
index f67a99f839..9c681c58c5 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -732,6 +732,7 @@ TESTPROGS-$(CONFIG_MOV_MUXER)            += movenc
 TESTPROGS-$(CONFIG_NETWORK)              += noproxy
 TESTPROGS-$(CONFIG_SRTP)                 += srtp
 TESTPROGS-$(CONFIG_IMF_DEMUXER)          += imf
+TESTPROGS-$(CONFIG_PIPE_PROTOCOL)        += pipe
 
 TOOLS     = aviocat                                                     \
             ismindex                                                    \
diff --git a/libavformat/tests/.gitignore b/libavformat/tests/.gitignore
index cdd0cce061..567d6f9e40 100644
--- a/libavformat/tests/.gitignore
+++ b/libavformat/tests/.gitignore
@@ -7,3 +7,4 @@
 /srtp
 /url
 /seek_utils
+/pipe
diff --git a/libavformat/tests/pipe.c b/libavformat/tests/pipe.c
new file mode 100644
index 0000000000..18a8551fd5
--- /dev/null
+++ b/libavformat/tests/pipe.c
@@ -0,0 +1,108 @@
+/*
+ * Copyright (c) 2022 Neil Roberts
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include <string.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <unistd.h>
+#include "libavformat/avio.h"
+#include "libavutil/error.h"
+
+static int check_pipe(const char *url, int mask, int expected)
+{
+    int flags = avio_check(url, mask);
+
+    if (flags < 0) {
+        fprintf(stderr,
+                "avio_check for %s with mask 0x%x failed: %s\n",
+                url,
+                mask,
+                av_err2str(flags));
+        return 0;
+    }
+
+    if (flags != expected) {
+        fprintf(stderr,
+                "Wrong result returned from avio_check for %s with mask 0x%x. "
+                "Expected 0x%x but received 0x%x\n",
+                url,
+                mask,
+                expected,
+                flags);
+        return 0;
+    }
+
+    return 1;
+}
+
+int main(int argc, char **argv)
+{
+    int ret = 0;
+    int pipe_fds[2];
+    char read_url[20], write_url[20];
+    int check_invalid_ret;
+    int pipe_ret;
+
+#ifdef _WIN32
+    pipe_ret = _pipe(pipe_fds, 1024 /* psize */, 0 /* textmode */);
+#else
+    pipe_ret = pipe(pipe_fds);
+#endif
+
+    if (pipe_ret == -1) {
+        fprintf(stderr, "error creating pipe: %s\n", strerror(errno));
+        return 1;
+    }
+
+    snprintf(read_url, sizeof(read_url), "pipe:%d", pipe_fds[0]);
+    snprintf(write_url, sizeof(write_url), "pipe:%d", pipe_fds[1]);
+
+    if (!check_pipe(read_url,
+                    AVIO_FLAG_READ | AVIO_FLAG_WRITE,
+                    AVIO_FLAG_READ))
+        ret = 1;
+
+    if (!check_pipe(write_url,
+                    AVIO_FLAG_READ | AVIO_FLAG_WRITE,
+                    AVIO_FLAG_WRITE))
+        ret = 1;
+
+    /* Ensure that we don't get flags that we didn't ask for */
+    if (!check_pipe(read_url, AVIO_FLAG_WRITE, 0))
+        ret = 1;
+
+    close(pipe_fds[0]);
+    close(pipe_fds[1]);
+
+    /* An invalid fd should return EBADF */
+    check_invalid_ret = avio_check(read_url, AVIO_FLAG_READ);
+
+    if (check_invalid_ret != AVERROR(EBADF)) {
+        fprintf(stderr,
+                "avio_check on invalid FD expected to return %i "
+                "but %i was received\n",
+                AVERROR(EBADF),
+                check_invalid_ret);
+        ret = 1;
+    }
+
+    return ret;
+}
diff --git a/tests/fate/libavformat.mak b/tests/fate/libavformat.mak
index d2acb4c9e0..7a22f54c04 100644
--- a/tests/fate/libavformat.mak
+++ b/tests/fate/libavformat.mak
@@ -26,6 +26,11 @@ FATE_LIBAVFORMAT-$(CONFIG_IMF_DEMUXER) += fate-imf
 fate-imf: libavformat/tests/imf$(EXESUF)
 fate-imf: CMD = run libavformat/tests/imf$(EXESUF)
 
+FATE_LIBAVFORMAT-$(CONFIG_PIPE_PROTOCOL) += fate-pipe
+fate-pipe: libavformat/tests/pipe$(EXESUF)
+fate-pipe: CMD = run libavformat/tests/pipe$(EXESUF)
+fate-pipe: CMP = null
+
 FATE_LIBAVFORMAT += fate-seek_utils
 fate-seek_utils: libavformat/tests/seek_utils$(EXESUF)
 fate-seek_utils: CMD = run libavformat/tests/seek_utils$(EXESUF)
-- 
2.37.2

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

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2 1/2] avformat/file: Add a specialized url_check callback for pipe protocol
  2022-08-30 12:09     ` [FFmpeg-devel] [PATCH v2 " Neil Roberts
  2022-08-30 12:09       ` [FFmpeg-devel] [PATCH v2 2/2] avformat/tests: Add a test for avio_check with the " Neil Roberts
@ 2022-11-09 14:31       ` Neil Roberts
  1 sibling, 0 replies; 6+ messages in thread
From: Neil Roberts @ 2022-11-09 14:31 UTC (permalink / raw)
  To: ffmpeg-devel

Bump.

Is anyone interested in looking at these two patches? I’m trying to get
into ffmpeg development and it would be nice to get some experience of
the full process of getting a patch landed. I don’t have commit rights.

Thanks in advance for any feedback.

Regards,
– Neil

Neil Roberts <bpeeluk-at-yahoo.co.uk@ffmpeg.org> writes:

> Using file_check for the pipe protocol doesn't make sense. For example,
> for a URL like “pipe:0” it would end up checking whether the “pipe:0”
> file exists. This patch instead makes it check the access modes on the
> corresponding file descriptor using fcntl on *nix and DuplicateHandle on
> Windows.
>
> v2: Use DuplicateHandle on Windows to check whether the duplicated
>     handle can have the corresponding access flags.
>
> Signed-off-by: Neil Roberts <bpeeluk@yahoo.co.uk>
> ---
>  libavformat/file.c | 74 ++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 71 insertions(+), 3 deletions(-)
>
> diff --git a/libavformat/file.c b/libavformat/file.c
> index 98c9e81bcb..b3f7bc9282 100644
> --- a/libavformat/file.c
> +++ b/libavformat/file.c
> @@ -375,9 +375,8 @@ const URLProtocol ff_file_protocol = {
>  
>  #if CONFIG_PIPE_PROTOCOL
>  
> -static int pipe_open(URLContext *h, const char *filename, int flags)
> +static int pipe_get_fd(const char *filename, int flags)
>  {
> -    FileContext *c = h->priv_data;
>      int fd;
>      char *final;
>      av_strstart(filename, "pipe:", &filename);
> @@ -390,6 +389,14 @@ static int pipe_open(URLContext *h, const char *filename, int flags)
>              fd = 0;
>          }
>      }
> +
> +    return fd;
> +}
> +
> +static int pipe_open(URLContext *h, const char *filename, int flags)
> +{
> +    FileContext *c = h->priv_data;
> +    int fd = pipe_get_fd(filename, flags);
>  #if HAVE_SETMODE
>      setmode(fd, O_BINARY);
>  #endif
> @@ -398,13 +405,74 @@ static int pipe_open(URLContext *h, const char *filename, int flags)
>      return 0;
>  }
>  
> +static int pipe_check(URLContext *h, int mask)
> +{
> +    int fd = pipe_get_fd(h->filename, mask);
> +    int access = 0;
> +
> +#ifdef _WIN32
> +
> +    HANDLE pipe_handle = (HANDLE) _get_osfhandle(fd);
> +    HANDLE tmp_handle;
> +
> +    if (pipe_handle == INVALID_HANDLE_VALUE)
> +        return AVERROR(errno);
> +
> +    if (mask & AVIO_FLAG_READ &&
> +        DuplicateHandle(GetCurrentProcess(), /* hSourceProcessHandle */
> +                        pipe_handle,
> +                        GetCurrentProcess(), /* hTargetProcessHandle */
> +                        &tmp_handle,
> +                        FILE_READ_DATA,
> +                        FALSE, /* bInheritHandle */
> +                        0 /* options */)) {
> +        access |= AVIO_FLAG_READ;
> +        CloseHandle(tmp_handle);
> +    }
> +
> +    if (mask & AVIO_FLAG_WRITE &&
> +        DuplicateHandle(GetCurrentProcess(), /* hSourceProcessHandle */
> +                        pipe_handle,
> +                        GetCurrentProcess(), /* hTargetProcessHandle */
> +                        &tmp_handle,
> +                        FILE_WRITE_DATA,
> +                        FALSE, /* bInheritHandle */
> +                        0 /* options */)) {
> +        access |= AVIO_FLAG_WRITE;
> +        CloseHandle(tmp_handle);
> +    }
> +
> +#else /* _WIN32 */
> +
> +    int status_flags = fcntl(fd, F_GETFL);
> +
> +    if (status_flags == -1)
> +        return AVERROR(errno);
> +
> +    switch (status_flags & O_ACCMODE) {
> +    case O_RDONLY:
> +        access = AVIO_FLAG_READ;
> +        break;
> +    case O_WRONLY:
> +        access = AVIO_FLAG_WRITE;
> +        break;
> +    case O_RDWR:
> +        access = AVIO_FLAG_READ | AVIO_FLAG_WRITE;
> +        break;
> +    }
> +
> +#endif /* _WIN32 */
> +
> +    return access & mask;
> +}
> +
>  const URLProtocol ff_pipe_protocol = {
>      .name                = "pipe",
>      .url_open            = pipe_open,
>      .url_read            = file_read,
>      .url_write           = file_write,
>      .url_get_file_handle = file_get_handle,
> -    .url_check           = file_check,
> +    .url_check           = pipe_check,
>      .priv_data_size      = sizeof(FileContext),
>      .priv_data_class     = &pipe_class,
>      .default_whitelist   = "crypto,data"
> -- 
> 2.37.2
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-11-09 14:31 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20220829144320.330685-1-bpeeluk.ref@yahoo.co.uk>
2022-08-29 14:43 ` [FFmpeg-devel] [PATCH 1/2] avformat/file: Add a specialized url_check callback for pipe protocol Neil Roberts
2022-08-29 14:43   ` [FFmpeg-devel] [PATCH 2/2] avformat/tests: Add a test for avio_check with the " Neil Roberts
2022-08-29 21:54   ` [FFmpeg-devel] [PATCH 1/2] avformat/file: Add a specialized url_check callback for " Michael Niedermayer
2022-08-30 12:09     ` [FFmpeg-devel] [PATCH v2 " Neil Roberts
2022-08-30 12:09       ` [FFmpeg-devel] [PATCH v2 2/2] avformat/tests: Add a test for avio_check with the " Neil Roberts
2022-11-09 14:31       ` [FFmpeg-devel] [PATCH v2 1/2] avformat/file: Add a specialized url_check callback for " Neil Roberts

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git