[FFmpeg-devel] [RFC]lavu/timecode: Increase AV_TIMECODE_STR_SIZE.

[Aaron Levinson]

On 5/5/2017 12:44 AM, Carl Eugen Hoyos wrote:
> 2017-05-05 9:42 GMT+02:00 Aaron Levinson <alevinsn at aracnet.com>:
>> On 5/5/2017 12:20 AM, Carl Eugen Hoyos wrote:
>>>
>>> 2017-05-05 3:29 GMT+02:00 Aaron Levinson <alevinsn at aracnet.com>:
>>>>
>>>> On 5/4/2017 4:27 PM, Carl Eugen Hoyos wrote:
>>>>>
>>>>>
>>>>> Hi!
>>>>>
>>>>> Attached patch is one possibility to fix the following warning with gcc
>>>>> 7:
>>>>> libavutil/timecode.c: In function ‘av_timecode_make_string’:
>>>>> libavutil/timecode.c:103:60: warning: ‘%02d’ directive output may be
>>>>> truncated
>>>>> writing between 2 and 10 bytes into a region of size between 0 and 7
>>>>> [-Wformat-truncation=]
>>>>>      snprintf(buf, AV_TIMECODE_STR_SIZE, "%s%02d:%02d:%02d%c%02d",
>>>>>                                                             ^~~~
>>>>> libavutil/timecode.c:103:41: note: directive argument in the range [0,
>>>>> 2147483647]
>>>>>      snprintf(buf, AV_TIMECODE_STR_SIZE, "%s%02d:%02d:%02d%c%02d",
>>>>>                                          ^~~~~~~~~~~~~~~~~~~~~~~~
>>>>> libavutil/timecode.c:103:5: note: ‘snprintf’ output between 12 and 32
>>>>> bytes
>>>>> into a destination of size 16
>>>>>      snprintf(buf, AV_TIMECODE_STR_SIZE, "%s%02d:%02d:%02d%c%02d",
>>>>>      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>>>               neg ? "-" : "",
>>>>>               ~~~~~~~~~~~~~~~
>>>>>               hh, mm, ss, drop ? ';' : ':', ff);
>>>>>               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>>>
>>>>> Several similar warnings are printed, an alternative is to disable the
>>>>> warning.
>>>>>
>>>>> The warning seemed wrong on first sight but may actually be correct, we
>>>>> accept invalid fps for timecode.
>>>>
>>>>
>>>>
>>>> Regarding the change in your patch:
>>>>
>>>> -#define AV_TIMECODE_STR_SIZE 16
>>>> +#define AV_TIMECODE_STR_SIZE 23
>>>>
>>>> It seems like 23 characters wouldn't be sufficient based on the warning:
>>>> "output between 12 and 32 bytes into a destination of size 16".  I would
>>>> guess that you would need at least 32 characters and perhaps one more (?)
>>>> for the terminating null character to avoid that warning.
>>>
>>>
>>> How can I reproduce / test your findings?
>>
>>
>> #define AV_TIMECODE_STR_SIZE 32 or 33 and confirm that gcc 7
>> warnings go away.
>
> Of course!
> Since the warnings go away with 23, they also go away with 32.
> I just wonder why 23 isn't enough in your opinion?

You hadn't previously indicated that the warnings disappeared when 23 is 
used, so I was operating based on the warning text, which said that the 
output text could consume as much as 32 bytes.  It seems odd that the 
warning text would disappear in this case, but if that's sufficient, it 
seems okay to apply.

Aaron Levinson