[FFmpeg-devel] Change type of spherical stuff to uint64_t

Submitted by Michael Niedermayer on March 12, 2017, 8:06 p.m.

Details

Message ID 20170312200626.26047-1-michael@niedermayer.cc
State New
Headers show

Commit Message

Michael Niedermayer March 12, 2017, 8:06 p.m.
Using the same type across platforms is more robust and avoids platform
specific issues from differences in range.
Also fixed point integers are on a semantical level not size_t

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 ffprobe.c             |  2 +-
 libavformat/dump.c    |  6 +++---
 libavutil/spherical.c | 10 +++++-----
 libavutil/spherical.h | 16 ++++++++--------
 4 files changed, 17 insertions(+), 17 deletions(-)

Comments

James Almer March 12, 2017, 8:41 p.m.
On 3/12/2017 5:06 PM, Michael Niedermayer wrote:
> Using the same type across platforms is more robust and avoids platform
> specific issues from differences in range.
> Also fixed point integers are on a semantical level not size_t

LGTM. You could even use uint32_t as it's more than enough for these
fields, which are all explicitly 0.32 fixed point values as mentioned
in the doxy.

Maybe a minor/micro bump? Even if doesn't really affect anything since
it's barely a five days old addition, it's still an API/ABI break and
it would be nice to signal it in some way.

> 
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  ffprobe.c             |  2 +-
>  libavformat/dump.c    |  6 +++---
>  libavutil/spherical.c | 10 +++++-----
>  libavutil/spherical.h | 16 ++++++++--------
>  4 files changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/ffprobe.c b/ffprobe.c
> index b104390990..a73566b7a3 100644
> --- a/ffprobe.c
> +++ b/ffprobe.c
> @@ -1793,7 +1793,7 @@ static void print_pkt_side_data(WriterContext *w,
>                  print_str("projection", "cubemap");
>                  print_int("padding", spherical->padding);
>              } else if (spherical->projection == AV_SPHERICAL_EQUIRECTANGULAR_TILE) {
> -                size_t l, t, r, b;
> +                uint64_t l, t, r, b;
>                  av_spherical_tile_bounds(spherical, par->width, par->height,
>                                           &l, &t, &r, &b);
>                  print_str("projection", "tiled equirectangular");
> diff --git a/libavformat/dump.c b/libavformat/dump.c
> index 505d572301..0a1208a375 100644
> --- a/libavformat/dump.c
> +++ b/libavformat/dump.c
> @@ -370,12 +370,12 @@ static void dump_spherical(void *ctx, AVCodecParameters *par, AVPacketSideData *
>      av_log(ctx, AV_LOG_INFO, "(%f/%f/%f) ", yaw, pitch, roll);
>  
>      if (spherical->projection == AV_SPHERICAL_EQUIRECTANGULAR_TILE) {
> -        size_t l, t, r, b;
> +        uint64_t l, t, r, b;
>          av_spherical_tile_bounds(spherical, par->width, par->height,
>                                   &l, &t, &r, &b);
> -        av_log(ctx, AV_LOG_INFO, "[%zu, %zu, %zu, %zu] ", l, t, r, b);
> +        av_log(ctx, AV_LOG_INFO, "[%"PRIu64", %"PRIu64", %"PRIu64", %"PRIu64"] ", 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 %"PRIu64"] ", spherical->padding);
>      }
>  }
>  
> diff --git a/libavutil/spherical.c b/libavutil/spherical.c
> index 0ca2dd367a..1e9805c2a5 100644
> --- a/libavutil/spherical.c
> +++ b/libavutil/spherical.c
> @@ -34,14 +34,14 @@ AVSphericalMapping *av_spherical_alloc(size_t *size)
>  }
>  
>  void av_spherical_tile_bounds(AVSphericalMapping *map,
> -                              size_t width, size_t height,
> -                              size_t *left, size_t *top,
> -                              size_t *right, size_t *bottom)
> +                              uint64_t width, uint64_t height,
> +                              uint64_t *left, uint64_t*top,
> +                              uint64_t *right, uint64_t *bottom)
>  {
>      /* conversion from 0.32 coordinates to pixels */
> -    uint64_t orig_width  = (uint64_t) width  * UINT32_MAX /
> +    uint64_t orig_width  = width  * UINT32_MAX /
>                             (UINT32_MAX - map->bound_right  - map->bound_left);
> -    uint64_t orig_height = (uint64_t) height * UINT32_MAX /
> +    uint64_t orig_height = height * UINT32_MAX /
>                             (UINT32_MAX - map->bound_bottom - map->bound_top);
>  
>      /* add a (UINT32_MAX - 1) to round up integer division */
> diff --git a/libavutil/spherical.h b/libavutil/spherical.h
> index db9bdc0be5..92a82199b2 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
> +    uint64_t bound_left;   ///< Distance from the left edge
> +    uint64_t bound_top;    ///< Distance from the top edge
> +    uint64_t bound_right;  ///< Distance from the right edge
> +    uint64_t bound_bottom; ///< Distance from the bottom edge

uint32_t is imo more than enough

>      /**
>       * @}
>       */
> @@ -179,7 +179,7 @@ typedef struct AVSphericalMapping {
>       *       (@ref AV_SPHERICAL_CUBEMAP), and should be ignored in all other
>       *       cases.
>       */
> -    size_t padding;
> +    uint64_t padding;
>  } AVSphericalMapping;
>  
>  /**
> @@ -203,9 +203,9 @@ AVSphericalMapping *av_spherical_alloc(size_t *size);
>   * @param bottom Pixels from the bottom edge.
>   */
>  void av_spherical_tile_bounds(AVSphericalMapping *map,
> -                              size_t width, size_t height,
> -                              size_t *left, size_t *top,
> -                              size_t *right, size_t *bottom);
> +                              uint64_t width, uint64_t height,
> +                              uint64_t *left, uint64_t *top,
> +                              uint64_t *right, uint64_t *bottom);
>  /**
>   * @}
>   * @}
>
wm4 March 13, 2017, 10:44 a.m.
On Sun, 12 Mar 2017 21:06:26 +0100
Michael Niedermayer <michael@niedermayer.cc> wrote:

> Using the same type across platforms is more robust and avoids platform
> specific issues from differences in range.
> Also fixed point integers are on a semantical level not size_t
> 


Please no. If you do this now we're going to get a type mismatch chaos
as Libav changes more types from int to size_t.

Patch hide | download patch | download mbox

diff --git a/ffprobe.c b/ffprobe.c
index b104390990..a73566b7a3 100644
--- a/ffprobe.c
+++ b/ffprobe.c
@@ -1793,7 +1793,7 @@  static void print_pkt_side_data(WriterContext *w,
                 print_str("projection", "cubemap");
                 print_int("padding", spherical->padding);
             } else if (spherical->projection == AV_SPHERICAL_EQUIRECTANGULAR_TILE) {
-                size_t l, t, r, b;
+                uint64_t l, t, r, b;
                 av_spherical_tile_bounds(spherical, par->width, par->height,
                                          &l, &t, &r, &b);
                 print_str("projection", "tiled equirectangular");
diff --git a/libavformat/dump.c b/libavformat/dump.c
index 505d572301..0a1208a375 100644
--- a/libavformat/dump.c
+++ b/libavformat/dump.c
@@ -370,12 +370,12 @@  static void dump_spherical(void *ctx, AVCodecParameters *par, AVPacketSideData *
     av_log(ctx, AV_LOG_INFO, "(%f/%f/%f) ", yaw, pitch, roll);
 
     if (spherical->projection == AV_SPHERICAL_EQUIRECTANGULAR_TILE) {
-        size_t l, t, r, b;
+        uint64_t l, t, r, b;
         av_spherical_tile_bounds(spherical, par->width, par->height,
                                  &l, &t, &r, &b);
-        av_log(ctx, AV_LOG_INFO, "[%zu, %zu, %zu, %zu] ", l, t, r, b);
+        av_log(ctx, AV_LOG_INFO, "[%"PRIu64", %"PRIu64", %"PRIu64", %"PRIu64"] ", 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 %"PRIu64"] ", spherical->padding);
     }
 }
 
diff --git a/libavutil/spherical.c b/libavutil/spherical.c
index 0ca2dd367a..1e9805c2a5 100644
--- a/libavutil/spherical.c
+++ b/libavutil/spherical.c
@@ -34,14 +34,14 @@  AVSphericalMapping *av_spherical_alloc(size_t *size)
 }
 
 void av_spherical_tile_bounds(AVSphericalMapping *map,
-                              size_t width, size_t height,
-                              size_t *left, size_t *top,
-                              size_t *right, size_t *bottom)
+                              uint64_t width, uint64_t height,
+                              uint64_t *left, uint64_t*top,
+                              uint64_t *right, uint64_t *bottom)
 {
     /* conversion from 0.32 coordinates to pixels */
-    uint64_t orig_width  = (uint64_t) width  * UINT32_MAX /
+    uint64_t orig_width  = width  * UINT32_MAX /
                            (UINT32_MAX - map->bound_right  - map->bound_left);
-    uint64_t orig_height = (uint64_t) height * UINT32_MAX /
+    uint64_t orig_height = height * UINT32_MAX /
                            (UINT32_MAX - map->bound_bottom - map->bound_top);
 
     /* add a (UINT32_MAX - 1) to round up integer division */
diff --git a/libavutil/spherical.h b/libavutil/spherical.h
index db9bdc0be5..92a82199b2 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
+    uint64_t bound_left;   ///< Distance from the left edge
+    uint64_t bound_top;    ///< Distance from the top edge
+    uint64_t bound_right;  ///< Distance from the right edge
+    uint64_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;
+    uint64_t padding;
 } AVSphericalMapping;
 
 /**
@@ -203,9 +203,9 @@  AVSphericalMapping *av_spherical_alloc(size_t *size);
  * @param bottom Pixels from the bottom edge.
  */
 void av_spherical_tile_bounds(AVSphericalMapping *map,
-                              size_t width, size_t height,
-                              size_t *left, size_t *top,
-                              size_t *right, size_t *bottom);
+                              uint64_t width, uint64_t height,
+                              uint64_t *left, uint64_t *top,
+                              uint64_t *right, uint64_t *bottom);
 /**
  * @}
  * @}