| Message ID | tencent_5A65487301485FF3F2D36935CCA908EEA605@qq.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avformat/file: add fd protocol | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
> On Nov 19, 2022, at 02:48, Zhao Zhili <quinkblack@foxmail.com> wrote: > > From: Zhao Zhili <zhilizhao@tencent.com> > > Unlike the pipe protocol, fd protocol has seek support if it > corresponding to a regular file. > --- > Sometimes it's the only way to access files via file descriptor, e.g., > requesting a shared file on Android: > https://developer.android.com/training/secure-file-sharing/request-file > > doc/protocols.texi | 24 +++++++++++++++++++ > libavformat/Makefile | 1 + > libavformat/file.c | 51 +++++++++++++++++++++++++++++++++++++++++ > libavformat/protocols.c | 1 + > libavformat/version.h | 4 ++-- > 5 files changed, 79 insertions(+), 2 deletions(-) > > Ping for review.
On Mon, 2022-12-05 at 10:51 +0800, zhilizhao(赵志立) wrote: > > On Nov 19, 2022, at 02:48, Zhao Zhili <quinkblack@foxmail.com> > > wrote: > > > > From: Zhao Zhili <zhilizhao@tencent.com> > > > > Unlike the pipe protocol, fd protocol has seek support if it > > corresponding to a regular file. > > --- > > Sometimes it's the only way to access files via file descriptor, > > e.g., > > requesting a shared file on Android: > > https://developer.android.com/training/secure-file-sharing/request-file > > > > doc/protocols.texi | 24 +++++++++++++++++++ > > libavformat/Makefile | 1 + > > libavformat/file.c | 51 > > +++++++++++++++++++++++++++++++++++++++++ > > libavformat/protocols.c | 1 + > > libavformat/version.h | 4 ++-- > > 5 files changed, 79 insertions(+), 2 deletions(-) > > > > > > Ping for review. > Will apply tomorrow unless there are objections.
On 8 Dec 2022, at 16:11, Zhao Zhili wrote: > On Mon, 2022-12-05 at 10:51 +0800, zhilizhao(赵志立) wrote: >>> On Nov 19, 2022, at 02:48, Zhao Zhili <quinkblack@foxmail.com> >>> wrote: >>> >>> From: Zhao Zhili <zhilizhao@tencent.com> >>> >>> Unlike the pipe protocol, fd protocol has seek support if it >>> corresponding to a regular file. >>> --- >>> Sometimes it's the only way to access files via file descriptor, >>> e.g., >>> requesting a shared file on Android: >>> https://developer.android.com/training/secure-file-sharing/request-file >>> >>> doc/protocols.texi | 24 +++++++++++++++++++ >>> libavformat/Makefile | 1 + >>> libavformat/file.c | 51 >>> +++++++++++++++++++++++++++++++++++++++++ >>> libavformat/protocols.c | 1 + >>> libavformat/version.h | 4 ++-- >>> 5 files changed, 79 insertions(+), 2 deletions(-) >>> >>> >> >> Ping for review. >> > > Will apply tomorrow unless there are objections. > Maybe I overlooked something but where does the CONFIG_FD_PROTOCOL define comes from? > _______________________________________________ > 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".
Marvin Scholz: > > > On 8 Dec 2022, at 16:11, Zhao Zhili wrote: > >> On Mon, 2022-12-05 at 10:51 +0800, zhilizhao(赵志立) wrote: >>>> On Nov 19, 2022, at 02:48, Zhao Zhili <quinkblack@foxmail.com> >>>> wrote: >>>> >>>> From: Zhao Zhili <zhilizhao@tencent.com> >>>> >>>> Unlike the pipe protocol, fd protocol has seek support if it >>>> corresponding to a regular file. >>>> --- >>>> Sometimes it's the only way to access files via file descriptor, >>>> e.g., >>>> requesting a shared file on Android: >>>> https://developer.android.com/training/secure-file-sharing/request-file >>>> >>>> doc/protocols.texi | 24 +++++++++++++++++++ >>>> libavformat/Makefile | 1 + >>>> libavformat/file.c | 51 >>>> +++++++++++++++++++++++++++++++++++++++++ >>>> libavformat/protocols.c | 1 + >>>> libavformat/version.h | 4 ++-- >>>> 5 files changed, 79 insertions(+), 2 deletions(-) >>>> >>>> >>> >>> Ping for review. >>> >> >> Will apply tomorrow unless there are objections. >> > > Maybe I overlooked something but where does the CONFIG_FD_PROTOCOL > define comes from? > It is autogenerated by configure due to the entry in lavf/protocols.c (and this change will trigger a request to rerun configure when compiling). (I haven't looked at the patch at all.) - Andreas
On Thu, 2022-12-08 at 16:28 +0100, Andreas Rheinhardt wrote: > Marvin Scholz: > > > > On 8 Dec 2022, at 16:11, Zhao Zhili wrote: > > > > > On Mon, 2022-12-05 at 10:51 +0800, zhilizhao(赵志立) wrote: > > > > > On Nov 19, 2022, at 02:48, Zhao Zhili <quinkblack@foxmail.com > > > > > > > > > > > wrote: > > > > > > > > > > From: Zhao Zhili <zhilizhao@tencent.com> > > > > > > > > > > Unlike the pipe protocol, fd protocol has seek support if it > > > > > corresponding to a regular file. > > > > > --- > > > > > Sometimes it's the only way to access files via file > > > > > descriptor, > > > > > e.g., > > > > > requesting a shared file on Android: > > > > > https://developer.android.com/training/secure-file-sharing/request-file > > > > > > > > > > doc/protocols.texi | 24 +++++++++++++++++++ > > > > > libavformat/Makefile | 1 + > > > > > libavformat/file.c | 51 > > > > > +++++++++++++++++++++++++++++++++++++++++ > > > > > libavformat/protocols.c | 1 + > > > > > libavformat/version.h | 4 ++-- > > > > > 5 files changed, 79 insertions(+), 2 deletions(-) > > > > > > > > > > > > > > > > > > Ping for review. > > > > > > > > > > Will apply tomorrow unless there are objections. > > > > > > > Maybe I overlooked something but where does the CONFIG_FD_PROTOCOL > > define comes from? > > > > It is autogenerated by configure due to the entry in lavf/protocols.c > (and this change will trigger a request to rerun configure when > compiling). > (I haven't looked at the patch at all.) > > - Andreas Yes, it was automatically generated by configure and included in config_components. For the patch itself, make pipe protocol is easy, but it doesn't sound right (a seekable pipe). So I chose to add a new one, which is supported by VLC too (URl is little different). A quick test case for this patch is: $ ./ffplay fd:0 < ~/video/cctv.mp4 You can seek back and forth.
Quoting Zhao Zhili (2022-11-18 19:48:02) > From: Zhao Zhili <zhilizhao@tencent.com> > > Unlike the pipe protocol, fd protocol has seek support if it > corresponding to a regular file. > --- > Sometimes it's the only way to access files via file descriptor, e.g., > requesting a shared file on Android: > https://developer.android.com/training/secure-file-sharing/request-file > Wouldn't dup()ing the file descriptor be safer?
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Anton Khirnov > Sent: 2022年12月9日 22:32 > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> > Cc: Zhao Zhili <zhilizhao@tencent.com> > Subject: Re: [FFmpeg-devel] [PATCH] avformat/file: add fd protocol > > Quoting Zhao Zhili (2022-11-18 19:48:02) > > From: Zhao Zhili <zhilizhao@tencent.com> > > > > Unlike the pipe protocol, fd protocol has seek support if it > > corresponding to a regular file. > > --- > > Sometimes it's the only way to access files via file descriptor, e.g., > > requesting a shared file on Android: > > https://developer.android.com/training/secure-file-sharing/request-file > > > > Wouldn't dup()ing the file descriptor be safer? Yes, I have thought that. The question is why pipe protocol doesn't do that. If there is no reason, I can update the patch then improve the pipe protocol. > > -- > Anton Khirnov > _______________________________________________ > 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".
Le maanantaina 5. joulukuuta 2022, 4.51.34 EET zhilizhao(赵志立) a écrit : > > On Nov 19, 2022, at 02:48, Zhao Zhili <quinkblack@foxmail.com> wrote: > > > > From: Zhao Zhili <zhilizhao@tencent.com> > > > > Unlike the pipe protocol, fd protocol has seek support if it > > corresponding to a regular file. > > --- > > Sometimes it's the only way to access files via file descriptor, e.g., > > requesting a shared file on Android: > > https://developer.android.com/training/secure-file-sharing/request-file > > > > doc/protocols.texi | 24 +++++++++++++++++++ > > libavformat/Makefile | 1 + > > libavformat/file.c | 51 +++++++++++++++++++++++++++++++++++++++++ > > libavformat/protocols.c | 1 + > > libavformat/version.h | 4 ++-- > > 5 files changed, 79 insertions(+), 2 deletions(-) > > Ping for review. VLC does this (with a slightly different syntax, i.e. fd://$NUM) and in hindsight, I think that it was a big mistake. It should not be possible to refer to an opaque handle from within the URL. This leads to all sorts of mischief, bordering on security issue, in the common case that the URL string is untrusted. To support this use case, IMO, the file descriptor should be passed explicitly via a trusted channel, *not* the URL.
On Mon, 2022-12-12 at 18:31 +0200, Rémi Denis-Courmont wrote: > Le maanantaina 5. joulukuuta 2022, 4.51.34 EET zhilizhao(赵志立) a écrit > : > > > On Nov 19, 2022, at 02:48, Zhao Zhili <quinkblack@foxmail.com> > > > wrote: > > > > > > From: Zhao Zhili <zhilizhao@tencent.com> > > > > > > Unlike the pipe protocol, fd protocol has seek support if it > > > corresponding to a regular file. > > > --- > > > Sometimes it's the only way to access files via file descriptor, > > > e.g., > > > requesting a shared file on Android: > > > https://developer.android.com/training/secure-file-sharing/request-file > > > > > > doc/protocols.texi | 24 +++++++++++++++++++ > > > libavformat/Makefile | 1 + > > > libavformat/file.c | 51 > > > +++++++++++++++++++++++++++++++++++++++++ > > > libavformat/protocols.c | 1 + > > > libavformat/version.h | 4 ++-- > > > 5 files changed, 79 insertions(+), 2 deletions(-) > > > > Ping for review. > > VLC does this (with a slightly different syntax, i.e. fd://$NUM) and > in > hindsight, I think that it was a big mistake. > > It should not be possible to refer to an opaque handle from within > the URL. > This leads to all sorts of mischief, bordering on security issue, in > the > common case that the URL string is untrusted. Could you elaborate on the security issue? > > To support this use case, IMO, the file descriptor should be passed > explicitly > via a trusted channel, *not* the URL. Does an explicit option works here? static const AVOption pipe_options[] = { { "blocksize", "set I/O operation maximum block size", offsetof(FileContext, blocksize), AV_OPT_TYPE_INT, { .i64 = INT_MAX }, 1, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM }, + { "fd", "set file descriptor", offsetof(FileContext, fd), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM }, { NULL } };
Le maanantaina 12. joulukuuta 2022, 19.23.52 EET Zhao Zhili a écrit : > On Mon, 2022-12-12 at 18:31 +0200, Rémi Denis-Courmont wrote: > > Le maanantaina 5. joulukuuta 2022, 4.51.34 EET zhilizhao(赵志立) a écrit > > > > > > On Nov 19, 2022, at 02:48, Zhao Zhili <quinkblack@foxmail.com> > > > > wrote: > > > > > > > > From: Zhao Zhili <zhilizhao@tencent.com> > > > > > > > > Unlike the pipe protocol, fd protocol has seek support if it > > > > corresponding to a regular file. > > > > --- > > > > Sometimes it's the only way to access files via file descriptor, > > > > e.g., > > > > requesting a shared file on Android: > > > > https://developer.android.com/training/secure-file-sharing/request-fil > > > > e > > > > > > > > doc/protocols.texi | 24 +++++++++++++++++++ > > > > libavformat/Makefile | 1 + > > > > libavformat/file.c | 51 > > > > +++++++++++++++++++++++++++++++++++++++++ > > > > libavformat/protocols.c | 1 + > > > > libavformat/version.h | 4 ++-- > > > > 5 files changed, 79 insertions(+), 2 deletions(-) > > > > > > Ping for review. > > > > VLC does this (with a slightly different syntax, i.e. fd://$NUM) and > > in > > hindsight, I think that it was a big mistake. > > > > It should not be possible to refer to an opaque handle from within > > the URL. > > This leads to all sorts of mischief, bordering on security issue, in > > the > > common case that the URL string is untrusted. > > Could you elaborate on the security issue? Just guess the FD of some critical process resource (.e.g D-Bus, X11 or Wayland connection), , and you can easily cause a DoS. It gets potentially worse if you can guess the FD that is used to carry sensitive data, such as another clients connection if FFmpeg runs inside some kind of network service. > > To support this use case, IMO, the file descriptor should be passed > > explicitly > > via a trusted channel, *not* the URL. > > Does an explicit option works here? > > static const AVOption pipe_options[] = { > { "blocksize", "set I/O operation maximum block size", > offsetof(FileContext, blocksize), AV_OPT_TYPE_INT, { .i64 = INT_MAX }, > 1, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM }, > + { "fd", "set file descriptor", offsetof(FileContext, fd), > AV_OPT_TYPE_INT, { .i64 = -1 }, -1, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM > }, > { NULL } > }; That seems more reasonable, so I cannot comment on the specifics of AVOption usage.
diff --git a/doc/protocols.texi b/doc/protocols.texi index 5e9198e67c..d9f2d2dec3 100644 --- a/doc/protocols.texi +++ b/doc/protocols.texi @@ -275,6 +275,30 @@ For example, to convert a GIF file given inline with @command{ffmpeg}: ffmpeg -i "data:image/gif;base64,R0lGODdhCAAIAMIEAAAAAAAA//8AAP//AP///////////////ywAAAAACAAIAAADF0gEDLojDgdGiJdJqUX02iB4E8Q9jUMkADs=" smiley.png @end example +@section fd + +File descriptor access protocol. + +The accepted syntax is: +@example +fd:[@var{number}] +@end example + +@var{number} is the number corresponding to a file descriptor. Unlike the pipe +protocol, fd protocol has seek support if it corresponding to a regular file. +If @var{number} is not specified, by default the stdout file descriptor will +be used for writing, stdin for reading. + +This protocol accepts the following options: + +@table @option +@item blocksize +Set I/O operation maximum block size, in bytes. Default value is +@code{INT_MAX}, which results in not limiting the requested block size. +Setting this value reasonably low improves user termination request reaction +time, which is valuable if data transmission is slow. +@end table + @section file File access protocol. diff --git a/libavformat/Makefile b/libavformat/Makefile index d7f198bf39..1452216e29 100644 --- a/libavformat/Makefile +++ b/libavformat/Makefile @@ -647,6 +647,7 @@ OBJS-$(CONFIG_DATA_PROTOCOL) += data_uri.o OBJS-$(CONFIG_FFRTMPCRYPT_PROTOCOL) += rtmpcrypt.o rtmpdigest.o rtmpdh.o OBJS-$(CONFIG_FFRTMPHTTP_PROTOCOL) += rtmphttp.o OBJS-$(CONFIG_FILE_PROTOCOL) += file.o +OBJS-$(CONFIG_FD_PROTOCOL) += file.o OBJS-$(CONFIG_FTP_PROTOCOL) += ftp.o urldecode.o OBJS-$(CONFIG_GOPHER_PROTOCOL) += gopher.o OBJS-$(CONFIG_GOPHERS_PROTOCOL) += gopher.o diff --git a/libavformat/file.c b/libavformat/file.c index 6103c37b34..765ad21d67 100644 --- a/libavformat/file.c +++ b/libavformat/file.c @@ -109,6 +109,13 @@ static const AVClass pipe_class = { .version = LIBAVUTIL_VERSION_INT, }; +static const AVClass fd_class = { + .class_name = "fd", + .item_name = av_default_item_name, + .option = pipe_options, + .version = LIBAVUTIL_VERSION_INT, +}; + static int file_read(URLContext *h, unsigned char *buf, int size) { FileContext *c = h->priv_data; @@ -412,3 +419,47 @@ const URLProtocol ff_pipe_protocol = { }; #endif /* CONFIG_PIPE_PROTOCOL */ + +#if CONFIG_FD_PROTOCOL + +static int fd_open(URLContext *h, const char *filename, int flags) +{ + FileContext *c = h->priv_data; + int fd; + char *final; + struct stat st; + + av_strstart(filename, "fd:", &filename); + + fd = strtol(filename, &final, 10); + if((filename == final) || *final ) {/* No digits found, or something like 10ab */ + if (flags & AVIO_FLAG_WRITE) { + fd = 1; + } else { + fd = 0; + } + } +#if HAVE_SETMODE + setmode(fd, O_BINARY); +#endif + if (fstat(fd, &st) < 0) + return AVERROR(errno); + h->is_streamed = !(S_ISREG(st.st_mode) || S_ISBLK(st.st_mode)); + c->fd = fd; + return 0; +} + +const URLProtocol ff_fd_protocol = { + .name = "fd", + .url_open = fd_open, + .url_read = file_read, + .url_write = file_write, + .url_seek = file_seek, + .url_get_file_handle = file_get_handle, + .url_check = file_check, + .priv_data_size = sizeof(FileContext), + .priv_data_class = &fd_class, + .default_whitelist = "crypto,data" +}; + +#endif /* CONFIG_FD_PROTOCOL */ diff --git a/libavformat/protocols.c b/libavformat/protocols.c index 8b7d1b940f..82cb1033e4 100644 --- a/libavformat/protocols.c +++ b/libavformat/protocols.c @@ -29,6 +29,7 @@ extern const URLProtocol ff_concat_protocol; extern const URLProtocol ff_concatf_protocol; extern const URLProtocol ff_crypto_protocol; extern const URLProtocol ff_data_protocol; +extern const URLProtocol ff_fd_protocol; extern const URLProtocol ff_ffrtmpcrypt_protocol; extern const URLProtocol ff_ffrtmphttp_protocol; extern const URLProtocol ff_file_protocol; diff --git a/libavformat/version.h b/libavformat/version.h index 7c9d50b7b3..f5a7f579b3 100644 --- a/libavformat/version.h +++ b/libavformat/version.h @@ -31,8 +31,8 @@ #include "version_major.h" -#define LIBAVFORMAT_VERSION_MINOR 34 -#define LIBAVFORMAT_VERSION_MICRO 101 +#define LIBAVFORMAT_VERSION_MINOR 35 +#define LIBAVFORMAT_VERSION_MICRO 100 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \ LIBAVFORMAT_VERSION_MINOR, \
From: Zhao Zhili <zhilizhao@tencent.com> Unlike the pipe protocol, fd protocol has seek support if it corresponding to a regular file. --- Sometimes it's the only way to access files via file descriptor, e.g., requesting a shared file on Android: https://developer.android.com/training/secure-file-sharing/request-file doc/protocols.texi | 24 +++++++++++++++++++ libavformat/Makefile | 1 + libavformat/file.c | 51 +++++++++++++++++++++++++++++++++++++++++ libavformat/protocols.c | 1 + libavformat/version.h | 4 ++-- 5 files changed, 79 insertions(+), 2 deletions(-)