VideoTrackRecorder: add support for texture backed ARGB VideoFrames

content/renderer/media/video_track_recorder.cc

 // Copyright 2015 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 "content/renderer/media/video_track_recorder.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/logging.h"
@@ skipping 167 matching lines @@
     base::TimeTicks capture_timestamp) {
   // Cache the thread sending frames on first frame arrival.
   if (!origin_task_runner_.get())
     origin_task_runner_ = base::ThreadTaskRunnerHandle::Get();
   DCHECK(origin_task_runner_->BelongsToCurrentThread());
   if (paused_)
     return;
 
   if (!(video_frame->format() == media::PIXEL_FORMAT_I420 ||
         video_frame->format() == media::PIXEL_FORMAT_YV12 ||
+        video_frame->format() == media::PIXEL_FORMAT_ARGB ||
         video_frame->format() == media::PIXEL_FORMAT_YV12A)) {
-    NOTREACHED();
+    NOTREACHED() << media::VideoPixelFormatToString(video_frame->format());
     return;
   }

[emircan, 2016/07/12 22:57:00]

This code looks very similar to CopyFrame() below, which handles texture
download as well. Can we refactor these into a common place?

Also, I am not sure about sending black frames as a solution. AFAIK, HW decode
is common in Android which sends textures that aren't mappable. SW decode would
send mappable I420. So, ARGB&Mappable() case is probably not going to be called.


Seeing lots of black frames may confuse the developer as well. Is there any way
we can notify about this in console? Or just download the texture like
CopyFrame() below.

https://cs.chromium.org/chromium/src/content/renderer/media/webmediaplayer_ms...

[mcasas, 2016/07/13 00:45:30]

Happy to do that, but I see that CopyFrame() has a parameter 
media::SkCanvasVideoRenderer* video_renderer
that holds the data we want to extract; in this case we have 
no such luxury and we simply know that we can't map the 
frame. WDYT?
Hmmm then I guess it doesn't make sense to have
such case  :)  I'll remove it if this is the case, but
since there's a bit of discussion now, I'll hold the 
removal until your next round of comments.
I thought that is better for the user to record black
frames, meaning "data couldn't be accessed" rather
than not recording anything, which developers (like
cpaulin@) interpreted as "the code is broken". 

Plus, if down the line we find a method to encode 
these non-mappable frames (e.g. using Android
MediaCodec) we can start encoding properly.

-  scoped_refptr<media::VideoFrame> frame = video_frame;
-  // Drop alpha channel since we do not support it yet.
-  if (frame->format() == media::PIXEL_FORMAT_YV12A)
+  scoped_refptr<media::VideoFrame> frame;
+  if (video_frame->format() == media::PIXEL_FORMAT_ARGB) {
+    // Certain Android (hardware) decoders produce media::PIXEL_FORMAT_ARGB,
+    // (see https://crbug.com/585242) that may or may not be opaque. In the
+    // former case, record black frames (yuv = {0, 0x80, 0x80}) instead.
+    if (!video_frame->IsMappable()) {
+      frame = media::VideoFrame::CreateColorFrame(
+          video_frame->coded_size(), 0u, 0x80, 0x80, video_frame->timestamp());

[emircan, 2016/07/12 22:57:00]

What if the coded_size() is not equal to visible_size()? These frames would have
the wrong visible_size() by default. 

[mcasas, 2016/07/13 00:45:30]

Well, I guess at this point the user is not going to 
get correct VideoFrames anyhow :)

+    } else {
+      frame = media::VideoFrame::CreateFrame(
+          media::PIXEL_FORMAT_I420, video_frame->coded_size(),
+          video_frame->visible_rect(), video_frame->natural_size(),
+          video_frame->timestamp());
+      const int result =
+          libyuv::ARGBToI420(video_frame->data(media::VideoFrame::kARGBPlane),
+                             video_frame->stride(media::VideoFrame::kARGBPlane),
+                             frame->data(media::VideoFrame::kYPlane),
+                             frame->stride(media::VideoFrame::kYPlane),
+                             frame->data(media::VideoFrame::kUPlane),
+                             frame->stride(media::VideoFrame::kUPlane),
+                             frame->data(media::VideoFrame::kVPlane),
+                             frame->stride(media::VideoFrame::kVPlane),
+                             video_frame->coded_size().width(),
+                             video_frame->coded_size().height());
+      DCHECK_EQ(0, result) << "Error converting ARGB input frame to I420";
+    }
+  } else if (video_frame->format() == media::PIXEL_FORMAT_YV12A) {
+    // Drop alpha channel since we do not support it yet.
     frame = media::WrapAsI420VideoFrame(video_frame);
+  } else {
+    frame = video_frame;
+  }
 
   encoding_task_runner_->PostTask(
       FROM_HERE, base::Bind(&Encoder::EncodeOnEncodingTaskRunner, this, frame,
                             capture_timestamp));
 }
 
 void VideoTrackRecorder::Encoder::SetPaused(bool paused) {
   if (!encoding_task_runner_->BelongsToCurrentThread()) {
     encoding_task_runner_->PostTask(
         FROM_HERE, base::Bind(&Encoder::SetPaused, this, paused));
@@ skipping 782 matching lines @@
   encoder_->SetPaused(false);
 }
 
 void VideoTrackRecorder::OnVideoFrameForTesting(
     const scoped_refptr<media::VideoFrame>& frame,
     base::TimeTicks timestamp) {
   encoder_->StartFrameEncode(frame, timestamp);
 }
 
 }  // namespace content