| Message ID | d5959287-76aa-f904-2b57-f36f10fa0278@googlemail.com |
|---|---|
| State | Superseded |
| Headers | show |
On Fri, Nov 25, 2016 at 12:03:30AM +0100, Andreas Cadhalpun wrote: > On 24.11.2016 17:57, Michael Niedermayer wrote: > > On Thu, Nov 24, 2016 at 05:45:38PM +0100, Michael Niedermayer wrote: > >> Is it correct that your cases uses > >> decode_wmv9() -> vc1_decode_i_blocks() ? > > Yes. > > >> these decode a rectangele up to end_mb_y, end_mb_x > >> does this mismatch with what later code accesses ? > > Yes, s->mb_width and s->mb_height are different from > v->end_mb_x and s->end_mb_y. > > >> would using end_mb_* in the EC code fix this ? > > I'm not sure how this could be done properly, simply setting > s->mb_width and s->mb_height to the other values does not work. replacing the mb_width / mb_height uses in libavcodec/error_resilience.c but its kind of messy and feels wrong also the way MSS2 works with multiple rectangeles doesnt fit into error concealment very well on a per rectangele. It probably should be done per whole image but thats a big change for no real gain. > > >> (or disabling EC if they mismatch) > > > > Note, for this sadly end_mb_* in MSS2 would need to be treated > > differently than other codecs as it has different semantics > > disabling EC on end_mb_ mismatch might be easier > > Disabling error correction in that case works, though. > Attached is a patch for that. > > Best regards, > Andreas > > mss2.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > 884b912643244a4205bac63faedfa0c048bcc97a 0001-mss2-only-use-error-correction-for-matching-block-co.patch > From df9241d8b575cc0fbf570e714c586ff37a4821fd Mon Sep 17 00:00:00 2001 > From: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> > Date: Thu, 24 Nov 2016 23:57:46 +0100 > Subject: [PATCH] mss2: only use error correction for matching block counts > > This fixes a heap-buffer-overflow in ff_er_frame_end when decoding mss2 > with coded_width/coded_height larger than width/height. > > Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> > --- > libavcodec/mss2.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/libavcodec/mss2.c b/libavcodec/mss2.c > index 1e24568..62761e8 100644 > --- a/libavcodec/mss2.c > +++ b/libavcodec/mss2.c > @@ -409,8 +409,6 @@ static int decode_wmv9(AVCodecContext *avctx, const uint8_t *buf, int buf_size, > return ret; > } > > - ff_mpeg_er_frame_start(s); > - > v->bits = buf_size * 8; > > v->end_mb_x = (w + 15) >> 4; > @@ -420,9 +418,18 @@ static int decode_wmv9(AVCodecContext *avctx, const uint8_t *buf, int buf_size, > if (v->respic & 2) > s->end_mb_y = s->end_mb_y + 1 >> 1; > > + if (v->end_mb_x == s->mb_width && s->end_mb_y == s->mb_height) { > + ff_mpeg_er_frame_start(s); > + } else { > + av_log(v->s.avctx, AV_LOG_WARNING, > + "disabling error correction due to block count mismatch %dx%d != %dx%d\n", > + v->end_mb_x, s->end_mb_y, s->mb_width, s->mb_height); > + } > + > ff_vc1_decode_blocks(v); > > - ff_er_frame_end(&s->er); > + if (v->end_mb_x == s->mb_width && s->end_mb_y == s->mb_height) > + ff_er_frame_end(&s->er); there are still ff_er_add_slice() calls in the block decode code i think It seems not to matter but skiping just ff_er_frame_end() and not ff_mpeg_er_frame_start() feels less inconsistent [...]
From df9241d8b575cc0fbf570e714c586ff37a4821fd Mon Sep 17 00:00:00 2001 From: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> Date: Thu, 24 Nov 2016 23:57:46 +0100 Subject: [PATCH] mss2: only use error correction for matching block counts This fixes a heap-buffer-overflow in ff_er_frame_end when decoding mss2 with coded_width/coded_height larger than width/height. Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> --- libavcodec/mss2.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/libavcodec/mss2.c b/libavcodec/mss2.c index 1e24568..62761e8 100644 --- a/libavcodec/mss2.c +++ b/libavcodec/mss2.c @@ -409,8 +409,6 @@ static int decode_wmv9(AVCodecContext *avctx, const uint8_t *buf, int buf_size, return ret; } - ff_mpeg_er_frame_start(s); - v->bits = buf_size * 8; v->end_mb_x = (w + 15) >> 4; @@ -420,9 +418,18 @@ static int decode_wmv9(AVCodecContext *avctx, const uint8_t *buf, int buf_size, if (v->respic & 2) s->end_mb_y = s->end_mb_y + 1 >> 1; + if (v->end_mb_x == s->mb_width && s->end_mb_y == s->mb_height) { + ff_mpeg_er_frame_start(s); + } else { + av_log(v->s.avctx, AV_LOG_WARNING, + "disabling error correction due to block count mismatch %dx%d != %dx%d\n", + v->end_mb_x, s->end_mb_y, s->mb_width, s->mb_height); + } + ff_vc1_decode_blocks(v); - ff_er_frame_end(&s->er); + if (v->end_mb_x == s->mb_width && s->end_mb_y == s->mb_height) + ff_er_frame_end(&s->er); ff_mpv_frame_end(s); -- 2.10.2