Revert "Remove default argument in VideoEncodeAccelerator::Client::In…

…itialize()"

This reverts commit a82e262.

Reason for revert: Broken ChromeOS video.EncodeAccel* tests
(with a segmentation fault, see bug below).

Bug: b:225386785

Original change's description:
> Remove default argument in VideoEncodeAccelerator::Client::Initialize()
>
> As default arguments are banned on virtual functions, per style guide,
> this CL makes sure we explicitly pass in a NullMediaLog in the cases
> where there isn't one, instead of making all the VideoEncodeAccelerator
> implementations check and create it.
> This CL also uses std::move() instead of calling Clone on media_log in
> VideoEncodeAcceleratorAdapter constructor, and moves
> VideoEncodeAccelerator media_log.h include to .cc files.
>
> Bug: 1296286
> Change-Id: I0b45ce6920be32dd788826d703388e210f2e94b2
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3516294
> Reviewed-by: Dan Sanders <sandersd@chromium.org>
> Reviewed-by: Markus Handell <handellm@google.com>
> Reviewed-by: Lambros Lambrou <lambroslambrou@chromium.org>
> Reviewed-by: Derek Schuff <dschuff@chromium.org>
> Reviewed-by: Chih-Yu Huang <akahuang@chromium.org>
> Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
> Commit-Queue: Lei Zhang <thestig@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#981339}

Bug: 1296286
Change-Id: I9cdd865420607ab63221861567796a08bbd3da95
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3539042
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Owners-Override: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#983171}

ash/components/arc/video_accelerator/gpu_arc_video_encode_accelerator.cc

@@ -18,7 +18,6 @@
 #include "media/base/bitrate.h"
 #include "media/base/color_plane_layout.h"
 #include "media/base/format_utils.h"
-#include "media/base/media_log.h"
 #include "media/base/video_types.h"
 #include "media/gpu/buffer_validation.h"
 #include "media/gpu/gpu_video_encode_accelerator_factory.h"

components/viz/service/gl/DEPS

@@ -14,7 +14,6 @@ include_rules = [
   "+gpu/vulkan",
   "+ipc",
   "+media/base/android/media_codec_util.h",
-  "+media/base/media_log.h",
   "+media/gpu",
   "+media/mojo",
   "+mojo/public/cpp",

components/viz/service/gl/gpu_service_impl.cc

@@ -49,7 +49,6 @@
 #include "ipc/ipc_channel_handle.h"
 #include "ipc/ipc_sync_channel.h"
 #include "ipc/ipc_sync_message_filter.h"
-#include "media/base/media_log.h"
 #include "media/gpu/buildflags.h"
 #include "media/gpu/gpu_video_accelerator_util.h"
 #include "media/gpu/gpu_video_encode_accelerator_factory.h"

content/renderer/pepper/pepper_video_encoder_host.cc

@@ -21,7 +21,6 @@
 #include "gpu/command_buffer/common/context_creation_attribs.h"
 #include "gpu/ipc/client/command_buffer_proxy_impl.h"
 #include "media/base/bitrate.h"
-#include "media/base/media_log.h"
 #include "media/video/video_encode_accelerator.h"
 #include "ppapi/c/pp_codecs.h"
 #include "ppapi/c/pp_errors.h"

content/renderer/pepper/video_encoder_shim.cc

@@ -17,7 +17,6 @@
 #include "base/threading/thread_task_runner_handle.h"
 #include "content/renderer/pepper/pepper_video_encoder_host.h"
 #include "content/renderer/render_thread_impl.h"
-#include "media/base/media_log.h"
 #include "media/video/video_encode_accelerator.h"
 #include "third_party/libvpx/source/libvpx/vpx/vp8cx.h"
 #include "third_party/libvpx/source/libvpx/vpx/vpx_encoder.h"

media/cast/sender/external_video_encoder.cc

@@ -22,7 +22,6 @@
 #include "media/base/bind_to_current_loop.h"
 #include "media/base/bitrate.h"
 #include "media/base/media_switches.h"
-#include "media/base/media_util.h"
 #include "media/base/video_frame.h"
 #include "media/base/video_types.h"
 #include "media/base/video_util.h"
@@ -141,8 +140,7 @@ class ExternalVideoEncoder::VEAClientImpl final
         base::saturated_cast<uint32_t>(start_bit_rate));
     const media::VideoEncodeAccelerator::Config config(
         media::PIXEL_FORMAT_I420, frame_size, codec_profile, bitrate);
-    encoder_active_ = video_encode_accelerator_->Initialize(
-        config, this, std::make_unique<media::NullMediaLog>());
+    encoder_active_ = video_encode_accelerator_->Initialize(config, this);
     next_frame_id_ = first_frame_id;
     codec_profile_ = codec_profile;
 

media/gpu/android/android_video_encode_accelerator.cc

@@ -20,6 +20,7 @@
 #include "media/base/bitstream_buffer.h"
 #include "media/base/limits.h"
 #include "media/base/media_log.h"
+#include "media/base/media_util.h"
 #include "media/base/unaligned_shared_memory.h"
 #include "media/video/picture.h"
 #include "third_party/libyuv/include/libyuv/convert_from.h"
@@ -150,6 +151,10 @@ bool AndroidVideoEncodeAccelerator::Initialize(
 
   client_ptr_factory_ = std::make_unique<base::WeakPtrFactory<Client>>(client);
 
+  // NullMediaLog silently and safely does nothing.
+  if (!media_log)
+    media_log = std::make_unique<media::NullMediaLog>();
+
   if (config.input_format != PIXEL_FORMAT_I420) {
     MEDIA_LOG(ERROR, media_log.get())
         << "Unexpected combo: " << config.input_format << ", "

media/gpu/gpu_video_encode_accelerator_factory.cc

@@ -13,6 +13,7 @@
 #include "gpu/config/gpu_preferences.h"
 #include "media/base/media_log.h"
 #include "media/base/media_switches.h"
+#include "media/base/media_util.h"
 #include "media/gpu/buildflags.h"
 #include "media/gpu/gpu_video_accelerator_util.h"
 #include "media/gpu/macros.h"
@@ -150,6 +151,10 @@ GpuVideoEncodeAcceleratorFactory::CreateVEA(
     const gpu::GpuPreferences& gpu_preferences,
     const gpu::GpuDriverBugWorkarounds& gpu_workarounds,
     std::unique_ptr<MediaLog> media_log) {
+  // NullMediaLog silently and safely does nothing.
+  if (!media_log)
+    media_log = std::make_unique<media::NullMediaLog>();
+
   for (const auto& create_vea :
        GetVEAFactoryFunctions(gpu_preferences, gpu_workarounds)) {
     std::unique_ptr<VideoEncodeAccelerator> vea = create_vea.Run();

media/gpu/mac/vt_video_encode_accelerator_mac.cc

@@ -16,6 +16,7 @@
 #include "media/base/bitrate.h"
 #include "media/base/mac/video_frame_mac.h"
 #include "media/base/media_log.h"
+#include "media/base/media_util.h"
 
 // This is a min version of macOS where we want to support SVC encoding via
 // EnableLowLatencyRateControl flag. The flag is actually supported since 11.3,
@@ -163,6 +164,10 @@ bool VTVideoEncodeAccelerator::Initialize(const Config& config,
   DCHECK_CALLED_ON_VALID_SEQUENCE(client_sequence_checker_);
   DCHECK(client);
 
+  // NullMediaLog silently and safely does nothing.
+  if (!media_log)
+    media_log = std::make_unique<media::NullMediaLog>();
+
   // Clients are expected to call Flush() before reinitializing the encoder.
   DCHECK_EQ(pending_encodes_, 0);
 

media/gpu/test/video_encoder/video_encoder_client.cc

@@ -17,7 +17,6 @@
 #include "gpu/ipc/service/gpu_memory_buffer_factory.h"
 #include "media/base/bind_to_current_loop.h"
 #include "media/base/bitrate.h"
-#include "media/base/media_log.h"
 #include "media/gpu/gpu_video_encode_accelerator_factory.h"
 #include "media/gpu/macros.h"
 #include "media/gpu/test/bitstream_helpers.h"

media/gpu/v4l2/v4l2_video_encode_accelerator.cc

@@ -31,6 +31,7 @@
 #include "media/base/bitstream_buffer.h"
 #include "media/base/color_plane_layout.h"
 #include "media/base/media_log.h"
+#include "media/base/media_util.h"
 #include "media/base/scopedfd_helper.h"
 #include "media/base/unaligned_shared_memory.h"
 #include "media/base/video_frame_layout.h"
@@ -205,6 +206,10 @@ bool V4L2VideoEncodeAccelerator::Initialize(
   TRACE_EVENT0("media,gpu", "V4L2VEA::Initialize");
   VLOGF(2) << ": " << config.AsHumanReadableString();
 
+  // NullMediaLog silently and safely does nothing.
+  if (!media_log)
+    media_log = std::make_unique<media::NullMediaLog>();
+
   // V4L2VEA doesn't support temporal layers but we let it pass here to support
   // simulcast.
   if (config.HasSpatialLayer()) {

media/gpu/vaapi/vaapi_video_encode_accelerator.cc

@@ -35,6 +35,7 @@
 #include "media/base/format_utils.h"
 #include "media/base/media_log.h"
 #include "media/base/media_switches.h"
+#include "media/base/media_util.h"
 #include "media/base/unaligned_shared_memory.h"
 #include "media/base/video_bitrate_allocation.h"
 #include "media/gpu/chromeos/platform_video_frame_utils.h"
@@ -181,11 +182,16 @@ VaapiVideoEncodeAccelerator::~VaapiVideoEncodeAccelerator() {
 bool VaapiVideoEncodeAccelerator::Initialize(
     const Config& config,
     Client* client,
+
     std::unique_ptr<MediaLog> media_log) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(child_sequence_checker_);
   DCHECK_EQ(state_, kUninitialized);
   VLOGF(2) << "Initializing VAVEA, " << config.AsHumanReadableString();
 
+  // NullMediaLog silently and safely does nothing.
+  if (!media_log)
+    media_log = std::make_unique<media::NullMediaLog>();
+
   if (AttemptedInitialization()) {
     MEDIA_LOG(ERROR, media_log.get())
         << "Initialize() cannot be called more than once.";

media/gpu/vaapi/vaapi_video_encode_accelerator_unittest.cc

@@ -12,7 +12,6 @@
 #include "base/run_loop.h"
 #include "base/test/gmock_callback_support.h"
 #include "base/test/task_environment.h"
-#include "media/base/media_util.h"
 #include "media/gpu/gpu_video_encode_accelerator_helpers.h"
 #include "media/gpu/vaapi/vaapi_utils.h"
 #include "media/gpu/vaapi/vaapi_wrapper.h"
@@ -300,8 +299,7 @@ class VaapiVideoEncodeAcceleratorTest
     vaapi_encoder->supported_profiles_for_testing_.push_back(profile);
     if (config.input_visible_size.IsEmpty())
       return false;
-    return encoder_->Initialize(config, &client_,
-                                std::make_unique<media::NullMediaLog>());
+    return encoder_->Initialize(config, &client_);
   }
 
   void InitializeSequenceForVP9(const VideoEncodeAccelerator::Config& config)

media/gpu/windows/media_foundation_video_encode_accelerator_win.cc

@@ -31,6 +31,7 @@
 #include "gpu/ipc/common/dxgi_helpers.h"
 #include "media/base/media_log.h"
 #include "media/base/media_switches.h"
+#include "media/base/media_util.h"
 #include "media/base/win/mf_helpers.h"
 #include "media/base/win/mf_initializer.h"
 #include "media/gpu/gpu_video_encode_accelerator_helpers.h"
@@ -331,6 +332,10 @@ bool MediaFoundationVideoEncodeAccelerator::Initialize(
   DVLOG(3) << __func__ << ": " << config.AsHumanReadableString();
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
+  // NullMediaLog silently and safely does nothing.
+  if (!media_log)
+    media_log = std::make_unique<media::NullMediaLog>();
+
   if (PIXEL_FORMAT_I420 != config.input_format &&
       PIXEL_FORMAT_NV12 != config.input_format) {
     MEDIA_LOG(ERROR, media_log.get())

media/mojo/clients/mojo_video_encode_accelerator.cc

@@ -12,6 +12,7 @@
 #include "build/build_config.h"
 #include "gpu/ipc/client/gpu_channel_host.h"
 #include "media/base/media_log.h"
+#include "media/base/media_util.h"
 #include "media/base/video_frame.h"
 #include "media/gpu/gpu_video_accelerator_util.h"
 #include "media/mojo/clients/mojo_media_log_service.h"
@@ -138,6 +139,10 @@ bool MojoVideoEncodeAccelerator::Initialize(
       base::BindOnce(&MojoVideoEncodeAccelerator::MojoDisconnectionHandler,
                      base::Unretained(this)));
 
+  // NullMediaLog silently and safely does nothing.
+  if (!media_log)
+    media_log = std::make_unique<media::NullMediaLog>();
+
   // Use `mojo::MakeSelfOwnedReceiver` for MediaLog so logs may go through even
   // after `MojoVideoEncodeAccelerator` is destructed.
   mojo::PendingReceiver<mojom::MediaLog> media_log_pending_receiver;

media/mojo/clients/mojo_video_encode_accelerator_unittest.cc

@@ -10,7 +10,6 @@
 #include "base/run_loop.h"
 #include "base/test/task_environment.h"
 #include "gpu/config/gpu_info.h"
-#include "media/base/media_util.h"
 #include "media/mojo/clients/mojo_video_encode_accelerator.h"
 #include "media/mojo/mojom/video_encode_accelerator.mojom.h"
 #include "media/video/video_encode_accelerator.h"
@@ -206,8 +205,7 @@ class MojoVideoEncodeAcceleratorTest : public ::testing::Test {
         PIXEL_FORMAT_I420, kInputVisibleSize, kOutputProfile, kInitialBitrate,
         absl::nullopt, absl::nullopt, absl::nullopt, false, absl::nullopt,
         kContentType);
-    EXPECT_TRUE(mojo_vea()->Initialize(
-        config, mock_vea_client, std::make_unique<media::NullMediaLog>()));
+    EXPECT_TRUE(mojo_vea()->Initialize(config, mock_vea_client));
     base::RunLoop().RunUntilIdle();
   }
 
@@ -335,8 +333,7 @@ TEST_F(MojoVideoEncodeAcceleratorTest, InitializeFailure) {
   const VideoEncodeAccelerator::Config config(
       PIXEL_FORMAT_I420, kInputVisibleSize, VIDEO_CODEC_PROFILE_UNKNOWN,
       kInitialBitrate);
-  EXPECT_FALSE(mojo_vea()->Initialize(config, mock_vea_client.get(),
-                                      std::make_unique<media::NullMediaLog>()));
+  EXPECT_FALSE(mojo_vea()->Initialize(config, mock_vea_client.get()));
   base::RunLoop().RunUntilIdle();
 }
 
@@ -349,8 +346,7 @@ TEST_F(MojoVideoEncodeAcceleratorTest, MojoDisconnect) {
   const VideoEncodeAccelerator::Config config(
       PIXEL_FORMAT_I420, kInputVisibleSize, VIDEO_CODEC_PROFILE_UNKNOWN,
       kInitialBitrate);
-  EXPECT_TRUE(mojo_vea()->Initialize(config, mock_vea_client.get(),
-                                     std::make_unique<media::NullMediaLog>()));
+  EXPECT_TRUE(mojo_vea()->Initialize(config, mock_vea_client.get()));
   mojo_vea_receiver_->Close();
   EXPECT_CALL(
       *mock_vea_client,

media/mojo/test/mojo_video_encode_accelerator_integration_test.cc

@@ -14,7 +14,6 @@
 #include "gpu/config/gpu_driver_bug_workarounds.h"
 #include "gpu/config/gpu_preferences.h"
 #include "media/base/limits.h"
-#include "media/base/media_util.h"
 #include "media/mojo/clients/mojo_video_encode_accelerator.h"
 #include "media/mojo/mojom/video_encode_accelerator.mojom.h"
 #include "media/mojo/services/mojo_video_encode_accelerator_service.h"
@@ -118,8 +117,7 @@ class MojoVideoEncodeAcceleratorIntegrationTest : public ::testing::Test {
     const VideoEncodeAccelerator::Config config(
         PIXEL_FORMAT_I420, kInputVisibleSize, kValidOutputProfile,
         kInitialBitrate);
-    EXPECT_TRUE(mojo_vea()->Initialize(
-        config, mock_vea_client, std::make_unique<media::NullMediaLog>()));
+    EXPECT_TRUE(mojo_vea()->Initialize(config, mock_vea_client));
     base::RunLoop().RunUntilIdle();
   }
 
@@ -152,8 +150,7 @@ TEST_F(MojoVideoEncodeAcceleratorIntegrationTest,
   const VideoEncodeAccelerator::Config config(
       PIXEL_FORMAT_I420, kInputVisibleSize, kValidOutputProfile,
       kInitialBitrate);
-  EXPECT_FALSE(mojo_vea()->Initialize(config, invalid_client,
-                                      std::make_unique<media::NullMediaLog>()));
+  EXPECT_FALSE(mojo_vea()->Initialize(config, invalid_client));
   base::RunLoop().RunUntilIdle();
 }
 
@@ -168,8 +165,7 @@ TEST_F(MojoVideoEncodeAcceleratorIntegrationTest,
   const VideoEncodeAccelerator::Config config(
       PIXEL_FORMAT_I420, kInvalidInputVisibleSize, kValidOutputProfile,
       kInitialBitrate);
-  EXPECT_FALSE(mojo_vea()->Initialize(config, mock_vea_client.get(),
-                                      std::make_unique<media::NullMediaLog>()));
+  EXPECT_FALSE(mojo_vea()->Initialize(config, mock_vea_client.get()));
   base::RunLoop().RunUntilIdle();
 }
 // This test verifies that Initialize() fails when called with an invalid codec
@@ -184,8 +180,7 @@ TEST_F(MojoVideoEncodeAcceleratorIntegrationTest,
   const VideoEncodeAccelerator::Config config(
       PIXEL_FORMAT_I420, kInputVisibleSize, kInvalidOutputProfile,
       kInitialBitrate);
-  EXPECT_FALSE(mojo_vea()->Initialize(config, mock_vea_client.get(),
-                                      std::make_unique<media::NullMediaLog>()));
+  EXPECT_FALSE(mojo_vea()->Initialize(config, mock_vea_client.get()));
   base::RunLoop().RunUntilIdle();
 }
 

media/video/fake_video_encode_accelerator.cc

@@ -9,7 +9,6 @@
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/task/sequenced_task_runner.h"
-#include "media/base/media_log.h"
 
 namespace media {
 

media/video/mock_video_encode_accelerator.cc

@@ -4,8 +4,6 @@
 
 #include "media/video/mock_video_encode_accelerator.h"
 
-#include "media/base/media_log.h"
-
 namespace media {
 
 using ::testing::Invoke;

media/video/video_encode_accelerator.h

@@ -18,6 +18,7 @@
 #include "media/base/bitrate.h"
 #include "media/base/bitstream_buffer.h"
 #include "media/base/media_export.h"
+#include "media/base/media_log.h"
 #include "media/base/svc_scalability_mode.h"
 #include "media/base/video_bitrate_allocation.h"
 #include "media/base/video_codecs.h"
@@ -369,7 +370,7 @@ class MEDIA_EXPORT VideoEncodeAccelerator {
   // TODO(sheu): handle resolution changes.  http://crbug.com/249944
   virtual bool Initialize(const Config& config,
                           Client* client,
-                          std::unique_ptr<MediaLog> media_log) = 0;
+                          std::unique_ptr<MediaLog> media_log = nullptr) = 0;
 
   // Encodes the given frame.
   // The storage type of |frame| must be the |storage_type| if it is specified

media/video/video_encode_accelerator_adapter.cc

@@ -122,7 +122,7 @@ VideoEncodeAcceleratorAdapter::VideoEncodeAcceleratorAdapter(
     : output_pool_(base::MakeRefCounted<base::UnsafeSharedMemoryPool>()),
       input_pool_(base::MakeRefCounted<base::UnsafeSharedMemoryPool>()),
       gpu_factories_(gpu_factories),
-      media_log_(std::move(media_log)),
+      media_log_(media_log->Clone()),
       accelerator_task_runner_(gpu_factories_->GetTaskRunner()),
       callback_task_runner_(std::move(callback_task_runner)) {
   DETACH_FROM_SEQUENCE(accelerator_sequence_checker_);

remoting/codec/webrtc_video_encoder_gpu.cc

@@ -24,7 +24,6 @@
 #include "build/build_config.h"
 #include "gpu/config/gpu_driver_bug_workarounds.h"
 #include "gpu/config/gpu_preferences.h"
-#include "media/base/media_log.h"
 #include "media/base/video_frame.h"
 #include "media/gpu/gpu_video_encode_accelerator_factory.h"
 #include "media/video/video_encode_accelerator.h"

third_party/blink/renderer/modules/mediarecorder/vea_encoder.cc

@@ -13,7 +13,6 @@
 #include "base/task/sequenced_task_runner.h"
 #include "media/base/bind_to_current_loop.h"
 #include "media/base/bitrate.h"
-#include "media/base/media_util.h"
 #include "media/base/video_frame.h"
 #include "media/video/gpu_video_accelerator_factories.h"
 #include "third_party/blink/public/platform/platform.h"
@@ -327,11 +326,8 @@ void VEAEncoder::ConfigureEncoderOnEncodingTaskRunner(const gfx::Size& size,
           base::saturated_cast<uint32_t>(bits_per_second_)),
       absl::nullopt, absl::nullopt, level_, false, storage_type,
       media::VideoEncodeAccelerator::Config::ContentType::kCamera);
-  if (!video_encoder_ ||
-      !video_encoder_->Initialize(config, this,
-                                  std::make_unique<media::NullMediaLog>())) {
+  if (!video_encoder_ || !video_encoder_->Initialize(config, this))
     NotifyError(media::VideoEncodeAccelerator::kPlatformFailureError);
-  }
 }
 
 void VEAEncoder::DestroyOnEncodingTaskRunner(

third_party/blink/renderer/platform/peerconnection/rtc_video_encoder.cc

@@ -28,7 +28,6 @@
 #include "media/base/bitrate.h"
 #include "media/base/bitstream_buffer.h"
 #include "media/base/media_switches.h"
-#include "media/base/media_util.h"
 #include "media/base/video_bitrate_allocation.h"
 #include "media/base/video_frame.h"
 #include "media/base/video_util.h"
@@ -725,8 +724,7 @@ void RTCVideoEncoder::Impl::CreateAndInitializeVEA(
           ? media::VideoEncodeAccelerator::Config::ContentType::kDisplay
           : media::VideoEncodeAccelerator::Config::ContentType::kCamera,
       spatial_layers, inter_layer_pred);
-  if (!video_encoder_->Initialize(config, this,
-                                  std::make_unique<media::NullMediaLog>())) {
+  if (!video_encoder_->Initialize(config, this)) {
     LogAndNotifyError(FROM_HERE, "Error initializing video_encoder",
                       media::VideoEncodeAccelerator::kInvalidArgumentError);
     return;