| Message ID | km1WYgTN6qXaOrGrInbQ3mWg5lbENGf_A_q0CWgeRy6TfK8bjDN8O-uTi5yhoZ3F8QidcBuF0UcvrJzS8zadrv_3yVu0i_dUSuFaoaLBuaY=@protonmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [FFmpeg-devel] libavcodec/libx264: fix reference frame computation based on level | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
Hi, > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > josh.brewster@protonmail.com > Sent: Friday, April 17, 2020 07:05 > To: ffmpeg-devel@ffmpeg.org > Subject: [FFmpeg-devel] [PATCH] libavcodec/libx264: fix reference frame > computation based on level > > Hell, can someone please review this patch? It fixes a wrong reference frame > computation problem when using parameters such as "-level 31" instead of > "-level 3.1". > > diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c > index a08fe0ce76..1149f2d668 100644 > --- a/libavcodec/libx264.c > +++ b/libavcodec/libx264.c > @@ -701,11 +701,14 @@ FF_ENABLE_DEPRECATION_WARNINGS > > if (!strcmp(x4->level, "1b")) { > level_id = 9; > - } else if (strlen(x4->level) <= 3){ > + } else if (av_strtod(x4->level, NULL) < 7){ > level_id = av_strtod(x4->level, &tail) * 10 + 0.5; > if (*tail) > level_id = -1; > } > + else { Wrong coding style for "else", should be "} else {" > + level_id = av_strtod(x4->level, NULL); > + } > if (level_id <= 0) > av_log(avctx, AV_LOG_WARNING, "Failed to parse level\n"); This part of code of parsing level_id seems redundant, since PARSE_X264_OPT("level", level) has been called and did the same thing inside libx264, which converts x4->level to x4->params.i_level_idc correctly (equals 31), regardless of "-level 3.1 or level 31". Hence it would be better to use x4->params.i_level_idc directly. - Linjie
From 9f9dcb3cceebb360468fea762b01780f65764a47 Mon Sep 17 00:00:00 2001 From: Josh Brewster <josh.brewster@protonmail.com> Date: Thu, 16 Apr 2020 22:50:29 +0200 Subject: [PATCH] libavcodec/libx264: fix reference frame computation based on level The current implementation allows passing levels to libavcodec as integers (such as "31" instead of "3.1"). However, in this case, the maximum reference frame value per level was ignored because libavcodec converted the string to 310 instead of 31. This commit changes the way levels are converted to int from strings, following an algoritm similar to that of x264 (currently defined in common/base.c). Signed-off-by: Josh Brewster <josh.brewster@protonmail.com> --- libavcodec/libx264.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c index a08fe0ce76..1149f2d668 100644 --- a/libavcodec/libx264.c +++ b/libavcodec/libx264.c @@ -701,11 +701,14 @@ FF_ENABLE_DEPRECATION_WARNINGS if (!strcmp(x4->level, "1b")) { level_id = 9; - } else if (strlen(x4->level) <= 3){ + } else if (av_strtod(x4->level, NULL) < 7){ level_id = av_strtod(x4->level, &tail) * 10 + 0.5; if (*tail) level_id = -1; } + else { + level_id = av_strtod(x4->level, NULL); + } if (level_id <= 0) av_log(avctx, AV_LOG_WARNING, "Failed to parse level\n"); -- 2.26.0