Bug 480042

# SUMMARY

The ffmpeg h264 software decoder used with a high profile in a GStreamer pipeline produces false-positive "uninitialised value" errors when run under Valgrind.

I tested it with Valgrind 3.18.1 and 3.22.0 and GStreamer from versions 1.20.3 to the git main branch and ffmpeg from 4.4.2 to 6.1.

# STEPS TO REPRODUCE

1. install GStreamer on your system (https://gstreamer.freedesktop.org/documentation/installing/on-linux.html)
2. open a terminal and call:
```
valgrind gst-launch-1.0 videotestsrc num-buffers=1 ! video/x-raw,width=256,height=66 ! x264enc ! h264parse ! video/x-h264,profile=high ! avdec_h264 max-threads=1 thread-type=frame direct-rendering=true ! filesink location=raw.bin
```

# OBSERVED RESULT

We have some errors for reading uninitialised memory.
```
==116012== Thread 2 videotestsrc0:sr:
==116012== Syscall param writev(vector[...]) points to uninitialised byte(s)
==116012==    at 0x4CCCA8D: __writev (writev.c:26)
==116012==    by 0x4CCCA8D: writev (writev.c:24)
==116012==    by 0xC52473B: gst_writev (gstelements_private.c:157)
==116012==    by 0xC524BE5: gst_writev_iovecs (gstelements_private.c:254)
==116012==    by 0xC525FA1: gst_writev_buffer_list (gstelements_private.c:554)
==116012==    by 0xC533860: gst_file_sink_render_list_internal (gstfilesink.c:775)
==116012==    by 0xC533B3A: gst_file_sink_flush_buffer (gstfilesink.c:837)
==116012==    by 0xC533211: gst_file_sink_event (gstfilesink.c:696)
==116012==    by 0x8BA8612: gst_base_sink_event (gstbasesink.c:3680)
==116012==    by 0x491ED3D: gst_pad_send_event_unchecked (gstpad.c:5963)
==116012==    by 0x491D55A: gst_pad_push_event_unchecked (gstpad.c:5596)
==116012==    by 0x4916BE7: push_sticky (gstpad.c:4080)
==116012==    by 0x490BD29: events_foreach (gstpad.c:613)
==116012==  Address 0x756f81f is 2,255 bytes inside a block of size 25,511 alloc'd
==116012==    at 0x484C899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==116012==    by 0x4A76738: g_malloc (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7200.4)
==116012==    by 0x48A919C: _sysmem_new_block (gstallocator.c:434)
==116012==    by 0x48A9561: default_alloc (gstallocator.c:533)
==116012==    by 0x48A8DCE: gst_allocator_alloc (gstallocator.c:336)
==116012==    by 0x48B8648: gst_buffer_new_allocate (gstbuffer.c:902)
==116012==    by 0x8AAF685: video_buffer_pool_alloc (gstvideopool.c:251)
==116012==    by 0x48BE2A8: do_alloc_buffer (gstbufferpool.c:286)
==116012==    by 0x48C00AF: default_acquire_buffer (gstbufferpool.c:1140)
==116012==    by 0x48C06D2: gst_buffer_pool_acquire_buffer (gstbufferpool.c:1299)
==116012==    by 0x8A9C66E: gst_video_decoder_allocate_output_frame_with_params (gstvideodecoder.c:4705)
==116012==    by 0x8A9C308: gst_video_decoder_allocate_output_frame (gstvideodecoder.c:4642)
==116012==
```

# EXPECTED RESULT

No errors (see below for more details).

# SOFTWARE/OS VERSIONS

Linux Mint 21.3 (based on Ubuntu 22.04.3)

# ADDITIONAL INFORMATION

I did different tests that show that the memory block is effectively fully initialised (see: https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/5882#note_2243694)

To sum up what happens:
- during the decoding of an H264 frame ffmpeg (through libavcodec) requires the allocation of a block of memory from GStreamer
- GStreamer allocates this block of memory with `malloc` and gives back to ffmpeg pointers for each image plane
- ffmpeg fills the pixels colors in each plane using this function: https://gitlab.freedesktop.org/gstreamer/meson-ports/ffmpeg/-/blob/meson-4.4/libavcodec/h264_mb_template.c?ref_type=heads#L41
- the decoded image is passed back to GStreamer that is going to write the packed image planes to a file. This is where the Valgrind error is triggered.

In this example, the image color format is YUV420 and because of memory alignments there is a lot of padding not only between the color planes but also at the end of each image line. This padding is never initialised, but I can also guaranty that it is not read either because if I flag the actual pixels areas (excluding any padding) with VALGRIND_MAKE_MEM_DEFINED right after getting the decoded image, the issue doesn't occur anymore.

In order to check that it was not a real issue in ffmpeg or GStreamer, I've also initialized the pixels areas (excluding any paddings) with 0xFF right before passing it to ffmpeg for the decoding. The resulting decoded image (as presented in the raw.bin file) doesn't have any single byte equal to 0xFF which proves that all pixels have effectively been written by ffmpeg.

The logical conclusion is that, somehow, Valgrind has not detected that memory has been effectively written during the image decoding by ffmpeg. Indeed, if I build ffmpeg disabling the ASM optimizations, the issue doesn't occur anymore. 

Therefore, it seems that some of the ASM instructions used by ffmpeg during the image decoding actually modify the memory but this is not detected by Valgrind. It seems that everything occurs during the call of this function: https://gitlab.freedesktop.org/gstreamer/meson-ports/ffmpeg/-/blob/meson-4.4/libavcodec/h264_mb_template.c?ref_type=heads#L41. Unfortunately, I haven't been able to isolate better which instruction is causing the issue. I did some small tests with direct copies using SSE instructions and in all cases it was perfectly detected by Valgrind.

The original issue in GStreamer with all the details is here: https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/3203. I've proposed a patch to fix it in GStreamer: https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/5882 but I finally closed it because it is just hiding the issue, not fixing its cause.