On Wed, Aug 12, 2020 at 08:58:30PM +0200, Alexander Strasser wrote:
> On 2020-08-09 14:56 +0200, Michael Niedermayer wrote:
> > On Sun, Aug 09, 2020 at 02:24:34PM +0200, Alexander Strasser wrote:
> > >
> > >
> > > Am 9. August 2020 12:56:53 MESZ schrieb Michael Niedermayer
> > > <mich...@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 <eclip...@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
>
> The rc_eq field was removed from AVCodecContext with the merge
> commit bfab430856 and lives only inside private contexts these
> days.
>
> Now I'm not 100% sure if there is a way to use av_opt API to set a
> string type option's value to NULL. Though it would worry me if that
> is possible, even for options that have a default string.
>
> If you know about that, I would be grateful to hear. Will see
> if I can it try out myself too.One can probably do it (maybe with av_opt_ptr()) how realistic an issue this is, i dont know. For the future we should ensure though its clearly documented if NULL is valid for AV_OPT_TYPE_STRING because part of the issue here is i think its not clearly documented if this is allowed thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB In fact, the RIAA has been known to suggest that students drop out of college or go to community college in order to be able to afford settlements. -- The RIAA
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
