[FFmpeg-devel] [RFC] Fixing libx264.c

Michael Niedermayer michaelni
Fri May 22 04:50:48 CEST 2009


On Thu, May 21, 2009 at 06:57:41PM -0700, Jason Garrett-Glaser wrote:
> I am sick of complaints of crappy quality, crappy streams, and general
> brokenness from the ffmpeg libx264 encoder.  This gets worse with
> ffserver, which doesn't pass all of the x264-appropriate options, so
> you can't even fix the options even if you know every single one of
> them. Per-codec defaults would solve the problem, but hardly anyone
> seems to want to back Baptiste in promoting this.  I could also cause
> ffmpeg to terminate if libx264 is selected as vcodec without an
> appropriate preset, but that would be obnoxious and would break
> ffserver.
> 
> I could also modify x264 itself to insta-terminate if it detects
> ffmpeg-level broken options, which may result in enough complaints
> that it gets fixed.  But that would be even more obnoxious.
> 
> I am all-ears for other ideas--but here's my list of problems and
> proposed solutions.  I have patches for these already, but there's no
> point in posting them until something is agreed upon, as they are
> trivial.  I do maintain libx264.c, but this needs discussion before I
> unilaterally modify tons of stuff.
> 

> 1.  ffmpeg sets the scenecut threshold to zero (disabled).
> Why it's broken currently: scenecut threshold at zero implies no
> scenecut detection.
> Solution: Set the default to something sane, or disconnect the option.
>  Does this affect the core mpeg encoder?

AVCodecContext.scenechange_threshold == 0 means default, not disabled
disabled would be INT_MAX
Its a simple bug in libx264.c of not translating from lavc API to x264 API

[...]


> 
> 3.  ffmpeg sets directpred to "temporal".
> Why it's broken currently: directpred temporal breaks horrifically
> when b-pyramid is applied, so it's easy to mix the two improperly
> (spatial is a better default anyways, and is the one x264 uses).
> Solution: set the default to spatial in options.c (which it should be).

its a little annying semantically as mpeg4 asp has no spatial and thus
strictly mpeg4 should fail with the default.
maybe none or auto would be better defaults.

though i have no real objection to change it to spatial if you want, its
just a little dirty for codecs that dont support spatial


> 
> 4.  ffmpeg turns off chroma ME.
> Why it's broken currently: It's forced off below subme=5 anyways, and
> above it's practically always useful.
> Solution: Disconnect the option.
> 
> 5.  ffmpeg turns off fast pskip.
> Why it's broken currently: This should practically always be on; it
> costs a lot of speed to leave it off (for no real benefit).
> Solution: Disconnect the option, or if only x264 uses the option, set
> it on by default.

> 8.  ffmpeg turns off adaptive b-frame placement by default.
> Why it's broken: Non-adaptive B-frame placement seriously sucks.  But
> this option is actually useful (0-off, 1-fast, 2-optimal), so I don't
> want to disconnect it.
> Solution: This is very important, but I can't come up with one.
> 
> 9.  Partitions are all off by default.
> Why it's broken: Turning off all partitions cripples compression.
> Partitions are actually useful to tweak, but the defaults should be
> "all on except p4x4," not all off.
> Solution: Adjust options.c to force the defaults correctly, since only
> x264 uses the partition options.
> 
> 10.  Loop filter is off by default.
> Why it's broken: Nobody should be turning it off except maybe to
> encode for some really slow CPU or something.  Critical for
> compression quality, costs nearly no speed.
> Solution: Turn it on by default?  Might affect other codecs.  Or
> disconnect the option.

The defaults in ffmpeg are generally all stuff disabled, changing that
requires per codec defaults as otherwise codecs would per default receive
settings they dont support (like a loop filter for mpeg1)


> 
> 6.  ffmpeg sets bitrate tolerance to a single default value regardless
> of input bitrate.
> Why it's broken currently: x264's ratecontrol doesn't like it when you
> start applying extremely small values to bitrate tolerance.  It's easy
> to set a very high bitrate and leave the low bitrate tolerance in, and
> the results can be rather crappy.
> Solution: Disconnect the option, or set it in ffmpeg.c in the same
> manner in which rc_initial_buffer_occupancy is set (see lines
> 3139-3140).  This would affect other encoders.

i suspect this issue affect all rc encoders so iam in favor of it being
set dynamically in ffmpeg.c


> 
> 7.  ffmpeg messes with ipratio/pbratio.
> Why it's broken currently: The values are tweaked for MPEG quant
> scale, not H.264.
> Solution: Disconnect the option (it's not very useful) or ignore it,
> since the values aren't too badly off.

hmm, are our options here even semantically identical to x264s?
qscale*x isnt the same as log(qscale)*x or am i missing something?


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090522/52c40d52/attachment.pgp>



More information about the ffmpeg-devel mailing list