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

[Vittorio Giovara]

On Wed, Mar 15, 2017 at 11:31 PM, James Almer <jamrial at 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 at 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.
-- 
Vittorio