[FFmpeg-devel] [PATCH] avcodec: Add AVClass to AVCodecParameters

[Michael Niedermayer]

On Tue, May 17, 2016 at 10:21:51AM -0400, Ronald S. Bultje wrote:
> Hi,
> 
> On Tue, May 17, 2016 at 10:04 AM, Michael Niedermayer <
> michael at niedermayer.cc> wrote:
> 
> > On Tue, May 17, 2016 at 06:49:44AM -0400, Ronald S. Bultje wrote:
> > > Hi,
> > >
> > > On Tue, May 17, 2016 at 6:07 AM, Michael Niedermayer
> > <michael at niedermayer.cc
> > > > wrote:
> > >
> > > > Most of the major public structures contain an AVClass.
> > >
> > >
> > > What are you hoping to accomplish with this struct that can currently not
> > > be done because it doesn't have an AVClass on top? I haven't seen this
> > > question answered anywhere yet, I think.
> >
> > Its primarely for consistency and cleanup
> > presenting a consistent API to the end user and not one thats
> > different for each struct
> >
> > What iam missing is a discussion about
> > 1. should all public structs have a AVClass, should some, should none
> > 2. should all public structs be accessible through AVOptions, should
> >    some, should none
> >
> > and then the resulting consens to be applied everywhere consistently
> >
> > what we have is that sometimes things get done one way and the next
> > time they get done the opposit way.
> > AVClass is one example, we for example use it for nearly all of the
> > many codecs private contexts, its used for most public structs
> > but here with AVCodecParameters theres a wall of people who seem
> > against it.
> 
> 
> I think we have to move away (for just one second) from "how it works" to
> "why does it work that way". Let me give an example:
> 
> Question: why do (some) private codec contexts have AVClass?
> Answer: private codec options!
> 
> OK, that was really simple. So now, we can answer the inverse question:
> 
> Question: which private codec contexts should have an AVClass?
> Answer: those that use private codec options!
> 
> Note that this might not be the only reason private codec contexts use
> AVClass. There may be other reasons, and as we extend the "why" cases, the
> "which" Q&A needs to be updated accordingly.
> 
> So, to answer the question of whether AVCodecParameter should have an
> AVClass associated with it, I believe we should answer the question why it
> should have one. Consistency might be a reason, but is a weak one, since
> we're entirely inconsistent anyway. Consistency can only be applied
> globally, not to this one specific case, because then by definition, it is
> not actually any more consistent.

we are 100% consistent in some areas like codecs from a public API
point of view. So its not at all "entirely inconsistent anyway"
Yes some private contexts have AVClasses and some
do not, but the public AVCodecContext always has a AVClass and it will
forward options it cannot handle to the private context only when that
supports it. it wont crash if the private context doesnt use an AVClass
The same is true for muxers and demuxers

AVCodecContext contains all the fields that AVCodecParameters has.
Examples are width, height, bitrate, profile, sample_rate, channels

In AVCodecContext these fields are accessible through AVOptions
in AVCodecParameters these fields are not accessible through AVOptions
and your code will crash mysteriously if you try to access them that
way

This is a inconsistency that was not there before


> 
> So, again my question: why do you want AVCodecParameter to have an AVClass?

Its for consistency, and for all the reasons we added AVClass and
AVOptions to other public structs

In every other public struct we added fields and extended
them over time, and user apps had or wanted to access the fields.
With AVOptions that can be done saftely no matter if a field exists
or not user apps need not maintain a list of version checks if they
have to support different libavcodec versions / different distributions
which ship sometimes years outdated libavcodec, ...
or apps want to work with more than one fork of a project, AVOptions
made that easier as it was safe to access a field even if that wasnt
there or was at a different location.

I think someone argued AVCodecParameters is different and user apps
do not need to access it and just pass it between libavformat and
libavcodec.
Now if your goal is to decouple libavcodec and libavformat then
of course the user app needs to access things and not pass "opaque"
structs. That would be like libvorbis and libogg
Someone might want to use libavcodec with some other demuxer lib
or libavformat with other decoders than libavcodec, in these cases
the user app certainly needs to access AVCodecParameters like any
other struct. And i think the same issues will aply

Or another hypothetical case, consider AVStream had a AVClass and
AVOptions
with that the replacement of AVCodecContext by AVCodecParameters
would not even have needed changes in some applications

as applications setting parameters for muxing like width & height via
AVOptions in a AVStream they would get passed into the AVCodecContext
in the old API and into AVCodecParameters in the new without changes

same for reading parameters out of such streams

That is AVClass and AVOptions usage by apps makes the implementation
more  flexible without user apps needing to update.


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

The educated differ from the uneducated as much as the living from the
dead. -- Aristotle 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160517/2ea1a60c/attachment.sig>