[FFmpeg-devel] [PATCH] mpegpicture: use coded_width/coded_height to allocate frame

[Michael Niedermayer]

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 at 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 at 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

[...]


-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you drop bombs on a foreign country and kill a hundred thousand
innocent people, expect your government to call the consequence
"unprovoked inhuman terrorist attacks" and use it to justify dropping
more bombs and killing more people. The technology changed, the idea is old.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20161125/644fc638/attachment.sig>