[FFmpeg-devel] [PATCH 3/3] TTML encoder and muxer

[Andreas Rheinhardt]

Jan Ekström:
> On Mon, Dec 7, 2020 at 4:03 PM Andreas Rheinhardt
> <andreas.rheinhardt at gmail.com> wrote:
>>>
>>> You are not copying the terminating NUL; and you also don't account for
>>> it in the above check. Is this intended?
>>>
>>
>> The srt encoder does it like you, the ass encoder is careful only to
>> output a NUL terminated string (with the NUL not accounted for in the
>> size); no documentation exists for avcodec_encode_subtitle(). ffmpeg.c
>> uses the size of the allocated (not-zeroed) buffer as bufsize; it also
>> does not add any padding at all (but it's buffer is huge (1MiB)).
>>
> 
> Yes, this is probably the roots of this code being based on the webvtt
> and srt encoders showing up. I do agree that especially for text
> buffers zero-termination makes sense, as long as it doesn't cause
> funky things to happen when the packet is being written by muxer(s).
> 
> As noted, I have vague memories of AVBPrint handling zero-termination
> in some cases, but my memory definitely needs a re-check.
> 
An AVBPrint's string is always zero-terminated and said zero is not
included in the length (like strlen); but the problem starts to exists
when you copy the string without the trailing zero. You should modify
your check to "if (s->buffer.len >= bufsize)" and copy the NUL (or
zero-terminate yourself, I don't care).

- Andreas