[FFmpeg-devel] [PATCH V8 2/2] avcodec/libx264: add support for ROI-based encoding

[Carl Eugen Hoyos]

2019-01-11 1:37 GMT+01:00, Guo, Yejun <yejun.guo at intel.com>:

>> 2019-01-10 9:54 GMT+01:00, Guo, Yejun <yejun.guo at intel.com>:
>>
>> > +                        roi = (AVRegionOfInterest*)((char*)roi +
>> > roi->self_size);
>>
>> Isn't this roi++?
>> If yes, please change this.
>
> no, it's not roi++, the reason is that struct AVRegionOfInterest might be
> extended,
> so to keep ABI compatible, we add the 'self_size'.  It is discussed in V4
> comments.

So after AVRegionOfInterest was extended, you as a user who had
compiled his application against old libavcodec will use new libavcodec
(with the extension) and it will be guaranteed that libx264 will still get
the correct information although only the part of the struct before the
extension was filled?
Does this guarantee really help?

>> I also wonder if the wording (elsewhere) of "returns EINVAL if
>> size is zero" is correct: Shouldn't it be "size must be >0"
>
> self_size is uint32_t, it is > 0 if not zero.

The comment I saw is in another file (probably another patch),
sorry if this is confusing (I also find it confusing).
I will hopefully not be affected by the documentation, I just wanted
to point out that the wording is misleading imo, because structs
do not return errors.

Carl Eugen