[FFmpeg-devel] [PATCH] Fix crash in libx264 when rc_eq is not set

[Jason Garrett-Glaser]

On Mon, Aug 25, 2008 at 3:39 PM, Stefano Sabatini
<stefano.sabatini-lala at poste.it> wrote:
> On date Monday 2008-08-25 21:24:39 +0200, Stefano Sabatini encoded:
>> On date Monday 2008-08-25 12:21:25 -0600, Loren Merritt encoded:
>> > On Mon, 25 Aug 2008, M?ns Rullg?rd wrote:
>> >> Stefano Sabatini <stefano.sabatini-lala at poste.it> writes:
>> >>> On date Monday 2008-08-25 17:41:05 +0200, Guillaume POIRIER encoded:
>> >>>>
>> >>>> no need to. x264 won't need rc_eq to be set in a few days...
>> >>>
>> >>> But older libx264 versions will lye around for a long time, so IMO is
>> >>> still a good idea to fix the problem.
>> >>
>> >> But we need to support the latest x264, which won't even have that
>> >> field in the struct.  What does old/current x264 do if it isn't
>> >> touched at all?
>> >
>> > x264_param_default sets rc_eq to a sane value. The problem only occurs
>> > when ffmpeg overrides that and sets it to null. So removing the line from
>> > ffmpeg will work with all x264 versions.
>>
>> Patch attached, OK to apply?
>>
>> Regards.
>> --
>> FFmpeg = Foolish & Freak Multipurpose Portable Entertaining God
>
>> Index: libavcodec/libx264.c
>> ===================================================================
>> --- libavcodec/libx264.c      (revision 14964)
>> +++ libavcodec/libx264.c      (working copy)
>> @@ -253,7 +253,6 @@
>>      x4->params.rc.f_ip_factor = 1/fabs(avctx->i_quant_factor);
>>      x4->params.rc.f_pb_factor = avctx->b_quant_factor;
>>      x4->params.analyse.i_chroma_qp_offset = avctx->chromaoffset;
>> -    x4->params.rc.psz_rc_eq = avctx->rc_eq;
>>
>>      x4->params.analyse.b_psnr = avctx->flags & CODEC_FLAG_PSNR;
>>      x4->params.i_log_level = X264_LOG_DEBUG;
>
> Again I hit the button too quickly... this it doesn't make sense to
> me, how is it then possible to set the rc_eq? Maybe I should leave this
> to someone else (libx264 maintainer?).

The default rceq is fine--I cannot imagine any reason one would need
to change it--which is why we are removing eval.c from x264 in a few
days anyways.

Dark Shikari