[FFmpeg-devel] [PATCH 2/2] lavc/ratecontrol: Fix error logging for parsing and evaluation of rc_eq
Alexander Strasser
eclipse7 at gmx.net
Sun Aug 9 15:24:34 EEST 2020
Am 9. August 2020 12:56:53 MESZ schrieb Michael Niedermayer <michael at niedermayer.cc>:
>On Fri, Aug 07, 2020 at 11:26:58PM +0200, Alexander Strasser wrote:
>> Don't pass a potential NULL pointer to av_log, instead use a default
>> in the rc_eq AVOption definitions. Now the context variable always
>> holds a string; even if it's the default expression.
>>
>> Note this also fixes the evaluation error path, where a similar
>> av_log call references the rc_eq string from the context too.
>>
>> Signed-off-by: Alexander Strasser <eclipse7 at gmx.net>
>> ---
>> libavcodec/mpegvideo.h | 2 +-
>> libavcodec/ratecontrol.c | 3 +--
>> libavcodec/snowenc.c | 2 +-
>> 3 files changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/libavcodec/mpegvideo.h b/libavcodec/mpegvideo.h
>> index 29e692f245..d2515a3bbf 100644
>> --- a/libavcodec/mpegvideo.h
>> +++ b/libavcodec/mpegvideo.h
>> @@ -637,7 +637,7 @@ FF_MPV_OPT_CMP_FUNC, \
>> "defined in the section 'Expression Evaluation', the
>following functions are available: "
> \
>> "bits2qp(bits), qp2bits(qp). Also the following constants
>are available: iTex pTex tex mv "
> \
>> "fCode iCount mcVar var isI isP isB avgQP qComp avgIITex
>avgPITex avgPPTex avgBPTex avgTex.",
> \
>> -
>FF_MPV_OFFSET(rc_eq), AV_OPT_TYPE_STRING,
>.flags = FF_MPV_OPT_FLAGS }, \
>> +
>FF_MPV_OFFSET(rc_eq), AV_OPT_TYPE_STRING, {.str = "tex^qComp" },
>.flags = FF_MPV_OPT_FLAGS }, \
>> {"rc_init_cplx", "initial complexity for 1-pass encoding",
>FF_MPV_OFFSET(rc_initial_cplx), AV_OPT_TYPE_FLOAT, {.dbl = 0 },
>-FLT_MAX, FLT_MAX, FF_MPV_OPT_FLAGS}, \
>> {"rc_buf_aggressivity", "currently useless",
>FF_MPV_OFFSET(rc_buffer_aggressivity), AV_OPT_TYPE_FLOAT, {.dbl = 1.0
>}, -FLT_MAX, FLT_MAX, FF_MPV_OPT_FLAGS}, \
>> {"border_mask", "increase the quantizer for macroblocks close to
>borders", FF_MPV_OFFSET(border_masking), AV_OPT_TYPE_FLOAT, {.dbl = 0
>}, -FLT_MAX, FLT_MAX, FF_MPV_OPT_FLAGS}, \
>> diff --git a/libavcodec/ratecontrol.c b/libavcodec/ratecontrol.c
>> index 6b77ccd006..03513177f7 100644
>> --- a/libavcodec/ratecontrol.c
>> +++ b/libavcodec/ratecontrol.c
>> @@ -515,8 +515,7 @@ av_cold int ff_rate_control_init(MpegEncContext
>*s)
>> s->avctx->rc_max_available_vbv_use = 1.0;
>> }
>>
>> - res = av_expr_parse(&rcc->rc_eq_eval,
>> - s->rc_eq ? s->rc_eq : "tex^qComp",
>> + res = av_expr_parse(&rcc->rc_eq_eval, s->rc_eq,
>> const_names, func1_names, func1,
>> NULL, NULL, 0, s->avctx);
>
>what happens if the user sets rc_eq explicitly to NULL ?
Not sure which user you mean.
If I'm not mistaken, a library user should not set it to NULL. Is it even possible while respecting public API?
If an internal user like an encoder sets rc_eq explicitly to NULL it will of course crash like in Snow without my previous patch
I guess I don't know what you prefer. Thus I'm waiting for a reply before attempting a new patch set if it's needed.
Alexander
[...]
More information about the ffmpeg-devel
mailing list