> On Apr 16, 2019, at 3:01 PM, James Almer <jamr...@gmail.com> wrote:
>
> On 4/16/2019 6:48 PM, Mark Thompson wrote:
>> 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?
>
> It's wildly inconsistent. There are both scalar values and arrays as
> uint32_t, int, and unsigned it, but in all cases they can store the
> correct range of values, so IMO, not worth changing unless the whole
> structs are made consistent, like you did with the CBS ones.Sounds like a good opportunity to make the whole structs consistent. — Baptiste
signature.asc
Description: Message signed with OpenPGP
_______________________________________________ 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".
