[FFmpeg-devel] [PATCH] configure: add pkg-config check for libopenvino
From: Zhao Zhili
Signed-off-by: Zhao Zhili
---
configure | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/configure b/configure
index bb7be67676..7be98ed2ba 100755
--- a/configure
+++ b/configure
@@ -6661,7 +6661,8 @@ enabled libopenh264 && require_pkg_config
libopenh264 openh264 wels/codec_
enabled libopenjpeg && { check_pkg_config libopenjpeg "libopenjp2 >=
2.1.0" openjpeg.h opj_version ||
{ require_pkg_config libopenjpeg "libopenjp2 >=
2.1.0" openjpeg.h opj_version -DOPJ_STATIC && add_cppflags -DOPJ_STATIC; } }
enabled libopenmpt&& require_pkg_config libopenmpt "libopenmpt >=
0.2.6557" libopenmpt/libopenmpt.h openmpt_module_create -lstdc++ && append
libopenmpt_extralibs "-lstdc++"
-enabled libopenvino && require libopenvino c_api/ie_c_api.h
ie_c_api_version -linference_engine_c_api
+enabled libopenvino && { check_pkg_config libopenvino openvino
c_api/ie_c_api.h ie_c_api_version ||
+ require libopenvino c_api/ie_c_api.h
ie_c_api_version -linference_engine_c_api; }
enabled libopus && {
enabled libopus_decoder && {
require_pkg_config libopus opus opus_multistream.h
opus_multistream_decoder_create
--
2.25.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/hls: fail on probing non hls/m3u8 file extensions
Michael Niedermayer 于2023年5月3日周三 20:30写道：
>
> Its unexpected that a .avi or other "standard" file turns into a playlist.
> The goal of this patch is to avoid this unexpected behavior and possible
> privacy or security differences.
>
> Signed-off-by: Michael Niedermayer
> ---
> libavformat/hls.c | 10 +-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/hls.c b/libavformat/hls.c
> index 8a96a37ff9..826135016d 100644
> --- a/libavformat/hls.c
> +++ b/libavformat/hls.c
> @@ -2530,10 +2530,18 @@ static int hls_probe(const AVProbeData *p)
> if (strncmp(p->buf, "#EXTM3U", 7))
> return 0;
>
> +
Empty line,
> if (strstr(p->buf, "#EXT-X-STREAM-INF:") ||
> strstr(p->buf, "#EXT-X-TARGETDURATION:") ||
> -strstr(p->buf, "#EXT-X-MEDIA-SEQUENCE:"))
> +strstr(p->buf, "#EXT-X-MEDIA-SEQUENCE:")) {
> +
> +if (!av_match_ext(p->filename, "m3u8,hls,m3u")) {
> +av_log(NULL, AV_LOG_ERROR, "Not detecting m3u8/hls with non
> standard extension\n");
> +return 0;
> +}
> +
> return AVPROBE_SCORE_MAX;
> +}
> return 0;
> }
>
> --
> 2.17.1
>
> ___
LGTM
Thanks
Steven
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] [RFC] avformat: Add basic same origin check
On Wed, May 03, 2023 at 11:01:43PM +0200, Timo Rothenpieler wrote: > On 03.05.2023 21:08, Michael Niedermayer wrote: > > > > > A quick check for example shows that even something as simple as the > > > > > HLS BBC Radio streams will fail _all_ checks, since the playlists are > > > > > hosted on another host entirely as the media, thanks to akamai live > > > > > streaming. > > > > > Playlist here, as an example: > > > > > http://a.files.bbci.co.uk/media/live/manifesto/audio/simulcast/hls/nonuk/sbr_low/ak/bbc_radio_one.m3u8 > > > > > > > > yes, thats why it says RFC in the subject, i had expected that a bit > > > > already > > > > > > > > still OTOH, blocking these by default is the safer option, i mean if a > > > > user > > > > does a > > > > ./ffplay http://trustedfoobar.org/cutevideo.avi > > > > > > > > would she expect that video to access http://127.0.0.1/ and later > > > > http://evilhost/localwebscan-success > > > > I think this should not be possible by default settings, its unexpected > > > > > > > > > > Coming from the other side -- If the user needs to set the flag for > > > nearly all streams, then they are not going to check in the future and > > > just set it, defeating the purpose of them. At which point we might as > > > well not burden them. > > > > Yes, we need a system that is secure and works in most cases. > > What about doing what actual browsers do, and reading the > Access-Control-Allow-Origin HTTP header, and checking if the current origin > is allowed? > > This does not really work for local files. Best you could do is check for > "*" or not. > But would at least fix the BBC+Akamai case. I like the idea, do you want to implement it ? thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Everything should be made as simple as possible, but not simpler. -- Albert Einstein signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/3] avfilter/vf_libplacebo: add fillcolor option
Merged as 4b11a0755036..ad417eb5fa1
On Mon, 01 May 2023 17:02:59 +0200 Niklas Haas wrote:
> From: Niklas Haas
>
> In some circumstances, libplacebo will clear the background as a result
> of cropping/padding. Currently, this uses the hard-coded default fill
> color of black. This option makes this behavior configurable.
> ---
> doc/filters.texi| 6 ++
> libavfilter/vf_libplacebo.c | 22 ++
> 2 files changed, 28 insertions(+)
>
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 6d2672063c1..35df5c339a7 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -16018,6 +16018,12 @@ content with black borders, while a value of
> @code{1.0} always crops off parts
> of the content. Intermediate values are possible, leading to a mix of the two
> approaches.
>
> +@item fillcolor
> +Set the color used to fill the output area not covered by the output image,
> for
> +example as a result of @ref{normalize_sar}. For the general syntax of this
> +option, check the @ref{color syntax,,"Color" section in the ffmpeg-utils
> +manual,ffmpeg-utils}. Defaults to @code{black}.
> +
> @item colorspace
> @item color_primaries
> @item color_trc
> diff --git a/libavfilter/vf_libplacebo.c b/libavfilter/vf_libplacebo.c
> index 66929223dd9..fcdc97e48e2 100644
> --- a/libavfilter/vf_libplacebo.c
> +++ b/libavfilter/vf_libplacebo.c
> @@ -18,6 +18,7 @@
>
> #include "libavutil/file.h"
> #include "libavutil/opt.h"
> +#include "libavutil/parseutils.h"
> #include "internal.h"
> #include "vulkan_filter.h"
> #include "scale_eval.h"
> @@ -73,6 +74,7 @@ typedef struct LibplaceboContext {
> /* settings */
> char *out_format_string;
> enum AVPixelFormat out_format;
> +char *fillcolor;
> char *w_expr;
> char *h_expr;
> AVRational target_sar;
> @@ -225,6 +227,24 @@ static int find_scaler(AVFilterContext *avctx,
> return AVERROR(EINVAL);
> }
>
> +static int parse_fillcolor(AVFilterContext *avctx,
> + struct pl_render_params *params,
> + const char *color_str)
> +{
> +int err = 0;
> +uint8_t color_rgba[4];
> +
> +RET(av_parse_color(color_rgba, color_str, -1, avctx));
> +params->background_color[0] = (float) color_rgba[0] / UINT8_MAX;
> +params->background_color[1] = (float) color_rgba[1] / UINT8_MAX;
> +params->background_color[2] = (float) color_rgba[2] / UINT8_MAX;
> +params->background_transparency = 1.0f - (float) color_rgba[3] /
> UINT8_MAX;
> +return 0;
> +
> +fail:
> +return err;
> +}
> +
> static void libplacebo_uninit(AVFilterContext *avctx);
>
> static int libplacebo_init(AVFilterContext *avctx)
> @@ -469,6 +489,7 @@ static int process_frames(AVFilterContext *avctx, AVFrame
> *out, AVFrame *in)
>
> RET(find_scaler(avctx, ¶ms.upscaler, s->upscaler));
> RET(find_scaler(avctx, ¶ms.downscaler, s->downscaler));
> +RET(parse_fillcolor(avctx, ¶ms, s->fillcolor));
>
> pl_render_image(s->renderer, &image, &target, ¶ms);
> pl_unmap_avframe(s->gpu, &image);
> @@ -703,6 +724,7 @@ static const AVOption libplacebo_options[] = {
> { "force_divisible_by", "enforce that the output resolution is divisible
> by a defined integer when force_original_aspect_ratio is used",
> OFFSET(force_divisible_by), AV_OPT_TYPE_INT, { .i64 = 1 }, 1, 256, STATIC },
> { "normalize_sar", "force SAR normalization to 1:1",
> OFFSET(normalize_sar), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, STATIC },
> { "pad_crop_ratio", "ratio between padding and cropping when normalizing
> SAR (0=pad, 1=crop)", OFFSET(pad_crop_ratio), AV_OPT_TYPE_FLOAT, {.dbl=0.0},
> 0.0, 1.0, DYNAMIC },
> +{ "fillcolor", "Background fill color", OFFSET(fillcolor),
> AV_OPT_TYPE_STRING, {.str = "black"}, .flags = DYNAMIC },
>
> {"colorspace", "select colorspace", OFFSET(colorspace), AV_OPT_TYPE_INT,
> {.i64=-1}, -1, AVCOL_SPC_NB-1, DYNAMIC, "colorspace"},
> {"auto", "keep the same colorspace", 0, AV_OPT_TYPE_CONST, {.i64=-1},
>INT_MIN, INT_MAX, STATIC, "colorspace"},
> --
> 2.40.0
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] [RFC] avformat: Add basic same origin check
On 03.05.2023 21:08, Michael Niedermayer wrote: A quick check for example shows that even something as simple as the HLS BBC Radio streams will fail _all_ checks, since the playlists are hosted on another host entirely as the media, thanks to akamai live streaming. Playlist here, as an example: http://a.files.bbci.co.uk/media/live/manifesto/audio/simulcast/hls/nonuk/sbr_low/ak/bbc_radio_one.m3u8 yes, thats why it says RFC in the subject, i had expected that a bit already still OTOH, blocking these by default is the safer option, i mean if a user does a ./ffplay http://trustedfoobar.org/cutevideo.avi would she expect that video to access http://127.0.0.1/ and later http://evilhost/localwebscan-success I think this should not be possible by default settings, its unexpected Coming from the other side -- If the user needs to set the flag for nearly all streams, then they are not going to check in the future and just set it, defeating the purpose of them. At which point we might as well not burden them. Yes, we need a system that is secure and works in most cases. What about doing what actual browsers do, and reading the Access-Control-Allow-Origin HTTP header, and checking if the current origin is allowed? This does not really work for local files. Best you could do is check for "*" or not. But would at least fix the BBC+Akamai case. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] [RFC] avformat: Add basic same origin check
Le keskiviikkona 3. toukokuuta 2023, 22.05.26 EEST Michael Niedermayer a écrit : > On Wed, May 03, 2023 at 07:07:09PM +0300, Rémi Denis-Courmont wrote: > The difference is with a dodgy link its the web browser that has to protect > the user. With a dodgy HLS file its ffmpeg that has to protect the user. Well, the browser does not protect the user in that case. At best, an antimalware plugin or proxy might catch it but that's pretty much it. In fact, origin checks don't protect against this, nor are they intended to (AFAIK). > > It is obviously not an ideal situation, but any restriction here will most > > definitely break existing use cases (and likely be abused by server > > operators to lock FFmpeg out). > > My goal is to make it more secure by default and to keep it reasonable > convenient to the user > > So to me at least, if i open an hls file and i get a popup asking me > "foobar.hls wants to access http://localhost/someexploit,"; > "[Allow] [Deny] [Allow Always] [Deny Always]" > thats a win and i wouldnt call that "Breaking" an existing usecase. > Nor is that allowing any server operator to lock FFmpeg out User dialogues are notoriously bad for security purposes, as users don't really have the necessary info to make the proper decision, and just learn to click Allow Always. All they achieve is absolve the developers from protecting the user, really. But even if, what will the criterion be? You'll match localhost. Okay. What about localhost.localdomain and 127.0.0.1? What about all the noncanonical writings of 127.0.0.1 and other addresses in 127.0.0.0/8, and their noncanonical writing? IPv6? What about the assigned external IP address of the host And so on, and that's just to prevent connections to localhost. Some browsers prevent connections to private IP addresses too, to "protect" the Intranet, but the flip side is to break corporate content and home file servers. > If we can make this more convenient to the user while keeping it secure we > should. But we should not make it more convenient than what can be done > securely. I appreciate the thought but without a clearly defined threat model, you can't define the security issue, and thus you can't hope to fix it. Meanwhile, you will break things, and if you're not careful, you might as well make it easier for publishers to deliberately block FFmpeg. > If you have a hls file that mixes local files and remote http(s) then you > need to override the default protocol whitelist. It makes sense for HLS manifests because HLS is by definition tied to HTTP, but that won't work for real playlists (M3U or other). -- レミ・デニ-クールモン http://www.remlab.net/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Embedded documentation?
Hi, Three years ago, I shared some brief thoughts about embedding the documentation in the libraries. For example, that would allow GUI applications to open help dialogs about specific options. To see what it would need, I wrote the following header. I did not work any further, because groundwork need to be laid first. But now that it was mentioned in another thread, I think it is a good idea to show it, to see how people like it. Please share your remarks. Even “+1” to say you like it, because people who will not like it will not hesitate to post “-1”. +1. I certainly like the idea. Not thinking ahead about any details except that for custom very small builds, one should be able to disable this feature. -Thilo ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] [RFC] avformat: Add basic same origin check
On Wed, May 03, 2023 at 02:24:34PM +0200, Hendrik Leppkes wrote:
> On Wed, May 3, 2023 at 12:49 PM Michael Niedermayer
> wrote:
> >
> > On Wed, May 03, 2023 at 12:05:54PM +0200, Hendrik Leppkes wrote:
> > > On Tue, May 2, 2023 at 10:57 PM James Almer wrote:
> > > > >
> > > > > added
> > > > > +{"same_none" , "same origin check off" , 0 ,
> > > > > AV_OPT_TYPE_CONST, { .i64 = AVFMT_SAME_ORIGIN_CHECK_NONE }, 0,
> > > > > INT_MAX, D|E, "same_origin"},
> > > >
> > > > "none" sounds more natural.
> > > >
> > > > >
> > > > >
> > > > >> And do we want check_path to be default? It's a change
> > > > >> in behavior.
> > > > >
> > > > > is it usefull if its not enabled by default ?
> > > >
> > > > It is, since it can be enabled, like the whitelists and blacklists, but
> > > > the question is if it's preferable to have it enabled. If you consider
> > > > it so, then it's good and i wont oppose it.
> > > >
> > >
> > > Is there any estimation how many legitimate streams would be broken by
> > > these options?
> > > If any major streams don't work with this, then its not a good option,
> > > and eg. library users will likely just turn it off or to a lower
> > > setting, as proper streams just have to work - and log output is
> > > pretty much useless for API usage cases.
> > >
> > > A quick check for example shows that even something as simple as the
> > > HLS BBC Radio streams will fail _all_ checks, since the playlists are
> > > hosted on another host entirely as the media, thanks to akamai live
> > > streaming.
> > > Playlist here, as an example:
> > > http://a.files.bbci.co.uk/media/live/manifesto/audio/simulcast/hls/nonuk/sbr_low/ak/bbc_radio_one.m3u8
> >
> > yes, thats why it says RFC in the subject, i had expected that a bit already
> >
> > still OTOH, blocking these by default is the safer option, i mean if a user
> > does a
> > ./ffplay http://trustedfoobar.org/cutevideo.avi
> >
> > would she expect that video to access http://127.0.0.1/ and later
> > http://evilhost/localwebscan-success
> > I think this should not be possible by default settings, its unexpected
> >
>
> Coming from the other side -- If the user needs to set the flag for
> nearly all streams, then they are not going to check in the future and
> just set it, defeating the purpose of them. At which point we might as
> well not burden them.
Yes, we need a system that is secure and works in most cases.
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Old school: Use the lowest level language in which you can solve the problem
conveniently.
New school: Use the highest level language in which the latest supercomputer
can solve the problem without the user falling asleep waiting.
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] [RFC] avformat: Add basic same origin check
On Wed, May 03, 2023 at 07:07:09PM +0300, Rémi Denis-Courmont wrote: > Le keskiviikkona 3. toukokuuta 2023, 16.33.59 EEST Michael Niedermayer a > écrit > : > > This patch was inspired by a report on ffmpeg-security about SSRF > > (for which custom io_open() callback or soem sort of sandboxing/VM can be > > used to avoid it) > > The patch here was intended to explore if we can provide something thats > > better tahn currently by default > > I am not sure how a dodgy HLS manifest would be any different from the user > clicking an hyperlink from a dodgy website - or opening a dodgy playlist file > in their FFmpeg-based media player application for that matter. Either way, > it > can open any URL. The difference is with a dodgy link its the web browser that has to protect the user. With a dodgy HLS file its ffmpeg that has to protect the user. > > It is obviously not an ideal situation, but any restriction here will most > definitely break existing use cases (and likely be abused by server operators > to lock FFmpeg out). My goal is to make it more secure by default and to keep it reasonable convenient to the user So to me at least, if i open an hls file and i get a popup asking me "foobar.hls wants to access http://localhost/someexploit,"; "[Allow] [Deny] [Allow Always] [Deny Always]" thats a win and i wouldnt call that "Breaking" an existing usecase. Nor is that allowing any server operator to lock FFmpeg out For a non GUI app thats a matter of adjusting the command line or defaults by more classical means. If we can make this more convenient to the user while keeping it secure we should. But we should not make it more convenient than what can be done securely. > > Even the "obvious" blocking of secure (HTTPS) to nonsecure (HTTP) references > is likely to break stuff. If the end result is that everybody just turns > origin > checking off, it's pretty pointless. > > > But the same issue with roles flipped occurs for the end user and the user > > cannot be expected to setup a custom io_open() callback for his player > > The current code can be also used to poke > > around the local network of the user. Which is unexpected by the user > > for example a avi file could be probed as a m3u8 playlist and then > > poke around on the local net while mixing that with remote urls > > from the timing of the remote accesses the remote party should be able > > to infer what happened with the local poking. > > I agree, but it is unrealistic to change anything here. People make playlists > mixed with local files and network file systems or cloud storage services. > Yes, > there is a slight information leakage. For instance, you can probe if a local > file exists by interleaving local and remote URLs in a playlist. I dont know what software you are using but FFmpeg will prevent this attack with default protocol whitelists If you have a hls file that mixes local files and remote http(s) then you need to override the default protocol whitelist. If iam the user i would do that for that one file which in fact in that case really has to be my file i wrote. Of course nothing stops the user to set that by default for all urls, thats the users choice, but i think its much wiser not to do that thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Elect your leaders based on what they did after the last election, not based on what they say before an election. signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Embedded documentation?
Timo Rothenpieler (12023-05-01): > Somewhat loosely related to this: > > A frequent issue is that it's entirely non-obvious which global libavcodec > options a codec might make use of. > Having a way to self-document that would be amazing, so those options show > up in the --help output, ideally with their codec-specific default. Interesting remark. I also thought in the past that knowing if a certain component uses or ignore a particular common option would be useful, but I had not pushed the reflexion to that point. > The obvious idea I had for this was to utilize the FFCodecDefault struct > which already exists, maybe expanding it a tiny bit to allow the second > value to be NULL, indicating "This codec uses that option, but does not > change the default". > > Main issue with this is that FFCodecDefault is a private struct. > It could just be made public and user-queryable, while making every current > user of it aware of possible NULL-values, which they can then just ignore. I can imagine a few other solutions. Note that the way we store the information in the library does not have to be the same as the way we give that information to the user. For example, internally we could store an array of used or unused options and to the user we can include only the used options in AVDocExcerpt. The most annoying task for this will be to look at the code component by component. Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Embedded documentation?
Diederick C. Niehorster (12023-05-01): > +1. Thanks. > I assume a lot of the AVDocNode can be automatically populated from its > corresponding option? We'd not want to maintain, e.g. option names and > aliases in more than one place. I cannot promise there will be no duplication at all, and AVOption is very limited and needs to be replaced anyway. But I can promise it will not have to be written as a C structure with all its fields initialized. I have not discussed it nor finalized any of the details, but I am thinking the build system will parse either doxy-style comments in the C source code or in a separate .md file with the same name to generate the structures for the built-in documentation. If the system parses comments in the C source, it can also parse the AVOption initializers, at least in the simple cases. Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] MAINTAINERS: add vanitous self to maintain RISC-V
--- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 854ccc3fa4..f95be01dc6 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -543,6 +543,7 @@ LoongArch Shiyou Yin Mac OS X / PowerPC Romain Dolbeau, Guillaume Poirier Amiga / PowerPC Colin Ward Linux / PowerPC Lauri Kasanen +RISC-V Rémi Denis-Courmont Windows MinGW Alex Beregszaszi, Ramiro Polla Windows Cygwin Victor Paesa Windows MSVCMatthew Oliver, Hendrik Leppkes -- 2.40.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] [RFC] avformat: Add basic same origin check
Le keskiviikkona 3. toukokuuta 2023, 16.33.59 EEST Michael Niedermayer a écrit : > This patch was inspired by a report on ffmpeg-security about SSRF > (for which custom io_open() callback or soem sort of sandboxing/VM can be > used to avoid it) > The patch here was intended to explore if we can provide something thats > better tahn currently by default I am not sure how a dodgy HLS manifest would be any different from the user clicking an hyperlink from a dodgy website - or opening a dodgy playlist file in their FFmpeg-based media player application for that matter. Either way, it can open any URL. It is obviously not an ideal situation, but any restriction here will most definitely break existing use cases (and likely be abused by server operators to lock FFmpeg out). Even the "obvious" blocking of secure (HTTPS) to nonsecure (HTTP) references is likely to break stuff. If the end result is that everybody just turns origin checking off, it's pretty pointless. > But the same issue with roles flipped occurs for the end user and the user > cannot be expected to setup a custom io_open() callback for his player > The current code can be also used to poke > around the local network of the user. Which is unexpected by the user > for example a avi file could be probed as a m3u8 playlist and then > poke around on the local net while mixing that with remote urls > from the timing of the remote accesses the remote party should be able > to infer what happened with the local poking. I agree, but it is unrealistic to change anything here. People make playlists mixed with local files and network file systems or cloud storage services. Yes, there is a slight information leakage. For instance, you can probe if a local file exists by interleaving local and remote URLs in a playlist. In practice, this is a well-known issue and has been for two at least decades, and the "solution" is to limit what the open file can do. To state the obvious extreme, one wouldn't want to execute a shell script or an executable from a playlist. -- Rémi Denis-Courmont http://www.remlab.net/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] [RFC] avformat: Add basic same origin check
Hi On Wed, May 03, 2023 at 02:16:03PM +0300, Rémi Denis-Courmont wrote: > Nit: different fixed > > But is there an actual threat model whence it is necessary or even useful for > a media framework to implement origin policies? On top of my head, this can > be used by content providers to prevent third parties from referencing their > media files... but that seems user-hostile; it does not provide any security > for the user of FFmpeg. > > I could be wrong, but IMU, origin policy is meant to prevent harmful > embedding of images and frames, and to prevent cross-site scripting, but > FFmpeg doesn't support either if these anyway, so it's not concerned. This patch was inspired by a report on ffmpeg-security about SSRF (for which custom io_open() callback or soem sort of sandboxing/VM can be used to avoid it) The patch here was intended to explore if we can provide something thats better tahn currently by default But the same issue with roles flipped occurs for the end user and the user cannot be expected to setup a custom io_open() callback for his player The current code can be also used to poke around the local network of the user. Which is unexpected by the user for example a avi file could be probed as a m3u8 playlist and then poke around on the local net while mixing that with remote urls from the timing of the remote accesses the remote party should be able to infer what happened with the local poking. Did it timeout? was the access rejected ? was there a file that was read and probed/played ? thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Whats the most studid thing your enemy could do ? Blow himself up Whats the most studid thing you could do ? Give up your rights and freedom because your enemy blew himself up. signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v4 1/6] ccfifo: Properly handle CEA-708 captions through framerate conversion
Hi Anton,
Thanks for your feedback. Comments inline:
On Wed, May 3, 2023 at 5:20 AM Anton Khirnov wrote:
>
> Quoting Devin Heitmueller (2023-04-28 18:37:46)
> > +void ff_ccfifo_freep(AVCCFifo **ccf)
> > +{
> > +if (ccf && *ccf) {
>
> Don't check for ccf, it makes no sense to call this function with
> ccf==NULL, so silently ignoring it can hide bugs.
Ok.
> > +AVCCFifo *tmp = *ccf;
> > +if (tmp->cc_608_fifo)
> > +av_fifo_freep2(&tmp->cc_608_fifo);
>
> Useless checks, av_fifo_freep2 can be called on &NULL.
Ok.
> > +if (tmp->cc_708_fifo)
> > +av_fifo_freep2(&tmp->cc_708_fifo);
> > +av_freep(*ccf);
> > +}
> > +}
> > +
> > +AVCCFifo *ff_ccfifo_alloc(AVRational *framerate, void *log_ctx)
>
> We typically pass AVRational directly, not as pointers. That also makes
> it clear that the passed value is not modified.
Ok.
> > +{
> > +AVCCFifo *ccf;
> > +int i;
> > +
> > +ccf = av_mallocz(sizeof(*ccf));
> > +if (!ccf)
> > +return NULL;
> > +
> > +if (!(ccf->cc_708_fifo = av_fifo_alloc2(MAX_CC_ELEMENTS,
> > CC_BYTES_PER_ENTRY, 0)))
> > +goto error;
> > +
> > +if (!(ccf->cc_608_fifo = av_fifo_alloc2(MAX_CC_ELEMENTS,
> > CC_BYTES_PER_ENTRY, 0)))
> > +goto error;
> > +
> > +/* Based on the target FPS, figure out the expected cc_count and
> > number of
> > + 608 tuples per packet. See ANSI/CTA-708-E Sec 4.3.6.1. */
> > +for (i = 0; i < FF_ARRAY_ELEMS(cc_lookup_vals); i++) {
> > +if (framerate->num == cc_lookup_vals[i].num &&
> > +framerate->den == cc_lookup_vals[i].den) {
> > +ccf->expected_cc_count = cc_lookup_vals[i].cc_count;
> > +ccf->expected_608 = cc_lookup_vals[i].num_608;
> > +break;
> > +}
> > +}
> > +
> > +if (ccf->expected_608 == 0) {
> > +/* We didn't find an output frame we support. We'll let the call
> > succeed
> > + and the FIFO to be allocated, but the extract/inject functions
> > will simply
> > + leave everything the way it is */
> > +av_log(ccf->log_ctx, AV_LOG_WARNING, "cc_fifo cannot transcode
> > captions fps=%d/%d\n",
> > + framerate->num, framerate->den);
>
> Won't this result in spurious warnings for users who are just converting
> framerates and don't have any captions. If so it should probably be
> printed the first time we actually see caption side data.
Yeah, I see your point. Let me investigate further and see what I can do here.
> > +ccf->passthrough = 1;
> > +}
> > +
> > +return ccf;
> > +
> > +error:
> > +ff_ccfifo_freep(&ccf);
> > +return NULL;
> > +}
> > +
> > +int ff_ccfifo_injectbytes(AVCCFifo *ccf, uint8_t **data, size_t *len)
> > +{
> > +char *cc_data;
> > +int cc_filled = 0;
> > +int i;
> > +
> > +if (!ccf)
> > +return AVERROR(EINVAL);
>
> For all the extract/inject functions: it should be invalid to call them
> with a NULL context, so this should be an av_assert0() or not be here at
> all.
Ok.
> > +
> > +if (ccf->passthrough) {
> > +*data = NULL;
> > +*len = 0;
> > +return 0;
> > +}
> > +
> > +cc_data = av_mallocz(ccf->expected_cc_count * CC_BYTES_PER_ENTRY);
>
> This buffer size is constant for a given AVCCFifo object, so perhaps
> ff_ccfifo_alloc() could return required buffer size and this function
> could write into a user-provided buffer and avoid constant dynamic
> allocation.
So I had previously considered the approach you suggested, but decided
against it. This is because while the AVCCFifo today always returns
the same number of bytes, there are cases where cc_count could vary on
a per frame basis (and thus the buffer size differs). In particular
with 30i with 3:2 pulldown the cc_count alternates between 20 and 30.
See CTA-708-E R-2018 Sec 4.6.3.1.
Having the queue return a properly sized buffer avoids situations
where you make a call to get the size and then make a separate call to
fill the buffer (where the size might not be correct).
I don't particularly love the approach, given it means callers have to
memcpy() the result into the final buffer and then free the buffer
created by the call to inject. But it seemed like the better approach
given the issue described above.
I guess because the API is private and we don't support that feature
today, we could do it as you described and then change the API later.
Suggestions welcome.
> > +if (!cc_data) {
> > +return AVERROR(ENOMEM);
> > +}
> > +
> > +for (i = 0; i < ccf->expected_608; i++) {
> > +if (av_fifo_can_read(ccf->cc_608_fifo) >= CC_BYTES_PER_ENTRY) {
> > +av_fifo_read(ccf->cc_608_fifo, &cc_data[cc_filled *
> > CC_BYTES_PER_ENTRY],
> > + CC_BYTES_PER_ENTRY);
>
> This looks wrong, as fifo operations are in elements, and your
> elements are already CC_BYTES_PER_ENTRY. So every read actually writes
> CC_B
[FFmpeg-devel] [PATCH v6] avformat: add MMTP parser and MMT/TLV demuxer
v1 -> v2: Refactor using GetByteContext; Fix compile error.
v2 -> v3: Remove debug statement.
v3 -> v4: Squash commits.
v4 -> v5: Improve portability; Cosmetic changes.
v5 -> v6: remove unnecessary NULL checks.
This patch adds an MPEG Media Transport Protocol (MMTP) parser, as defined in
ISO/IEC 23008-1, and an MMT protocol over TLV packets (MMT/TLV) demuxer, as
defined in ARIB STD-B32. Currently, it supports HEVC, AAC LATM, and ARIB-TTML
demuxing.
Since MMTP is designed to transmit over IP, there is no size information within
each MMTP packet, and there is no on-disk format defined alongside the
protocol. One industrial solution is a simple container format using
type–length–value packets, which is defined in ARIB STD-B32.
Another known container format for MMTP is using packet capture (pcap) files
which records network packets. This patch does not include the demuxer for this
container format.
Signed-off-by: SuperFashi
---
Changelog|1 +
doc/demuxers.texi|4 +
libavformat/Makefile |1 +
libavformat/allformats.c |1 +
libavformat/mmtp.c | 1525 ++
libavformat/mmtp.h | 64 ++
libavformat/mmttlv.c | 335 +
libavformat/version.h|2 +-
8 files changed, 1932 insertions(+), 1 deletion(-)
create mode 100644 libavformat/mmtp.c
create mode 100644 libavformat/mmtp.h
create mode 100644 libavformat/mmttlv.c
diff --git a/Changelog b/Changelog
index 4901ef6ad7..594c445ea2 100644
--- a/Changelog
+++ b/Changelog
@@ -7,6 +7,7 @@ version :
- Extend VAAPI support for libva-win32 on Windows
- afireqsrc audio source filter
- arls filter
+- MMTP parser and MMT/TLV demuxer
version 6.0:
- Radiance HDR image support
diff --git a/doc/demuxers.texi b/doc/demuxers.texi
index 2d33b47a56..56aab251b2 100644
--- a/doc/demuxers.texi
+++ b/doc/demuxers.texi
@@ -689,6 +689,10 @@ Set the sample rate for libopenmpt to output.
Range is from 1000 to INT_MAX. The value default is 48000.
@end table
+@section mmttlv
+
+Demuxer for MMT protocol over TLV packets (MMT/TLV), as defined in ARIB
STD-B32.
+
@section mov/mp4/3gp
Demuxer for Quicktime File Format & ISO/IEC Base Media File Format (ISO/IEC
14496-12 or MPEG-4 Part 12, ISO/IEC 15444-12 or JPEG 2000 Part 12).
diff --git a/libavformat/Makefile b/libavformat/Makefile
index f8ad7c6a11..e32d6e71a3 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -354,6 +354,7 @@ OBJS-$(CONFIG_MLV_DEMUXER) += mlvdec.o
riffdec.o
OBJS-$(CONFIG_MM_DEMUXER)+= mm.o
OBJS-$(CONFIG_MMF_DEMUXER) += mmf.o
OBJS-$(CONFIG_MMF_MUXER) += mmf.o rawenc.o
+OBJS-$(CONFIG_MMTTLV_DEMUXER)+= mmtp.o mmttlv.o
OBJS-$(CONFIG_MODS_DEMUXER) += mods.o
OBJS-$(CONFIG_MOFLEX_DEMUXER)+= moflex.o
OBJS-$(CONFIG_MOV_DEMUXER) += mov.o mov_chan.o mov_esds.o \
diff --git a/libavformat/allformats.c b/libavformat/allformats.c
index efdb34e29d..d5f4f5680e 100644
--- a/libavformat/allformats.c
+++ b/libavformat/allformats.c
@@ -270,6 +270,7 @@ extern const AVInputFormat ff_mlv_demuxer;
extern const AVInputFormat ff_mm_demuxer;
extern const AVInputFormat ff_mmf_demuxer;
extern const FFOutputFormat ff_mmf_muxer;
+extern const AVInputFormat ff_mmttlv_demuxer;
extern const AVInputFormat ff_mods_demuxer;
extern const AVInputFormat ff_moflex_demuxer;
extern const AVInputFormat ff_mov_demuxer;
diff --git a/libavformat/mmtp.c b/libavformat/mmtp.c
new file mode 100644
index 00..6f002cc827
--- /dev/null
+++ b/libavformat/mmtp.c
@@ -0,0 +1,1525 @@
+/*
+ * MPEG Media Transport Protocol (MMTP) parser, as defined in ISO/IEC 23008-1.
+ * Copyright (c) 2023 SuperFashi
+ *
+ * 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
+
+#include "libavcodec/bytestream.h"
+#include "libavutil/avassert.h"
+#include "libavutil/intreadwrite.h"
+#include "libavutil/mem.h"
+#include "demux.h"
+#include "internal.h"
+#include "mmtp.h"
+#include "network.h"
+
+struct MMTGeneralLocationInfo {
+uint8_t location_type;
+union {
+struct {
+uint16_t packet_id;
+} type0;
+struct {
+struct in_addr ipv4_src_addr;
+
[FFmpeg-devel] [PATCH] avformat/hls: fail on probing non hls/m3u8 file extensions
Its unexpected that a .avi or other "standard" file turns into a playlist.
The goal of this patch is to avoid this unexpected behavior and possible
privacy or security differences.
Signed-off-by: Michael Niedermayer
---
libavformat/hls.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/libavformat/hls.c b/libavformat/hls.c
index 8a96a37ff9..826135016d 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -2530,10 +2530,18 @@ static int hls_probe(const AVProbeData *p)
if (strncmp(p->buf, "#EXTM3U", 7))
return 0;
+
if (strstr(p->buf, "#EXT-X-STREAM-INF:") ||
strstr(p->buf, "#EXT-X-TARGETDURATION:") ||
-strstr(p->buf, "#EXT-X-MEDIA-SEQUENCE:"))
+strstr(p->buf, "#EXT-X-MEDIA-SEQUENCE:")) {
+
+if (!av_match_ext(p->filename, "m3u8,hls,m3u")) {
+av_log(NULL, AV_LOG_ERROR, "Not detecting m3u8/hls with non
standard extension\n");
+return 0;
+}
+
return AVPROBE_SCORE_MAX;
+}
return 0;
}
--
2.17.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] Added support for MB_INFO
Sorry to revive an old thread, but I updated the patch for ffmpeg 6 and this new patch should address the comments. Still this is a libx264-only patch, and provides a means to specify that only portions of the frame have changed from the previous one while the others should be P_SKIP-ped. More specifically, now the data is fully contained in the side data and the actual map of macroblocks which changed from the previous frame is created from the side information at encoding time, much like what is done for the region of interest. best, Elias On Fri, 2022-06-17 at 17:10 +0200, Timo Rothenpieler wrote: > CAUTION: This email originated from outside of the organization. Do > not click links or open attachments unless you can confirm the sender > and know the content is safe. > > > > On 17.06.2022 12:59, Carotti, Elias wrote: > > > > > > Yes, exactly. It relies on x264 to free it. > > > > Not really. It can rely on x264 if you explicitly want that > > behavior. > > If you do not set a deallocator, it remains the caller > > responsibility. > > > > > What happens if x264 is not involved, but some other encoder, > > > which > > > does > > > not check for that kind of side data? > > > > > > How does the caller know that the data was consumed, and the > > > ownership > > > passed on? > > > > Again, you don't have to pass the ownership, and in fact in my use > > case > > I do not pass it since I actually recycle and update the same > > buffer > > for subsequent frames. If you do intentionally pass the ownership > > you > > need to be aware of what you are doing. As I said, I see a leak > > only in > > case of a programming error. > > Maybe we could explicitly state it in the comment section in > > mb_info.h: > > if you set the deallocator you're relinquishing ownership of the > > buffer. > > I'm not sure if that's something you can sensibly do with side data? > What if it gets buffered, copied, and so on? > > > Plus, there's one more flag (b_mb_info_update) in libx264 which > > allows > > to pass back information to the caller about which macroblocks > > among > > the flagged ones were actually encoded as P_SKIP. I did not add > > support > > for that though but in the future somebody may want to. > > Yes, it's very x264 specific. > But the side data is generic. If for some reason x264 does not > process a > frame, or any other encoder ends up getting used, the data will leak > if > it relied on x264 to free it. > > > In principle I could've put the buffer into the side information > > and > > not just pass a pointer to it but I thought that it would require > > adding more semantics which would imply a stronger dependency on > > libx264. > > Right now, mb_info is a vector of uint8_t flags and the only > > possible > > value - to date - is X264_MBINFO_CONSTANT. What if, say, one day > > libx264 decides the buffer has to be a vector of uint16_t or still > > uint8_t but the flags are packed? On the other hand, this, AFAIK, > > is > > only supported by libx264. Other codecs may want to choose a > > different > > semantic for a similar field and the only possibility to make it > > generic is letting the caller handling the low level details. > > I'm not aware of any other side data with a similar semantic. And I'm > really not sure if it's sane or even valid to do it like that. > Can't the data be entirely contained within the side data? > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". NICE SRL, viale Monte Grappa 3/5, 20124 Milano, Italia, Registro delle Imprese di Milano Monza Brianza Lodi REA n. 2096882, Capitale Sociale: 10.329,14 EUR i.v., Cod. Fisc. e P.IVA 01133050052, Societa con Socio Unico From 6c9024de2d0cdb03706ef8894a36f954185cefbb Mon Sep 17 00:00:00 2001 From: Elias Carotti Date: Wed, 19 Apr 2023 11:49:39 +0200 Subject: [PATCH] Add support for x264's MB_INFO --- libavcodec/libx264.c | 61 libavutil/Makefile | 4 +++ libavutil/frame.h| 10 libavutil/mb_info.c | 48 ++ libavutil/mb_info.h | 46 + 5 files changed, 169 insertions(+) create mode 100644 libavutil/mb_info.c create mode 100644 libavutil/mb_info.h diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c index cfdd422236..a98d702c92 100644 --- a/libavcodec/libx264.c +++ b/libavcodec/libx264.c @@ -30,6 +30,7 @@ #include "libavutil/stereo3d.h" #include "libavutil/time.h" #include "libavutil/intreadwrite.h" +#include "libavutil/mb_info.h" #include "avcodec.h" #include "codec_internal.h" #include "encode.h" @@ -48,6 +49,9 @@ // from x264.h, for quant_offsets, Macroblocks are 16x16 // blocks of pixels (with respect to the luma plane) #define MB_SIZE 16 +#define MB_LSIZE 4 +#def
Re: [FFmpeg-devel] [PATCH] [RFC] avformat: Add basic same origin check
On Wed, May 3, 2023 at 12:49 PM Michael Niedermayer
wrote:
>
> On Wed, May 03, 2023 at 12:05:54PM +0200, Hendrik Leppkes wrote:
> > On Tue, May 2, 2023 at 10:57 PM James Almer wrote:
> > > >
> > > > added
> > > > +{"same_none" , "same origin check off" , 0 ,
> > > > AV_OPT_TYPE_CONST, { .i64 = AVFMT_SAME_ORIGIN_CHECK_NONE }, 0, INT_MAX,
> > > > D|E, "same_origin"},
> > >
> > > "none" sounds more natural.
> > >
> > > >
> > > >
> > > >> And do we want check_path to be default? It's a change
> > > >> in behavior.
> > > >
> > > > is it usefull if its not enabled by default ?
> > >
> > > It is, since it can be enabled, like the whitelists and blacklists, but
> > > the question is if it's preferable to have it enabled. If you consider
> > > it so, then it's good and i wont oppose it.
> > >
> >
> > Is there any estimation how many legitimate streams would be broken by
> > these options?
> > If any major streams don't work with this, then its not a good option,
> > and eg. library users will likely just turn it off or to a lower
> > setting, as proper streams just have to work - and log output is
> > pretty much useless for API usage cases.
> >
> > A quick check for example shows that even something as simple as the
> > HLS BBC Radio streams will fail _all_ checks, since the playlists are
> > hosted on another host entirely as the media, thanks to akamai live
> > streaming.
> > Playlist here, as an example:
> > http://a.files.bbci.co.uk/media/live/manifesto/audio/simulcast/hls/nonuk/sbr_low/ak/bbc_radio_one.m3u8
>
> yes, thats why it says RFC in the subject, i had expected that a bit already
>
> still OTOH, blocking these by default is the safer option, i mean if a user
> does a
> ./ffplay http://trustedfoobar.org/cutevideo.avi
>
> would she expect that video to access http://127.0.0.1/ and later
> http://evilhost/localwebscan-success
> I think this should not be possible by default settings, its unexpected
>
Coming from the other side -- If the user needs to set the flag for
nearly all streams, then they are not going to check in the future and
just set it, defeating the purpose of them. At which point we might as
well not burden them.
- Hendrik
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] [RFC] avformat: Add basic same origin check
Nit: different But is there an actual threat model whence it is necessary or even useful for a media framework to implement origin policies? On top of my head, this can be used by content providers to prevent third parties from referencing their media files... but that seems user-hostile; it does not provide any security for the user of FFmpeg. I could be wrong, but IMU, origin policy is meant to prevent harmful embedding of images and frames, and to prevent cross-site scripting, but FFmpeg doesn't support either if these anyway, so it's not concerned. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] [RFC] avformat: Add basic same origin check
On Wed, May 03, 2023 at 12:05:54PM +0200, Hendrik Leppkes wrote:
> On Tue, May 2, 2023 at 10:57 PM James Almer wrote:
> > >
> > > added
> > > +{"same_none" , "same origin check off" , 0 ,
> > > AV_OPT_TYPE_CONST, { .i64 = AVFMT_SAME_ORIGIN_CHECK_NONE }, 0, INT_MAX,
> > > D|E, "same_origin"},
> >
> > "none" sounds more natural.
> >
> > >
> > >
> > >> And do we want check_path to be default? It's a change
> > >> in behavior.
> > >
> > > is it usefull if its not enabled by default ?
> >
> > It is, since it can be enabled, like the whitelists and blacklists, but
> > the question is if it's preferable to have it enabled. If you consider
> > it so, then it's good and i wont oppose it.
> >
>
> Is there any estimation how many legitimate streams would be broken by
> these options?
> If any major streams don't work with this, then its not a good option,
> and eg. library users will likely just turn it off or to a lower
> setting, as proper streams just have to work - and log output is
> pretty much useless for API usage cases.
>
> A quick check for example shows that even something as simple as the
> HLS BBC Radio streams will fail _all_ checks, since the playlists are
> hosted on another host entirely as the media, thanks to akamai live
> streaming.
> Playlist here, as an example:
> http://a.files.bbci.co.uk/media/live/manifesto/audio/simulcast/hls/nonuk/sbr_low/ak/bbc_radio_one.m3u8
yes, thats why it says RFC in the subject, i had expected that a bit already
still OTOH, blocking these by default is the safer option, i mean if a user
does a
./ffplay http://trustedfoobar.org/cutevideo.avi
would she expect that video to access http://127.0.0.1/ and later
http://evilhost/localwebscan-success
I think this should not be possible by default settings, its unexpected
maybe a whitelist of hosts or urls. Something the user could add
*.akamaized.net
to may be an option
Thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The real ebay dictionary, page 2
"100% positive feedback" - "All either got their money back or didnt complain"
"Best seller ever, very honest" - "Seller refunded buyer after failed scam"
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] [RFC] avformat: Add basic same origin check
On Tue, May 2, 2023 at 10:57 PM James Almer wrote:
> >
> > added
> > +{"same_none" , "same origin check off" , 0 ,
> > AV_OPT_TYPE_CONST, { .i64 = AVFMT_SAME_ORIGIN_CHECK_NONE }, 0, INT_MAX,
> > D|E, "same_origin"},
>
> "none" sounds more natural.
>
> >
> >
> >> And do we want check_path to be default? It's a change
> >> in behavior.
> >
> > is it usefull if its not enabled by default ?
>
> It is, since it can be enabled, like the whitelists and blacklists, but
> the question is if it's preferable to have it enabled. If you consider
> it so, then it's good and i wont oppose it.
>
Is there any estimation how many legitimate streams would be broken by
these options?
If any major streams don't work with this, then its not a good option,
and eg. library users will likely just turn it off or to a lower
setting, as proper streams just have to work - and log output is
pretty much useless for API usage cases.
A quick check for example shows that even something as simple as the
HLS BBC Radio streams will fail _all_ checks, since the playlists are
hosted on another host entirely as the media, thanks to akamai live
streaming.
Playlist here, as an example:
http://a.files.bbci.co.uk/media/live/manifesto/audio/simulcast/hls/nonuk/sbr_low/ak/bbc_radio_one.m3u8
- Hendrik
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] [RFC] avformat: Add basic same origin check
Quoting Michael Niedermayer (2023-05-02 23:15:46) > the problem with default-disabled is that the user needs to know > 1. that the option exist > 2. what the option does > 3. what an attacker can do with such urls > 4. that its not enabled by default > > OTOH if its enabled by default, the worst it can do is fail with a error > the user can lookup the error and disable the option > > but i may be missing something here, also comments both from people > who regularly work with hls and anything else contaning urls in files > and also people who dealt with any related attacks is welcome. > > The goal is that this actually does something useful in reality. This changes behavior in an incompatible way, so IMO this should happen on a major bump. There should also be a note in the changelog. Perhaps there could be a special 'auto' value that would initially default to no effect, but would print a warning if a URL would stop working after the bump. -- 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] [RFC] avformat: Add basic same origin check
Quoting Michael Niedermayer (2023-05-02 21:36:31)
> TODO: bump minor version, add docs
>
> Signed-off-by: Michael Niedermayer
> ---
> libavformat/avformat.h | 10 ++
> libavformat/options.c | 29 +
> libavformat/options_table.h | 3 +++
> 3 files changed, 42 insertions(+)
>
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 1916aa2dc5..5ff77323ba 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -1713,6 +1713,16 @@ typedef struct AVFormatContext {
> * @return 0 on success, a negative AVERROR code on failure
> */
> int (*io_close2)(struct AVFormatContext *s, AVIOContext *pb);
> +
> +/**
> + * Perform basic same origin checks in default io_open()
> + * - encoding: set by user
> + * - decoding: set by user
> + */
> +int same_origin_check;
> +#define AVFMT_SAME_ORIGIN_CHECK_NONE 0 //no check
> +#define AVFMT_SAME_ORIGIN_CHECK_HOST 1 //protocol, host, auth, port
> +#define AVFMT_SAME_ORIGIN_CHECK_PATH 2 //protocol, host, auth, port, parent
> path
Shouldn't these be flags instead?
--
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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v4 1/6] ccfifo: Properly handle CEA-708 captions through framerate conversion
Quoting Devin Heitmueller (2023-04-28 18:37:46)
> +void ff_ccfifo_freep(AVCCFifo **ccf)
> +{
> +if (ccf && *ccf) {
Don't check for ccf, it makes no sense to call this function with
ccf==NULL, so silently ignoring it can hide bugs.
> +AVCCFifo *tmp = *ccf;
> +if (tmp->cc_608_fifo)
> +av_fifo_freep2(&tmp->cc_608_fifo);
Useless checks, av_fifo_freep2 can be called on &NULL.
> +if (tmp->cc_708_fifo)
> +av_fifo_freep2(&tmp->cc_708_fifo);
> +av_freep(*ccf);
> +}
> +}
> +
> +AVCCFifo *ff_ccfifo_alloc(AVRational *framerate, void *log_ctx)
We typically pass AVRational directly, not as pointers. That also makes
it clear that the passed value is not modified.
> +{
> +AVCCFifo *ccf;
> +int i;
> +
> +ccf = av_mallocz(sizeof(*ccf));
> +if (!ccf)
> +return NULL;
> +
> +if (!(ccf->cc_708_fifo = av_fifo_alloc2(MAX_CC_ELEMENTS,
> CC_BYTES_PER_ENTRY, 0)))
> +goto error;
> +
> +if (!(ccf->cc_608_fifo = av_fifo_alloc2(MAX_CC_ELEMENTS,
> CC_BYTES_PER_ENTRY, 0)))
> +goto error;
> +
> +/* Based on the target FPS, figure out the expected cc_count and number
> of
> + 608 tuples per packet. See ANSI/CTA-708-E Sec 4.3.6.1. */
> +for (i = 0; i < FF_ARRAY_ELEMS(cc_lookup_vals); i++) {
> +if (framerate->num == cc_lookup_vals[i].num &&
> +framerate->den == cc_lookup_vals[i].den) {
> +ccf->expected_cc_count = cc_lookup_vals[i].cc_count;
> +ccf->expected_608 = cc_lookup_vals[i].num_608;
> +break;
> +}
> +}
> +
> +if (ccf->expected_608 == 0) {
> +/* We didn't find an output frame we support. We'll let the call
> succeed
> + and the FIFO to be allocated, but the extract/inject functions
> will simply
> + leave everything the way it is */
> +av_log(ccf->log_ctx, AV_LOG_WARNING, "cc_fifo cannot transcode
> captions fps=%d/%d\n",
> + framerate->num, framerate->den);
Won't this result in spurious warnings for users who are just converting
framerates and don't have any captions. If so it should probably be
printed the first time we actually see caption side data.
> +ccf->passthrough = 1;
> +}
> +
> +return ccf;
> +
> +error:
> +ff_ccfifo_freep(&ccf);
> +return NULL;
> +}
> +
> +int ff_ccfifo_injectbytes(AVCCFifo *ccf, uint8_t **data, size_t *len)
> +{
> +char *cc_data;
> +int cc_filled = 0;
> +int i;
> +
> +if (!ccf)
> +return AVERROR(EINVAL);
For all the extract/inject functions: it should be invalid to call them
with a NULL context, so this should be an av_assert0() or not be here at
all.
> +
> +if (ccf->passthrough) {
> +*data = NULL;
> +*len = 0;
> +return 0;
> +}
> +
> +cc_data = av_mallocz(ccf->expected_cc_count * CC_BYTES_PER_ENTRY);
This buffer size is constant for a given AVCCFifo object, so perhaps
ff_ccfifo_alloc() could return required buffer size and this function
could write into a user-provided buffer and avoid constant dynamic
allocation.
> +if (!cc_data) {
> +return AVERROR(ENOMEM);
> +}
> +
> +for (i = 0; i < ccf->expected_608; i++) {
> +if (av_fifo_can_read(ccf->cc_608_fifo) >= CC_BYTES_PER_ENTRY) {
> +av_fifo_read(ccf->cc_608_fifo, &cc_data[cc_filled *
> CC_BYTES_PER_ENTRY],
> + CC_BYTES_PER_ENTRY);
This looks wrong, as fifo operations are in elements, and your
elements are already CC_BYTES_PER_ENTRY. So every read actually writes
CC_BYTES_PER_ENTRY * CC_BYTES_PER_ENTRY bytes to cc_data.
I think you can also do this as a single av_fifo_read() call without a
loop.
--
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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v4 6/6] decklink_enc: add support for playout of 608 captions in MOV files
On Tue, May 2, 2023 at 10:48 PM Devin Heitmueller < devin.heitmuel...@ltnglobal.com> wrote: > Hi Lance, > > On Sun, Apr 30, 2023 at 7:01 PM Lance Wang wrote: > > This implementation is limited to decklink SDI output only, If > possible, > > can we implement the function from demuxer layer, and then passthrough > > by SEI side data? By this way, we can convert such stream in streaming > > to embedded CC to video stream easily also. > > I did consider this approach, and it does raise the more fundamental > issue about trying to minimize the number of ways we have to process > CC data depending on whether it originated in SEI metadata or in > separate packets. There are a number of problems with what you are > proposing though: > > 1. There could be multiple CC streams within an MOV file but only a > single CC stream can be embedded into AVFrame side data. Hence you > would have to specify some sort of argument to the demux to decide > which stream to embed. This makes it much more difficult to do things > like ingest a stream with multiple CC streams and have separate > outputs with different CC streams. Performing the work on the output > side allows you to use the standard "-map" mechanism to dictate which > CC streams are routed to which outputs, and to deliver the content to > different outputs with different CC streams. > > 2. I have use cases in mind where the captions originate from sources > other than MOV files, where the video framerate is not known (or there > is no video at all in the source). For example, I want to be able to > consume video from a TS source while simultaneously demuxing an SCC or > MCC file and sending the result in the output. In such cases the > correct rate control for the captions can only be implemented on the > output side, since in such cases the SCC/MCC demux doesn't have access > to the corresponding video stream (it won't know the video framerate, > nor is it able to embed the captions into the AVFrame side data). > > I can indeed imagine there are use cases where doing it further up the > pipeline could be useful. For example, if you were taking in an MOV > file and wanting to produce a TS where the captions need to be > embedded as SEI metadata (hence you would need the e608 packets > converted to AVFrame side data prior to reaching the encoder). > However I don't see this as a substitute for being able to do it on > the output side when that is the most flexible approach for those > other use cases described above. > Much of this comes down to the fundamental limitations of the ffmpeg > framework related to being able to move data back/forth between data > packets and side data. You can't feed data packets into > AVFilterGraphs. You can't easily combine data from data packets into > AVFrames carrying video (or extract side data from AVFrames to > generate data packets), etc. You can't use BSF filters to combine > data from multiple inputs such as compressed video streams and data > streams after encoding. I've run across all these limitations over > the years, and at this point I'm trying to take the least invasive > approach possible that doesn't require changes to the fundamental > frameworks for handling data packets. > It's worth noting that nothing you have suggested is an "either/or" > situation. Because caption processing is inexpensive, there isn't any > significant overhead in having multiple AvCCFifo instances in the > pipeline. In other words, if you added a feature to the MOV demuxer, > it wouldn't prevent us from running the packets through an AvCCFifo > instance on the output side. The patch proposed doesn't preclude you > adding such a feature on the demux side in the future. > > Thanks for the answer. It's acceptable from my side. Devin > > -- > Devin Heitmueller, Senior Software Engineer > LTN Global Communications > o: +1 (301) 363-1001 > w: https://ltnglobal.com e: devin.heitmuel...@ltnglobal.com > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".
