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

[Rostislav Pehlivanov]

On Fri, 4 Jan 2019 at 20:44, Vittorio Giovara <vittorio.giovara at gmail.com>
wrote:

> On Fri, Jan 4, 2019 at 7:57 PM Rostislav Pehlivanov <atomnuker at gmail.com>
> wrote:
>
> > On Fri, 4 Jan 2019 at 16:28, Vittorio Giovara <
> vittorio.giovara at gmail.com>
> > wrote:
> >
> > > 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.
> > >
> >
> > Hence we should avoid making a new one now.
>
>
> Right, we should do things that make sense and argument them with something
> more than saying "it's confusing".
>
> Pixels shouldn't have the size_t type, it just makes things confusing.
> >
> --
> Vittorio
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Its illogical to have pixels be typed with a size_t, its not something
we've done before either (which makes sense), and its different size across
platforms doesn't help nor does it keep things consistent when there's no
reason for them to be different.
Its a noop type change on 32 bit arches anyway as uint32_t == size_t. Won't
break anything. And the author's having to change another field in the
struct anyway so its not like its more work.
All you've argued thus far for is that since no one said anything for a
month, and that its the author's choice, it should remain that way, and
that coordinates within an avframe are memory sizes (which they can be, but
width and height are not, they're in pixels, and I'd argue they should
remain such). I don't see the reasoning here.