[FFmpeg-devel] [PATCH] avformat/av1: Initialize padding in ff_isom_write_av1c
jamrial at gmail.com
Mon Apr 8 17:26:27 EEST 2019
On 4/8/2019 10:53 AM, Jeremy Dorfman via ffmpeg-devel wrote:
> Judging by the code, I think flush_put_bits only pads out to a byte
> boundary rather than the end of the buffer. By my read it will run the body
> of the while loop 3 times in this case, for bit_left = 8, 16, 24, so only 3
> bytes are filled in header.
> I'd expect this to only affect the actual output in uncommon cases. It
> requires the reuse of a stack with non-zero values at the location of
> uint8_t header. In the common case of running FFmpeg, it looks like this
> is called from the main thread so it should typically be zeroed by the OS.
> MemorySanitizer is tagging that region of the stack as uninitialized, so I
> don't think this is a false positive -- it doesn't appear to ever be
> intentionally written.
Alright, thanks for the explanation. Pushed.
> On Mon, Apr 8, 2019 at 9:33 AM James Almer <jamrial at gmail.com> wrote:
>> On 4/8/2019 9:14 AM, Jeremy Dorfman via ffmpeg-devel wrote:
>>> Otherwise, AV1 encodes with FFmpeg trigger use-of-uninitialized-value
>>> warnings under MemorySanitizer, and the output buffer potentially
>>> changes from run to run.
>>> libavformat/av1.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>> diff --git a/libavformat/av1.c b/libavformat/av1.c
>>> index a0aad436a6..5fde8df97e 100644
>>> --- a/libavformat/av1.c
>>> +++ b/libavformat/av1.c
>>> @@ -372,6 +372,7 @@ int ff_isom_write_av1c(AVIOContext *pb, const
>> uint8_t *buf, int size)
>>> put_bits(&pbc, 1, seq_params.chroma_subsampling_x);
>>> put_bits(&pbc, 1, seq_params.chroma_subsampling_y);
>>> put_bits(&pbc, 2, seq_params.chroma_sample_position);
>>> + put_bits(&pbc, 8, 0); // padding
>> Shouldn't flush_put_bits() below do this? The doxy says "Pad the end of
>> the output stream with zeros".
>> In any case, i just tried remuxing a single file several times, and in
>> all cases the hash of the output file was the same, so it could be a
>> false positive from MemorySanitizer.
>> Patch is fine either way, but i'd like to know why you're getting those
>>> avio_write(pb, header, sizeof(header));
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
More information about the ffmpeg-devel