From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 3027240A77 for ; Sat, 5 Mar 2022 19:19:11 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id EA2FA68B0DB; Sat, 5 Mar 2022 21:19:09 +0200 (EET) Received: from mail8.parnet.fi (mail8.parnet.fi [77.234.108.134]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 9E73F680266 for ; Sat, 5 Mar 2022 21:19:02 +0200 (EET) Received: from mail9.parnet.fi (mail9.parnet.fi [77.234.108.21]) by mail8.parnet.fi with ESMTP id 225JJ1qN020859-225JJ1qO020859 for ; Sat, 5 Mar 2022 21:19:01 +0200 Received: from foo.martin.st (host-97-187.parnet.fi [77.234.97.187]) by mail9.parnet.fi (Postfix) with ESMTPS id 4F9FAA142C for ; Sat, 5 Mar 2022 21:19:01 +0200 (EET) Date: Sat, 5 Mar 2022 21:18:59 +0200 (EET) From: =?ISO-8859-15?Q?Martin_Storsj=F6?= To: FFmpeg development discussions and patches In-Reply-To: <20220305101544.507071-1-onemda@gmail.com> Message-ID: <9e7bfc61-9758-5dca-dd26-63be0e994b1@martin.st> References: <20220305101544.507071-1-onemda@gmail.com> MIME-Version: 1.0 X-FE-Policy-ID: 3:14:2:SYSTEM Subject: Re: [FFmpeg-devel] [PATCH] avcodec/dnxhdenc: retry increasing qscale to not overflow max_bits X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On Sat, 5 Mar 2022, Paul B Mahol wrote: > Increase mb_bits type from uint16_t to uint32_t to fix possible oveflows > in bit size calculations. > > Update fate test that needs change. > > Signed-off-by: Paul B Mahol > --- > libavcodec/dnxhdenc.c | 8 +++++--- > libavcodec/dnxhdenc.h | 2 +- > tests/ref/lavf/mxf_opatom | 4 ++-- > 3 files changed, 8 insertions(+), 6 deletions(-) > > diff --git a/libavcodec/dnxhdenc.c b/libavcodec/dnxhdenc.c > index 374fec499b..9bdecec284 100644 > --- a/libavcodec/dnxhdenc.c > +++ b/libavcodec/dnxhdenc.c > @@ -924,7 +924,7 @@ static void dnxhd_setup_threads_slices(DNXHDEncContext *ctx) > unsigned mb = mb_y * ctx->m.mb_width + mb_x; > ctx->slice_size[mb_y] += ctx->mb_bits[mb]; > } > - ctx->slice_size[mb_y] = (ctx->slice_size[mb_y] + 31) & ~31; > + ctx->slice_size[mb_y] = (ctx->slice_size[mb_y] + 31U) & ~31U; > ctx->slice_size[mb_y] >>= 3; > thread_size = ctx->slice_size[mb_y]; > offset += thread_size; > @@ -1220,17 +1220,19 @@ static int dnxhd_encode_fast(AVCodecContext *avctx, DNXHDEncContext *ctx) > avctx->execute2(avctx, dnxhd_mb_var_thread, > NULL, NULL, ctx->m.mb_height); > radix_sort(ctx->mb_cmp, ctx->mb_cmp_tmp, ctx->m.mb_num); > +retry: > for (x = 0; x < ctx->m.mb_num && max_bits > ctx->frame_bits; x++) { > int mb = ctx->mb_cmp[x].mb; > int rc = (ctx->qscale * ctx->m.mb_num ) + mb; > max_bits -= ctx->mb_rc[rc].bits - > ctx->mb_rc[rc + ctx->m.mb_num].bits; > - ctx->mb_qscale[mb] = ctx->qscale + 1; > + if (ctx->mb_qscale[mb] < 255) > + ctx->mb_qscale[mb]++; > ctx->mb_bits[mb] = ctx->mb_rc[rc + ctx->m.mb_num].bits; > } > > if (max_bits > ctx->frame_bits) > - return AVERROR(EINVAL); > + goto retry; > } > return 0; > } > diff --git a/libavcodec/dnxhdenc.h b/libavcodec/dnxhdenc.h > index 30ae8c15e3..7726a3915f 100644 > --- a/libavcodec/dnxhdenc.h > +++ b/libavcodec/dnxhdenc.h > @@ -100,7 +100,7 @@ typedef struct DNXHDEncContext { > unsigned qscale; > unsigned lambda; > > - uint16_t *mb_bits; > + uint32_t *mb_bits; > uint8_t *mb_qscale; > > RCCMPEntry *mb_cmp; > diff --git a/tests/ref/lavf/mxf_opatom b/tests/ref/lavf/mxf_opatom > index e34cf2559e..359422ce9a 100644 > --- a/tests/ref/lavf/mxf_opatom > +++ b/tests/ref/lavf/mxf_opatom > @@ -1,3 +1,3 @@ > -aab6397829bd90f0c77a3f9fde53bb9c *tests/data/lavf/lavf.mxf_opatom > +b1e32792b835ac51b0b1d5dcb9497e4e *tests/data/lavf/lavf.mxf_opatom > 4717625 tests/data/lavf/lavf.mxf_opatom > -tests/data/lavf/lavf.mxf_opatom CRC=0xf55aa22a > +tests/data/lavf/lavf.mxf_opatom CRC=0xb13ba2f8 > -- > 2.33.0 Thanks! I'm not very familiar with the codebase so I can't say for sure, but the reasoning sounds sensible, and if it fixes the test, I'm all for it, so please go ahead! // Martin _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".