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

[Alexander Strasser]

Hi Michael!

On 2020-07-31 19:54 +0200, Michael Niedermayer wrote:
> On Thu, Jul 30, 2020 at 02:42:30PM +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 references the rc_eq string from the context too.
> > ---
> >  libavcodec/mpegvideo.h   | 2 +-
> >  libavcodec/ratecontrol.c | 3 +--
> >  2 files changed, 2 insertions(+), 3 deletions(-)
>
> crashes with:
> ./ffmpeg -i matrixbench_mpeg2.mpg -vcodec snow  -y file.avi
> ==22043== Invalid read of size 1
> ==22043==    at 0x4C32CF2: strlen (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==22043==    by 0x10A83AD: av_expr_parse (in ffmpeg-git/ffmpeg/ffmpeg_g)
> ==22043==    by 0x2858ED: ff_rate_control_init (in ffmpeg-git/ffmpeg/ffmpeg_g)
> ==22043==    by 0x28A668: encode_init (in ffmpeg-git/ffmpeg/ffmpeg_g)
> ==22043==    by 0xA7E2D0: avcodec_open2 (in ffmpeg-git/ffmpeg/ffmpeg_g)
> ==22043==    by 0x2F196A: init_output_stream.constprop.24 (in ffmpeg-git/ffmpeg/ffmpeg_g)
> ==22043==    by 0x2F383A: reap_filters (in ffmpeg-git/ffmpeg/ffmpeg_g)
> ==22043==    by 0x2F78D7: transcode (in ffmpeg-git/ffmpeg/ffmpeg_g)
> ==22043==    by 0x2D093B: main (in ffmpeg-git/ffmpeg/ffmpeg_g)
> ==22043==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
>
> if you cannot reproduce, say so and ill make a more detailed backtrace

Good catch.

I can reproduce all the time. I looked into it and the snow encoder
uses the rate control, that is elsewhere only used by the codecs that
use the MpegEncContext with initialization and common options. So in
case of snow, the rc_eq expression string is always initialized to
the default with the current code and with my patch it isn't
initialized any more.

What should I aim for here?

* Always initializing the rc_eq with the default expression
* Exposing the rc_eq option


[...]

  Alexander