On 2020-08-02 20:49 +0200, Michael Niedermayer wrote: > On Sat, Aug 01, 2020 at 11:23:36PM +0200, Alexander Strasser wrote: > > 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 > > The user should definitly be able to set rc_eq as (s)he prefers for snow too > > thx
Thanks for the comment. New patch set finally sent. Alexander _______________________________________________ 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".
