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

Michael Niedermayer michaelni
Fri May 22 12:32:44 CEST 2009


On Fri, May 22, 2009 at 09:55:59AM +0000, Loren Merritt wrote:
> On Fri, 22 May 2009, Michael Niedermayer wrote:
>> On Thu, May 21, 2009 at 06:57:41PM -0700, Jason Garrett-Glaser wrote:
>>
>>> 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
>
> Is that an official ffmpeg policy: codecs should refuse to encode if you 
> enable any option they don't support?

no, its not official policy

its just that a user asking explicitly for some feature that a encoder doesnt
support, should receive an error message. That being something occasionally
requested ...


> Every time anyone adds a new option 
> for one codec, they have to modify all the other codecs to check it too?

no, this certainly was not my intent.
Rather AVCodecs could have a list of supported options and leave the checking
to some common code ...

That may be during avcodec_open/encode or during parsing/setting of the
options


> Should intra-only codecs refuse to encode with the default gop_size of 12? 
> Should codecs that don't have quantizers refuse to encode if qmin/qmax are 
> set to anything at all?
>
> Surely the only sane thing to do is to ignore all options that are 
> irrelevant to the format being encoded. mpeg4 doesn't have any choice of 
> direct modes, so the direct mode option shouldn't matter.
>
> With the common mpegvideo codepath we do have to check that options used by 
> some mpeglike codecs but not the one in use aren't enabled. But once we're 
> checking that, is it better to error out or to disable the option?

There are a varity of constraining points here
A The user should receive an error if she requests nonsensical options like
  an explicit gop size or b frames with intra only codecs
B Variables should not be set to semantically incorrect values, its a receipe
  for bugs and maintaince issues
C Variables that are used (mpegvideo common code) must not be set to incorrect
  values
D code should of course be simple & readable & small & fast ...

A & B are not really achived currently, C & D are

and again, i dont mind the global default for directpred to be set to spatial
i just think its not a pretty solution and might byte us in the future (when
there is comon code using directpred for example)

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you really think that XML is the answer, then you definitly missunderstood
the question -- Attila Kinali
-------------- 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/f2ee3441/attachment.pgp>



More information about the ffmpeg-devel mailing list