[FFmpeg-devel] [PATCH 2/2] lavc/ratecontrol: Fix error logging for parsing and evaluation of rc_eq

[Michael Niedermayer]

On Sun, Aug 09, 2020 at 02:24:34PM +0200, Alexander Strasser wrote:
> 
> 
> 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.

lib user


> 
> If I'm not mistaken, a library user should not set it to NULL. Is it even possible while respecting public API?

if its not possible then the patch should be fine. I was just asking becasue
thats the thing that would change if it can be set

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Into a blind darkness they enter who follow after the Ignorance,
they as if into a greater darkness enter who devote themselves
to the Knowledge alone. -- Isha Upanishad
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200809/b9408631/attachment.sig>