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

[Stefano Sabatini]

On date Monday 2008-08-25 22:46:34 +0200, Michael Niedermayer encoded:
> On Mon, Aug 25, 2008 at 03:51:36PM -0400, Jason Garrett-Glaser wrote:
> > 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-
> 
> Well i can imagine a few reasons, after all iam the one who originally
> wrote this stuff ...
> The original idea IIRC was that some user may wish to increase or
> decrease the number of bits available to some things like frames differing
> more from previous frames, frames with long motion vectors or complex
> motion or ...
> Iam well aware that either it isnt usefull in practice or its to complex
> for everyone who might want to change it ...

I'm also for keeping that feature.

Ideally libx264 should be able to set its default value if libavcodec
leaves it to NULL, but this would break (at least) old libx264
versions.

So I propose again my first patch, which I attach.

Regards.
-- 
FFmpeg = Frightening and Faboulous Meaningless Prodigious Ecstatic Guide
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix-libx64-rc-eq-crash-00.patch
Type: text/x-diff
Size: 616 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080826/a1543396/attachment.patch>