| Message ID | a7451140937532e426c960a441ee67a280520ba7.1596835157.git.eclipse7@gmx.net |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/2] lavc/snowenc: Expose an option to set the rc_eq expression | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
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@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 ? thx [...]
Am 9. August 2020 12:56:53 MESZ schrieb Michael Niedermayer <michael@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@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 [...]
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@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@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 [...]
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 <michael@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@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. [...] Alexander
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 <michael@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@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 [...]
On 2020-08-13 22:32 +0200, Michael Niedermayer wrote: > On Wed, Aug 12, 2020 at 08:58:30PM +0200, Alexander Strasser wrote: > > On 2020-08-09 14:56 +0200, Michael Niedermayer wrote: [...] > > > > > > 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 Unfortunately I think ATM it is always possible to set (unset?) a string option to NULL with a simple av_set_opt call. I think in particular in combination with defaults this is problematic. I'm sure many places in our code don't expect a NULL for an AV_OPT_TYPE_STRING field that has a default. What do you and others think about the matter? How should we deal with this? As I see the situation, we could either decide 1. to fix a lot of places to handle NULL? 2. to document for a lot of options that NULL is invalid 3. to disallow a) setting options to NULL b) setting options to NULL if they have a non-NULL default 4. to reset options to their defaults if they are set to NULL This may be a bit over-simplified because of the av_opt APIs and different ways the AV_OPT_TYPE_ opts are treated. So this should probably be looked into a bit closer. [...] Alexander
On Sun, Aug 16, 2020 at 10:57:26PM +0200, Alexander Strasser wrote: > On 2020-08-13 22:32 +0200, Michael Niedermayer wrote: > > On Wed, Aug 12, 2020 at 08:58:30PM +0200, Alexander Strasser wrote: > > > On 2020-08-09 14:56 +0200, Michael Niedermayer wrote: > [...] > > > > > > > > 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 > > Unfortunately I think ATM it is always possible to set (unset?) a > string option to NULL with a simple av_set_opt call. > > I think in particular in combination with defaults this is problematic. > > I'm sure many places in our code don't expect a NULL for an > AV_OPT_TYPE_STRING field that has a default. > > What do you and others think about the matter? > How should we deal with this? > > As I see the situation, we could either decide > > 1. to fix a lot of places to handle NULL? > 2. to document for a lot of options that NULL is invalid > 3. to disallow > a) setting options to NULL > b) setting options to NULL if they have a non-NULL default > 4. to reset options to their defaults if they are set to NULL Are there cases where NULL is needed ? If so, a flag that specifies that the value may be NULL could be a possibility too to seperate the fields that do not allow or support NULL vs the ones where it is needed and supported thx [...]
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); if (res < 0) { diff --git a/libavcodec/snowenc.c b/libavcodec/snowenc.c index 16d2b7c302..71fb5654cd 100644 --- a/libavcodec/snowenc.c +++ b/libavcodec/snowenc.c @@ -1952,7 +1952,7 @@ static const AVOption options[] = { "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.", - OFFSET(m.rc_eq), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, VE }, + OFFSET(m.rc_eq), AV_OPT_TYPE_STRING, { .str = "tex^qComp" }, 0, 0, VE }, { NULL }, };
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@gmx.net> --- libavcodec/mpegvideo.h | 2 +- libavcodec/ratecontrol.c | 3 +-- libavcodec/snowenc.c | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) --