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

[James Almer]

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:
>>> When we will backport this, it will be inevitably in a different location
>>> in AVCodecContext in each release and master. 3.0, 3.1, 3.2 and master
>>> use the same soname though and must have a binary compatible interface.
>>> It thus can only saftely be accessed through AVOptions
>>>
>>> It may be enough to require this only in the releases but that could be
>>> rather confusing.
>>
>> Wouldn't it be better to initially add such new fields inside
>> AVCodecContext->internal and accessible by AVOptions only so they
>> may be trivial to backport, then once a major bump occurs they can
>> be moved to the actual public struct with enabled direct access?
>>
>> Not sure how feasible is to have options_table.h options change
>> values from that internal struct, but it would solve the above
>> concerns.
> 
> AVCodecInternal does not start with a AVClass / AVOption table
> (thats solvable though would need to be done to past releases)
> 
> AVCodecContext->internal is not a child class of AVCodecContext
> (that is solvable too, if we want to take the risk to do that in
>  point releases)
> 
> AVCodecContext->internal is allocated in avcodec_open2() so nothing
> can be set in it by the user before
> (i dont see a solution to this that we would want to backport)
> 
> If you suggest to make these changes now to for future use.
> that will make backports across this change very annoying, as theres
> a point before AVCodecInternal cannot be used.

If this is impossible now for the above reasons then it's something
that could be considered for the next bump, or the one after that
if it requires careful planning and design.
I was simply suggesting something that could prevent backporting
fields on public structs that would not have a fixed offset.

The plan was to undo all the current ones in the next bump now that
we dropped ABI compatibility with Libav, so it would be nice to find
a way to avoid something like that happening again.

> 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.

No objections or other comments for this patch from me.