[FFmpeg-devel] [PATCH 3/5] avcodec/cbs_h264: fix storage type for time_offset in Pic Timing SEI

[Mark Thompson]

On 15/04/2019 22:17, James Almer wrote:
> The spec defines it as a signed value.
> 
> Signed-off-by: James Almer <jamrial at gmail.com>
> ---
> The only sample i could find with time_offset values it's in the fate suite,
> and in all cases it's 0.
> 
>  libavcodec/cbs_h264.h                 | 2 +-
>  libavcodec/cbs_h264_syntax_template.c | 5 +++--
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/cbs_h264.h b/libavcodec/cbs_h264.h
> index 92277e4750..b5eee7c370 100644
> --- a/libavcodec/cbs_h264.h
> +++ b/libavcodec/cbs_h264.h
> @@ -253,7 +253,7 @@ typedef struct H264RawSEIPicTimestamp {
>      uint8_t minutes_value;
>      uint8_t hours_flag;
>      uint8_t hours_value;
> -    uint32_t time_offset;
> +    int32_t time_offset;
>  } H264RawSEIPicTimestamp;
>  
>  typedef struct H264RawSEIPicTiming {
> diff --git a/libavcodec/cbs_h264_syntax_template.c b/libavcodec/cbs_h264_syntax_template.c
> index 4da4c5da67..07b4cddb5e 100644
> --- a/libavcodec/cbs_h264_syntax_template.c
> +++ b/libavcodec/cbs_h264_syntax_template.c
> @@ -592,8 +592,9 @@ static int FUNC(sei_pic_timestamp)(CodedBitstreamContext *ctx, RWContext *rw,
>          time_offset_length = 24;
>  
>      if (time_offset_length > 0)
> -        u(time_offset_length, time_offset,
> -          0, MAX_UINT_BITS(time_offset_length));
> +        i(time_offset_length, time_offset,
> +          MIN_INT_BITS(time_offset_length),
> +          MAX_INT_BITS(time_offset_length));
>      else
>          infer(time_offset, 0);
>  
> 

LGTM.

I'm glad the standard gets plenty of use out of that i(v) syntax element type definition.

- Mark