Re: [FFmpeg-devel] [PATCH v4] vf_drawtext: Add pkt_pos, pkt_duration, pkt_size as variables
On 05-07-2019 10:15 AM, Gyan wrote: On 05-07-2019 07:32 AM, greg Luce wrote: If the joined version is preferred it's been submitted at http://ffmpeg.org/pipermail/ffmpeg-devel/2019-June/245662.html Pinging. Would it be possible to get one of these two versions applied, if no other changes are required? Will test and apply. Updated version bump and applied as 2bd21b96096320bc12532119a6b0f7a974db6c19 Thanks, Gyan ___ 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 1/2] lavc/decode: recreate hw_frames_ctx instead of return if already exists
If hw_frames_ctx exists when calling ff_decode_get_hw_frames_ctx, it
is allowed to be recreated instead of just return.
Move hw_frames_ctx check outside ff_decode_get_hw_frames_ctx, and check
in relevant code.
Signed-off-by: Linjie Fu
---
libavcodec/decode.c | 2 +-
libavcodec/dxva2.c | 8 +---
libavcodec/vdpau.c | 9 +
3 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 6c31166..0863b82 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1230,7 +1230,7 @@ int ff_decode_get_hw_frames_ctx(AVCodecContext *avctx,
return AVERROR(ENOSYS);
if (avctx->hw_frames_ctx)
-return 0;
+av_buffer_unref(>hw_frames_ctx);
if (!avctx->hw_device_ctx) {
av_log(avctx, AV_LOG_ERROR, "A hardware frames or device context is "
"required for hardware accelerated decoding.\n");
diff --git a/libavcodec/dxva2.c b/libavcodec/dxva2.c
index 3241611..0404064 100644
--- a/libavcodec/dxva2.c
+++ b/libavcodec/dxva2.c
@@ -661,9 +661,11 @@ int ff_dxva2_decode_init(AVCodecContext *avctx)
// (avctx->pix_fmt is not updated yet at this point)
sctx->pix_fmt = avctx->hwaccel->pix_fmt;
-ret = ff_decode_get_hw_frames_ctx(avctx, dev_type);
-if (ret < 0)
-return ret;
+if (!avctx->hw_frames_ctx) {
+ret = ff_decode_get_hw_frames_ctx(avctx, dev_type);
+if (ret < 0)
+return ret;
+}
frames_ctx = (AVHWFramesContext*)avctx->hw_frames_ctx->data;
sctx->device_ctx = frames_ctx->device_ctx;
diff --git a/libavcodec/vdpau.c b/libavcodec/vdpau.c
index 167f06d..b7a4e9c 100644
--- a/libavcodec/vdpau.c
+++ b/libavcodec/vdpau.c
@@ -178,10 +178,11 @@ int ff_vdpau_common_init(AVCodecContext *avctx,
VdpDecoderProfile profile,
AVHWFramesContext *frames_ctx;
AVVDPAUDeviceContext *dev_ctx;
-ret = ff_decode_get_hw_frames_ctx(avctx, AV_HWDEVICE_TYPE_VDPAU);
-if (ret < 0)
-return ret;
-
+if (!avctx->hw_frames_ctx) {
+ret = ff_decode_get_hw_frames_ctx(avctx, AV_HWDEVICE_TYPE_VDPAU);
+if (ret < 0)
+return ret;
+}
frames_ctx = (AVHWFramesContext*)avctx->hw_frames_ctx->data;
dev_ctx = frames_ctx->device_ctx->hwctx;
--
2.7.4
___
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 2/2] lavc/vaapi_decode: recreate hw_frames_ctx without destroy va_context
VP9 allows resolution changes per frame. Currently in VAAPI, resolution
changes leads to va context destroy and reinit. This will cause
reference frame surface lost and produce garbage.
As libva allows re-create surface separately without changing the
context, this issue could be handled by only recreating hw_frames_ctx.
Could be verified by:
ffmpeg -hwaccel vaapi -hwaccel_device /dev/dri/renderD128 -i
./resolutions.ivf
-pix_fmt p010le -f rawvideo -vframes 20 -y vaapi.yuv
Signed-off-by: Linjie Fu
---
libavcodec/decode.c | 8
libavcodec/vaapi_decode.c | 40 ++--
2 files changed, 26 insertions(+), 22 deletions(-)
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 0863b82..a81b857 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1254,7 +1254,6 @@ int ff_decode_get_hw_frames_ctx(AVCodecContext *avctx,
frames_ctx = (AVHWFramesContext*)avctx->hw_frames_ctx->data;
-
if (frames_ctx->initial_pool_size) {
// We guarantee 4 base work surfaces. The function above guarantees 1
// (the absolute minimum), so add the missing count.
@@ -1333,7 +1332,7 @@ static int hwaccel_init(AVCodecContext *avctx,
return AVERROR_PATCHWELCOME;
}
-if (hwaccel->priv_data_size) {
+if (hwaccel->priv_data_size && !avctx->internal->hwaccel_priv_data) {
avctx->internal->hwaccel_priv_data =
av_mallocz(hwaccel->priv_data_size);
if (!avctx->internal->hwaccel_priv_data)
@@ -1397,8 +1396,9 @@ int ff_get_format(AVCodecContext *avctx, const enum
AVPixelFormat *fmt)
for (;;) {
// Remove the previous hwaccel, if there was one.
-hwaccel_uninit(avctx);
-
+// VAAPI allows reinit hw_frames_ctx only
+if (choices[0] != AV_PIX_FMT_VAAPI_VLD)
+hwaccel_uninit(avctx);
user_choice = avctx->get_format(avctx, choices);
if (user_choice == AV_PIX_FMT_NONE) {
// Explicitly chose nothing, give up.
diff --git a/libavcodec/vaapi_decode.c b/libavcodec/vaapi_decode.c
index 69512e1..13f0cf0 100644
--- a/libavcodec/vaapi_decode.c
+++ b/libavcodec/vaapi_decode.c
@@ -613,8 +613,10 @@ int ff_vaapi_decode_init(AVCodecContext *avctx)
VAStatus vas;
int err;
-ctx->va_config = VA_INVALID_ID;
-ctx->va_context = VA_INVALID_ID;
+if (!ctx->va_config && !ctx->va_context){
+ctx->va_config = VA_INVALID_ID;
+ctx->va_context = VA_INVALID_ID;
+}
#if FF_API_STRUCT_VAAPI_CONTEXT
if (avctx->hwaccel_context) {
@@ -642,7 +644,6 @@ int ff_vaapi_decode_init(AVCodecContext *avctx)
// present, so set it here to avoid the behaviour changing.
ctx->hwctx->driver_quirks =
AV_VAAPI_DRIVER_QUIRK_RENDER_PARAM_BUFFERS;
-
}
#endif
@@ -655,7 +656,8 @@ int ff_vaapi_decode_init(AVCodecContext *avctx)
"context: %#x/%#x.\n", ctx->va_config, ctx->va_context);
} else {
#endif
-
+// Get a new hw_frames_ctx even if there is already one
+// recreate surface without reconstuct va_context
err = ff_decode_get_hw_frames_ctx(avctx, AV_HWDEVICE_TYPE_VAAPI);
if (err < 0)
goto fail;
@@ -670,21 +672,23 @@ int ff_vaapi_decode_init(AVCodecContext *avctx)
if (err)
goto fail;
-vas = vaCreateContext(ctx->hwctx->display, ctx->va_config,
- avctx->coded_width, avctx->coded_height,
- VA_PROGRESSIVE,
- ctx->hwfc->surface_ids,
- ctx->hwfc->nb_surfaces,
- >va_context);
-if (vas != VA_STATUS_SUCCESS) {
-av_log(avctx, AV_LOG_ERROR, "Failed to create decode "
- "context: %d (%s).\n", vas, vaErrorStr(vas));
-err = AVERROR(EIO);
-goto fail;
-}
+if (ctx->va_context == VA_INVALID_ID) {
+vas = vaCreateContext(ctx->hwctx->display, ctx->va_config,
+ avctx->coded_width, avctx->coded_height,
+ VA_PROGRESSIVE,
+ ctx->hwfc->surface_ids,
+ ctx->hwfc->nb_surfaces,
+ >va_context);
+if (vas != VA_STATUS_SUCCESS) {
+av_log(avctx, AV_LOG_ERROR, "Failed to create decode "
+ "context: %d (%s).\n", vas, vaErrorStr(vas));
+err = AVERROR(EIO);
+goto fail;
+}
-av_log(avctx, AV_LOG_DEBUG, "Decode context initialised: "
- "%#x/%#x.\n", ctx->va_config, ctx->va_context);
+av_log(avctx, AV_LOG_DEBUG, "Decode context initialised: "
+ "%#x/%#x.\n", ctx->va_config, ctx->va_context);
+}
#if FF_API_STRUCT_VAAPI_CONTEXT
}
#endif
--
2.7.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
Re: [FFmpeg-devel] [PATCH] avutil: add av_memcpy() to avoid undefined behavior with NULL, NULL, 0
lör 2019-07-06 klockan 18:34 +0200 skrev Michael Niedermayer:
> On Sat, Jul 06, 2019 at 02:34:34PM +0200, Tomas Härdin wrote:
> > lör 2019-07-06 klockan 00:08 +0200 skrev Michael Niedermayer:
> > > As we are already off topic, heres an example to test static
> > > analysis, does this trigger undefined behavior by executing the
> > > memcpy
> > > for some user input ?
> > >
> > > void f(unsigned bigint a) {
> > > bigint i;
> > > for (i = 2; (((bigint)1 << a) + 1) % i; i++)
> > > ;
> > > if (a > 20 && i > ((bigint)1 << a))
> > > memcpy(NULL, NULL, 0);
> > > }
> > >
> > > i know its a lame example but just to remind that static analysis
> > > has
> > > limitations. (your mail sounds a bit like static analysis could
> > > replace
> > > everything ...)
> >
> > That is acually perfectly legal since the intersection between
> > [NULL,NULL) and [NULL,NULL) is empty and thus they do not overlap.
> > Here's an example that validates:
> >
> > #include
> > #include
> >
> > /*@ axiomatic Foo {
> > axiom Bar: \forall integer a;
> > 0 <= a <= 62 ==>
> > 1 <= (1< > }
> > */
> >
> > /*@ requires 0 <= a <= 62;
> > assigns ((char*)NULL)[0..-1];
> > */
> > void f(uint64_t a) {
> > int64_t i = 2;
> > int64_t a2 = (1LL << a) + 1;
> > /*@ loop invariant 2 <= i <= a2;
> > loop assigns i;
> > */
> > for (; a2 % i; i++)
> > ;
> > //@ assert a2 % i == 0;
> > if (a > 20 && i > ((int64_t)1 << a))
> > memcpy(NULL, NULL, 0);
> > }
>
> this code is wrong.
> Imagine this was real code, and to make it fit in the static
> analyzer one changes it like this
>
> why is it worng ?
> the range should not have a upper bound of 62 in fact there is no
> reason to run this function with input below 1LL<<33 that is not
> 33 that is 1LL<<33
All bignum implementations I've seen have upper bounds. Maybe you have
an infinite tape laying around somewhere?
It is possible to define custom data types and reason about them just
as well as integers. In fact, I've seen elliptic curve implementations
that do just that. It's just that it's a bit of work
>
> also you can restrict a to powers of 2 if you like
> thats
> for (b = 33; ; b++)
> f((bigint)1<
> can the analyzer help check this ?
If you have a bigint_shift() function with an appropriate contract,
sure. b will overflow at some point howeveer, which the prover will
catch by default if b is signed. It can also be told to catch unsigned
overflow.
> I am pretty sure its not UB below, and i suspect its not UB for most
> values
> above but iam really curious and like to know for sure.
>
>
> [...]
>
> > If you change the memcpy() to copy n=1 bytes then the verification
> > fails, as expected. It also fails if let src and dst equal to the
> > same
> > valid memory area, because of overlap.
>
> are you saying that code which never executes causes failure of the
> analyzer because if it did execute it would be wrong ?
This is equivalent to asking the prover to show that there are no
Fermat primes above (1<<20), which is a tall order. If your code
depends on tricky-to-prove theorems then obviously the prover is not
going to do that job for you.
Whether dead code is removed is a good question nonetheless, let me
check:
void f(uint64_t a) {
[...]
if (a > 20 && i > ((int64_t)1 << a)) {
char aa[1];
memcpy(aa, aa, 1);
}
}
fails
void f(uint64_t a) {
[...]
if (a > 62 && i > ((int64_t)1 << a)) {
char aa[1];
memcpy(aa, aa, 1);
}
}
succeeds (because 0 <= a <= 62)
/Tomas
___
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 3/7] avcodec/alsdec: Fix integer overflow with buffer number
Am 06.07.19 um 22:10 schrieb Michael Niedermayer: > On Sat, Jul 06, 2019 at 09:39:32PM +0200, Thilo Borgmann wrote: >> Am 21.06.19 um 09:00 schrieb Reimar Döffinger: >>> >>> >>> On 21.06.2019, at 00:47, Michael Niedermayer wrote: >>> Fixes: signed integer overflow: 65313 * 65313 cannot be represented in type 'int' Fixes: 15290/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ALS_fuzzer-5738074249625600 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer --- libavcodec/alsdec.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libavcodec/alsdec.c b/libavcodec/alsdec.c index 79d22b7c2b..8e0d3e5f83 100644 --- a/libavcodec/alsdec.c +++ b/libavcodec/alsdec.c @@ -1990,6 +1990,8 @@ static av_cold int decode_init(AVCodecContext *avctx) // allocate quantized parcor coefficient buffer num_buffers = sconf->mc_coding ? avctx->channels : 1; +if (num_buffers * (uint64_t)num_buffers > INT_MAX) +return AVERROR_INVALIDDATA; >>> >>> It would be nice if it was clear which code this check protects, i.e. some >>> connection between the check and the code that overflows. >>> I guess one might also ask if > 30 000 channels might not be something to >>> catch and disallow earlier and generally... >> >> AFAICT the specs allow all 16 bit aka 65536 (+1) channels. For the case that >> remark from Raimar had been addressed.. > > its the chan_data_buffer allocation. Ill add a comment > > >> >> The rest lgtm. I would appreciate s.o. bumping me if I miss something about >> ALS on devel, pls 0:-) > > not sure i understand the abbreviation but i will apply the patchset as it > seems, thats the consensus and ill try to ping you in the future of als > patches unless > i forget ... sadly i tend to forget these things ... "Someone". Do apply. The rest is just in regard of redundance, I'll not miss every patch ^^ Thanks, 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 3/7] avcodec/alsdec: Fix integer overflow with buffer number
On Sat, Jul 06, 2019 at 09:39:32PM +0200, Thilo Borgmann wrote: > Am 21.06.19 um 09:00 schrieb Reimar Döffinger: > > > > > > On 21.06.2019, at 00:47, Michael Niedermayer wrote: > > > >> Fixes: signed integer overflow: 65313 * 65313 cannot be represented in > >> type 'int' > >> Fixes: > >> 15290/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ALS_fuzzer-5738074249625600 > >> > >> Found-by: continuous fuzzing process > >> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > >> Signed-off-by: Michael Niedermayer > >> --- > >> libavcodec/alsdec.c | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/libavcodec/alsdec.c b/libavcodec/alsdec.c > >> index 79d22b7c2b..8e0d3e5f83 100644 > >> --- a/libavcodec/alsdec.c > >> +++ b/libavcodec/alsdec.c > >> @@ -1990,6 +1990,8 @@ static av_cold int decode_init(AVCodecContext *avctx) > >> > >> // allocate quantized parcor coefficient buffer > >> num_buffers = sconf->mc_coding ? avctx->channels : 1; > >> +if (num_buffers * (uint64_t)num_buffers > INT_MAX) > >> +return AVERROR_INVALIDDATA; > > > > It would be nice if it was clear which code this check protects, i.e. some > > connection between the check and the code that overflows. > > I guess one might also ask if > 30 000 channels might not be something to > > catch and disallow earlier and generally... > > AFAICT the specs allow all 16 bit aka 65536 (+1) channels. For the case that > remark from Raimar had been addressed.. its the chan_data_buffer allocation. Ill add a comment > > The rest lgtm. I would appreciate s.o. bumping me if I miss something about > ALS on devel, pls 0:-) not sure i understand the abbreviation but i will apply the patchset as it seems, thats the consensus and ill try to ping you in the future of als patches unless i forget ... sadly i tend to forget these things ... Thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB What does censorship reveal? It reveals fear. -- Julian Assange 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 3/7] avcodec/alsdec: Fix integer overflow with buffer number
Am 21.06.19 um 09:00 schrieb Reimar Döffinger: > > > On 21.06.2019, at 00:47, Michael Niedermayer wrote: > >> Fixes: signed integer overflow: 65313 * 65313 cannot be represented in type >> 'int' >> Fixes: >> 15290/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ALS_fuzzer-5738074249625600 >> >> Found-by: continuous fuzzing process >> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg >> Signed-off-by: Michael Niedermayer >> --- >> libavcodec/alsdec.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/libavcodec/alsdec.c b/libavcodec/alsdec.c >> index 79d22b7c2b..8e0d3e5f83 100644 >> --- a/libavcodec/alsdec.c >> +++ b/libavcodec/alsdec.c >> @@ -1990,6 +1990,8 @@ static av_cold int decode_init(AVCodecContext *avctx) >> >> // allocate quantized parcor coefficient buffer >> num_buffers = sconf->mc_coding ? avctx->channels : 1; >> +if (num_buffers * (uint64_t)num_buffers > INT_MAX) >> +return AVERROR_INVALIDDATA; > > It would be nice if it was clear which code this check protects, i.e. some > connection between the check and the code that overflows. > I guess one might also ask if > 30 000 channels might not be something to > catch and disallow earlier and generally... AFAICT the specs allow all 16 bit aka 65536 (+1) channels. For the case that remark from Raimar had been addressed.. The rest lgtm. I would appreciate s.o. bumping me if I miss something about ALS on devel, pls 0:-) -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] add FF_DECODE_ERROR_DECODE_SLICES flag for AVFrame.decode_error_flags
Thanks Michael, Could you please also apply the patch [PATCH] set AVFrame decode_error_flags in case of decoding error by h264dec It is the code that uses those flags. Thanks Amir On Sat, Jun 29, 2019 at 11:35 AM Michael Niedermayer wrote: > On Fri, Jun 28, 2019 at 02:21:18AM -0700, Amir Pauker wrote: > > avutil: add FF_DECODE_ERROR_DECODE_SLICES for AVFrame.decode_error_flags > > > > Signed-off-by: Amir Pauker > > --- > > doc/APIchanges | 3 +++ > > libavutil/frame.h | 1 + > > libavutil/version.h | 2 +- > > 3 files changed, 5 insertions(+), 1 deletion(-) > > will apply > > thanks > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Modern terrorism, a quick summary: Need oil, start war with country that > has oil, kill hundread thousand in war. Let country fall into chaos, > be surprised about raise of fundamantalists. Drop more bombs, kill more > people, be surprised about them taking revenge and drop even more bombs > and strip your own citizens of their rights and freedoms. to be continued > ___ > 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".
Re: [FFmpeg-devel] [PATCH] set AVFrame decode_error_flags in case of decoding error by h264dec
Michael hey,
Could you please apply this patch as well.
Thanks
On Fri, Jun 21, 2019 at 9:15 AM Amir Pauker wrote:
> set AVFrame decode_error_flags in case h->slice_ctx->er.error_occurred is
> set
> after the call to ff_h264_execute_decode_slices. This allows the user to
> detect
> concealed decoding errors in the call to avcodec_receive_frame
>
> Signed-off-by: Amir Pauker
> ---
> libavcodec/error_resilience.c | 2 ++
> libavcodec/h264dec.c | 5 +
> 2 files changed, 7 insertions(+)
>
> diff --git a/libavcodec/error_resilience.c b/libavcodec/error_resilience.c
> index 35d0c60..ca22871 100644
> --- a/libavcodec/error_resilience.c
> +++ b/libavcodec/error_resilience.c
> @@ -1121,6 +1121,8 @@ void ff_er_frame_end(ERContext *s)
> av_log(s->avctx, AV_LOG_INFO, "concealing %d DC, %d AC, %d MV errors
> in %c frame\n",
> dc_error, ac_error, mv_error,
> av_get_picture_type_char(s->cur_pic.f->pict_type));
>
> +s->cur_pic.f->decode_error_flags |=
> FF_DECODE_ERROR_CONCEALMENT_ACTIVE;
> +
> is_intra_likely = is_intra_more_likely(s);
>
> /* set unknown mb-type to most likely */
> diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
> index 837c3b7..8d1bd16 100644
> --- a/libavcodec/h264dec.c
> +++ b/libavcodec/h264dec.c
> @@ -761,6 +761,11 @@ static int decode_nal_units(H264Context *h, const
> uint8_t *buf, int buf_size)
> if (ret < 0 && (h->avctx->err_recognition & AV_EF_EXPLODE))
> goto end;
>
> +// set decode_error_flags to allow users to detect concealed decoding
> errors
> +if ((ret < 0 || h->slice_ctx->er.error_occurred) && h->cur_pic_ptr) {
> +h->cur_pic_ptr->f->decode_error_flags |=
> FF_DECODE_ERROR_DECODE_SLICES;
> +}
> +
> ret = 0;
> end:
>
> --
> 2.1.4
>
>
___
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 3/3] avcodec/alsdec: Fix integer overflow with shifting samples
On Wed, Jun 19, 2019 at 11:54:22PM +0200, Michael Niedermayer wrote: > Fixes: signed integer overflow: -346039050 * 8 cannot be represented in type > 'int' > Fixes: > 15283/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ALS_fuzzer-5692700268953600 > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer > --- > libavcodec/alsdec.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) will apply [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB If a bugfix only changes things apparently unrelated to the bug with no further explanation, that is a good sign that the bugfix is wrong. 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 2/3] avcodec/alsdec: Fix undefined behavior in decode_rice()
On Wed, Jun 19, 2019 at 11:54:21PM +0200, Michael Niedermayer wrote: > Fixes: left shift of 72 by 26 places cannot be represented in type 'int' > Fixes: > 15279/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ALS_fuzzer-5700665621348352 > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer > --- > libavcodec/alsdec.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) will apply [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Those who would give up essential Liberty, to purchase a little temporary Safety, deserve neither Liberty nor Safety -- Benjamin Franklin 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 6/7] avcodec/alsdec: Fix another integer overflow in INTERLEAVE_OUTPUT()
On Fri, Jun 21, 2019 at 12:47:20AM +0200, Michael Niedermayer wrote: > Fixes: signed integer overflow: 41582592 * 256 cannot be represented in type > 'int' > Fixes: > 15296/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ALS_fuzzer-5739558227935232 > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer > --- > libavcodec/alsdec.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) will combine this with the other overflow fix in here and apply [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Democracy is the form of government in which you can choose your dictator 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] avcodec/alsdec: Fixes invalid shifts in read_var_block_data() and INTERLEAVE_OUTPUT()
On Wed, Jun 19, 2019 at 11:54:20PM +0200, Michael Niedermayer wrote: > Fixes: left shift of negative value -6 > Fixes: > 15275/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ALS_fuzzer-5742361767837696 > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer > --- > libavcodec/alsdec.c | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) will apply [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB While the State exists there can be no freedom; when there is freedom there will be no State. -- Vladimir Lenin 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] matroskadec: Fix overflow introduced in a569a7b3
On 7/6/2019 1:59 PM, Andreas Rheinhardt wrote:
> This commit fixes an overflow introduced in a569a7b3 that affected EBML
> elements that the Matroska demuxer doesn't want to parse like CRC-32
> elements. The return value of avio_skip (the new position on success or
> an AVERROR on failure) has been assigned to an integer which meant that
> new positions in the range of 2GB to 4GB-1 etc. were considered errors.
>
> Fixes ticket #8001.
>
> Signed-off-by: Andreas Rheinhardt
> ---
> libavformat/matroskadec.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index bc73bfed11..4d7076fa26 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -1259,12 +1259,13 @@ static int ebml_parse_elem(MatroskaDemuxContext
> *matroska,
> return 1;
> default:
> if (length) {
> +int64_t res2;
> if (ffio_limit(pb, length) != length) {
> // ffio_limit emits its own error message,
> // so we don't have to.
> return AVERROR(EIO);
> }
> -if ((res = avio_skip(pb, length - 1)) >= 0) {
> +if ((res2 = avio_skip(pb, length - 1)) >= 0) {
> // avio_skip might take us past EOF. We check for this
> // by skipping only length - 1 bytes, reading a byte and
> // checking the error flags. This is done in order to check
> @@ -1272,7 +1273,8 @@ static int ebml_parse_elem(MatroskaDemuxContext
> *matroska,
> // no filesize (that ffio_limit relies on) is available.
> avio_r8(pb);
> res = NEEDS_CHECKING;
> -}
> +} else
> +res = res2;
> } else
> res = 0;
> }
Pushed.
___
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] matroskadec: Fix overflow introduced in a569a7b3
This commit fixes an overflow introduced in a569a7b3 that affected EBML
elements that the Matroska demuxer doesn't want to parse like CRC-32
elements. The return value of avio_skip (the new position on success or
an AVERROR on failure) has been assigned to an integer which meant that
new positions in the range of 2GB to 4GB-1 etc. were considered errors.
Fixes ticket #8001.
Signed-off-by: Andreas Rheinhardt
---
libavformat/matroskadec.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index bc73bfed11..4d7076fa26 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -1259,12 +1259,13 @@ static int ebml_parse_elem(MatroskaDemuxContext
*matroska,
return 1;
default:
if (length) {
+int64_t res2;
if (ffio_limit(pb, length) != length) {
// ffio_limit emits its own error message,
// so we don't have to.
return AVERROR(EIO);
}
-if ((res = avio_skip(pb, length - 1)) >= 0) {
+if ((res2 = avio_skip(pb, length - 1)) >= 0) {
// avio_skip might take us past EOF. We check for this
// by skipping only length - 1 bytes, reading a byte and
// checking the error flags. This is done in order to check
@@ -1272,7 +1273,8 @@ static int ebml_parse_elem(MatroskaDemuxContext *matroska,
// no filesize (that ffio_limit relies on) is available.
avio_r8(pb);
res = NEEDS_CHECKING;
-}
+} else
+res = res2;
} else
res = 0;
}
--
2.21.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] avutil: add av_memcpy() to avoid undefined behavior with NULL, NULL, 0
On Sat, Jul 06, 2019 at 02:34:34PM +0200, Tomas Härdin wrote:
> lör 2019-07-06 klockan 00:08 +0200 skrev Michael Niedermayer:
> > As we are already off topic, heres an example to test static
> > analysis, does this trigger undefined behavior by executing the memcpy
> > for some user input ?
> >
> > void f(unsigned bigint a) {
> > bigint i;
> > for (i = 2; (((bigint)1 << a) + 1) % i; i++)
> > ;
> > if (a > 20 && i > ((bigint)1 << a))
> > memcpy(NULL, NULL, 0);
> > }
> >
> > i know its a lame example but just to remind that static analysis has
> > limitations. (your mail sounds a bit like static analysis could replace
> > everything ...)
>
> That is acually perfectly legal since the intersection between
> [NULL,NULL) and [NULL,NULL) is empty and thus they do not overlap.
> Here's an example that validates:
>
> #include
> #include
>
> /*@ axiomatic Foo {
> axiom Bar: \forall integer a;
> 0 <= a <= 62 ==>
> 1 <= (1< }
> */
>
> /*@ requires 0 <= a <= 62;
> assigns ((char*)NULL)[0..-1];
> */
> void f(uint64_t a) {
> int64_t i = 2;
> int64_t a2 = (1LL << a) + 1;
> /*@ loop invariant 2 <= i <= a2;
> loop assigns i;
> */
> for (; a2 % i; i++)
> ;
> //@ assert a2 % i == 0;
> if (a > 20 && i > ((int64_t)1 << a))
> memcpy(NULL, NULL, 0);
> }
this code is wrong.
Imagine this was real code, and to make it fit in the static
analyzer one changes it like this
why is it worng ?
the range should not have a upper bound of 62 in fact there is no
reason to run this function with input below 1LL<<33 that is not
33 that is 1LL<<33
also you can restrict a to powers of 2 if you like
thats
for (b = 33; ; b++)
f((bigint)1< If you change the memcpy() to copy n=1 bytes then the verification
> fails, as expected. It also fails if let src and dst equal to the same
> valid memory area, because of overlap.
are you saying that code which never executes causes failure of the
analyzer because if it did execute it would be wrong ?
[...]
Thanks
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Never trust a computer, one day, it may think you are the virus. -- Compn
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 3/6] truehd_core: Return error in case of error
Several checks (e.g. when the size of the input packet is too small)
simply used "goto fail", but didn't set the return value appropriately
for an error.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/truehd_core_bsf.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/libavcodec/truehd_core_bsf.c b/libavcodec/truehd_core_bsf.c
index 83f2b16e3d..f858c2d4d5 100644
--- a/libavcodec/truehd_core_bsf.c
+++ b/libavcodec/truehd_core_bsf.c
@@ -53,8 +53,10 @@ static int truehd_core_filter(AVBSFContext *ctx, AVPacket
*out)
if (ret < 0)
return ret;
-if (in->size < 4)
+if (in->size < 4) {
+ret = AVERROR_INVALIDDATA;
goto fail;
+}
ret = init_get_bits(, in->data, 32);
if (ret < 0)
@@ -62,8 +64,10 @@ static int truehd_core_filter(AVBSFContext *ctx, AVPacket
*out)
skip_bits(, 4);
in_size = get_bits(, 12) * 2;
-if (in_size < 4 || in_size > in->size)
+if (in_size < 4 || in_size > in->size) {
+ret = AVERROR_INVALIDDATA;
goto fail;
+}
out_size = in_size;
dts = get_bits(, 16);
@@ -73,13 +77,15 @@ static int truehd_core_filter(AVBSFContext *ctx, AVPacket
*out)
goto fail;
if (show_bits_long(, 32) == 0xf8726fba) {
-if ((ret = ff_mlp_read_major_sync(ctx, >hdr, )) != 0)
+if ((ret = ff_mlp_read_major_sync(ctx, >hdr, )) < 0)
goto fail;
have_header = 1;
}
-if (s->hdr.num_substreams > MAX_SUBSTREAMS)
+if (s->hdr.num_substreams > MAX_SUBSTREAMS) {
+ret = AVERROR_INVALIDDATA;
goto fail;
+}
for (i = 0; i < s->hdr.num_substreams; i++) {
for (int j = 0; j < 4; j++)
--
2.21.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".
[FFmpeg-devel] [PATCH 6/6] truehd_core: Switch to in-place modifications
The truehd_core bitstream filter decreases the sizes of the
major_sync_info structure (if present), of the
substream_directory and of the substreams themselves. As a consequence,
there is enough space available in front of the actual substream data
for the new header, so that one only needs to modify the header in front
of the actual data (which apart from shrinking is left untouched) and
the packet's size and buffer pointer (after having made sure that the
packet is writable).
This and switching to bsf_get_packet_ref also removed the need for
having separate packets for in- and output.
Even if the input is not writable, there are noticable performance
improvements: The average of 10 iterations of processing a file with 262144
runs each (inlcuding about 20 skips per iteration) went down from 5669
to 4362 decicycles. If the input is writable, it goes down to 1363
decicycles.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/truehd_core_bsf.c | 64 +---
1 file changed, 31 insertions(+), 33 deletions(-)
diff --git a/libavcodec/truehd_core_bsf.c b/libavcodec/truehd_core_bsf.c
index 8ea80d3015..dbd05b34ca 100644
--- a/libavcodec/truehd_core_bsf.c
+++ b/libavcodec/truehd_core_bsf.c
@@ -36,34 +36,33 @@ typedef struct TrueHDCoreContext {
MLPHeaderInfo hdr;
} TrueHDCoreContext;
-static int truehd_core_filter(AVBSFContext *ctx, AVPacket *out)
+static int truehd_core_filter(AVBSFContext *ctx, AVPacket *pkt)
{
TrueHDCoreContext *s = ctx->priv_data;
GetBitContext gbc;
AccessUnit units[MAX_SUBSTREAMS];
-AVPacket *in;
int ret, i, last_offset = 0;
int in_size, out_size;
int have_header = 0;
int substream_bytes = 0;
int end;
-ret = ff_bsf_get_packet(ctx, );
+ret = ff_bsf_get_packet_ref(ctx, pkt);
if (ret < 0)
return ret;
-if (in->size < 4) {
+if (pkt->size < 4) {
ret = AVERROR_INVALIDDATA;
goto fail;
}
-in_size = (AV_RB16(in->data) & 0xFFF) * 2;
-if (in_size < 4 || in_size > in->size) {
+in_size = (AV_RB16(pkt->data) & 0xFFF) * 2;
+if (in_size < 4 || in_size > pkt->size) {
ret = AVERROR_INVALIDDATA;
goto fail;
}
-ret = init_get_bits8(, in->data + 4, in->size - 4);
+ret = init_get_bits8(, pkt->data + 4, pkt->size - 4);
if (ret < 0)
goto fail;
@@ -99,27 +98,31 @@ static int truehd_core_filter(AVBSFContext *ctx, AVPacket
*out)
out_size = end + 4 + last_offset;
if (out_size < in_size) {
int bpos = 0, reduce = end - have_header * 28 - substream_bytes;
-uint16_t parity_nibble, dts = AV_RB16(in->data + 2);
+uint16_t parity_nibble, dts = AV_RB16(pkt->data + 2);
uint16_t auheader;
+uint8_t header[28];
-av_assert1(reduce % 2 == 0);
+av_assert1(reduce >= 0 && reduce % 2 == 0);
-ret = av_new_packet(out, out_size);
+if (have_header) {
+memcpy(header, pkt->data + 4, 28);
+header[16] = (header[16] & 0x0c) | (FFMIN(s->hdr.num_substreams,
3) << 4);
+header[17] &= 0x7f;
+header[25] &= 0xfe;
+AV_WL16(header + 26, ff_mlp_checksum16(header, 26));
+}
+
+pkt->data += reduce;
+out_size -= reduce;
+pkt->size = out_size;
+
+ret = av_packet_make_writable(pkt);
if (ret < 0)
goto fail;
-AV_WB16(out->data + 2, dts);
+AV_WB16(pkt->data + 2, dts);
parity_nibble = dts;
-out->size -= reduce;
-parity_nibble ^= out->size / 2;
-
-if (have_header) {
-memcpy(out->data + 4, in->data + 4, 28);
-out->data[16 + 4] = (out->data[16 + 4] & 0x0c) |
(FFMIN(s->hdr.num_substreams, 3) << 4);
-out->data[17 + 4]&= 0x7f;
-out->data[25 + 4] = out->data[25 + 4] & 0xfe;
-AV_WL16(out->data + 4 + 26, ff_mlp_checksum16(out->data + 4, 26));
-}
+parity_nibble ^= out_size / 2;
for (i = 0; i < FFMIN(s->hdr.num_substreams, 3); i++) {
uint16_t substr_hdr = 0;
@@ -130,13 +133,13 @@ static int truehd_core_filter(AVBSFContext *ctx, AVPacket
*out)
substr_hdr |= (units[i].bits[3] << 12);
substr_hdr |= units[i].offset;
-AV_WB16(out->data + have_header * 28 + 4 + bpos, substr_hdr);
+AV_WB16(pkt->data + have_header * 28 + 4 + bpos, substr_hdr);
parity_nibble ^= substr_hdr;
bpos += 2;
if (units[i].bits[0]) {
-AV_WB16(out->data + have_header * 28 + 4 + bpos,
units[i].optional);
+AV_WB16(pkt->data + have_header * 28 + 4 + bpos,
units[i].optional);
parity_nibble ^= units[i].optional;
bpos += 2;
@@ -147,22 +150,17 @@ static int truehd_core_filter(AVBSFContext *ctx, AVPacket
*out)
parity_nibble ^= parity_nibble >> 4;
[FFmpeg-devel] [PATCH 4/6] truehd_core: Miscellaneous improvements
1. The loop counter of the substream_directory loop is always less than
the number of substreams, yet within the loop it is checked whether it
is less than FFMIN(3, s->hdr.num_substreams), although the check for < 3
would suffice.
2. In case the packet is a major sync packet, the last two bytes of the
major sync structure were initialized to 0xff and then immediately
overwritten afterwards without ever making use of the values just set.
3. When updating the parity_nibble during writing the new
substream_directory, the parity_nibble is updated one byte at a time
with bytes that might be read from the output packet's data. But one can
do both bytes at the same time without resorting to the data just
written by XOR'ing with the variable that contains the value that has
just been written as a big endian number. This changes the intermediate
value of parity_nibble, but in the end it just amounts to a reordering
of the sum modulo two that will eventually be written as parity_nibble.
Due to associativity and commutativity, this value is unchanged.
4. init_get_bits8 already checks that no overflow happens during the
conversion of its argument from bytes to bits. ff_mlp_read_major_sync
makes sure not to overread (the maximum size of a major_sync_info is 60
bytes anyway) and last_offset is < 2^13, so that no overflow in the
calculation of size can happen, i.e. the check for whether size is >= 0
is unnecessary. But then size is completely unnecessary and can be
removed.
5. In case the packet is just passed through, it is unnecessary to read
the packet's dts. This is therefore postponed to when we know that the
packet is not passed through.
6. Given that it seems overkill to use a bitreader just for one
variable, the size of the input access unit is now read directly.
7. A substream's offset (of the end of the substream) is now stored as is
(i.e. in units of words).
These changes amount to a slight performance improvement: It improved
from 5897 decicycles of ten runs with about 262144 runs each (including
an insignificant amount -- about 20-25 usually of skips) to 5747
decicycles under the same conditions.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/truehd_core_bsf.c | 39
1 file changed, 13 insertions(+), 26 deletions(-)
diff --git a/libavcodec/truehd_core_bsf.c b/libavcodec/truehd_core_bsf.c
index f858c2d4d5..47684235db 100644
--- a/libavcodec/truehd_core_bsf.c
+++ b/libavcodec/truehd_core_bsf.c
@@ -42,12 +42,11 @@ static int truehd_core_filter(AVBSFContext *ctx, AVPacket
*out)
GetBitContext gbc;
AccessUnit units[MAX_SUBSTREAMS];
AVPacket *in;
-int ret, i, size, last_offset = 0;
+int ret, i, last_offset = 0;
int in_size, out_size;
int have_header = 0;
int substream_bits = 0;
int end;
-uint16_t dts;
ret = ff_bsf_get_packet(ctx, );
if (ret < 0)
@@ -58,20 +57,12 @@ static int truehd_core_filter(AVBSFContext *ctx, AVPacket
*out)
goto fail;
}
-ret = init_get_bits(, in->data, 32);
-if (ret < 0)
-goto fail;
-
-skip_bits(, 4);
-in_size = get_bits(, 12) * 2;
+in_size = (AV_RB16(in->data) & 0xFFF) * 2;
if (in_size < 4 || in_size > in->size) {
ret = AVERROR_INVALIDDATA;
goto fail;
}
-out_size = in_size;
-dts = get_bits(, 16);
-
ret = init_get_bits8(, in->data + 4, in->size - 4);
if (ret < 0)
goto fail;
@@ -91,26 +82,24 @@ static int truehd_core_filter(AVBSFContext *ctx, AVPacket
*out)
for (int j = 0; j < 4; j++)
units[i].bits[j] = get_bits1();
-units[i].offset = get_bits(, 12) * 2;
-if (i < FFMIN(s->hdr.num_substreams, 3)) {
-last_offset = units[i].offset;
+units[i].offset = get_bits(, 12);
+if (i < 3) {
+last_offset = units[i].offset * 2;
substream_bits += 16;
}
if (units[i].bits[0]) {
units[i].optional = get_bits(, 16);
-if (i < FFMIN(s->hdr.num_substreams, 3))
+if (i < 3)
substream_bits += 16;
}
}
end = get_bits_count();
-size = ((end + 7) >> 3) + 4 + last_offset;
-if (size >= 0 && size <= in->size)
-out_size = size;
+out_size = ((end + 7) >> 3) + 4 + last_offset;
if (out_size < in_size) {
int bpos = 0, reduce = (end - have_header * 28 * 8 - substream_bits)
>> 4;
-uint16_t parity_nibble = 0;
+uint16_t parity_nibble, dts = AV_RB16(in->data + 2);
uint16_t auheader;
ret = av_new_packet(out, out_size);
@@ -127,8 +116,6 @@ static int truehd_core_filter(AVBSFContext *ctx, AVPacket
*out)
out->data[16 + 4] = (out->data[16 + 4] & 0x0c) |
(FFMIN(s->hdr.num_substreams, 3) << 4);
out->data[17 + 4]&= 0x7f;
out->data[25 + 4] = out->data[25 + 4] & 0xfe;
-out->data[26 + 4] = 0xff;
-out->data[27 + 4] = 0xff;
[FFmpeg-devel] [PATCH 5/6] truehd_core: Use byte offsets instead of bit offsets
Words of 16 bit are the unit for TrueHD's size and offset fields;
in particular the sizes of the high-level structures of TrueHD are
always a multiple of a byte; yet truehd_core unnecessarily used
bit offsets at several places. This has been changed.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/truehd_core_bsf.c | 20 +++-
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/libavcodec/truehd_core_bsf.c b/libavcodec/truehd_core_bsf.c
index 47684235db..8ea80d3015 100644
--- a/libavcodec/truehd_core_bsf.c
+++ b/libavcodec/truehd_core_bsf.c
@@ -45,7 +45,7 @@ static int truehd_core_filter(AVBSFContext *ctx, AVPacket
*out)
int ret, i, last_offset = 0;
int in_size, out_size;
int have_header = 0;
-int substream_bits = 0;
+int substream_bytes = 0;
int end;
ret = ff_bsf_get_packet(ctx, );
@@ -85,30 +85,32 @@ static int truehd_core_filter(AVBSFContext *ctx, AVPacket
*out)
units[i].offset = get_bits(, 12);
if (i < 3) {
last_offset = units[i].offset * 2;
-substream_bits += 16;
+substream_bytes += 2;
}
if (units[i].bits[0]) {
units[i].optional = get_bits(, 16);
if (i < 3)
-substream_bits += 16;
+substream_bytes += 2;
}
}
-end = get_bits_count();
+end = get_bits_count() >> 3;
-out_size = ((end + 7) >> 3) + 4 + last_offset;
+out_size = end + 4 + last_offset;
if (out_size < in_size) {
-int bpos = 0, reduce = (end - have_header * 28 * 8 - substream_bits)
>> 4;
+int bpos = 0, reduce = end - have_header * 28 - substream_bytes;
uint16_t parity_nibble, dts = AV_RB16(in->data + 2);
uint16_t auheader;
+av_assert1(reduce % 2 == 0);
+
ret = av_new_packet(out, out_size);
if (ret < 0)
goto fail;
AV_WB16(out->data + 2, dts);
parity_nibble = dts;
-out->size -= reduce * 2;
+out->size -= reduce;
parity_nibble ^= out->size / 2;
if (have_header) {
@@ -146,8 +148,8 @@ static int truehd_core_filter(AVBSFContext *ctx, AVPacket
*out)
parity_nibble &= 0xF;
memcpy(out->data + have_header * 28 + 4 + bpos,
- in->data + 4 + (end >> 3),
- out_size - (4 + (end >> 3)));
+ in->data + 4 + end,
+ out_size - (4 + end));
auheader = (parity_nibble ^ 0xF) << 12;
auheader |= (out->size / 2) & 0x0fff;
AV_WB16(out->data, auheader);
--
2.21.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".
[FFmpeg-devel] [PATCH 2/6] truehd_core: Correct output size
If truehd_core strips Atmos data away, three parts of the output differ
in size compared to the input access unit: a) The major_sync_info block
if the extra_channel_meaning_data is present, as the newly written
output never contains said block; b) the substream_directory (because
entries relating to discarded substreams are discarded, too); and c)
the actual substream data. b) and c) have already been taken into account
when choosing the size of the output packet, but a) has been forgotten.
This is also the reason behind the end of the output buffer having been
uninitialized until 801d78f0. The workaround added in said commit has
been removed, too.
Signed-off-by: Andreas Rheinhardt
---
The extra_channel stuff can be 32 bytes at most, so the workaround might
have initialized too few bytes for some files.
libavcodec/truehd_core_bsf.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/libavcodec/truehd_core_bsf.c b/libavcodec/truehd_core_bsf.c
index 757d26a10d..83f2b16e3d 100644
--- a/libavcodec/truehd_core_bsf.c
+++ b/libavcodec/truehd_core_bsf.c
@@ -46,7 +46,7 @@ static int truehd_core_filter(AVBSFContext *ctx, AVPacket
*out)
int in_size, out_size;
int have_header = 0;
int substream_bits = 0;
-int start, end;
+int end;
uint16_t dts;
ret = ff_bsf_get_packet(ctx, );
@@ -81,7 +81,6 @@ static int truehd_core_filter(AVBSFContext *ctx, AVPacket
*out)
if (s->hdr.num_substreams > MAX_SUBSTREAMS)
goto fail;
-start = get_bits_count();
for (i = 0; i < s->hdr.num_substreams; i++) {
for (int j = 0; j < 4; j++)
units[i].bits[j] = get_bits1();
@@ -104,7 +103,7 @@ static int truehd_core_filter(AVBSFContext *ctx, AVPacket
*out)
if (size >= 0 && size <= in->size)
out_size = size;
if (out_size < in_size) {
-int bpos = 0, reduce = (end - start - substream_bits) >> 4;
+int bpos = 0, reduce = (end - have_header * 28 * 8 - substream_bits)
>> 4;
uint16_t parity_nibble = 0;
uint16_t auheader;
@@ -117,8 +116,6 @@ static int truehd_core_filter(AVBSFContext *ctx, AVPacket
*out)
out->size -= reduce * 2;
parity_nibble ^= out->size / 2;
-if (out_size > 8)
-AV_WN64(out->data + out_size - 8, 0);
if (have_header) {
memcpy(out->data + 4, in->data + 4, 28);
out->data[16 + 4] = (out->data[16 + 4] & 0x0c) |
(FFMIN(s->hdr.num_substreams, 3) << 4);
--
2.21.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".
[FFmpeg-devel] [PATCH 1/6] truehd_core: Disable 16-channel presentation
The most serious bit of the substream_info header field (in a mayor sync
packet) indicates whether a 16-channel presentation is present in the
bitstream. If set, the extended_substream_info header field contains
information about the 16-channel presentation. This presentation always
uses substream 3, a substream that is discarded by truehd_core. So
substream_info needs to be changed to no longer indicate the presence
of a 16-channel presentation in order for truehd_core's output to be
consistent. This is implemented in this commit.
This change also makes MediaInfo no longer display the presence of Atmos
in the output of truehd_core.
Also, set the (now irrelevant) extended_substream_info field to zero as
this seems to be the common value for ordinary TrueHD.
Signed-off-by: Andreas Rheinhardt
---
The info this patchset relies on can be found in Dolby's TrueHD (MLP)
high-level bitstream description [1]. See sections 4.2.8 and 4.2.9 for
this commit. Thanks to Hendrik Leppkes for the link.
[1]:
https://developer.dolby.com/globalassets/technology/dolby-truehd/dolbytruehdhighlevelbitstreamdescription.pdf
libavcodec/truehd_core_bsf.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/libavcodec/truehd_core_bsf.c b/libavcodec/truehd_core_bsf.c
index 9e3ee07eed..757d26a10d 100644
--- a/libavcodec/truehd_core_bsf.c
+++ b/libavcodec/truehd_core_bsf.c
@@ -121,7 +121,8 @@ static int truehd_core_filter(AVBSFContext *ctx, AVPacket
*out)
AV_WN64(out->data + out_size - 8, 0);
if (have_header) {
memcpy(out->data + 4, in->data + 4, 28);
-out->data[16 + 4] = (out->data[16 + 4] & 0x0f) |
(FFMIN(s->hdr.num_substreams, 3) << 4);
+out->data[16 + 4] = (out->data[16 + 4] & 0x0c) |
(FFMIN(s->hdr.num_substreams, 3) << 4);
+out->data[17 + 4]&= 0x7f;
out->data[25 + 4] = out->data[25 + 4] & 0xfe;
out->data[26 + 4] = 0xff;
out->data[27 + 4] = 0xff;
--
2.21.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] avutil: add av_memcpy() to avoid undefined behavior with NULL, NULL, 0
lör 2019-07-06 klockan 00:08 +0200 skrev Michael Niedermayer:
> As we are already off topic, heres an example to test static
> analysis, does this trigger undefined behavior by executing the memcpy
> for some user input ?
>
> void f(unsigned bigint a) {
> bigint i;
> for (i = 2; (((bigint)1 << a) + 1) % i; i++)
> ;
> if (a > 20 && i > ((bigint)1 << a))
> memcpy(NULL, NULL, 0);
> }
>
> i know its a lame example but just to remind that static analysis has
> limitations. (your mail sounds a bit like static analysis could replace
> everything ...)
That is acually perfectly legal since the intersection between
[NULL,NULL) and [NULL,NULL) is empty and thus they do not overlap.
Here's an example that validates:
#include
#include
/*@ axiomatic Foo {
axiom Bar: \forall integer a;
0 <= a <= 62 ==>
1 <= (1< 20 && i > ((int64_t)1 << a))
memcpy(NULL, NULL, 0);
}
I got a bit lazy with the axiom. I think newer versions of Frama-C are
better able to reason about shifts. I'm on version Silicon-20161101. To
validate, put in a file called mini.c and run:
frama-c -wp -wp-rte -wp-prover z3,cvc4,alt-ergo,qed mini.c
You'll need the frama-c package installed, and all the provers listed.
Curiously, replacing the assignment in the contract with assigns
\nothing doesn't work (should eq. assigning the empty set). Maybe a
newer version of Frama-C fixes this.
If you change the memcpy() to copy n=1 bytes then the verification
fails, as expected. It also fails if let src and dst equal to the same
valid memory area, because of overlap.
The point of all this is that it's possible to specify and verify
whether a function has side effects, that it doesn't write out of
bounds, that it terminates etc. This removes the need for unit tests
(if FFmpeg were to have any)
/Tomas
___
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 2/5] avcodec/dnxhd_parser: remove unneeded code
Signed-off-by: Michael Niedermayer
---
libavcodec/dnxhd_parser.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/libavcodec/dnxhd_parser.c b/libavcodec/dnxhd_parser.c
index 7c16e251a4..f657af8f41 100644
--- a/libavcodec/dnxhd_parser.c
+++ b/libavcodec/dnxhd_parser.c
@@ -81,8 +81,6 @@ static int dnxhd_find_frame_end(DNXHDParserContext *dctx,
}
dctx->remaining = remaining;
if (buf_size - i + 47 >= dctx->remaining) {
-int remaining = dctx->remaining;
-
pc->frame_start_found = 0;
pc->state64 = -1;
dctx->cur_byte = 0;
--
2.22.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".
[FFmpeg-devel] [PATCH 1/5] avcodec/parser: Check next index validity in ff_combine_frame()
Fixes: out of array access Fixes: 15522/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_DNXHD_fuzzer-5747756078989312 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer --- libavcodec/parser.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libavcodec/parser.c b/libavcodec/parser.c index 0a994a3f30..3e19810a94 100644 --- a/libavcodec/parser.c +++ b/libavcodec/parser.c @@ -245,6 +245,9 @@ int ff_combine_frame(ParseContext *pc, int next, for (; pc->overread > 0; pc->overread--) pc->buffer[pc->index++] = pc->buffer[pc->overread_index++]; +if (next > *buf_size) +return AVERROR(EINVAL); + /* flush remaining if EOF */ if (!*buf_size && next == END_NOT_FOUND) next = 0; -- 2.22.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".
[FFmpeg-devel] [PATCH 3/5] avcodec/dnxhd_parser: Optimize insufficient buf size case
Signed-off-by: Michael Niedermayer
---
libavcodec/dnxhd_parser.c | 4
1 file changed, 4 insertions(+)
diff --git a/libavcodec/dnxhd_parser.c b/libavcodec/dnxhd_parser.c
index f657af8f41..31a3158fea 100644
--- a/libavcodec/dnxhd_parser.c
+++ b/libavcodec/dnxhd_parser.c
@@ -88,6 +88,10 @@ static int dnxhd_find_frame_end(DNXHDParserContext *dctx,
return remaining;
} else {
dctx->remaining -= buf_size;
+// Update variables for correctness, they are currently
not used beyond here
+state = -1;
+dctx->cur_byte += buf_size - i;
+break;
}
}
}
--
2.22.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".
[FFmpeg-devel] [PATCH 4/5] avformat/rawdec: Make the raw packet size configurable
This allows testing parsers with a wider range of input packet sizes.
Which is important and usefull for regression testing, some of our
parsers in fact to not work if the packet size is changed from 1024
Signed-off-by: Michael Niedermayer
---
libavformat/ac3dec.c | 6 ++
libavformat/acm.c| 3 +++
libavformat/dtsdec.c | 4 +++-
libavformat/flacdec.c| 4
libavformat/g722.c | 4 +++-
libavformat/loasdec.c| 3 +++
libavformat/mlpdec.c | 6 ++
libavformat/rawdec.c | 12 +++-
libavformat/rawdec.h | 19 +--
libavformat/sbcdec.c | 3 +++
libavformat/shortendec.c | 3 +++
libavformat/takdec.c | 4
libavformat/wsddec.c | 3 +++
13 files changed, 69 insertions(+), 5 deletions(-)
diff --git a/libavformat/ac3dec.c b/libavformat/ac3dec.c
index 3736b118d3..1f87939388 100644
--- a/libavformat/ac3dec.c
+++ b/libavformat/ac3dec.c
@@ -102,6 +102,7 @@ static int ac3_probe(const AVProbeData *p)
return ac3_eac3_probe(p, AV_CODEC_ID_AC3);
}
+FF_RAW_DEMUXER_CLASS(ac3)
AVInputFormat ff_ac3_demuxer = {
.name = "ac3",
.long_name = NULL_IF_CONFIG_SMALL("raw AC-3"),
@@ -111,6 +112,8 @@ AVInputFormat ff_ac3_demuxer = {
.flags= AVFMT_GENERIC_INDEX,
.extensions = "ac3",
.raw_codec_id = AV_CODEC_ID_AC3,
+.priv_data_size = sizeof(FFRawDemuxerContext),
+.priv_class = _demuxer_class,
};
#endif
@@ -120,6 +123,7 @@ static int eac3_probe(const AVProbeData *p)
return ac3_eac3_probe(p, AV_CODEC_ID_EAC3);
}
+FF_RAW_DEMUXER_CLASS(eac3)
AVInputFormat ff_eac3_demuxer = {
.name = "eac3",
.long_name = NULL_IF_CONFIG_SMALL("raw E-AC-3"),
@@ -129,5 +133,7 @@ AVInputFormat ff_eac3_demuxer = {
.flags = AVFMT_GENERIC_INDEX,
.extensions = "eac3",
.raw_codec_id = AV_CODEC_ID_EAC3,
+.priv_data_size = sizeof(FFRawDemuxerContext),
+.priv_class = _demuxer_class,
};
#endif
diff --git a/libavformat/acm.c b/libavformat/acm.c
index 125352a8f6..5e03cf8bff 100644
--- a/libavformat/acm.c
+++ b/libavformat/acm.c
@@ -60,6 +60,7 @@ static int acm_read_header(AVFormatContext *s)
return 0;
}
+FF_RAW_DEMUXER_CLASS(acm)
AVInputFormat ff_acm_demuxer = {
.name = "acm",
.long_name = NULL_IF_CONFIG_SMALL("Interplay ACM"),
@@ -69,4 +70,6 @@ AVInputFormat ff_acm_demuxer = {
.flags = AVFMT_NOBINSEARCH | AVFMT_NOGENSEARCH |
AVFMT_NO_BYTE_SEEK | AVFMT_NOTIMESTAMPS,
.extensions = "acm",
.raw_codec_id = AV_CODEC_ID_INTERPLAY_ACM,
+.priv_data_size = sizeof(FFRawDemuxerContext),
+.priv_class = _demuxer_class,
};
diff --git a/libavformat/dtsdec.c b/libavformat/dtsdec.c
index 8ec7925c59..ab59a56dfc 100644
--- a/libavformat/dtsdec.c
+++ b/libavformat/dtsdec.c
@@ -127,6 +127,7 @@ static int dts_probe(const AVProbeData *p)
return 0;
}
+FF_RAW_DEMUXER_CLASS(dts)
AVInputFormat ff_dts_demuxer = {
.name = "dts",
.long_name = NULL_IF_CONFIG_SMALL("raw DTS"),
@@ -136,4 +137,5 @@ AVInputFormat ff_dts_demuxer = {
.flags = AVFMT_GENERIC_INDEX,
.extensions = "dts",
.raw_codec_id = AV_CODEC_ID_DTS,
-};
+.priv_data_size = sizeof(FFRawDemuxerContext),
+.priv_class = _demuxer_class,};
diff --git a/libavformat/flacdec.c b/libavformat/flacdec.c
index ea803f57dd..8394e47483 100644
--- a/libavformat/flacdec.c
+++ b/libavformat/flacdec.c
@@ -31,6 +31,8 @@
#define SEEKPOINT_SIZE 18
typedef struct FLACDecContext {
+AVClass *class;
+int raw_packet_size;
int found_seektable;
} FLACDecContext;
@@ -327,6 +329,7 @@ static int flac_seek(AVFormatContext *s, int stream_index,
int64_t timestamp, in
return -1;
}
+FF_RAW_DEMUXER_CLASS(flac)
AVInputFormat ff_flac_demuxer = {
.name = "flac",
.long_name = NULL_IF_CONFIG_SMALL("raw FLAC"),
@@ -339,4 +342,5 @@ AVInputFormat ff_flac_demuxer = {
.extensions = "flac",
.raw_codec_id = AV_CODEC_ID_FLAC,
.priv_data_size = sizeof(FLACDecContext),
+.priv_class = _demuxer_class,
};
diff --git a/libavformat/g722.c b/libavformat/g722.c
index 2feec01211..fe8c4ae7bb 100644
--- a/libavformat/g722.c
+++ b/libavformat/g722.c
@@ -46,6 +46,7 @@ static int g722_read_header(AVFormatContext *s)
return 0;
}
+FF_RAW_DEMUXER_CLASS(g722)
AVInputFormat ff_g722_demuxer = {
.name = "g722",
.long_name = NULL_IF_CONFIG_SMALL("raw G.722"),
@@ -54,4 +55,5 @@ AVInputFormat ff_g722_demuxer = {
.flags = AVFMT_GENERIC_INDEX,
.extensions = "g722,722",
.raw_codec_id = AV_CODEC_ID_ADPCM_G722,
-};
+.priv_data_size = sizeof(FFRawDemuxerContext),
+.priv_class = _demuxer_class,};
diff --git a/libavformat/loasdec.c b/libavformat/loasdec.c
index c7e124e340..e166a5928a 100644
--- a/libavformat/loasdec.c
+++ b/libavformat/loasdec.c
@@ -83,6 +83,7 @@ static int
[FFmpeg-devel] [PATCH 5/5] avcodec/dnxhd_parser: Fix parser when input does not have nicely sized packets
Fixes: out of array access
Fixes:
15522/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_DNXHD_fuzzer-5747756078989312
Found-by: continuous fuzzing process
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer
---
libavcodec/dnxhd_parser.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/libavcodec/dnxhd_parser.c b/libavcodec/dnxhd_parser.c
index 31a3158fea..63b4ff89e1 100644
--- a/libavcodec/dnxhd_parser.c
+++ b/libavcodec/dnxhd_parser.c
@@ -79,8 +79,9 @@ static int dnxhd_find_frame_end(DNXHDParserContext *dctx,
if (remaining <= 0)
continue;
}
+remaining += i - 47;
dctx->remaining = remaining;
-if (buf_size - i + 47 >= dctx->remaining) {
+if (buf_size >= dctx->remaining) {
pc->frame_start_found = 0;
pc->state64 = -1;
dctx->cur_byte = 0;
--
2.22.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".
