[libav-devel] [RFC] [PATCH] lavc: Drop exporting 2-pass encoding stats

Anton Khirnov anton at khirnov.net
Thu Dec 3 21:00:11 CET 2015


Quoting Vittorio Giovara (2015-12-03 19:19:34)
> On Thu, Dec 3, 2015 at 1:04 PM, Anton Khirnov <anton at khirnov.net> wrote:
> >> These variables leaked from mpegvideoenc where are supposedly used for
> >> statistics.
> >
> > "leaked"? I already said it's not statistics. And they are very
> > deliberately exported. One can argue that we don't want to provide this
> > information for whatever reason, but you cannot pretend that they are
> > somehow exported by accident.
> 
> it looked like that to me, there was not a real reason to make these
> fields public (or at least the rationale was not well explained in the
> commit message) and the fact that only mpegenc used them made me very
> suspicious about them.

Very little is explained in commit messages from that time, that does
not mean there was not a reason. Not saying it must necessarily have
been a good one.

> 
> >> However they might very well be private
> >
> > I have no idea what is this trying to say
> 
> I mean to say that there is no reason to have those variables public
> 
> >> and, due to their
> >> absolute lack of documentation, they are hardly used in the wild. There is also
> >> a spurious frame_bits which is used in aacenc but it can be replaced
> >> with a simple local variable.
> >
> > Are you quite sure it's spurious?
> 
> Somewhat, yes, it looks like it's using a global context variable only
> because it's there.

What makes you think it's not trying to export the information to the
caller, however useless it may be?

> 
> >> inability to instruct applications how to use the values
> >
> > What does this mean exactly?
> 
> I mean to say that these variables are not documented at all.

The fact that something is undocumented is not an argument for removing
it, it's an argument for documenting it.

> 
> >> and the presence of better means to share this kind of information
> >
> > Well, you are not using this better means.
> 
> How would you word this sentence instead?

I don't see why it should be mentioned there at all when it's not
relevant to the patch.

You don't need to have ten million arguments here, one is enough if it's
a good one.

-- 
Anton Khirnov


More information about the libav-devel mailing list