| Message ID | 20170312200626.26047-1-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
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); > /** > * @} > * @} >
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.
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); /** * @} * @}
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(-)