ppapi: Change endianness in libyuv calls, from BGRA to ARGB

content/renderer/media/webrtc/video_destination_handler.cc

 // Copyright (c) 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 "content/renderer/media/webrtc/video_destination_handler.h"
 
 #include <string>
 
 #include "base/base64.h"
 #include "base/debug/trace_event.h"
@@ skipping 23 matching lines @@
   // Starts forwarding frames to |frame_callback| on the  IO-thread that are
   // delivered to this class by calling DeliverFrame on the main render thread.
   void StartDeliver(const VideoCaptureDeliverFrameCB& frame_callback);
   void StopDeliver();
 
   void DeliverFrame(const scoped_refptr<PPB_ImageData_Impl>& image_data,
                     int64 time_stamp_ns);
 
  private:
   friend class base::RefCountedThreadSafe<FrameWriterDelegate>;
+  // Endian in memory order, e.g. AXXX stands for uint8 pixel[4] = {A, x, x, x};
+  enum PixelEndian {
+    UNKNOWN,
+    AXXX,
+    XXXA,
+  };
+
   virtual ~FrameWriterDelegate();
 
   void StartDeliverOnIO(const VideoCaptureDeliverFrameCB& frame_callback);
   void StopDeliverOnIO();
 
   void DeliverFrameOnIO(uint8* data, int stride, int width, int height,
                         int64 time_stamp_ns);
   void FrameDelivered(const scoped_refptr<PPB_ImageData_Impl>& image_data);
 
   scoped_refptr<base::MessageLoopProxy> io_message_loop_;
 
-  // |frame_pool_| and |new_frame_callback_| are only used on the IO-thread.
+  // |frame_pool_|, |new_frame_callback_| and |endian_| are only used on the
+  // IO-thread.
   media::VideoFramePool frame_pool_;
   VideoCaptureDeliverFrameCB new_frame_callback_;
+  PixelEndian endian_;
 
   // Used to DCHECK that we are called on the main render thread.
   base::ThreadChecker thread_checker_;
 };
 
 PpFrameWriter::FrameWriterDelegate::FrameWriterDelegate(
     const scoped_refptr<base::MessageLoopProxy>& io_message_loop_proxy)
-    : io_message_loop_(io_message_loop_proxy) {
+    : io_message_loop_(io_message_loop_proxy), endian_(UNKNOWN) {
 }
 
 PpFrameWriter::FrameWriterDelegate::~FrameWriterDelegate() {
 }
 
 void PpFrameWriter::FrameWriterDelegate::StartDeliver(
     const VideoCaptureDeliverFrameCB& frame_callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
   io_message_loop_->PostTask(
       FROM_HERE,
@@ skipping 18 matching lines @@
                << "The image could not be mapped and is unusable.";
     return;
   }
 
   const SkBitmap* bitmap = image_data->GetMappedBitmap();
   if (!bitmap) {
     LOG(ERROR) << "PpFrameWriter::PutFrame - "
                << "The image_data's mapped bitmap is NULL.";
     return;
   }
+  // We only support PP_IMAGEDATAFORMAT_BGRA_PREMUL at the moment.
+  DCHECK(image_data->format() == PP_IMAGEDATAFORMAT_BGRA_PREMUL);
   io_message_loop_->PostTaskAndReply(
       FROM_HERE,
       base::Bind(&FrameWriterDelegate::DeliverFrameOnIO, this,
                  static_cast<uint8*>(bitmap->getPixels()),
                  bitmap->rowBytes(),
                  bitmap->width(),
                  bitmap->height(),
                  time_stamp_ns),
       base::Bind(&FrameWriterDelegate::FrameDelivered, this,
                  image_data));
@@ skipping 32 matching lines @@
   // WebRtcVideoCapturerAdapter before the frame is delivered to libJingle.
   // crbug/359587.
   scoped_refptr<media::VideoFrame> new_frame =
       frame_pool_.CreateFrame(media::VideoFrame::YV12, frame_size,
                               gfx::Rect(frame_size), frame_size, timestamp);
   media::VideoCaptureFormat format(
       frame_size,
       MediaStreamVideoSource::kUnknownFrameRate,
       media::PIXEL_FORMAT_YV12);
 
-  libyuv::BGRAToI420(data,
-                     stride,
-                     new_frame->data(media::VideoFrame::kYPlane),
-                     new_frame->stride(media::VideoFrame::kYPlane),
-                     new_frame->data(media::VideoFrame::kUPlane),
-                     new_frame->stride(media::VideoFrame::kUPlane),
-                     new_frame->data(media::VideoFrame::kVPlane),
-                     new_frame->stride(media::VideoFrame::kVPlane),
-                     frame_size.width(), frame_size.height());
+  // TODO(magjed): Remove this and always use libyuv::ARGBToI420 when
+  // crbug/426020 is fixed.
+  // Due to confusion about endianness, we try to determine it from the data.

[fbarchard, 2014/10/22 21:47:43]

I'm not aware of any 'confusion'.  Pepper originally only supported BGRA - big
endian, A,R,G,B.   Now that we can use ARGB, we're switching.  But we dont have
an easy way to confirm the chrome version, so we're auto detecting the pixel
channel order.

[magjed_chromium, 2014/10/23 10:59:35]

I removed 'confusion' from the comment. Something is inconsistent in the current
code though, because the comments for PP_ImageDataFormat state:
* The third part of each enumeration value describes the memory layout from
 * the lowest address to the highest. For example, BGRA means the B component
 * is stored in the lowest address, no matter what endianness the platform is
 * using.
and this is currently not true. Link to PP_ImageDataFormat:
https://code.google.com/p/chromium/codesearch#chromium/src/ppapi/c/ppb_image_...

+  // The alpha channel is always 255. It is unlikely for other color channels to
+  // be 255, so we will most likely break on the first few pixels in the first
+  // frame.
+  uint8* row_ptr = data;
+  for (int y = 0; y < height && endian_ == UNKNOWN; ++y) {

[fbarchard, 2014/10/22 21:47:43]

consider making endian_ static so it detects once.

[magjed_chromium, 2014/10/23 10:59:35]

I prefer to keep it per stream. static variables for POD is allowed, but not
encouraged. We recently moved the color conversion to the IO thread in this CL:
https://codereview.chromium.org/631903003/
so it should be thread safe now, but we might revert that CL in the future.
Also, making it static is a minor optimization.

+    for (int x = 0; x < width; ++x) {
+      if (row_ptr[x * 4 + 0] != 255) {  // First byte is not Alpha => XXXA.
+        endian_ = XXXA;
+        break;
+      }
+      if (row_ptr[x * 4 + 3] != 255) {  // Fourth byte is not Alpha => AXXX.
+        endian_ = AXXX;
+        break;
+      }
+    }
+    row_ptr += stride;
+  }
+  if (endian_ == UNKNOWN) {
+    LOG(WARNING) << "PpFrameWriter::FrameWriterDelegate::DeliverFrameOnIO - "
+                 << "Could not determine endianness.";
+  }
+  // libyuv uses reversed memory byte order, that is why the naming is reversed.

[fbarchard, 2014/10/22 21:47:43]

comment misleading.  libyuv specifies fourcc/channel ordering the same as
webrtc.
FYI Chrome supports 4 channel orderings: ARGB, BGRA, RGBA, and ABGR.

[magjed_chromium, 2014/10/23 10:59:36]

Done.

+  const auto xxxxToI420 =
+      (endian_ == AXXX) ? libyuv::BGRAToI420 : libyuv::ARGBToI420;
+  xxxxToI420(data,

[fbarchard, 2014/10/22 21:47:43]

consider using ConvertToI420, which takes the fourcc as a parameter.

is 'auto' safe to use?

[magjed_chromium, 2014/10/23 10:59:36]

I can't pass the src stride to ConvertToI420 - it assumes it is tightly packed.
'auto' was recently approved for chromium, ref: http://chromium-cpp.appspot.com/

+             stride,
+             new_frame->data(media::VideoFrame::kYPlane),
+             new_frame->stride(media::VideoFrame::kYPlane),
+             new_frame->data(media::VideoFrame::kUPlane),
+             new_frame->stride(media::VideoFrame::kUPlane),
+             new_frame->data(media::VideoFrame::kVPlane),
+             new_frame->stride(media::VideoFrame::kVPlane),
+             width,
+             height);
 
   // The local time when this frame is generated is unknown so give a null
   // value to |estimated_capture_time|.
   new_frame_callback_.Run(new_frame, format, base::TimeTicks());
 }
 
 PpFrameWriter::PpFrameWriter() {
   DVLOG(3) << "PpFrameWriter ctor";
   delegate_ = new FrameWriterDelegate(io_message_loop());
 }
@@ skipping 78 matching lines @@
   bool track_enabled = true;
 
   stream.addTrack(MediaStreamVideoTrack::CreateVideoTrack(
       writer, constraints, MediaStreamVideoSource::ConstraintsCallback(),
       track_enabled));
 
   return true;
 }
 
 }  // namespace content