[FFmpeg-devel] [PATCH] Change type of spherical stuff to uint32_t

[James Almer]

On 3/14/2017 12:37 PM, Vittorio Giovara wrote:
> On Sun, Mar 12, 2017 at 11:21 PM, Michael Niedermayer
> <michael at niedermayer.cc> wrote:
>> On Sun, Mar 12, 2017 at 08:54:07PM -0400, Ronald S. Bultje wrote:
>>> Hi,
>>>
>>> On Sun, Mar 12, 2017 at 7:32 PM, James Almer <jamrial at gmail.com> wrote:
>>>
>>>> On 3/12/2017 6:16 PM, Michael Niedermayer wrote:
>>>>> Using the same type across platforms is more robust and avoids platform
>>>>> specific issues from differences in range.
> 
> I still think you are curing the symptom rather than the illness.
> 
> Besides, you can't just change types on a whim, you should wait for
> the major bump (if at all).

As mentioned by Hendrik, it's only six days old and not part of any
release, so it can be changed just fine.

> 
>>>>> Also fixed point integers are on a semantical level not size_t
> 
> This is only theoretical,

You specifically wrote the API to have the fields store 0.32 fixed point
values. Why you choose size_t for a field that's meant to store exactly
32 bits is the question.

Afaik, size_t could even be 16 bit in some systems. FreeDOS is probably
one, and we supposedly support it. Libav does too, so maybe this change
should be done in both projects.

> and, since we're talking about semantics,
> you're breaking ABI by using sizeof(AVSphericalMapping) outside of
> libavutil.

Well, you're the one that introduced the only current sizeof() check
outside of libavutil, in both projects.

> 
>>>>> Previous version reviewed-by: James Almer <jamrial at gmail.com>
>>>>> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
>>>>> ---
>>>>>  ffprobe.c             |  2 +-
>>>>>  libavformat/dump.c    |  6 +++---
>>>>>  libavutil/spherical.c |  6 +++---
>>>>>  libavutil/spherical.h | 16 ++++++++--------
>>>>>  4 files changed, 15 insertions(+), 15 deletions(-)
>>>>
>>>> Needs FATE update.
>>>
>>>
>>> Since this is Vittorio's code, it's probably good to ask him [CC]? From
>>
>> yes,
>>
>> semi on topic: but i think people interrested in FFmpeg
>> development and the direction the code goes should subscribe to
>> ffmpeg-devel. This CC-ing is not too practical
> 
> My subscription to ffmpeg-security was rejected, if you can rectify
> that, I'll subscribe to ffmpeg-devel too.
>