[FFmpeg-devel] [RFC] Fixing libx264.c
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
> 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
Size: 189 bytes
Desc: Digital signature
More information about the ffmpeg-devel