* [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