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

Vittorio Giovara vittorio.giovara at gmail.com
Fri Jan 4 18:28:03 EET 2019


On Fri, Jan 4, 2019 at 2:37 PM Nicolas George <george at nsup.org> wrote:

> Vittorio Giovara (12019-01-04):
> > I personally disagree, what are coordinates within an AVFrame if not the
> > length/size of an object in memory?
>
> That would be an argument for making AVFrame.width and AVFrame.height
> size_t. But they are not, and therefore these ROI values have no reason
> to be either. There is no point in being able to express ROI coordinates
> in the quadrillion when the size of the frame is bounded by much less.
>

That seems a poor argument since the code base is so old that there are a
plethora of bad design decisions that should not dictate what choices are
made now.
size_t really seems the right choice here, and since it's the one chosen by
the author of the patch (revision 5) I think we should respect that.
Using AVRational instead of float for qoffset has the added benefit of
making the field encoder independent, so it's a sensible change, while
using uint32 instead of sizet does not bring anything to the table, as far
as I can tell, so let's not ask Yejun to change it.
-- 
Vittorio


More information about the ffmpeg-devel mailing list