On 11/04/2019 04:10, James Almer wrote:
> On 4/10/2019 3:30 PM, James Almer wrote:
>> The spec defines the valid range of values to be INT32_MIN + 1 to INT32_MAX, 
>> inclusive.
>>
>> Signed-off-by: James Almer <jamr...@gmail.com>
>> ---
>> A good example of why making offsets and sizes of structs like this tied to 
>> the
>> ABI is not a good idea.
>>
>>  libavcodec/h264_ps.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/h264_ps.h b/libavcodec/h264_ps.h
>> index e967b9cbcf..9014326dfb 100644
>> --- a/libavcodec/h264_ps.h
>> +++ b/libavcodec/h264_ps.h
>> @@ -81,7 +81,7 @@ typedef struct SPS {
>>      uint32_t num_units_in_tick;
>>      uint32_t time_scale;
>>      int fixed_frame_rate_flag;
>> -    short offset_for_ref_frame[256]; // FIXME dyn aloc?
>> +    int32_t offset_for_ref_frame[256];
> 
> The doxy for get_se_golomb() doesn't mention the range of values it can
> handle, but seeing there's also a get_se_golomb_long(), I guess the
> relevant line in h264_ps.c should now use the latter instead?

I think it's correct to do that.  Seems highly unlikely anyone would ever hit 
it outside a conformance-checking context, though - using anything other than 
pic_order_cnt_type 0 for nontrivial reference structures is madness.

>>      int bitstream_restriction_flag;
>>      int num_reorder_frames;
>>      int scaling_matrix_present;

There are some other fields with int32_t range which are using get_se_golomb() 
- e.g. offset_for_non_ref_pic.  I guess they should use get_se_golomb_long() as 
above.  They're also plain ints - do they want to be explicitly int32_t?

- Mark
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to