Re: [FFmpeg-devel] [PATCH] avcodec/rscc: Avoid returning frames that have nearly no undamaged pixels in them
On 17/01/2019 23:33, Michael Niedermayer wrote: > Would you be ok with rejecting RSCC files without a keyframe ? > or more precissely all frames before a keyframe and thus if there is > no keyframe the whole file > (that would be a superset of what this patch rejects) This, to me, soundsp preferable, if hidden under ER being disable or something. > > I suggested that arbitrary check above as it would allow decoding more data > in case of a damaged or lost keyframe [...] > The gain these changes have is they increase the cost for an > attacker in a DOS attack. (they also improve the efficiency of the fuzzer but > thats secondary) By this logic, every single optimization is an anti-DoS measure and can be argued ad nauseam in favor of every such changed, regardless of cost. > As is you can have a file with huge resolution where each frame only encodes > 1 pixel in about a dozen bytes per frame. That transforms a very low cost of > network bandwidth from an attacker to a rather large computational cost on > anything processing such file. You don't need to explain what a DoS is to us... > Requiring more than 1 valid pixels in a billion or Requiring a valid keyframe > would increase the cost for an attacker. This is not codec specific, either. > Also do you see a cleaner way to achive this as the author of this > decoder ? People are disagreeing with the premise, not the implementation. - Derek ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/proresdec2: only set avctx->color* when values are specified
On Thu, Jan 17, 2019 at 10:57 PM Neil Birkbeck wrote: > On Thu, Jan 17, 2019 at 7:43 PM Neil Birkbeck > wrote: > > > This allows preservation of color values set from the container, > > while still letting the bitstream take precedent when its values > > are specified to some actual value (e.g., not *UNSPECIFIED). > > > > Signed-off-by: Neil Birkbeck > > --- > > libavcodec/proresdec2.c | 9 ++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/libavcodec/proresdec2.c b/libavcodec/proresdec2.c > > index 6209c229c9..662ca7d48b 100644 > > --- a/libavcodec/proresdec2.c > > +++ b/libavcodec/proresdec2.c > > @@ -262,9 +262,12 @@ static int decode_frame_header(ProresContext *ctx, > > const uint8_t *buf, > > } > > } > > > > -avctx->color_primaries = buf[14]; > > -avctx->color_trc = buf[15]; > > -avctx->colorspace = buf[16]; > > +if (buf[14] != AVCOL_PRI_UNSPECIFIED) > > +avctx->color_primaries = buf[14]; > > +if (buf[15] != AVCOL_TRC_UNSPECIFIED) > > +avctx->color_trc = buf[15]; > > +if (buf[16] != AVCOL_SPC_UNSPECIFIED) > > +avctx->colorspace = buf[16]; > > avctx->color_range = AVCOL_RANGE_MPEG; > > > > ptr = buf + 20; > > -- > > 2.20.1.321.g9e740568ce-goog > > > > > To add a bit more context. The patch is a fix for the case when prores > bitstream code points are explicitly set to unspecified and are overriding > what may have been in the container (unlike h264/h265 where such values can > behind the color_description flag, these fields always must be present in > the prores header). Of course, ideally these should be the same. > > We noticed this inconsistency on some HDR content, as prior to > > https://github.com/FFmpeg/FFmpeg/commit/81d78fe13bc1fd94845c002f3439fe44d9e9e0d9 > the color information in the prores bitstream (which may have been > unspecified) was simply ignored. > > In practice, I guess some workflows may not have known about the new code > points and put unspecified in the bitstream (or worse where some headers > will contain non-HDR code points). > > The patch seemed like a situation where we could resolve the inconsistency > in favor of the container (given my understanding, and from what I see in > the code, I'm assuming the codec is typically given preference). But I'm > interested in hearing ffmpeg-devel's opinions on whether this is most > consistent (actually, for the HDR files that I've seen, the container is > correct--although I'm sure there are cases where the opposite is true). > > I guess the most closely related discussion about codecs overriding these > types of values is here > https://patchwork.ffmpeg.org/patch/11279/ > but this case seemed a bit different. > This makes a lot of sense, but it should be part of the commit message instead of the attached thread. So ok with me. -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 5/5] lavf/tls: enable server verification by default when not on mbedtls
2019-01-18 9:46 GMT+01:00, Rodger Combs : > All other TLS wrappers now have a mechanism to load a system trust store > by default, without setting the cafile option. For Secure Transport and > Secure Channel, it's the OS. For OpenSSL and libtls, it's a path set at > compile-time. For GNUTLS, it's either a path set at compile-time, or the > OS trust store (if on macOS, iOS, or Windows). It's possible to configure > OpenSSL, GNUTLS, and libtls without a working trust store, but these are > broken configurations and I don't have a problem with requiring users with > that kind of install to either fix it, or explicitly opt in to insecure > behavior. mbedtls doesn't have a default trust store (it's assumed that the > application will provide one), so it continues to require the user to pass > in a path and enable verification manually. I believe the current behaviour is more desirable as default for a multimedia library. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] tools/target_dec_fate.sh: print some statistics
2019-01-17 22:21 GMT+01:00, Michael Niedermayer : > On Tue, Jan 15, 2019 at 11:41:22PM +0100, Carl Eugen Hoyos wrote: >> 2019-01-15 23:36 GMT+01:00, Michael Niedermayer : >> > On Fri, Jan 04, 2019 at 08:29:36PM +0100, Michael Niedermayer wrote: >> >> On Sat, Dec 29, 2018 at 10:47:16AM +0100, Moritz Barsnick wrote: >> >> > On Sat, Dec 29, 2018 at 02:35:18 +0100, Michael Niedermayer wrote: >> >> > > +CACHED=$((CACHED+1)) >> >> > >> >> > I believe this is the sort of math that won't work on old, non-POSIX >> >> > Bourne shells. (I'm thinking Solaris /bin/sh here.) In case that >> >> > even matters. >> >> >> >> which platform without a posix /bin/sh supports the libfuzz stuff ? >> > >> > ping, is there any suggestion for improving this patch ? >> > or are all concerns resolved ? >> >> I believe adding more things that will not work on sunos is not >> ideal. >> But if there is no other way solving the issues that this patch >> fixes, sunos is of course no show-stopper. > > how can i test easily that a change work with such a non posix shell ? That is exactly what I don't know: Even on sunos it appears to depend on settings that I don't understand. Sorry, Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/libx265: add support for ROI-based encoding
On 18/01/2019 15:53, Guo, Yejun wrote:
[...]
> +static av_cold int libx265_encode_set_roi(libx265Context *ctx, const AVFrame
> *frame, x265_picture* pic)
> +{
> +// From x265.h:
> +/* An array of quantizer offsets to be applied to this image during
> encoding.
> +* These are added on top of the decisions made by rateControl.
> +* Adaptive quantization must be enabled to use this feature. These
> quantizer
> +* offsets should be given for each 16x16 block (8x8 block, when qg-size
> is 8).
> +* Behavior if quant offsets differ between encoding passes is undefined.
> +*/
I don't think you need to copy the API documentation from x265.
> +// 25 is a number that I think it is a possible proper scale
> value.
> +qoffset = qoffset * 25;
Is this based off anything?
> +
> +if (x265pic.quantOffsets) {
> +av_free(x265pic.quantOffsets);
> +x265pic.quantOffsets = NULL;
> +}
av_freep();
- Derek
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/proresdec2: only set avctx->color* when values are specified
On Fri, Jan 18, 2019 at 6:43 AM Carl Eugen Hoyos wrote: > 2019-01-18 4:48 GMT+01:00, Neil Birkbeck : > > On Thu, Jan 17, 2019 at 7:43 PM Neil Birkbeck > > wrote: > > > >> This allows preservation of color values set from the container, > >> while still letting the bitstream take precedent when its values > >> are specified to some actual value (e.g., not *UNSPECIFIED). > >> > >> Signed-off-by: Neil Birkbeck > >> --- > >> libavcodec/proresdec2.c | 9 ++--- > >> 1 file changed, 6 insertions(+), 3 deletions(-) > >> > >> diff --git a/libavcodec/proresdec2.c b/libavcodec/proresdec2.c > >> index 6209c229c9..662ca7d48b 100644 > >> --- a/libavcodec/proresdec2.c > >> +++ b/libavcodec/proresdec2.c > >> @@ -262,9 +262,12 @@ static int decode_frame_header(ProresContext *ctx, > >> const uint8_t *buf, > >> } > >> } > >> > >> -avctx->color_primaries = buf[14]; > >> -avctx->color_trc = buf[15]; > >> -avctx->colorspace = buf[16]; > >> +if (buf[14] != AVCOL_PRI_UNSPECIFIED) > >> +avctx->color_primaries = buf[14]; > >> +if (buf[15] != AVCOL_TRC_UNSPECIFIED) > >> +avctx->color_trc = buf[15]; > >> +if (buf[16] != AVCOL_SPC_UNSPECIFIED) > >> +avctx->colorspace = buf[16]; > >> avctx->color_range = AVCOL_RANGE_MPEG; > >> > >> ptr = buf + 20; > >> -- > >> 2.20.1.321.g9e740568ce-goog > >> > >> > > To add a bit more context. The patch is a fix for the case when prores > > bitstream code points are explicitly set to unspecified and are > overriding > > what may have been in the container (unlike h264/h265 where such values > can > > behind the color_description flag, these fields always must be present in > > the prores header). > > Isn't this even more of a reason to prefer the container value over > the bitstream value? > Absolutely not -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/rscc: Avoid returning frames that have nearly no undamaged pixels in them
On Thu, Jan 17, 2019 at 6:34 PM Michael Niedermayer
wrote:
> On Wed, Jan 16, 2019 at 09:05:18PM -0500, Vittorio Giovara wrote:
> > On Wed, Jan 16, 2019 at 7:44 PM Michael Niedermayer
>
> > wrote:
> >
> > > Fixes: Timeout
> > > Fixes:
> > >
> 12192/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_RSCC_fuzzer-6279038004363264
> > >
> > > Before:
> > >
> clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_RSCC_fuzzer-6279038004363264
> > > in 15423 ms
> > > After:
> > >
> clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_RSCC_fuzzer-6279038004363264
> > > in 190 ms
> > >
> > > Found-by: continuous fuzzing process
> > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > Signed-off-by
> > > <
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpegSigned-off-by
> >:
> > > Michael Niedermayer
> > > ---
> > > libavcodec/rscc.c | 7 ++-
> > > 1 file changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/libavcodec/rscc.c b/libavcodec/rscc.c
> > > index 7921f149ed..fa066afd7f 100644
> > > --- a/libavcodec/rscc.c
> > > +++ b/libavcodec/rscc.c
> > > @@ -64,6 +64,7 @@ typedef struct RsccContext {
> > > /* zlib interaction */
> > > uint8_t *inflated_buf;
> > > uLongf inflated_size;
> > > +int valid_pixels
> > > } RsccContext;
> > >
> > > static av_cold int rscc_init(AVCodecContext *avctx)
> > > @@ -348,7 +349,11 @@ static int rscc_decode_frame(AVCodecContext
> *avctx,
> > > void *data,
> > > memcpy (frame->data[1], ctx->palette, AVPALETTE_SIZE);
> > > }
> > >
> > > -*got_frame = 1;
> > > +// We only return a picture when more than 5% is undameged, this
> > > avoids copying nearly broken frames around
> > > +if (ctx->valid_pixels < ctx->inflated_size)
> > > +ctx->valid_pixels += pixel_size;
> > > +if (ctx->valid_pixels >= ctx->inflated_size/20)
> > >
> >
> > this feels arbitrary and hackish, for very little gain, IMO
>
> Would you be ok with rejecting RSCC files without a keyframe ?
> or more precissely all frames before a keyframe and thus if there is
> no keyframe the whole file
> (that would be a superset of what this patch rejects)
>
If that could be achieved in a clean way without codec-specific additions,
sure why not. It could maybe exploit the AV_FRAME_FLAG_CORRUPT flag and
some other combination of codec flag to expose this feature. However I'm
still adamant at "fixing" issues in the code found by external code tools
in this way: at most it is the fuzzer that should be better instrumented to
accept longer or no timeouts for this codec.
--
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH]lavu/frame: Try to improve the documentation wording
Hi!
Attached patch tries to improve the ROI documentation wording, C
structs cannot return errors.
Please comment, Carl Eugen
From 4abd545e7ab463c97bf816b270544eee25c4755b Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos
Date: Fri, 18 Jan 2019 12:35:44 +0100
Subject: [PATCH] lavu/frame: Try to improve the documentation wording.
C structs cannot return errors.
---
libavutil/frame.h |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libavutil/frame.h b/libavutil/frame.h
index 8aa3e88..1990320 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -218,8 +218,8 @@ typedef struct AVFrameSideData {
* if the codec requires an alignment.
* If the regions overlap, the last value in the list will be used.
*
- * qoffset is quant offset, and base rule here:
- * returns EINVAL if AVRational.den is zero.
+ * qoffset is quantization offset:
+ * Encoders will return EINVAL if qoffset.den is zero.
* the value (num/den) range is [-1.0, 1.0], clamp to +-1.0 if out of range.
* 0 means no picture quality change,
* negative offset asks for better quality (and the best with value -1.0),
--
1.7.10.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/rscc: Avoid returning frames that have nearly no undamaged pixels in them
On 18/01/2019 11:46, Carl Eugen Hoyos wrote: > No, you are completely missing the point. I am not. I fully understand the argument in favour of these, I just don't agree. > Possible security issues in this decoder will only be > searched (and therefore found) if the decoder doesn't > timeout quickly on damaged files. I am aware, and I disagree with the premise of dumping all over the code and its complexity/readability in order to make a particular fuzzer happy, so we can be 100% sure it won't miss an issue. To that end, I've opened a bug with oss-fuzz for some guidance: https://github.com/google/oss-fuzz/issues/2095 > I assume this is the result of a (simple) cost-benefit- > analysis by the people running the fuzzing systems. Yes, the cost of them running the tests, not dev/complexity costs for downstream. > Nobody asks you to fix the issues, blocking them is an > interesting concept security-wise. It makes plenty of code horrible and unnecessarily complex, so you cannot simply argue "well you're not the one fixing them so bugger off". - Derek ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/rscc: Avoid returning frames that have nearly no undamaged pixels in them
2019-01-17 22:58 GMT+01:00, Derek Buitenhuis : > On 17/01/2019 03:06, Carl Eugen Hoyos wrote: >> You mean searching for security issues makes no sense? > > This isn't a security and it isn't a fix. It's a completely > arbitrary statistic to make an arbitrary program happy. No, you are completely missing the point. Possible security issues in this decoder will only be searched (and therefore found) if the decoder doesn't timeout quickly on damaged files. I assume this is the result of a (simple) cost-benefit- analysis by the people running the fuzzing systems. Nobody asks you to fix the issues, blocking them is an interesting concept security-wise. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/proresdec2: only set avctx->color* when values are specified
2019-01-18 4:48 GMT+01:00, Neil Birkbeck : > On Thu, Jan 17, 2019 at 7:43 PM Neil Birkbeck > wrote: > >> This allows preservation of color values set from the container, >> while still letting the bitstream take precedent when its values >> are specified to some actual value (e.g., not *UNSPECIFIED). >> >> Signed-off-by: Neil Birkbeck >> --- >> libavcodec/proresdec2.c | 9 ++--- >> 1 file changed, 6 insertions(+), 3 deletions(-) >> >> diff --git a/libavcodec/proresdec2.c b/libavcodec/proresdec2.c >> index 6209c229c9..662ca7d48b 100644 >> --- a/libavcodec/proresdec2.c >> +++ b/libavcodec/proresdec2.c >> @@ -262,9 +262,12 @@ static int decode_frame_header(ProresContext *ctx, >> const uint8_t *buf, >> } >> } >> >> -avctx->color_primaries = buf[14]; >> -avctx->color_trc = buf[15]; >> -avctx->colorspace = buf[16]; >> +if (buf[14] != AVCOL_PRI_UNSPECIFIED) >> +avctx->color_primaries = buf[14]; >> +if (buf[15] != AVCOL_TRC_UNSPECIFIED) >> +avctx->color_trc = buf[15]; >> +if (buf[16] != AVCOL_SPC_UNSPECIFIED) >> +avctx->colorspace = buf[16]; >> avctx->color_range = AVCOL_RANGE_MPEG; >> >> ptr = buf + 20; >> -- >> 2.20.1.321.g9e740568ce-goog >> >> > To add a bit more context. The patch is a fix for the case when prores > bitstream code points are explicitly set to unspecified and are overriding > what may have been in the container (unlike h264/h265 where such values can > behind the color_description flag, these fields always must be present in > the prores header). Isn't this even more of a reason to prefer the container value over the bitstream value? Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCHv2 2/6] avcodec/vp3dsp: add 12 pixel loop filter functions
On Tue, Jan 15, 2019 at 12:18:44AM +0100, Hendrik Leppkes wrote:
> On Mon, Jan 14, 2019 at 9:48 PM Peter Ross wrote:
> > diff --git a/libavcodec/vp3dsp.h b/libavcodec/vp3dsp.h
> > index f55a7f834f..30a76100d9 100644
> > --- a/libavcodec/vp3dsp.h
> > +++ b/libavcodec/vp3dsp.h
> > @@ -43,8 +43,13 @@ typedef struct VP3DSPContext {
> > void (*idct_dc_add)(uint8_t *dest, ptrdiff_t stride, int16_t *block);
> > void (*v_loop_filter)(uint8_t *src, ptrdiff_t stride, int
> > *bounding_values);
> > void (*h_loop_filter)(uint8_t *src, ptrdiff_t stride, int
> > *bounding_values);
> > +void (*v_loop_filter_12)(uint8_t *src, ptrdiff_t stride, int
> > *bounding_values);
> > +void (*h_loop_filter_12)(uint8_t *src, ptrdiff_t stride, int
> > *bounding_values);
> > } VP3DSPContext;
> >
> > +void ff_vp3dsp_v_loop_filter_12(uint8_t *first_pixel, ptrdiff_t stride,
> > int *bounding_values);
> > +void ff_vp3dsp_h_loop_filter_12(uint8_t *first_pixel, ptrdiff_t stride,
> > int *bounding_values);
> > +
>
> It seems weird to have DSP member methods for the 12 pixel loop
> filter, but then never assign or use them. Is that a remnant from an
> earlier approach?
Yeah. I will remove those unused context function pointers.
Thanks for noticing this.
-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] [RFC] channel_layout: add support for ambisonics
On Thu, Nov 29, 2018 at 1:55 PM Marton Balint wrote:
>
>
> On Thu, 29 Nov 2018, Rostislav Pehlivanov wrote:
>
> > This is an RFC to add support for tagging channel layouts as ambisonics
> > in a backward-compatible way.
> > For now ambisonics up to third order are supported.
> > The functions have been updated to support and propagate the
> > AV_CH_LAYOUT_AMBISONICS flag.
> > This is messy but does not require a new API for layouts. Perhaps the
> > new proposed API might be a better solution, comments are welcome.
> > ---
> > doc/APIchanges | 4 ++
> > libavutil/channel_layout.c | 85 +-
> > libavutil/channel_layout.h | 19 -
> > libavutil/version.h| 4 +-
> > 4 files changed, 72 insertions(+), 40 deletions(-)
>
> Using separate channel_layout bits for each (up to 16) ambisonic channel
> seems a lot cleaner.
>
+1 for separate bits, e.g.
static const struct channel_name channel_names[] = {
...
+[36] = { "ACN_0", "ambisonics component 0" },
+[37] = { "ACN_1", "ambisonics component 1" },
as it would also enable signalling ambisonics with an additional
head-locked stereo track.
What do you think of maintaining ACN order throughout as {0, 1, 2, 3, 4, 5
...} instead of {1, 2, 0, 3, 4, 5 ...}?
Thanks,
Felicia
> Regards,
> Marton
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 5/5] lavf/tls: enable server verification by default when not on mbedtls
> On Jan 18, 2019, at 05:41, Carl Eugen Hoyos wrote: > > 2019-01-18 9:46 GMT+01:00, Rodger Combs : >> All other TLS wrappers now have a mechanism to load a system trust store >> by default, without setting the cafile option. For Secure Transport and >> Secure Channel, it's the OS. For OpenSSL and libtls, it's a path set at >> compile-time. For GNUTLS, it's either a path set at compile-time, or the >> OS trust store (if on macOS, iOS, or Windows). It's possible to configure >> OpenSSL, GNUTLS, and libtls without a working trust store, but these are >> broken configurations and I don't have a problem with requiring users with >> that kind of install to either fix it, or explicitly opt in to insecure >> behavior. mbedtls doesn't have a default trust store (it's assumed that the >> application will provide one), so it continues to require the user to pass >> in a path and enable verification manually. > > I believe the current behaviour is more desirable as default for a multimedia > library. That's patent nonsense. Requests for media are as likely to contain authentication tokens as anything else today, and users have as much of a right to privacy in the specific media they consume as in anything else they use online. Without any verification of peer certificates, our current defaults are little better than using plaintext. > > Carl Eugen > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/proresdec2: only set avctx->color* when values are specified
On Fri, Jan 18, 2019 at 7:48 AM Vittorio Giovara wrote: > On Fri, Jan 18, 2019 at 6:43 AM Carl Eugen Hoyos > wrote: > > > 2019-01-18 4:48 GMT+01:00, Neil Birkbeck : > > > On Thu, Jan 17, 2019 at 7:43 PM Neil Birkbeck > > > > wrote: > > > > > >> This allows preservation of color values set from the container, > > >> while still letting the bitstream take precedent when its values > > >> are specified to some actual value (e.g., not *UNSPECIFIED). > > >> > > >> Signed-off-by: Neil Birkbeck > > >> --- > > >> libavcodec/proresdec2.c | 9 ++--- > > >> 1 file changed, 6 insertions(+), 3 deletions(-) > > >> > > >> diff --git a/libavcodec/proresdec2.c b/libavcodec/proresdec2.c > > >> index 6209c229c9..662ca7d48b 100644 > > >> --- a/libavcodec/proresdec2.c > > >> +++ b/libavcodec/proresdec2.c > > >> @@ -262,9 +262,12 @@ static int decode_frame_header(ProresContext > *ctx, > > >> const uint8_t *buf, > > >> } > > >> } > > >> > > >> -avctx->color_primaries = buf[14]; > > >> -avctx->color_trc = buf[15]; > > >> -avctx->colorspace = buf[16]; > > >> +if (buf[14] != AVCOL_PRI_UNSPECIFIED) > > >> +avctx->color_primaries = buf[14]; > > >> +if (buf[15] != AVCOL_TRC_UNSPECIFIED) > > >> +avctx->color_trc = buf[15]; > > >> +if (buf[16] != AVCOL_SPC_UNSPECIFIED) > > >> +avctx->colorspace = buf[16]; > > >> avctx->color_range = AVCOL_RANGE_MPEG; > > >> > > >> ptr = buf + 20; > > >> -- > > >> 2.20.1.321.g9e740568ce-goog > > >> > > >> > > > To add a bit more context. The patch is a fix for the case when prores > > > bitstream code points are explicitly set to unspecified and are > > overriding > > > what may have been in the container (unlike h264/h265 where such values > > can > > > behind the color_description flag, these fields always must be present > in > > > the prores header). > > > > Isn't this even more of a reason to prefer the container value over > > the bitstream value? > > > > Absolutely not > I was initially going to check and see if preferring the container (when set) was a valid alternative, since that was what the old behavior was. But I couldn't find precedent for that elsewhere in the code (except when the codec was just assuming some fixed constant not specified in the bitstream). If a library user wants to prefer/detect container metadata, I take it they have to avformat_open_input, inspect metadata before find_stream_info, and use their application-specific logic to do add any bitstream filters or adjust parameters of the filter graph? For posterity, there are a couple more slightly relevant links I found when looking for an authoritative reference on such inconsistencies in prores. I only found [1] that confirms the tooling/workflows were inconsistent at some point, and [2] which suggested that the colr atom took precedence over headers on whatever viewer they were talking about (presumably quicktime, on mac). [1] https://github.com/bbc/qtff-parameter-editor [2] https://www.drastic.tv/support-59/supporttipstechnical/202-prores-colour-shifts-in-post-production From ab77fa3035bace9dfff3b9f83e623e823cac80cf Mon Sep 17 00:00:00 2001 From: Neil Birkbeck Date: Tue, 15 Jan 2019 17:19:44 -0800 Subject: [PATCH] avcodec/proresdec2: only set avctx->color* when values are specified This allows preservation of color values set from the container, while still letting the bitstream take precedent when its values are specified to some actual value (e.g., not *UNSPECIFIED). The patch is a fix for the case when prores bitstream code points are explicitly set to unspecified and are overriding what may have been in the container (unlike h264/h265 where such values can behind the color_description flag, these fields always must be present in the prores header). Of course, ideally these should be the same. We noticed this inconsistency on some HDR content, as prior to https://github.com/FFmpeg/FFmpeg/commit/81d78fe13bc1fd94845c002f3439fe44d9e9e0d9 the color information in the prores bitstream (which may have been unspecified) was simply ignored. In practice, I guess some workflows may not have known about the new code points and put unspecified in the bitstream (or worse where some headers will contain non-HDR code points). A bit more info on that topic related to HDR workflows here: https://github.com/bbc/qtff-parameter-editor The patch seemed like a situation where we could resolve the inconsistency in favor of the container (given my understanding, and from what I see in the code, I'm assuming the codec is typically given preference). Signed-off-by: Neil Birkbeck --- libavcodec/proresdec2.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/libavcodec/proresdec2.c b/libavcodec/proresdec2.c index 6209c229c9..662ca7d48b 100644 --- a/libavcodec/proresdec2.c +++ b/libavcodec/proresdec2.c @@ -262,9 +262,12 @@ static int decode_frame_header(ProresContext *ctx, const uint8_t *buf,
[FFmpeg-devel] [PATCH] tools/target_dec_fate.list: add entries upto 1214
Signed-off-by: Michael Niedermayer --- tools/target_dec_fate.list | 63 ++ 1 file changed, 63 insertions(+) diff --git a/tools/target_dec_fate.list b/tools/target_dec_fate.list index f6b56261bb..b20137bfef 100644 --- a/tools/target_dec_fate.list +++ b/tools/target_dec_fate.list @@ -61,6 +61,69 @@ 722/clusterfuzz-testcase-5711268868521984 target_dec_dca_fuzzer 723/clusterfuzz-testcase-6471394663596032 target_dec_wavpack_fuzzer 724/clusterfuzz-testcase-6738249571631104 target_dec_pictor_fuzzer +729/clusterfuzz-testcase-5154831595470848 target_dec_wavpack_fuzzer +730/clusterfuzz-testcase-5265113739165696 target_dec_vp8_fuzzer +731/clusterfuzz-testcase-5391628980191232 target_dec_flac_fuzzer +732/clusterfuzz-testcase-4872990070145024 target_dec_dca_fuzzer +733/clusterfuzz-testcase-4682158096515072 target_dec_mjpeg_fuzzer +734/clusterfuzz-testcase-4821293192970240 target_dec_h264_fuzzer +736/clusterfuzz-testcase-5580263943831552 target_dec_mpeg4_fuzzer +741/clusterfuzz-testcase-5869962004529152 target_dec_movtext_fuzzer +755/clusterfuzz-testcase-5369072516595712 target_dec_h264_fuzzer +758/clusterfuzz-testcase-4720832028868608 target_dec_vp5_fuzzer +761/clusterfuzz-testcase-544252097536 target_dec_wavpack_fuzzer +762/clusterfuzz-testcase-5927683747741696 target_dec_dca_fuzzer +763/clusterfuzz-testcase-6007567320875008 target_dec_amrwb_fuzzer +764/clusterfuzz-testcase-6273034652483584 target_dec_mpeg2video_fuzzer +766/clusterfuzz-testcase-4603047080624128 target_dec_vp6_fuzzer +767/clusterfuzz-testcase-6743603416137728 target_dec_vp6f_fuzzer +768/clusterfuzz-testcase-4807444305805312 target_dec_rv40_fuzzer +772/clusterfuzz-testcase-5453962780082176 target_dec_h264_fuzzer +773/clusterfuzz-testcase-6362160458366976 target_dec_vp6f_fuzzer +779/clusterfuzz-testcase-5568669545398272 target_dec_mpeg4_fuzzer +780/clusterfuzz-testcase-6393552642768896 target_dec_gif_fuzzer +807/clusterfuzz-testcase-6470061042696192 target_dec_vp6f_fuzzer +808/clusterfuzz-testcase-4715513349406720 target_dec_wavpack_fuzzer +809/clusterfuzz-testcase-6172687908995072 target_dec_vp6f_fuzzer +810/clusterfuzz-testcase-5249282825256960 target_dec_targa_fuzzer +811/clusterfuzz-testcase-6465493076541440 target_dec_mjpeg_fuzzer +822/clusterfuzz-testcase-4873433189974016 target_dec_wavpack_fuzzer +823/clusterfuzz-testcase-6727060074528768 target_dec_pictor_fuzzer +826/clusterfuzz-testcase-5316921379520512 target_dec_tiff_fuzzer +830/clusterfuzz-testcase-6253175327686656 target_dec_mp2_fuzzer +839/clusterfuzz-testcase-4871084446842880 target_dec_wavpack_fuzzer +842/clusterfuzz-testcase-6361547318231040 target_dec_tiff_fuzzer +847/clusterfuzz-testcase-5291877358108672 target_dec_vp5_fuzzer +848/clusterfuzz-testcase-5432155620507648 target_dec_vp6f_fuzzer +850/clusterfuzz-testcase-5721296509861888 target_dec_vp6f_fuzzer
[FFmpeg-devel] [PATCH] avformat/hlsenc: fix fmp4_init_filename file name with %v problem
when set option fmp4_init_filename to init_%v.mp4
before patch:
the init file will be init_%v_0.mp4, init_%v_1.mp4
after patch:
the init file will be init_0.mp4, init_1.mp4
Reported-By: Gyan Doshi
Signed-off-by: Steven Liu
---
libavformat/hlsenc.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index ca57694e9e..d98d52a332 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -2715,7 +2715,11 @@ static int hls_init(AVFormatContext *s)
av_strlcpy(vs->fmp4_init_filename, hls->fmp4_init_filename,
fmp4_init_filename_len);
if (hls->nb_varstreams > 1) {
-ret = append_postfix(vs->fmp4_init_filename,
fmp4_init_filename_len, i);
+if (av_stristr(vs->fmp4_init_filename, "%v")) {
+format_name(vs->fmp4_init_filename,
fmp4_init_filename_len, i);
+} else {
+ret = append_postfix(vs->fmp4_init_filename,
fmp4_init_filename_len, i);
+}
if (ret < 0)
goto fail;
}
--
2.15.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/2] avcodec/rscc: Avoid returning frames that have nearly no undamaged pixels in them
Fixes: Timeout
Fixes:
12192/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_RSCC_fuzzer-6279038004363264
Before:
clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_RSCC_fuzzer-6279038004363264
in 15423 ms
After:
clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_RSCC_fuzzer-6279038004363264
in 190 ms
Found-by: continuous fuzzing process
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer
---
libavcodec/rscc.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/libavcodec/rscc.c b/libavcodec/rscc.c
index 7921f149ed..3868c1cb1b 100644
--- a/libavcodec/rscc.c
+++ b/libavcodec/rscc.c
@@ -64,6 +64,7 @@ typedef struct RsccContext {
/* zlib interaction */
uint8_t *inflated_buf;
uLongf inflated_size;
+int valid_pixels
} RsccContext;
static av_cold int rscc_init(AVCodecContext *avctx)
@@ -348,7 +349,11 @@ static int rscc_decode_frame(AVCodecContext *avctx, void
*data,
memcpy (frame->data[1], ctx->palette, AVPALETTE_SIZE);
}
-*got_frame = 1;
+// We only return a picture when too little is undameged, this avoids
copying nearly broken frames around
+if (ctx->valid_pixels < ctx->inflated_size)
+ctx->valid_pixels += pixel_size;
+if (ctx->valid_pixels >= ctx->inflated_size * (100 -
avctx->discard_damaged_percentage) / 100)
+*got_frame = 1;
ret = avpkt->size;
end:
--
2.20.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter/extractplanes: add support for 12-bit YUVA formats
On Thu, Jan 17, 2019 at 20:38:44 +0100, Carl Eugen Hoyos wrote: > > Fixes alpha extraction for sample at > > > > https://08488297638989341201.googlegroups.com/attach/85d30cd6b8095/Circle%20Ink%20Bleed_10.mov > > This is a textfile afaict. The correct download link seems to be https://groups.google.com/group/qlab/attach/85d30cd6b8095/Circle%20Ink%20Bleed_10.mov?part=0.1 Moritz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] avutil/imgutils: Optimize writing 4 bytes in memset_bytes()
On Thu, 17 Jan 2019, Michael Niedermayer wrote:
On Wed, Jan 16, 2019 at 08:00:22PM +0100, Marton Balint wrote:
On Tue, 15 Jan 2019, Michael Niedermayer wrote:
On Sun, Dec 30, 2018 at 07:15:49PM +0100, Marton Balint wrote:
On Fri, 28 Dec 2018, Michael Niedermayer wrote:
On Wed, Dec 26, 2018 at 10:16:47PM +0100, Marton Balint wrote:
On Wed, 26 Dec 2018, Paul B Mahol wrote:
On 12/26/18, Michael Niedermayer wrote:
On Wed, Dec 26, 2018 at 04:32:17PM +0100, Paul B Mahol wrote:
On 12/25/18, Michael Niedermayer wrote:
Fixes: Timeout
Fixes:
11502/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920
Before: Executed
clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920
in 11294 ms
After : Executed
clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920
in 4249 ms
Found-by: continuous fuzzing process
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer
---
libavutil/imgutils.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/libavutil/imgutils.c b/libavutil/imgutils.c
index 4938a7ef67..cc38f1e878 100644
--- a/libavutil/imgutils.c
+++ b/libavutil/imgutils.c
@@ -529,6 +529,12 @@ static void memset_bytes(uint8_t *dst, size_t
dst_size,
uint8_t *clear,
}
} else if (clear_size == 4) {
uint32_t val = AV_RN32(clear);
+uint64_t val8 = val * 0x10001ULL;
+for (; dst_size >= 32; dst_size -= 32) {
+AV_WN64(dst , val8); AV_WN64(dst+ 8, val8);
+AV_WN64(dst+16, val8); AV_WN64(dst+24, val8);
+dst += 32;
+}
for (; dst_size >= 4; dst_size -= 4) {
AV_WN32(dst, val);
dst += 4;
--
2.20.1
NAK, implement special memset function instead.
I can move the added loop into a seperate function, if thats what you
suggest ?
No, don't do that.
All the code is already in a "special" memset though, this is
memset_bytes()
I guess function is less useful if its static. So any duplicate should
be avoided in codebase.
Isn't av_memcpy_backptr does almost exactly what is needed here? That can
also be optimized further if needed.
av_memcpy_backptr() copies data with overlap, its more like a recursive
memmove().
So? As far as I see the memset_bytes function in imgutils.c can be replaced
with this:
if (clear_size > dst_size)
clear_size = dst_size;
memcpy(dst, clear, clear_size);
av_memcpy_backptr(dst + clear_size, clear_size, dst_size - clear_size);
I am not against an av_memset_bytes API addition, but I believe it should
share code with av_memcpy_backptr to avoid duplication.
ive implemented this, it does not seem to be really faster in the testcase
I guess it is not faster because you have not applied your original
optimalization to fill32 in libavutil/mem.c. Could you compare speed after
optimizing that the same way your original patch did it with imgutils
memset_bytes?
sure, that makes it faster:
Thanks, both patches LGTM.
Regards,
Marton
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] avcodec/rscc: Avoid returning frames that have nearly no undamaged pixels in them
> On 18 Jan 2019, at 23:00, Michael Niedermayer wrote:
>
> Fixes: Timeout
> Fixes:
> 12192/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_RSCC_fuzzer-6279038004363264
>
> Before:
> clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_RSCC_fuzzer-6279038004363264
> in 15423 ms
> After:
> clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_RSCC_fuzzer-6279038004363264
> in 190 ms
>
> Found-by: continuous fuzzing process
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer
> ---
> libavcodec/rscc.c | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/libavcodec/rscc.c b/libavcodec/rscc.c
> index 7921f149ed..3868c1cb1b 100644
> --- a/libavcodec/rscc.c
> +++ b/libavcodec/rscc.c
> @@ -64,6 +64,7 @@ typedef struct RsccContext {
> /* zlib interaction */
> uint8_t *inflated_buf;
> uLongf inflated_size;
> +int valid_pixels
> } RsccContext;
>
> static av_cold int rscc_init(AVCodecContext *avctx)
> @@ -348,7 +349,11 @@ static int rscc_decode_frame(AVCodecContext *avctx, void
> *data,
> memcpy (frame->data[1], ctx->palette, AVPALETTE_SIZE);
> }
>
> -*got_frame = 1;
> +// We only return a picture when too little is undameged, this avoids
> copying nearly broken frames around
Hi,
FWIW, I think “too little is undameged” should say: “enough of it is undamaged”
(i.e. invert the logic, fix the typo).
> +if (ctx->valid_pixels < ctx->inflated_size)
> +ctx->valid_pixels += pixel_size;
> +if (ctx->valid_pixels >= ctx->inflated_size * (100 -
> avctx->discard_damaged_percentage) / 100)
> +*got_frame = 1;
>
> ret = avpkt->size;
> end:
> --
> 2.20.1
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/2] avcodec: Add discard_sample_percentage
Suggested-by: BBB
Signed-off-by: Michael Niedermayer
---
doc/APIchanges | 3 +++
libavcodec/avcodec.h | 8
libavcodec/options_table.h | 1 +
tests/ref/fate/api-mjpeg-codec-param | 2 ++
tests/ref/fate/api-png-codec-param | 2 ++
5 files changed, 16 insertions(+)
diff --git a/doc/APIchanges b/doc/APIchanges
index 5889fb246b..a146a487ef 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -21,6 +21,9 @@ API changes, most recent first:
8< - FFmpeg 4.1 was cut here 8< -
+2019-XX-XX - XX - lavc 58.XX.100 - avcodec.h
+ Add discard_damaged_percentage
+
2018-10-27 - 718044dc19 - lavu 56.21.100 - pixdesc.h
Add av_read_image_line2(), av_write_image_line2()
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 90f9f08289..969cf83081 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -3344,6 +3344,14 @@ typedef struct AVCodecContext {
* used as reference pictures).
*/
int extra_hw_frames;
+
+/**
+ * The percentage of damaged samples to discard a frame.
+ *
+ * - decoding: set by user
+ * - encoding: unused
+ */
+int discard_damaged_percentage;
} AVCodecContext;
#if FF_API_CODEC_GET_SET
diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h
index 2f36ec7b8e..a3235bcd57 100644
--- a/libavcodec/options_table.h
+++ b/libavcodec/options_table.h
@@ -479,6 +479,7 @@ static const AVOption avcodec_options[] = {
{"allow_high_depth", "allow to output YUV pixel formats with a different
chroma sampling than 4:2:0 and/or other than 8 bits per component", 0,
AV_OPT_TYPE_CONST, {.i64 = AV_HWACCEL_FLAG_ALLOW_HIGH_DEPTH }, INT_MIN,
INT_MAX, V | D, "hwaccel_flags"},
{"allow_profile_mismatch", "attempt to decode anyway if HW accelerated
decoder's supported profiles do not exactly match the stream", 0,
AV_OPT_TYPE_CONST, {.i64 = AV_HWACCEL_FLAG_ALLOW_PROFILE_MISMATCH }, INT_MIN,
INT_MAX, V | D, "hwaccel_flags"},
{"extra_hw_frames", "Number of extra hardware frames to allocate for the
user", OFFSET(extra_hw_frames), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, INT_MAX,
V|D },
+{"discard_damaged_percentage", "Percentage of damaged samples to discard a
frame", OFFSET(discard_damaged_percentage), AV_OPT_TYPE_INT, {.i64 = 95 }, 0,
100, V|D },
{NULL},
};
diff --git a/tests/ref/fate/api-mjpeg-codec-param
b/tests/ref/fate/api-mjpeg-codec-param
index 0d2b55cff1..0815919d7d 100644
--- a/tests/ref/fate/api-mjpeg-codec-param
+++ b/tests/ref/fate/api-mjpeg-codec-param
@@ -138,6 +138,7 @@ stream=0, decode=0
max_pixels=2147483647
hwaccel_flags=0x0001
extra_hw_frames=-1
+discard_damaged_percentage=95
stream=0, decode=1
b=0
ab=0
@@ -278,3 +279,4 @@ stream=0, decode=1
max_pixels=2147483647
hwaccel_flags=0x0001
extra_hw_frames=-1
+discard_damaged_percentage=95
diff --git a/tests/ref/fate/api-png-codec-param
b/tests/ref/fate/api-png-codec-param
index 1ae94a2e91..a47d0963da 100644
--- a/tests/ref/fate/api-png-codec-param
+++ b/tests/ref/fate/api-png-codec-param
@@ -138,6 +138,7 @@ stream=0, decode=0
max_pixels=2147483647
hwaccel_flags=0x0001
extra_hw_frames=-1
+discard_damaged_percentage=95
stream=0, decode=1
b=0
ab=0
@@ -278,3 +279,4 @@ stream=0, decode=1
max_pixels=2147483647
hwaccel_flags=0x0001
extra_hw_frames=-1
+discard_damaged_percentage=95
--
2.20.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 3/5] lavf/tls_openssl: on 1.1 or later, verify the server's hostname
---
libavformat/tls_openssl.c | 22 ++
1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/libavformat/tls_openssl.c b/libavformat/tls_openssl.c
index ae3fd6e236..ba233c0229 100644
--- a/libavformat/tls_openssl.c
+++ b/libavformat/tls_openssl.c
@@ -35,6 +35,7 @@
#include
#include
#include
+#include
static int openssl_init;
@@ -269,8 +270,6 @@ static int tls_open(URLContext *h, const char *uri, int
flags, AVDictionary **op
ret = AVERROR(EIO);
goto fail;
}
-// Note, this doesn't check that the peer certificate actually matches
-// the requested hostname.
if (c->verify)
SSL_CTX_set_verify(p->ctx,
SSL_VERIFY_PEER|SSL_VERIFY_FAIL_IF_NO_PEER_CERT, NULL);
p->ssl = SSL_new(p->ctx);
@@ -294,8 +293,23 @@ static int tls_open(URLContext *h, const char *uri, int
flags, AVDictionary **op
bio->ptr = c->tcp;
#endif
SSL_set_bio(p->ssl, bio, bio);
-if (!c->listen && !c->numerichost)
-SSL_set_tlsext_host_name(p->ssl, c->host);
+if (!c->listen && !c->numerichost) {
+#if OPENSSL_VERSION_NUMBER >= 0x101fL
+X509_VERIFY_PARAM *param = SSL_get0_param(p->ssl);
+X509_VERIFY_PARAM_set_hostflags(param,
X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS);
+#endif
+if (
+#if OPENSSL_VERSION_NUMBER >= 0x101fL
+// Note, if on OpenSSL prior to 1.1.0, we won't check that
+// the peer certificate actually matches the requested hostname.
+!X509_VERIFY_PARAM_set1_host(param, c->host, 0) ||
+#endif
+!SSL_set_tlsext_host_name(p->ssl, c->host)) {
+av_log(h, AV_LOG_ERROR, "%s\n", ERR_error_string(ERR_get_error(),
NULL));
+ret = AVERROR(EIO);
+goto fail;
+}
+}
ret = c->listen ? SSL_accept(p->ssl) : SSL_connect(p->ssl);
if (ret == 0) {
av_log(h, AV_LOG_ERROR, "Unable to negotiate TLS/SSL session\n");
--
2.19.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 5/5] lavf/tls: enable server verification by default when not on mbedtls
All other TLS wrappers now have a mechanism to load a system trust store
by default, without setting the cafile option. For Secure Transport and
Secure Channel, it's the OS. For OpenSSL and libtls, it's a path set at
compile-time. For GNUTLS, it's either a path set at compile-time, or the
OS trust store (if on macOS, iOS, or Windows). It's possible to configure
OpenSSL, GNUTLS, and libtls without a working trust store, but these are
broken configurations and I don't have a problem with requiring users with
that kind of install to either fix it, or explicitly opt in to insecure
behavior. mbedtls doesn't have a default trust store (it's assumed that the
application will provide one), so it continues to require the user to pass
in a path and enable verification manually.
---
libavformat/tls.c | 3 +++
libavformat/tls.h | 2 +-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/libavformat/tls.c b/libavformat/tls.c
index a6dcd3cc96..c564b1252b 100644
--- a/libavformat/tls.c
+++ b/libavformat/tls.c
@@ -62,6 +62,9 @@ int ff_tls_open_underlying(TLSShared *c, URLContext *parent,
const char *uri, AV
const char *proxy_path;
int use_proxy;
+if (c->verify == -1)
+c->verify = !c->listen && !CONFIG_MBEDTLS;
+
set_options(c, uri);
if (c->listen)
diff --git a/libavformat/tls.h b/libavformat/tls.h
index beb19d6d55..bc4ee1c216 100644
--- a/libavformat/tls.h
+++ b/libavformat/tls.h
@@ -45,7 +45,7 @@ typedef struct TLSShared {
#define TLS_COMMON_OPTIONS(pstruct, options_field) \
{"ca_file","Certificate Authority database file", offsetof(pstruct,
options_field . ca_file), AV_OPT_TYPE_STRING, .flags = TLS_OPTFL }, \
{"cafile", "Certificate Authority database file", offsetof(pstruct,
options_field . ca_file), AV_OPT_TYPE_STRING, .flags = TLS_OPTFL }, \
-{"tls_verify", "Verify the peer certificate", offsetof(pstruct,
options_field . verify),AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 1, .flags =
TLS_OPTFL }, \
+{"tls_verify", "Verify the peer certificate", offsetof(pstruct,
options_field . verify),AV_OPT_TYPE_BOOL, { .i64 = -1 }, -1, 1, .flags =
TLS_OPTFL }, \
{"cert_file", "Certificate file",offsetof(pstruct,
options_field . cert_file), AV_OPT_TYPE_STRING, .flags = TLS_OPTFL }, \
{"key_file", "Private key file",offsetof(pstruct,
options_field . key_file), AV_OPT_TYPE_STRING, .flags = TLS_OPTFL }, \
{"listen", "Listen for incoming connections", offsetof(pstruct,
options_field . listen),AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 1, .flags =
TLS_OPTFL }, \
--
2.19.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/5] lavf/tls_openssl: disable obsolete locking code on OpenSSL 1.1 and later
These functions are now all no-ops and locking is handled internally.
This fixes a small memory leak on init.
---
libavformat/tls_openssl.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/libavformat/tls_openssl.c b/libavformat/tls_openssl.c
index 7ae71bdaf3..9dd53c6fc0 100644
--- a/libavformat/tls_openssl.c
+++ b/libavformat/tls_openssl.c
@@ -48,7 +48,7 @@ typedef struct TLSContext {
#endif
} TLSContext;
-#if HAVE_THREADS
+#if HAVE_THREADS && (OPENSSL_VERSION_NUMBER < 0x101fL)
#include
pthread_mutex_t *openssl_mutexes;
static void openssl_lock(int mode, int type, const char *file, int line)
@@ -72,7 +72,7 @@ int ff_openssl_init(void)
if (!openssl_init) {
SSL_library_init();
SSL_load_error_strings();
-#if HAVE_THREADS
+#if HAVE_THREADS && (OPENSSL_VERSION_NUMBER < 0x101fL)
if (!CRYPTO_get_locking_callback()) {
int i;
openssl_mutexes = av_malloc_array(sizeof(pthread_mutex_t),
CRYPTO_num_locks());
@@ -101,7 +101,7 @@ void ff_openssl_deinit(void)
ff_lock_avformat();
openssl_init--;
if (!openssl_init) {
-#if HAVE_THREADS
+#if HAVE_THREADS && (OPENSSL_VERSION_NUMBER < 0x101fL)
if (CRYPTO_get_locking_callback() == openssl_lock) {
int i;
CRYPTO_set_locking_callback(NULL);
--
2.19.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/5] lavf/tls_openssl: if no CA path is set, use the system default
This is consistent with the other TLS wrappers
---
libavformat/tls_openssl.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/libavformat/tls_openssl.c b/libavformat/tls_openssl.c
index 9dd53c6fc0..ae3fd6e236 100644
--- a/libavformat/tls_openssl.c
+++ b/libavformat/tls_openssl.c
@@ -253,6 +253,9 @@ static int tls_open(URLContext *h, const char *uri, int
flags, AVDictionary **op
if (c->ca_file) {
if (!SSL_CTX_load_verify_locations(p->ctx, c->ca_file, NULL))
av_log(h, AV_LOG_ERROR, "SSL_CTX_load_verify_locations %s\n",
ERR_error_string(ERR_get_error(), NULL));
+} else {
+if (!SSL_CTX_set_default_verify_paths(p->ctx))
+av_log(h, AV_LOG_ERROR, "SSL_CTX_set_default_verify_paths %s\n",
ERR_error_string(ERR_get_error(), NULL));
}
if (c->cert_file && !SSL_CTX_use_certificate_chain_file(p->ctx,
c->cert_file)) {
av_log(h, AV_LOG_ERROR, "Unable to load cert file %s: %s\n",
--
2.19.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 4/5] lavf/tls: apply numerichost check to verifyhost
---
libavformat/tls.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/libavformat/tls.c b/libavformat/tls.c
index 10e0792e29..a6dcd3cc96 100644
--- a/libavformat/tls.c
+++ b/libavformat/tls.c
@@ -80,15 +80,15 @@ int ff_tls_open_underlying(TLSShared *c, URLContext
*parent, const char *uri, AV
ff_url_join(buf, sizeof(buf), "tcp", NULL, c->underlying_host, port, "%s",
p);
+if (!c->host && !(c->host = av_strdup(c->underlying_host)))
+return AVERROR(ENOMEM);
+
hints.ai_flags = AI_NUMERICHOST;
-if (!getaddrinfo(c->underlying_host, NULL, , )) {
+if (!getaddrinfo(c->host, NULL, , )) {
c->numerichost = 1;
freeaddrinfo(ai);
}
-if (!c->host && !(c->host = av_strdup(c->underlying_host)))
-return AVERROR(ENOMEM);
-
proxy_path = getenv("http_proxy");
use_proxy = !ff_http_match_no_proxy(getenv("no_proxy"),
c->underlying_host) &&
proxy_path && av_strstart(proxy_path, "http://;, NULL);
--
2.19.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avcodec/libx265: add support for ROI-based encoding
Signed-off-by: Guo, Yejun
---
libavcodec/libx265.c | 74
1 file changed, 74 insertions(+)
diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c
index 27c90b3..6b1cc74 100644
--- a/libavcodec/libx265.c
+++ b/libavcodec/libx265.c
@@ -285,6 +285,70 @@ static av_cold int libx265_encode_init(AVCodecContext
*avctx)
return 0;
}
+static av_cold int libx265_encode_set_roi(libx265Context *ctx, const AVFrame
*frame, x265_picture* pic)
+{
+// From x265.h:
+/* An array of quantizer offsets to be applied to this image during
encoding.
+* These are added on top of the decisions made by rateControl.
+* Adaptive quantization must be enabled to use this feature. These
quantizer
+* offsets should be given for each 16x16 block (8x8 block, when qg-size is
8).
+* Behavior if quant offsets differ between encoding passes is undefined.
+*/
+AVFrameSideData *sd = av_frame_get_side_data(frame,
AV_FRAME_DATA_REGIONS_OF_INTEREST);
+if (sd) {
+if (ctx->params->rc.aqMode == X265_AQ_NONE) {
+av_log(ctx, AV_LOG_WARNING, "Adaptive quantization must be enabled
to use ROI encoding, skipping ROI.\n");
+} else {
+int mb_size = (ctx->params->rc.qgSize == 8) ? 8 : 16;
+int mbx = (frame->width + mb_size - 1) / mb_size;
+int mby = (frame->height + mb_size - 1) / mb_size;
+int nb_rois;
+AVRegionOfInterest* roi;
+float* qoffsets; // will be freed after encode is called
+qoffsets = av_mallocz_array(mbx * mby, sizeof(*qoffsets));
+if (!qoffsets)
+return AVERROR(ENOMEM);
+
+nb_rois = sd->size / sizeof(AVRegionOfInterest);
+roi = (AVRegionOfInterest*)sd->data;
+for (int count = 0; count < nb_rois; count++) {
+int starty = FFMIN(mby, roi->top / mb_size);
+int endy = FFMIN(mby, (roi->bottom + mb_size - 1)/ mb_size);
+int startx = FFMIN(mbx, roi->left / mb_size);
+int endx = FFMIN(mbx, (roi->right + mb_size - 1)/ mb_size);
+float qoffset;
+
+if (roi->qoffset.den == 0) {
+av_free(qoffsets);
+av_log(ctx, AV_LOG_ERROR, "AVRegionOfInterest.qoffset.den
should not be zero.\n");
+return AVERROR(EINVAL);
+}
+qoffset = roi->qoffset.num * 1.0f / roi->qoffset.den;
+qoffset = av_clipf(qoffset, -1.0f, 1.0f);
+
+// 25 is a number that I think it is a possible proper scale
value.
+qoffset = qoffset * 25;
+
+for (int y = starty; y < endy; y++) {
+for (int x = startx; x < endx; x++) {
+qoffsets[x + y*mbx] = qoffset;
+}
+}
+
+if (roi->self_size == 0) {
+av_free(qoffsets);
+av_log(ctx, AV_LOG_ERROR, "AVRegionOfInterest.self_size
should be set to sizeof(AVRegionOfInterest).\n");
+return AVERROR(EINVAL);
+}
+roi = (AVRegionOfInterest*)((char*)roi + roi->self_size);
+}
+
+pic->quantOffsets = qoffsets;
+}
+}
+return 0;
+}
+
static int libx265_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
const AVFrame *pic, int *got_packet)
{
@@ -314,10 +378,20 @@ static int libx265_encode_frame(AVCodecContext *avctx,
AVPacket *pkt,
pic->pict_type == AV_PICTURE_TYPE_P ? X265_TYPE_P :
pic->pict_type == AV_PICTURE_TYPE_B ? X265_TYPE_B :
X265_TYPE_AUTO;
+
+ret = libx265_encode_set_roi(ctx, pic, );
+if (ret < 0)
+return ret;
}
ret = ctx->api->encoder_encode(ctx->encoder, , ,
pic ? : NULL, _out);
+
+if (x265pic.quantOffsets) {
+av_free(x265pic.quantOffsets);
+x265pic.quantOffsets = NULL;
+}
+
if (ret < 0)
return AVERROR_EXTERNAL;
--
2.7.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
