diff mbox

[FFmpeg-devel] spherical: Change types of bounding and pad to uint32_t

Message ID 20170315213930.24234-1-vittorio.giovara@gmail.com
State Accepted
Commit 5f90ad99bb7e53383fefab5107b861e4c4600700
Headers show

Commit Message

Vittorio Giovara March 15, 2017, 9:39 p.m. UTC
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 <vittorio.giovara@gmail.com>
---
Updated commit message, changed a couple of internal types.
Please CC or I can't see replies.
Vittorio

 libavformat/dump.c        |  2 +-
 libavformat/matroskadec.c |  6 +++---
 libavformat/mov.c         |  7 +++----
 libavutil/spherical.h     | 10 +++++-----
 4 files changed, 12 insertions(+), 13 deletions(-)

Comments

James Almer March 16, 2017, 3:31 a.m. UTC | #1
On 3/15/2017 6:39 PM, 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 <vittorio.giovara@gmail.com>
> ---
> Updated commit message, changed a couple of internal types.
> Please CC or I can't see replies.
> Vittorio

Mention it's based on a patch by Michael Niedermayer.

> 
>  libavformat/dump.c        |  2 +-
>  libavformat/matroskadec.c |  6 +++---
>  libavformat/mov.c         |  7 +++----
>  libavutil/spherical.h     | 10 +++++-----
>  4 files changed, 12 insertions(+), 13 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 *
>                                   &l, &t, &r, &b);
>          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/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index 4fbf4b9a96..c6e1a190a8 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -1601,8 +1601,8 @@ static int mkv_parse_video_projection(AVStream *st, const MatroskaTrack *track)
>      AVSphericalMapping *spherical;
>      enum AVSphericalProjection projection;
>      size_t spherical_size;
> -    size_t l = 0, t = 0, r = 0, b = 0;
> -    size_t padding = 0;
> +    uint32_t l = 0, t = 0, r = 0, b = 0;
> +    uint32_t padding = 0;
>      int ret;
>      GetByteContext gb;
>  
> @@ -1627,7 +1627,7 @@ static int mkv_parse_video_projection(AVStream *st, const MatroskaTrack *track)
>              if (b >= UINT_MAX - t || r >= UINT_MAX - l) {
>                  av_log(NULL, AV_LOG_ERROR,
>                         "Invalid bounding rectangle coordinates "
> -                       "%zu,%zu,%zu,%zu\n", l, t, r, b);
> +                       "%"PRIu32",%"PRIu32",%"PRIu32",%"PRIu32"\n", l, t, r, b);

This chunk doesn't apply.

>                  return AVERROR_INVALIDDATA;
>              }
>          } else if (track->video.projection.private.size != 0) {
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index c6e7a38398..1c1857eaf9 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -3237,9 +3237,8 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>      MOVStreamContext *sc;
>      int size, version, layout;
>      int32_t yaw, pitch, roll;
> -    size_t l = 0, t = 0, r = 0, b = 0;
> -    size_t padding = 0;
> -    uint32_t tag;
> +    uint32_t l = 0, t = 0, r = 0, b = 0;
> +    uint32_t tag, padding = 0;
>      enum AVSphericalProjection projection;
>  
>      if (c->fc->nb_streams < 1)
> @@ -3335,7 +3334,7 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>          if (b >= UINT_MAX - t || r >= UINT_MAX - l) {
>              av_log(c->fc, AV_LOG_ERROR,
>                     "Invalid bounding rectangle coordinates "
> -                   "%zu,%zu,%zu,%zu\n", l, t, r, b);
> +                   "%"PRIu32",%"PRIu32",%"PRIu32",%"PRIu32"\n", l, t, r, b);

Same.

>              return AVERROR_INVALIDDATA;
>          }
>  
> 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;
>  
>  /**

LGTM otherwise.
Vittorio Giovara March 16, 2017, 7:18 p.m. UTC | #2
On Wed, Mar 15, 2017 at 11:31 PM, James Almer <jamrial@gmail.com> wrote:
> On 3/15/2017 6:39 PM, 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 <vittorio.giovara@gmail.com>
>> ---
>> Updated commit message, changed a couple of internal types.
>> Please CC or I can't see replies.
>> Vittorio
>
> Mention it's based on a patch by Michael Niedermayer.
>
>>
>>  libavformat/dump.c        |  2 +-
>>  libavformat/matroskadec.c |  6 +++---
>>  libavformat/mov.c         |  7 +++----
>>  libavutil/spherical.h     | 10 +++++-----
>>  4 files changed, 12 insertions(+), 13 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 *
>>                                   &l, &t, &r, &b);
>>          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/libavformat/matroskadec.c b/libavformat/matroskadec.c
>> index 4fbf4b9a96..c6e1a190a8 100644
>> --- a/libavformat/matroskadec.c
>> +++ b/libavformat/matroskadec.c
>> @@ -1601,8 +1601,8 @@ static int mkv_parse_video_projection(AVStream *st, const MatroskaTrack *track)
>>      AVSphericalMapping *spherical;
>>      enum AVSphericalProjection projection;
>>      size_t spherical_size;
>> -    size_t l = 0, t = 0, r = 0, b = 0;
>> -    size_t padding = 0;
>> +    uint32_t l = 0, t = 0, r = 0, b = 0;
>> +    uint32_t padding = 0;
>>      int ret;
>>      GetByteContext gb;
>>
>> @@ -1627,7 +1627,7 @@ static int mkv_parse_video_projection(AVStream *st, const MatroskaTrack *track)
>>              if (b >= UINT_MAX - t || r >= UINT_MAX - l) {
>>                  av_log(NULL, AV_LOG_ERROR,
>>                         "Invalid bounding rectangle coordinates "
>> -                       "%zu,%zu,%zu,%zu\n", l, t, r, b);
>> +                       "%"PRIu32",%"PRIu32",%"PRIu32",%"PRIu32"\n", l, t, r, b);
>
> This chunk doesn't apply.
>
>>                  return AVERROR_INVALIDDATA;
>>              }
>>          } else if (track->video.projection.private.size != 0) {
>> diff --git a/libavformat/mov.c b/libavformat/mov.c
>> index c6e7a38398..1c1857eaf9 100644
>> --- a/libavformat/mov.c
>> +++ b/libavformat/mov.c
>> @@ -3237,9 +3237,8 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>>      MOVStreamContext *sc;
>>      int size, version, layout;
>>      int32_t yaw, pitch, roll;
>> -    size_t l = 0, t = 0, r = 0, b = 0;
>> -    size_t padding = 0;
>> -    uint32_t tag;
>> +    uint32_t l = 0, t = 0, r = 0, b = 0;
>> +    uint32_t tag, padding = 0;
>>      enum AVSphericalProjection projection;
>>
>>      if (c->fc->nb_streams < 1)
>> @@ -3335,7 +3334,7 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>>          if (b >= UINT_MAX - t || r >= UINT_MAX - l) {
>>              av_log(c->fc, AV_LOG_ERROR,
>>                     "Invalid bounding rectangle coordinates "
>> -                   "%zu,%zu,%zu,%zu\n", l, t, r, b);
>> +                   "%"PRIu32",%"PRIu32",%"PRIu32",%"PRIu32"\n", l, t, r, b);
>
> Same.
>
>>              return AVERROR_INVALIDDATA;
>>          }
>>
>> 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;
>>
>>  /**
>
> LGTM otherwise.
>

Ok thank you

The "other side" will apply this as well at the same time this is committed.

Can you (or someone else) also approve the ffprobe change too please?
I'd like to push these two together.
Carl Eugen Hoyos March 16, 2017, 7:33 p.m. UTC | #3
2017-03-16 20:18 GMT+01:00 Vittorio Giovara <vittorio.giovara@gmail.com>:

>> LGTM otherwise.
>>
>
> Ok thank you
>
> The "other side" will apply this as well at the same time this is committed.

Don't forget to fix the patch author!

Carl Eugen
diff mbox

Patch

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 *
                                  &l, &t, &r, &b);
         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/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 4fbf4b9a96..c6e1a190a8 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -1601,8 +1601,8 @@  static int mkv_parse_video_projection(AVStream *st, const MatroskaTrack *track)
     AVSphericalMapping *spherical;
     enum AVSphericalProjection projection;
     size_t spherical_size;
-    size_t l = 0, t = 0, r = 0, b = 0;
-    size_t padding = 0;
+    uint32_t l = 0, t = 0, r = 0, b = 0;
+    uint32_t padding = 0;
     int ret;
     GetByteContext gb;
 
@@ -1627,7 +1627,7 @@  static int mkv_parse_video_projection(AVStream *st, const MatroskaTrack *track)
             if (b >= UINT_MAX - t || r >= UINT_MAX - l) {
                 av_log(NULL, AV_LOG_ERROR,
                        "Invalid bounding rectangle coordinates "
-                       "%zu,%zu,%zu,%zu\n", l, t, r, b);
+                       "%"PRIu32",%"PRIu32",%"PRIu32",%"PRIu32"\n", l, t, r, b);
                 return AVERROR_INVALIDDATA;
             }
         } else if (track->video.projection.private.size != 0) {
diff --git a/libavformat/mov.c b/libavformat/mov.c
index c6e7a38398..1c1857eaf9 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -3237,9 +3237,8 @@  static int mov_read_sv3d(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     MOVStreamContext *sc;
     int size, version, layout;
     int32_t yaw, pitch, roll;
-    size_t l = 0, t = 0, r = 0, b = 0;
-    size_t padding = 0;
-    uint32_t tag;
+    uint32_t l = 0, t = 0, r = 0, b = 0;
+    uint32_t tag, padding = 0;
     enum AVSphericalProjection projection;
 
     if (c->fc->nb_streams < 1)
@@ -3335,7 +3334,7 @@  static int mov_read_sv3d(MOVContext *c, AVIOContext *pb, MOVAtom atom)
         if (b >= UINT_MAX - t || r >= UINT_MAX - l) {
             av_log(c->fc, AV_LOG_ERROR,
                    "Invalid bounding rectangle coordinates "
-                   "%zu,%zu,%zu,%zu\n", l, t, r, b);
+                   "%"PRIu32",%"PRIu32",%"PRIu32",%"PRIu32"\n", l, t, r, b);
             return AVERROR_INVALIDDATA;
         }
 
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;
 
 /**