media/gpu/linux: MailboxVideoFrameConverter cleanups

This CL has a few cleanups that were punted during crrev.com/c/1772642

- Using UniqueID ISO int
- Use base::small_map
- cleanup includes
- forward declare some classes
- using base::Location in OnError()

Bug: 998279
Change-Id: I654411ab5f7dbc6527e3805d76887eb202ddd40a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1819681
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Andres Calderon Jaramillo <andrescj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#701020}

media/gpu/linux/mailbox_video_frame_converter.cc

@@ -6,6 +6,7 @@
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
+#include "base/location.h"
 #include "base/memory/ptr_util.h"
 #include "base/single_thread_task_runner.h"
 #include "base/task/post_task.h"
@@ -16,6 +17,7 @@
 #include "gpu/ipc/service/gpu_channel.h"
 #include "gpu/ipc/service/shared_image_stub.h"
 #include "media/base/format_utils.h"
+#include "media/base/video_frame.h"
 #include "media/gpu/linux/platform_video_frame_utils.h"
 #include "media/gpu/macros.h"
 #include "ui/gfx/gpu_memory_buffer.h"
@@ -141,14 +143,14 @@ void MailboxVideoFrameConverter::ConvertFrame(scoped_refptr<VideoFrame> frame) {
   DVLOGF(4);
 
   if (!frame || !frame->HasDmaBufs())
-    return OnError("Invalid frame.");
+    return OnError(FROM_HERE, "Invalid frame.");
 
   VideoFrame* origin_frame = unwrap_frame_cb_.Run(*frame);
   if (!origin_frame)
-    return OnError("Failed to get origin frame.");
+    return OnError(FROM_HERE, "Failed to get origin frame.");
 
   gpu::Mailbox mailbox;
-  const int origin_frame_id = origin_frame->unique_id();
+  const UniqueID origin_frame_id = origin_frame->unique_id();
   if (shared_images_.find(origin_frame_id) != shared_images_.end())
     mailbox = shared_images_[origin_frame_id]->mailbox();
 
@@ -170,7 +172,7 @@ void MailboxVideoFrameConverter::WrapMailboxAndVideoFrameAndOutput(
   DCHECK(parent_task_runner_->RunsTasksInCurrentSequence());
   DCHECK(!mailbox.IsZero());
 
-  const int origin_frame_id = origin_frame->unique_id();
+  const UniqueID origin_frame_id = origin_frame->unique_id();
   DCHECK(base::Contains(shared_images_, origin_frame_id));
 
   // While we were on |gpu_task_runner_|, AbortPendingFrames() might have been
@@ -270,15 +272,15 @@ MailboxVideoFrameConverter::GenerateSharedImageOnGPUThread(
 
   // TODO(crbug.com/998279): consider eager initialization.
   if (!InitializeOnGPUThread()) {
-    OnError("InitializeOnGPUThread failed");
+    OnError(FROM_HERE, "InitializeOnGPUThread failed");
     return nullptr;
   }
 
   const auto buffer_format =
       VideoPixelFormatToGfxBufferFormat(video_frame->format());
   if (!buffer_format) {
-    OnError("Unsupported format: " +
-            VideoPixelFormatToString(video_frame->format()));
+    OnError(FROM_HERE, "Unsupported format: " +
+                           VideoPixelFormatToString(video_frame->format()));
     return nullptr;
   }
 
@@ -289,7 +291,7 @@ MailboxVideoFrameConverter::GenerateSharedImageOnGPUThread(
   gpu::Mailbox mailbox = gpu::Mailbox::GenerateForSharedImage();
 
   if (!gpu_channel_) {
-    OnError("GpuChannel is gone!");
+    OnError(FROM_HERE, "GpuChannel is gone!");
     return nullptr;
   }
   gpu::SharedImageStub* shared_image_stub = gpu_channel_->shared_image_stub();
@@ -305,7 +307,7 @@ MailboxVideoFrameConverter::GenerateSharedImageOnGPUThread(
       gpu::kNullSurfaceHandle, video_frame->coded_size(),
       video_frame->ColorSpace(), shared_image_usage);
   if (!success) {
-    OnError("Failed to create shared image.");
+    OnError(FROM_HERE, "Failed to create shared image.");
     return nullptr;
   }
   // There's no need to UpdateSharedImage() after CreateSharedImage().
@@ -328,7 +330,7 @@ void MailboxVideoFrameConverter::RegisterSharedImage(
   origin_frame->AddDestructionObserver(base::BindOnce(
       [](scoped_refptr<base::SequencedTaskRunner> parent_task_runner,
          base::WeakPtr<MailboxVideoFrameConverter> parent_weak_ptr,
-         int origin_frame_id) {
+         UniqueID origin_frame_id) {
         if (parent_task_runner->RunsTasksInCurrentSequence()) {
           if (parent_weak_ptr)
             parent_weak_ptr->UnregisterSharedImage(origin_frame_id);
@@ -346,13 +348,13 @@ bool MailboxVideoFrameConverter::UpdateSharedImageOnGPUThread(
     const gpu::Mailbox& mailbox) {
   DCHECK(gpu_task_runner_->BelongsToCurrentThread());
   if (!gpu_channel_) {
-    OnError("GpuChannel is gone!");
+    OnError(FROM_HERE, "GpuChannel is gone!");
     return false;
   }
   gpu::SharedImageStub* shared_image_stub = gpu_channel_->shared_image_stub();
   DCHECK(shared_image_stub);
   if (!shared_image_stub->UpdateSharedImage(mailbox, gfx::GpuFenceHandle())) {
-    OnError("Could not update shared image");
+    OnError(FROM_HERE, "Could not update shared image");
     return false;
   }
   return true;
@@ -364,7 +366,7 @@ void MailboxVideoFrameConverter::WaitOnSyncTokenAndReleaseFrameOnGPUThread(
   DCHECK(gpu_task_runner_->BelongsToCurrentThread());
 
   if (!gpu_channel_)
-    return OnError("GpuChannel is gone!");
+    return OnError(FROM_HERE, "GpuChannel is gone!");
   gpu::SharedImageStub* shared_image_stub = gpu_channel_->shared_image_stub();
   DCHECK(shared_image_stub);
 
@@ -377,7 +379,8 @@ void MailboxVideoFrameConverter::WaitOnSyncTokenAndReleaseFrameOnGPUThread(
       std::vector<gpu::SyncToken>({sync_token})));
 }
 
-void MailboxVideoFrameConverter::UnregisterSharedImage(int origin_frame_id) {
+void MailboxVideoFrameConverter::UnregisterSharedImage(
+    UniqueID origin_frame_id) {
   DCHECK(parent_task_runner_->RunsTasksInCurrentSequence());
   DVLOGF(4);
 
@@ -400,8 +403,9 @@ bool MailboxVideoFrameConverter::HasPendingFrames() const {
   return !input_frame_queue_.empty();
 }
 
-void MailboxVideoFrameConverter::OnError(const std::string& msg) {
-  VLOGF(1) << msg;
+void MailboxVideoFrameConverter::OnError(const base::Location& location,
+                                         const std::string& msg) {
+  VLOGF(1) << "(" << location.ToString() << ") " << msg;
 
   parent_task_runner_->PostTask(
       FROM_HERE, base::BindOnce(&MailboxVideoFrameConverter::AbortPendingFrames,

media/gpu/linux/mailbox_video_frame_converter.h

@@ -5,20 +5,20 @@
 #ifndef MEDIA_GPU_LINUX_MAILBOX_VIDEO_FRAME_CONVERTER_H_
 #define MEDIA_GPU_LINUX_MAILBOX_VIDEO_FRAME_CONVERTER_H_
 
-#include <map>
-
 #include "base/callback_forward.h"
 #include "base/containers/queue.h"
-#include "base/memory/ref_counted.h"
+#include "base/containers/small_map.h"
 #include "base/memory/scoped_refptr.h"
 #include "base/memory/weak_ptr.h"
-#include "base/single_thread_task_runner.h"
 #include "gpu/command_buffer/common/mailbox.h"
-#include "media/base/video_decoder.h"
-#include "media/base/video_frame.h"
 #include "media/gpu/media_gpu_export.h"
 #include "media/gpu/video_frame_converter.h"
 
+namespace base {
+class Location;
+class SingleThreadTaskRunner;
+}  // namespace base
+
 namespace gpu {
 class GpuChannel;
 class CommandBufferStub;
@@ -26,6 +26,8 @@ class CommandBufferStub;
 
 namespace media {
 
+class VideoFrame;
+
 // This class is used for converting DMA-buf backed VideoFrames to mailbox-based
 // VideoFrames. See ConvertFrame() for more details.
 // After conversion, the mailbox VideoFrame will retain a reference of the
@@ -59,6 +61,9 @@ class MEDIA_GPU_EXPORT MailboxVideoFrameConverter : public VideoFrameConverter {
   bool HasPendingFrames() const override;
 
  private:
+  // Use VideoFrame::unique_id() as internal VideoFrame indexing.
+  using UniqueID = decltype(std::declval<VideoFrame>().unique_id());
+
   // A self-cleaning SharedImage, with move-only semantics.
   class ScopedSharedImage;
 
@@ -100,7 +105,7 @@ class MEDIA_GPU_EXPORT MailboxVideoFrameConverter : public VideoFrameConverter {
       VideoFrame* origin_frame,
       std::unique_ptr<ScopedSharedImage> scoped_shared_image);
   // Unregisters the |origin_frame_id| and associated SharedImage.
-  void UnregisterSharedImage(int origin_frame_id);
+  void UnregisterSharedImage(UniqueID origin_frame_id);
 
   // Updates the SharedImage associated to |mailbox|. Returns true if the update
   // could be carried out, false otherwise.
@@ -113,7 +118,7 @@ class MEDIA_GPU_EXPORT MailboxVideoFrameConverter : public VideoFrameConverter {
       const gpu::SyncToken& sync_token);
 
   // Invoked when any error occurs. |msg| is the error message.
-  void OnError(const std::string& msg);
+  void OnError(const base::Location& location, const std::string& msg);
 
   // In DmabufVideoFramePool, we recycle the unused frames. To do that, each
   // time a frame is requested from the pool it is wrapped inside another frame.
@@ -134,14 +139,14 @@ class MEDIA_GPU_EXPORT MailboxVideoFrameConverter : public VideoFrameConverter {
 
   // Mapping from the unique id of the frame to its corresponding SharedImage.
   // Accessed only on |parent_task_runner_|.
-  // TODO(crbug.com/998279): use base::small_map.
-  // TODO(crbug.com/998279): use VideoFrame::unique_id() return type.
-  std::map<int, std::unique_ptr<ScopedSharedImage>> shared_images_;
+  base::small_map<std::map<UniqueID, std::unique_ptr<ScopedSharedImage>>>
+      shared_images_;
 
   // The queue of input frames and the unique_id of their origin frame.
   // Accessed only on |parent_task_runner_|.
   // TODO(crbug.com/998279): remove this member entirely.
-  base::queue<std::pair<scoped_refptr<VideoFrame>, int>> input_frame_queue_;
+  base::queue<std::pair<scoped_refptr<VideoFrame>, UniqueID>>
+      input_frame_queue_;
 
   // The weak pointer of this, bound to |parent_task_runner_|.
   // Used at the VideoFrame destruction callback.