Revert to zero-initializing buffers for FFmpegVideoDecoder

media/base/video_frame_pool.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/base/video_frame_pool.h"
 
 #include <list>
 
 #include "base/bind.h"
 #include "base/memory/ref_counted.h"
 #include "base/synchronization/lock.h"
 
 namespace media {
 
 class VideoFramePool::PoolImpl
     : public base::RefCountedThreadSafe<VideoFramePool::PoolImpl> {
  public:
   PoolImpl();
 
-  // Returns a frame from the pool that matches the specified
-  // parameters or creates a new frame if no suitable frame exists in
-  // the pool. The pool is drained if no matching frame is found.
+  // See VideoFramePool::CreateFrame() for usage.
   scoped_refptr<VideoFrame> CreateFrame(VideoPixelFormat format,
                                         const gfx::Size& coded_size,
                                         const gfx::Rect& visible_rect,
                                         const gfx::Size& natural_size,
-                                        base::TimeDelta timestamp);
+                                        base::TimeDelta timestamp,
+                                        bool zero_new_frames);
 
   // Shuts down the frame pool and releases all frames in |frames_|.
   // Once this is called frames will no longer be inserted back into
   // |frames_|.
   void Shutdown();
 
   size_t GetPoolSizeForTesting() const { return frames_.size(); }
 
  private:
   friend class base::RefCountedThreadSafe<VideoFramePool::PoolImpl>;
@@ skipping 15 matching lines @@
 
 VideoFramePool::PoolImpl::~PoolImpl() {
   DCHECK(is_shutdown_);
 }
 
 scoped_refptr<VideoFrame> VideoFramePool::PoolImpl::CreateFrame(
     VideoPixelFormat format,
     const gfx::Size& coded_size,
     const gfx::Rect& visible_rect,
     const gfx::Size& natural_size,
-    base::TimeDelta timestamp) {
+    base::TimeDelta timestamp,
+    bool zero_new_frames) {
   base::AutoLock auto_lock(lock_);
   DCHECK(!is_shutdown_);
 
   scoped_refptr<VideoFrame> frame;
 
   while (!frame.get() && !frames_.empty()) {
       scoped_refptr<VideoFrame> pool_frame = frames_.front();
       frames_.pop_front();
 
       if (pool_frame->format() == format &&
           pool_frame->coded_size() == coded_size &&
           pool_frame->visible_rect() == visible_rect &&
           pool_frame->natural_size() == natural_size) {
         frame = pool_frame;
         frame->set_timestamp(timestamp);
         break;
       }
   }
 
   if (!frame.get()) {
     frame = VideoFrame::CreateFrame(
         format, coded_size, visible_rect, natural_size, timestamp);
+    if (zero_new_frames)
+      memset(frame->data(0), 0, frame->allocated_size());

[miu, 2015/08/04 20:16:22]

You have enough information to memset() all the memory without having this extra
allocated_size() property.  Meaning:

  if (zero_new_frames) {
    const size_t num_planes = VideoFrame::NumPlanes(frame->format());
    for (size_t i = 0; i < num_planes; ++i) {
      memset(frame->data(i),
             0,
             VideoFrame::PlaneSize(frame->format(),
                                   i,
                                   frame->coded_size()).GetArea()));
    }
  }

[DaleCurtis, 2015/08/04 21:20:52]

This doesn't include the padding around the frame or the extra U plane
allocation.  See l.889

[miu, 2015/08/06 21:23:30]

Okay, but is it enough to ensure FFMPEG isn't reading uninitialized data? 
Basically, I'm opposed to adding |allocated_size| to VideoFrames just for this
one edge case.  Let's be sure we absolutely have to do this.

BTW--If we "absolutely have to do this," we should just have
VideoFrame::CreateFrame() do it (e.g., in AllocateYUV() or nearby), rather than
expose this as an externally-handled thing.

   }
 
   scoped_refptr<VideoFrame> wrapped_frame = VideoFrame::WrapVideoFrame(
       frame, frame->visible_rect(), frame->natural_size());
   wrapped_frame->AddDestructionObserver(
       base::Bind(&VideoFramePool::PoolImpl::FrameReleased, this, frame));
   return wrapped_frame;
 }
 
 void VideoFramePool::PoolImpl::Shutdown() {
@@ skipping 16 matching lines @@
 
 VideoFramePool::~VideoFramePool() {
   pool_->Shutdown();
 }
 
 scoped_refptr<VideoFrame> VideoFramePool::CreateFrame(
     VideoPixelFormat format,
     const gfx::Size& coded_size,
     const gfx::Rect& visible_rect,
     const gfx::Size& natural_size,
-    base::TimeDelta timestamp) {
+    base::TimeDelta timestamp,
+    bool zero_new_frames) {
   return pool_->CreateFrame(format, coded_size, visible_rect, natural_size,
-                            timestamp);
+                            timestamp, zero_new_frames);
 }
 
 size_t VideoFramePool::GetPoolSizeForTesting() const {
   return pool_->GetPoolSizeForTesting();
 }
 
 }  // namespace media