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

Vittorio Giovara vittorio.giovara at gmail.com
Mon Jan 7 14:35:45 EET 2019


On Sat, Jan 5, 2019 at 2:15 AM James Almer <jamrial at gmail.com> wrote:

> On 1/4/2019 10:08 AM, Vittorio Giovara wrote:
> > On Fri, Jan 4, 2019 at 12:22 PM Nicolas George <george at nsup.org> wrote:
> >
> >> Rostislav Pehlivanov (12019-01-04):
> >>>> +typedef struct AVRegionOfInterest {
> >>>> +    size_t self_size;
> >>>> +    size_t top;
> >>>> +    size_t bottom;
> >>>> +    size_t left;
> >>>> +    size_t right;
> >>> I'd still much rather have uints with fixed sizes than these platform
> >>> dependent types.
> >>
> >> Guo, Yejun said:
> >>
> >>>> I usually choose 'size_t' for the meanings with length/size.
> >>
> >> But that is a mistake. size_t is for length/size of objects in memory,
> >> not any length/size.
> >>
> >> These numbers, unless I am mistaken, are coordinates within an AVFrame.
> >> In that case, the only correct type is the same as AVFrame.width and
> >> AVFrame.height.
> >>
> >
> > I personally disagree, what are coordinates within an AVFrame if not the
> > length/size of an object in memory?
> > A buffer containing video data is still an object in memory after all, so
> > IMHO using size_t makes a lot of sense here.
>
> 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, and it will generate the same issues we already
> experienced with AVSphericalMapping in fate tests.
>
>
Just for the sake of information, the issue with the spherical fate tests
is that some mov tests included the side data size which was thankfully
removed because it did not made any sense. Even then, the
AVSphericalMapping structure was changed only to make it more similar to
the specification -- this is not the case here: size_t represents a length
so using it for video dimensions makes perfect sense. That being said, use
whatever type you want, as long as it's correctly documented and argumented.
Regards
-- 
Vittorio


More information about the ffmpeg-devel mailing list