[FFmpeg-devel] [PATCH] avcodec: Require avoptions for the user to set max_pixels.

[wm4]

On Mon, 12 Dec 2016 19:55:59 +0100
Michael Niedermayer <michael at niedermayer.cc> wrote:

> On Mon, Dec 12, 2016 at 05:59:32PM +0100, wm4 wrote:
> > On Mon, 12 Dec 2016 17:34:15 +0100
> > Michael Niedermayer <michael at niedermayer.cc> wrote:
> >   
> > > On Mon, Dec 12, 2016 at 09:48:01AM +0100, wm4 wrote:  
> > > > On Mon, 12 Dec 2016 03:34:27 +0100
> > > > Michael Niedermayer <michael at niedermayer.cc> wrote:
> > > >     
> > > > > On Sun, Dec 11, 2016 at 10:51:08PM -0300, James Almer wrote:    
> > > > > > On 12/11/2016 10:29 PM, Michael Niedermayer wrote:      
> > > > > > > On Sat, Dec 10, 2016 at 09:52:08PM -0300, James Almer wrote:      
> > > > > > >> On 12/10/2016 9:23 PM, Michael Niedermayer wrote:      
> > > > > > >>> On Sat, Dec 10, 2016 at 08:31:57PM -0300, James Almer wrote:      
> > > > > > >>>> On 12/10/2016 7:01 PM, Michael Niedermayer wrote:      
> > > > > > > [...]
> > > > > > >       
> > > > > > >>> And also theres more work for us to maintain seperate implementations
> > > > > > >>> for the options, all code accessing them has to deal with them being
> > > > > > >>> in different places, making all related backports harder.
> > > > > > >>>
> > > > > > >>> To me that looks like a disadvantage from every side
> > > > > > >>>
> > > > > > >>> I think the real solution is to start liking AVOptions, they make
> > > > > > >>> our work easier.      
> > > > > > >>
> > > > > > >> AVOptions are fine. Private-but-not-really and no-direct-access fields
> > > > > > >> in public structs are what's kinda ugly an unpopular.      
> > > > > > > 
> > > > > > > a slightly off topic question but
> > > > > > > if people care about these existing "no direct access" fields
> > > > > > > why do they not change them ?
> > > > > > > its a bit reading and thinking and droping the "no direct access"
> > > > > > > comments, this is not much work
> > > > > > > It requires a tiny bit of care so that future added fields dont do
> > > > > > > bad things and we should document that past releases still in some
> > > > > > > cases need the indirect access  ...
> > > > > > > 
> > > > > > > just seems a bit odd to me in relation to the opposition to add such
> > > > > > > a field were its needed for a bugfix but total apparent lack of
> > > > > > > interrest in removing such "no direct access" restrctions  where there
> > > > > > > is no reason at all to keep them      
> > > > > > 
> > > > > > They are all supposed to stop being no direct access with the next bump.
> > > > > > Afaik most are remnants of the libav ABI compatibility days, same with
> > > > > > the get/setter functions, but they can't just be removed right now because
> > > > > > there are several versions using the current ABI (3.0 and newer i think).      
> > > > > 
> > > > > theres no need to wait for the next bump with removing many of the
> > > > > "no direct access" rules because the fields always where in the same
> > > > > position in the current major version
> > > > > 
> > > > > Removing them before the bump would allow people to stop using
> > > > > the accessors earlier
> > > > > 
> > > > > someone needs to read through the struct and think about it though
> > > > > so nothing is missed but from just the diff between master and
> > > > > the releases there seem to be many that did not move    
> > > > 
> > > > Can I send a patch that makes them "real" public fields, and which
> > > > deprecates the accessors? In all libs?    
> > > 
> > > If someone first checks that they are in the same offset in their
> > > structs in all FFmpeg releases with the same soname.
> > > And someone checks that we have no "insert new ... field above/below
> > > this line" things above them
> > > 
> > > Yes, definitly  
> > 
> > The offsets wouldn't have to be the same, since they were allowed to be
> > accessed by accessor only.  
> 
> thats true but someone writing code to access it would likely use
> the latest documentation and if that doesnt mention that earlier
> versions needed a different access then likely not realize and as
> that app would build and work fine against a release he wouldnt notice
> until that app built against a older release is then linked to a
> newer

We've never cared about such cases before.