diff mbox

[FFmpeg-devel,4/5] avcodec/cbs_h265: fix storage type for time_offset_value in Time Code SEI

Message ID 20190415211734.3968-4-jamrial@gmail.com
State Accepted
Commit 137c0fcad15139921279038351048970e99838f4
Headers show

Commit Message

James Almer April 15, 2019, 9:17 p.m. UTC 
The spec defines it as an array of signed values, inferred to 0 when not
present.

Signed-off-by: James Almer <jamrial@gmail.com>
---
Couldn't find any sample using it.

 libavcodec/cbs_h265.h                 | 2 +-
 libavcodec/cbs_h265_syntax_template.c | 7 +++++--
 2 files changed, 6 insertions(+), 3 deletions(-)

Comments

Mark Thompson April 16, 2019, 11:03 p.m. UTC | #1 
On 15/04/2019 22:17, James Almer wrote:
> The spec defines it as an array of signed values, inferred to 0 when not
> present.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> Couldn't find any sample using it.
> 
>  libavcodec/cbs_h265.h                 | 2 +-
>  libavcodec/cbs_h265_syntax_template.c | 7 +++++--
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/cbs_h265.h b/libavcodec/cbs_h265.h
> index d216caca2b..0c0e4f84b0 100644
> --- a/libavcodec/cbs_h265.h
> +++ b/libavcodec/cbs_h265.h
> @@ -659,7 +659,7 @@ typedef struct H265RawSEITimeCode {
>      uint8_t  minutes_flag[3];
>      uint8_t  hours_flag[3];
>      uint8_t  time_offset_length[3];
> -    uint32_t time_offset_value[3];
> +    int32_t  time_offset_value[3];
>  } H265RawSEITimeCode;
>  
>  typedef struct H265RawSEIMasteringDisplayColourVolume {
> diff --git a/libavcodec/cbs_h265_syntax_template.c b/libavcodec/cbs_h265_syntax_template.c
> index f1e1bb0e7e..bbd363a389 100644
> --- a/libavcodec/cbs_h265_syntax_template.c
> +++ b/libavcodec/cbs_h265_syntax_template.c
> @@ -1986,8 +1986,11 @@ static int FUNC(sei_time_code)(CodedBitstreamContext *ctx, RWContext *rw,
>  
>              us(5, time_offset_length[i], 0, 31, 1, i);
>              if (current->time_offset_length[i] > 0)
> -                us(current->time_offset_length[i], time_offset_value[i],
> -                   0, MAX_UINT_BITS(current->time_offset_length[i]), 1, i);
> +                is(current->time_offset_length[i], time_offset_value[i],
> +                   MIN_INT_BITS(current->time_offset_length[i]),
> +                   MAX_INT_BITS(current->time_offset_length[i]), 1, i);
> +            else
> +                infer(time_offset_value[i], 0);
>          }
>      }
>  
> 

LGTM.

Thanks,

- Mark
James Almer April 16, 2019, 11:48 p.m. UTC | #2 
On 4/16/2019 8:03 PM, Mark Thompson wrote:
> On 15/04/2019 22:17, James Almer wrote:
>> The spec defines it as an array of signed values, inferred to 0 when not
>> present.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>> Couldn't find any sample using it.
>>
>>  libavcodec/cbs_h265.h                 | 2 +-
>>  libavcodec/cbs_h265_syntax_template.c | 7 +++++--
>>  2 files changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/libavcodec/cbs_h265.h b/libavcodec/cbs_h265.h
>> index d216caca2b..0c0e4f84b0 100644
>> --- a/libavcodec/cbs_h265.h
>> +++ b/libavcodec/cbs_h265.h
>> @@ -659,7 +659,7 @@ typedef struct H265RawSEITimeCode {
>>      uint8_t  minutes_flag[3];
>>      uint8_t  hours_flag[3];
>>      uint8_t  time_offset_length[3];
>> -    uint32_t time_offset_value[3];
>> +    int32_t  time_offset_value[3];
>>  } H265RawSEITimeCode;
>>  
>>  typedef struct H265RawSEIMasteringDisplayColourVolume {
>> diff --git a/libavcodec/cbs_h265_syntax_template.c b/libavcodec/cbs_h265_syntax_template.c
>> index f1e1bb0e7e..bbd363a389 100644
>> --- a/libavcodec/cbs_h265_syntax_template.c
>> +++ b/libavcodec/cbs_h265_syntax_template.c
>> @@ -1986,8 +1986,11 @@ static int FUNC(sei_time_code)(CodedBitstreamContext *ctx, RWContext *rw,
>>  
>>              us(5, time_offset_length[i], 0, 31, 1, i);
>>              if (current->time_offset_length[i] > 0)
>> -                us(current->time_offset_length[i], time_offset_value[i],
>> -                   0, MAX_UINT_BITS(current->time_offset_length[i]), 1, i);
>> +                is(current->time_offset_length[i], time_offset_value[i],
>> +                   MIN_INT_BITS(current->time_offset_length[i]),
>> +                   MAX_INT_BITS(current->time_offset_length[i]), 1, i);
>> +            else
>> +                infer(time_offset_value[i], 0);
>>          }
>>      }
>>  
>>
> 
> LGTM.
> 
> Thanks,
> 
> - Mark

Pushed, thanks!
diff mbox

Patch

diff --git a/libavcodec/cbs_h265.h b/libavcodec/cbs_h265.h
index d216caca2b..0c0e4f84b0 100644
--- a/libavcodec/cbs_h265.h
+++ b/libavcodec/cbs_h265.h
@@ -659,7 +659,7 @@  typedef struct H265RawSEITimeCode {
     uint8_t  minutes_flag[3];
     uint8_t  hours_flag[3];
     uint8_t  time_offset_length[3];
-    uint32_t time_offset_value[3];
+    int32_t  time_offset_value[3];
 } H265RawSEITimeCode;
 
 typedef struct H265RawSEIMasteringDisplayColourVolume {
diff --git a/libavcodec/cbs_h265_syntax_template.c b/libavcodec/cbs_h265_syntax_template.c
index f1e1bb0e7e..bbd363a389 100644
--- a/libavcodec/cbs_h265_syntax_template.c
+++ b/libavcodec/cbs_h265_syntax_template.c
@@ -1986,8 +1986,11 @@  static int FUNC(sei_time_code)(CodedBitstreamContext *ctx, RWContext *rw,
 
             us(5, time_offset_length[i], 0, 31, 1, i);
             if (current->time_offset_length[i] > 0)
-                us(current->time_offset_length[i], time_offset_value[i],
-                   0, MAX_UINT_BITS(current->time_offset_length[i]), 1, i);
+                is(current->time_offset_length[i], time_offset_value[i],
+                   MIN_INT_BITS(current->time_offset_length[i]),
+                   MAX_INT_BITS(current->time_offset_length[i]), 1, i);
+            else
+                infer(time_offset_value[i], 0);
         }
     }