Remove memset from VideoFrame and mark buffer as unpoisoned

media/filters/ffmpeg_video_decoder.cc

Index: media/filters/ffmpeg_video_decoder.cc
diff --git a/media/filters/ffmpeg_video_decoder.cc b/media/filters/ffmpeg_video_decoder.cc
index fdf8dc5841a8085c4831026dd693b6fe3e994acf..a356fab6418596556836d6e41fa548fb05303268 100644
--- a/media/filters/ffmpeg_video_decoder.cc
+++ b/media/filters/ffmpeg_video_decoder.cc
@@ -127,12 +127,17 @@ int FFmpegVideoDecoder::GetVideoBuffer(struct AVCodecContext* codec_context,
 
   scoped_refptr<VideoFrame> video_frame = frame_pool_.CreateFrame(
       format, coded_size, gfx::Rect(size), natural_size, kNoTimestamp());
+  // FFmpeg expects the initialize allocation to be zero-initialized.  Failure
+  // to do so can lead to unitialized value usage.  See http://crbug.com/390941
+  const size_t data_size = VideoFrame::AlignedAllocationSize(video_frame);
+  memset(video_frame->data(0), 0, data_size);

[DaleCurtis, 2015/07/09 21:35:11]

We don't want to do this here actually, as now memset() is called for every
video frame instead of for every new video frame.  Instead we should have an
option for the VideoFramePool to do this on initial allocation only.

[emircan, 2015/07/09 22:12:18]

Done. I added |zero_out_memory| bool to carry that information to the creation. 

+
   if (codec_context->colorspace == AVCOL_SPC_BT709) {
     video_frame->metadata()->SetInteger(VideoFrameMetadata::COLOR_SPACE,
                                         VideoFrame::COLOR_SPACE_HD_REC709);
   }
 
-  for (int i = 0; i < 3; i++) {
+  for (size_t i = 0; i < VideoFrame::NumPlanes(video_frame->format()); i++) {
     frame->data[i] = video_frame->data(i);
     frame->linesize[i] = video_frame->stride(i);
   }
@@ -148,7 +153,7 @@ int FFmpegVideoDecoder::GetVideoBuffer(struct AVCodecContext* codec_context,
   video_frame.swap(reinterpret_cast<VideoFrame**>(&opaque));
   frame->buf[0] =
       av_buffer_create(frame->data[0],
-                       VideoFrame::AllocationSize(format, coded_size),

[DaleCurtis, 2015/07/09 21:36:22]

Hmm, I think we were intentionally using the lower size here such that ffmpeg
didn't think it had further padding areas to read/write from (which it does in
many places).

[emircan, 2015/07/09 22:12:17]

I see. It looked like a mistake to me in the beginning as frame->data[] points
to the aligned&padded buffers and av_buffer_create goes for the packed size,
which is a mismatch. 
If we are not using such padded areas, should we remove them from allocation and
only allocate the packed size instead?

[DaleCurtis, 2015/07/09 22:21:16]

Hmm, I see, we can try it your way and see what falls out.

[emircan, 2015/07/09 23:16:27]

Done.

[emircan, 2015/07/10 18:43:40]

Well, bots showed that it didn't work out. Reverting this.

+                       data_size,
                        ReleaseVideoBufferImpl,
                        opaque,
                        0);