> -----Original Message-----
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Carl Eugen Hoyos
> Sent: Friday, January 11, 2019 1:19 AM
> To: FFmpeg development discussions and patches <ffmpeg-
> de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH V8 2/2] avcodec/libx264: add support
> for ROI-based encoding
> 
> 2019-01-10 9:54 GMT+01:00, Guo, Yejun <yejun....@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.

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


> or similar? A struct can hardly return an error, no?

it is caller's responsibility to set the value to be sizeof(AVRegionOfInterest).
There will be an error if the caller does not set it explicitly.

> 
> Sorry for the late comment, Carl Eugen
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to