Re: [libav-devel] [PATCH] spherical: Change types of bounding and pad to uint32_t

2017-03-16 Thread Luca Barbato 
On 15/03/2017 22:53, Vittorio Giovara wrote:
> On Wed, Mar 15, 2017 at 5:52 PM, Luca Barbato  wrote:
>> On 15/03/2017 22:38, Vittorio Giovara wrote:
>>> These values are defined to be 32bit in the specification,
>>> so it makes more sense to store them as fixed width.
>>>
>>> Signed-off-by: Vittorio Giovara 
>>> ---
>>> Updated commit message, changed a couple of internal types.
>>> Vittorio
>>>
>>
>> While you are at it would you point that the values are defined a 0.32
>> fixed point?
> 
> Do you mean in the commit log? They are documented in spherical.h.

Yes, but I guess it is good enough as is.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] spherical: Change types of bounding and pad to uint32_t

2017-03-15 Thread Vittorio Giovara 
On Wed, Mar 15, 2017 at 5:52 PM, Luca Barbato  wrote:
> On 15/03/2017 22:38, Vittorio Giovara wrote:
>> These values are defined to be 32bit in the specification,
>> so it makes more sense to store them as fixed width.
>>
>> Signed-off-by: Vittorio Giovara 
>> ---
>> Updated commit message, changed a couple of internal types.
>> Vittorio
>>
>
> While you are at it would you point that the values are defined a 0.32
> fixed point?

Do you mean in the commit log? They are documented in spherical.h.
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] spherical: Change types of bounding and pad to uint32_t

2017-03-15 Thread Luca Barbato 
On 15/03/2017 22:38, Vittorio Giovara wrote:
> These values are defined to be 32bit in the specification,
> so it makes more sense to store them as fixed width.
> 
> Signed-off-by: Vittorio Giovara 
> ---
> Updated commit message, changed a couple of internal types.
> Vittorio
> 

While you are at it would you point that the values are defined a 0.32
fixed point?

lu

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] spherical: Change types of bounding and pad to uint32_t

2017-03-15 Thread wm4 
On Wed, 15 Mar 2017 11:31:48 -0400
Vittorio Giovara  wrote:

> On Wed, Mar 15, 2017 at 10:54 AM, Anton Khirnov  wrote:
> >
> > I'm not against this change, but as this is an ABI break, it should be
> > done during the major bump (which should be happening around now anyway).
> > It should also be mentioned in apichages.  
> 
> I am aware that this is an ABI break, but people claimed that since it
> was so recent in practice it does not matter.

I support this opinion.

> I don't have a strong opinion either way (I'd just like this to be over tbh).

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] spherical: Change types of bounding and pad to uint32_t

2017-03-15 Thread Vittorio Giovara 
On Wed, Mar 15, 2017 at 10:54 AM, Anton Khirnov  wrote:
>
> I'm not against this change, but as this is an ABI break, it should be
> done during the major bump (which should be happening around now anyway).
> It should also be mentioned in apichages.

I am aware that this is an ABI break, but people claimed that since it
was so recent in practice it does not matter.
I don't have a strong opinion either way (I'd just like this to be over tbh).
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] spherical: Change types of bounding and pad to uint32_t

2017-03-15 Thread Anton Khirnov 
Quoting Vittorio Giovara (2017-03-14 22:59:35)
> These types better reflect the ones described in the specification and
> avoid any platform-specific implementation issues.

What "platform specific implementation issues" would those be? There's
no need to invent such weasel words when the reason is pretty clear --
the values are defined to be 32bit so it makes more sense to store them
as fixed width.

> 
> Signed-off-by: Vittorio Giovara 
> ---
>  libavformat/dump.c|  2 +-
>  libavutil/spherical.h | 10 +-
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/libavformat/dump.c b/libavformat/dump.c
> index 7514aee7ac..c56895628d 100644
> --- a/libavformat/dump.c
> +++ b/libavformat/dump.c
> @@ -339,7 +339,7 @@ static void dump_spherical(void *ctx, AVCodecParameters 
> *par, AVPacketSideData *
>   , , , );
>  av_log(ctx, AV_LOG_INFO, "[%zu, %zu, %zu, %zu] ", l, t, r, b);
>  } else if (spherical->projection == AV_SPHERICAL_CUBEMAP) {
> -av_log(ctx, AV_LOG_INFO, "[pad %zu] ", spherical->padding);
> +av_log(ctx, AV_LOG_INFO, "[pad %"PRIu32"] ", spherical->padding);
>  }
>  }
>  
> diff --git a/libavutil/spherical.h b/libavutil/spherical.h
> index e7fc1d69fb..fd662cf676 100644
> --- a/libavutil/spherical.h
> +++ b/libavutil/spherical.h
> @@ -164,10 +164,10 @@ typedef struct AVSphericalMapping {
>   *   projection type (@ref AV_SPHERICAL_EQUIRECTANGULAR_TILE),
>   *   and should be ignored in all other cases.
>   */
> -size_t bound_left;   ///< Distance from the left edge
> -size_t bound_top;///< Distance from the top edge
> -size_t bound_right;  ///< Distance from the right edge
> -size_t bound_bottom; ///< Distance from the bottom edge
> +uint32_t bound_left;   ///< Distance from the left edge
> +uint32_t bound_top;///< Distance from the top edge
> +uint32_t bound_right;  ///< Distance from the right edge
> +uint32_t bound_bottom; ///< Distance from the bottom edge
>  /**
>   * @}
>   */
> @@ -179,7 +179,7 @@ typedef struct AVSphericalMapping {
>   *   (@ref AV_SPHERICAL_CUBEMAP), and should be ignored in all other
>   *   cases.
>   */
> -size_t padding;
> +uint32_t padding;
>  } AVSphericalMapping;
>  
>  /**
> -- 
> 2.12.0

I'm not against this change, but as this is an ABI break, it should be
done during the major bump (which should be happening around now anyway).
It should also be mentioned in apichages.

-- 
Anton Khirnov
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] spherical: Change types of bounding and pad to uint32_t

2017-03-14 Thread wm4 
On Tue, 14 Mar 2017 17:59:35 -0400
Vittorio Giovara  wrote:

> These types better reflect the ones described in the specification and
> avoid any platform-specific implementation issues.
> 
> Signed-off-by: Vittorio Giovara 
> ---
>  libavformat/dump.c|  2 +-
>  libavutil/spherical.h | 10 +-
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/libavformat/dump.c b/libavformat/dump.c
> index 7514aee7ac..c56895628d 100644
> --- a/libavformat/dump.c
> +++ b/libavformat/dump.c
> @@ -339,7 +339,7 @@ static void dump_spherical(void *ctx, AVCodecParameters 
> *par, AVPacketSideData *
>   , , , );
>  av_log(ctx, AV_LOG_INFO, "[%zu, %zu, %zu, %zu] ", l, t, r, b);
>  } else if (spherical->projection == AV_SPHERICAL_CUBEMAP) {
> -av_log(ctx, AV_LOG_INFO, "[pad %zu] ", spherical->padding);
> +av_log(ctx, AV_LOG_INFO, "[pad %"PRIu32"] ", spherical->padding);
>  }
>  }
>  
> diff --git a/libavutil/spherical.h b/libavutil/spherical.h
> index e7fc1d69fb..fd662cf676 100644
> --- a/libavutil/spherical.h
> +++ b/libavutil/spherical.h
> @@ -164,10 +164,10 @@ typedef struct AVSphericalMapping {
>   *   projection type (@ref AV_SPHERICAL_EQUIRECTANGULAR_TILE),
>   *   and should be ignored in all other cases.
>   */
> -size_t bound_left;   ///< Distance from the left edge
> -size_t bound_top;///< Distance from the top edge
> -size_t bound_right;  ///< Distance from the right edge
> -size_t bound_bottom; ///< Distance from the bottom edge
> +uint32_t bound_left;   ///< Distance from the left edge
> +uint32_t bound_top;///< Distance from the top edge
> +uint32_t bound_right;  ///< Distance from the right edge
> +uint32_t bound_bottom; ///< Distance from the bottom edge
>  /**
>   * @}
>   */
> @@ -179,7 +179,7 @@ typedef struct AVSphericalMapping {
>   *   (@ref AV_SPHERICAL_CUBEMAP), and should be ignored in all other
>   *   cases.
>   */
> -size_t padding;
> +uint32_t padding;
>  } AVSphericalMapping;
>  
>  /**

+1

We should change the AVFrame crop fields too.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel