Fix crash on unsupported pixel formats.

Replaces the NOTREACHED() calls with DLOG(WARNING), avoids creating
frames when invalid pixel formats are detected, and finally calls
OnInitializeComplete(false) in FFmpegVideoDecodeEngine::Initialize.

BUG=101803
TEST=Ran unittests. Ran test video.


Review URL: http://codereview.chromium.org/8437021

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@108338 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 34 insertions, 18 deletions

diff --git a/media/ffmpeg/ffmpeg_common.cc b/media/ffmpeg/ffmpeg_common.cc
index cbcb7c6..396ee03 100644
--- a/media/ffmpeg/ffmpeg_common.cc
+++ b/media/ffmpeg/ffmpeg_common.cc
@@ -289,9 +289,9 @@ VideoFrame::Format PixelFormatToVideoFormat(PixelFormat pixel_format) {
     case PIX_FMT_YUV420P:
       return VideoFrame::YV12;
     default:
-      NOTREACHED() << "Unsupported PixelFormat: " << pixel_format;
+      DLOG(WARNING) << "Unsupported PixelFormat: " << pixel_format;
+      return VideoFrame::INVALID;
   }
-  return VideoFrame::INVALID;
 }
 
 PixelFormat VideoFormatToPixelFormat(VideoFrame::Format video_format) {
@@ -301,9 +301,9 @@ PixelFormat VideoFormatToPixelFormat(VideoFrame::Format video_format) {
     case VideoFrame::YV12:
       return PIX_FMT_YUV420P;
     default:
-      NOTREACHED() << "Unsupported VideoFrame Format: " << video_format;
+      DLOG(WARNING) << "Unsupported VideoFrame Format: " << video_format;
+      return PIX_FMT_NONE;
   }
-  return PIX_FMT_NONE;
 }
 
 base::TimeDelta GetFrameDuration(const VideoDecoderConfig& config) {
diff --git a/media/video/ffmpeg_video_decode_engine.cc b/media/video/ffmpeg_video_decode_engine.cc
index dad9fc1..ceaa802 100644
--- a/media/video/ffmpeg_video_decode_engine.cc
+++ b/media/video/ffmpeg_video_decode_engine.cc
@@ -88,22 +88,27 @@ void FFmpegVideoDecodeEngine::Initialize(
   // If we do not have enough buffers, we will report error too.
   frame_queue_available_.clear();
 
-  // Create output buffer pool when direct rendering is not used.
-  for (size_t i = 0; i < Limits::kMaxVideoFrames; ++i) {
-    VideoFrame::Format format =
-        PixelFormatToVideoFormat(codec_context_->pix_fmt);
-
-    scoped_refptr<VideoFrame> video_frame =
-        VideoFrame::CreateFrame(format,
-                                config.visible_rect().width(),
-                                config.visible_rect().height(),
-                                kNoTimestamp,
-                                kNoTimestamp);
-    frame_queue_available_.push_back(video_frame);
+  // Convert the pixel format to video format and ensure we support it.
+  VideoFrame::Format format =
+      PixelFormatToVideoFormat(codec_context_->pix_fmt);
+
+  bool success = false;
+  if (format != VideoFrame::INVALID) {
+      // Create output buffer pool when direct rendering is not used.
+      for (size_t i = 0; i < Limits::kMaxVideoFrames; ++i) {
+        scoped_refptr<VideoFrame> video_frame =
+            VideoFrame::CreateFrame(format,
+                                    config.visible_rect().width(),
+                                    config.visible_rect().height(),
+                                    kNoTimestamp,
+                                    kNoTimestamp);
+        frame_queue_available_.push_back(video_frame);
+      }
+
+      // Open the codec!
+      success = codec && avcodec_open(codec_context_, codec) >= 0;
   }
 
-  // Open the codec!
-  bool success = codec && avcodec_open(codec_context_, codec) >= 0;
   event_handler_ = event_handler;
   event_handler_->OnInitializeComplete(success);
 }
diff --git a/media/video/ffmpeg_video_decode_engine_unittest.cc b/media/video/ffmpeg_video_decode_engine_unittest.cc
index 8670dc8..f2cf348 100644
--- a/media/video/ffmpeg_video_decode_engine_unittest.cc
+++ b/media/video/ffmpeg_video_decode_engine_unittest.cc
@@ -168,6 +168,17 @@ TEST_F(FFmpegVideoDecodeEngineTest, Initialize_OpenDecoderFails) {
   test_engine_->Initialize(this, config);
 }
 
+TEST_F(FFmpegVideoDecodeEngineTest, Initialize_UnsupportedPixelFormat) {
+  // Ensure decoder handles unsupport pixel formats without crashing.
+  VideoDecoderConfig config(kCodecVP8, VideoFrame::INVALID,
+                            kCodedSize, kVisibleRect,
+                            kFrameRate.num, kFrameRate.den,
+                            kAspectRatio.num, kAspectRatio.den,
+                            NULL, 0);
+  EXPECT_CALL(*this, OnInitializeComplete(false));
+  test_engine_->Initialize(this, config);
+}
+
 TEST_F(FFmpegVideoDecodeEngineTest, DecodeFrame_Normal) {
   Initialize();