[FFmpeg-devel] [PATCH V5 1/2] avutil: add ROI (Region Of Interest) data struct and bump version

[Guo, Yejun]

thanks a lot for all your valuable comments.

> -----Original Message-----
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf
> Of Nicolas George
> Sent: Saturday, January 05, 2019 12:51 PM
> To: FFmpeg development discussions and patches <ffmpeg-
> devel at ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH V5 1/2] avutil: add ROI (Region Of
> Interest) data struct and bump version
> 
> James Almer (12019-01-04):
> > We already did size_t for AVSphericalMapping and had to change them to
> > uint32_t.
> > size_t is not the correct type at all, as video dimensions are not
> > system dependent,
> 
> I fully agree with that.
> 
> >		    and it will generate the same issues we already
> experienced with
> >AVSphericalMapping in fate tests.
> 
> I disagree with that: for some things, size_t would be the correct type, and in
> that case, the FATE test should work. A misdesigned FATE test should not
> direct the choice of type. Also, IIUC, somebody said the issue was fixed.
> 
> For me, the definite argument is: All the rest of the code uses int for frame
> size and coordinate, so new code should use int too unless there is a very
> good and specific reason.
> 
> Note that while we disagree on the reasons, I think we agree on the
> conclusion, which is what matter.
> 
> Regards,
> 
> --
>   Nicolas George
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

For the type of ROI coordinate, in struct AVFrame, there are ' int width, height;'  and  'size_t crop_top;' as reference, I'll choose int.

For the type of qoffset, I'll change to AVRational.