Revert "media/video: Add SpatialLayer to VideoEncodeAccelerator::Config"

This reverts commit ee98d31c.

Reason for revert:
After the commit, SpatialLayerStructTraitTest.RoundTrip has been
consistently failing on msan.

I was able to reproduce the failure locally on my linux machine by
building and running media_unittests with these gn args:
is_component_build = false
is_debug = false
is_msan = true
msan_track_origins = 2
use_goma = true

Original change's description:
> media/video: Add SpatialLayer to VideoEncodeAccelerator::Config
> 
> This CL adds SpatialLayer to VideoEncodeAccelerator::Config so
> that a VEA client is able to configure a spatial layer encoding.
> The configuration is useful to configure temporal layer encoding
> too. Supporting temporal layer with vp9 hw encoder is a short
> term goal today.
> 
> Design doc: https://docs.google.com/document/d/1yeCV36yhk9zna4qwngPiJGhSUkDyJHRlfEpziFgJfHo#heading=h.aeh6kjktcuzy
> 
> Bug: 1030199
> Test: media_unittests
> Change-Id: I1e0524a71b043ccfa0e41d08e8de4b86e9c17c9e
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2131444
> Commit-Queue: Hirokazu Honda <hiroh@chromium.org>
> Reviewed-by: Dominick Ng <dominickn@chromium.org>
> Reviewed-by: Dan Sanders <sandersd@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#756300}

TBR=sandersd@chromium.org,sprang@chromium.org,dominickn@chromium.org,hiroh@chromium.org,ilnik@chromium.org

Change-Id: I6a86cb5a8f7d5fd2d1b7dbd4b3d6d99d2298c5dd
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1030199,1067758
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2136113Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#756427}

media/mojo/mojom/BUILD.gn

@@ -115,7 +115,7 @@ source_set("unit_tests") {
     "audio_decoder_config_mojom_traits_unittest.cc",
     "cdm_key_information_mojom_traits_unittest.cc",
     "video_decoder_config_mojom_traits_unittest.cc",
-    "video_encode_accelerator_mojom_traits_unittest.cc",
+    "video_encoder_info_mojom_traits_unittest.cc",
     "video_frame_mojom_traits_unittest.cc",
   ]
 

media/mojo/mojom/video_encode_accelerator.mojom

@@ -60,17 +60,6 @@ struct VideoBitrateAllocation {
   array<int32> bitrates;
 };
 
-// This defines a mojo transport format for
-// media::VideoEncodeAccelerator::Config::SpatialLayer.
-struct SpatialLayer {
-  int32 width;
-  int32 height;
-  uint32 bitrate_bps;
-  uint32 framerate;
-  uint8 max_qp;
-  uint8 num_of_temporal_layers;
-};
-
 // This defines a mojo transport format for
 // media::VideoEncodeAccelerator::Config.
 struct VideoEncodeAcceleratorConfig {
@@ -99,7 +88,6 @@ struct VideoEncodeAcceleratorConfig {
   StorageType storage_type;
   bool has_storage_type;  // Whether or not config has storage type config
   ContentType content_type;
-  array<SpatialLayer> spatial_layers;
 };
 
 interface VideoEncodeAccelerator {

media/mojo/mojom/video_encode_accelerator.typemap

@@ -31,7 +31,6 @@ type_mappings = [
   "media.mojom.VideoBitrateAllocation=::media::VideoBitrateAllocation",
   "media.mojom.VideoEncodeAccelerator.Error=::media::VideoEncodeAccelerator::Error",
   "media.mojom.VideoEncodeAcceleratorConfig=::media::VideoEncodeAccelerator::Config",
-  "media.mojom.SpatialLayer=::media::VideoEncodeAccelerator::Config::SpatialLayer",
   "media.mojom.VideoEncodeAcceleratorSupportedProfile=::media::VideoEncodeAccelerator::SupportedProfile",
   "media.mojom.Vp8Metadata=::media::Vp8Metadata",
 ]

media/mojo/mojom/video_encode_accelerator_mojom_traits.cc

@@ -195,20 +195,6 @@ bool EnumTraits<media::mojom::VideoEncodeAcceleratorConfig::ContentType,
   return false;
 }
 
-// static
-bool StructTraits<media::mojom::SpatialLayerDataView,
-                  media::VideoEncodeAccelerator::Config::SpatialLayer>::
-    Read(media::mojom::SpatialLayerDataView input,
-         media::VideoEncodeAccelerator::Config::SpatialLayer* output) {
-  output->width = input.width();
-  output->height = input.height();
-  output->bitrate_bps = input.bitrate_bps();
-  output->framerate = input.framerate();
-  output->max_qp = input.max_qp();
-  output->num_of_temporal_layers = input.num_of_temporal_layers();
-  return true;
-}
-
 // static
 bool StructTraits<media::mojom::VideoEncodeAcceleratorConfigDataView,
                   media::VideoEncodeAccelerator::Config>::
@@ -249,15 +235,10 @@ bool StructTraits<media::mojom::VideoEncodeAcceleratorConfigDataView,
   if (!input.ReadContentType(&content_type))
     return false;
 
-  std::vector<media::VideoEncodeAccelerator::Config::SpatialLayer>
-      spatial_layers;
-  if (!input.ReadSpatialLayers(&spatial_layers))
-    return false;
-
   *output = media::VideoEncodeAccelerator::Config(
       input_format, input_visible_size, output_profile, input.initial_bitrate(),
       initial_framerate, gop_length, h264_output_level, storage_type,
-      content_type, spatial_layers);
+      content_type);
   return true;
 }
 

media/mojo/mojom/video_encode_accelerator_mojom_traits.h

@@ -131,43 +131,6 @@ struct EnumTraits<media::mojom::VideoEncodeAcceleratorConfig::ContentType,
       media::VideoEncodeAccelerator::Config::ContentType* output);
 };
 
-template <>
-struct StructTraits<media::mojom::SpatialLayerDataView,
-                    media::VideoEncodeAccelerator::Config::SpatialLayer> {
-  static int32_t width(
-      const media::VideoEncodeAccelerator::Config::SpatialLayer& input) {
-    return input.width;
-  }
-
-  static int32_t height(
-      const media::VideoEncodeAccelerator::Config::SpatialLayer& input) {
-    return input.height;
-  }
-
-  static uint32_t bitrate_bps(
-      const media::VideoEncodeAccelerator::Config::SpatialLayer& input) {
-    return input.bitrate_bps;
-  }
-
-  static uint32_t framerate(
-      const media::VideoEncodeAccelerator::Config::SpatialLayer& input) {
-    return input.framerate;
-  }
-
-  static uint8_t max_qp(
-      const media::VideoEncodeAccelerator::Config::SpatialLayer& input) {
-    return input.max_qp;
-  }
-
-  static uint8_t num_of_temporal_layers(
-      const media::VideoEncodeAccelerator::Config::SpatialLayer& input) {
-    return input.num_of_temporal_layers;
-  }
-
-  static bool Read(media::mojom::SpatialLayerDataView input,
-                   media::VideoEncodeAccelerator::Config::SpatialLayer* output);
-};
-
 template <>
 struct StructTraits<media::mojom::VideoEncodeAcceleratorConfigDataView,
                     media::VideoEncodeAccelerator::Config> {
@@ -237,11 +200,6 @@ struct StructTraits<media::mojom::VideoEncodeAcceleratorConfigDataView,
     return input.content_type;
   }
 
-  static const std::vector<media::VideoEncodeAccelerator::Config::SpatialLayer>&
-  spatial_layers(const media::VideoEncodeAccelerator::Config& input) {
-    return input.spatial_layers;
-  }
-
   static bool Read(media::mojom::VideoEncodeAcceleratorConfigDataView input,
                    media::VideoEncodeAccelerator::Config* output);
 };

media/mojo/mojom/video_encode_accelerator_mojom_traits_unittest.cc → media/mojo/mojom/video_encoder_info_mojom_traits_unittest.cc

@@ -2,11 +2,10 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "media/mojo/mojom/video_encode_accelerator_mojom_traits.h"
 #include "media/mojo/mojom/video_encoder_info_mojom_traits.h"
 
-#include "media/video/video_encode_accelerator.h"
 #include "media/video/video_encoder_info.h"
+
 #include "mojo/public/cpp/test_support/test_utils.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -61,28 +60,6 @@ bool operator==(const ::media::VideoEncoderInfo& l,
   return true;
 }
 
-bool operator==(
-    const ::media::VideoEncodeAccelerator::Config::SpatialLayer& l,
-    const ::media::VideoEncodeAccelerator::Config::SpatialLayer& r) {
-  return l.width == r.width && l.height == r.height &&
-         l.bitrate_bps == r.bitrate_bps && l.framerate == r.framerate &&
-         l.max_qp == r.max_qp && l.num_of_temporal_layers &&
-         r.num_of_temporal_layers;
-}
-
-bool operator==(const ::media::VideoEncodeAccelerator::Config& l,
-                const ::media::VideoEncodeAccelerator::Config& r) {
-  return l.input_format == r.input_format &&
-         l.input_visible_size == r.input_visible_size &&
-         l.output_profile == r.output_profile &&
-         l.initial_bitrate == r.initial_bitrate &&
-         l.initial_framerate == r.initial_framerate &&
-         l.gop_length == r.gop_length &&
-         l.h264_output_level == r.h264_output_level &&
-         l.storage_type == r.storage_type && l.content_type == r.content_type &&
-         l.spatial_layers == r.spatial_layers;
-}
-
 TEST(VideoEncoderInfoStructTraitTest, RoundTrip) {
   ::media::VideoEncoderInfo input;
   input.implementation_name = "FakeVideoEncodeAccelerator";
@@ -108,50 +85,4 @@ TEST(VideoEncoderInfoStructTraitTest, RoundTrip) {
       &input, &output));
   EXPECT_EQ(input, output);
 }
-
-TEST(SpatialLayerStructTraitTest, RoundTrip) {
-  ::media::VideoEncodeAccelerator::Config::SpatialLayer input_spatial_layer;
-  input_spatial_layer.width = 320u;
-  input_spatial_layer.width = 180u;
-  input_spatial_layer.bitrate_bps = 12345678;
-  input_spatial_layer.framerate = 24;
-  input_spatial_layer.max_qp = 30;
-  input_spatial_layer.num_of_temporal_layers = 3;
-  ::media::VideoEncodeAccelerator::Config::SpatialLayer output_spatial_layer{};
-  ASSERT_TRUE(mojo::test::SerializeAndDeserialize<mojom::SpatialLayer>(
-      &input_spatial_layer, &output_spatial_layer));
-  EXPECT_EQ(input_spatial_layer, output_spatial_layer);
-}
-
-TEST(VideoEncodeAcceleratorConfigStructTraitTest, RoundTrip) {
-  std::vector<::media::VideoEncodeAccelerator::Config::SpatialLayer>
-      input_spatial_layers(3);
-  gfx::Size kBaseSize(320, 180);
-  uint32_t kBaseBitrateBps = 123456;
-  uint32_t kBaseFramerate = 24;
-  for (size_t i = 0; i < input_spatial_layers.size(); ++i) {
-    input_spatial_layers[i].width =
-        static_cast<uint32_t>(kBaseSize.width() * (i + 1));
-    input_spatial_layers[i].height =
-        static_cast<uint32_t>(kBaseSize.height() * (i + 1));
-    input_spatial_layers[i].bitrate_bps = kBaseBitrateBps * (i + 1) / 2;
-    input_spatial_layers[i].framerate = kBaseFramerate * 2 / (i + 1);
-    input_spatial_layers[i].max_qp = 30 * (i + 1) / 2;
-    input_spatial_layers[i].num_of_temporal_layers = 3 - i;
-  }
-  ::media::VideoEncodeAccelerator::Config input_config(
-      ::media::PIXEL_FORMAT_NV12, kBaseSize, ::media::VP9PROFILE_PROFILE0,
-      kBaseBitrateBps, kBaseFramerate, base::nullopt, base::nullopt,
-      ::media::VideoEncodeAccelerator::Config::StorageType::kDmabuf,
-      ::media::VideoEncodeAccelerator::Config::ContentType::kCamera,
-      input_spatial_layers);
-  DVLOG(4) << input_config.AsHumanReadableString();
-
-  ::media::VideoEncodeAccelerator::Config output_config{};
-  ASSERT_TRUE(
-      mojo::test::SerializeAndDeserialize<mojom::VideoEncodeAcceleratorConfig>(
-          &input_config, &output_config));
-  DVLOG(4) << output_config.AsHumanReadableString();
-  EXPECT_EQ(input_config, output_config);
-}
 }  // namespace media

media/video/video_encode_accelerator.cc

@@ -41,8 +41,7 @@ VideoEncodeAccelerator::Config::Config(
     base::Optional<uint32_t> gop_length,
     base::Optional<uint8_t> h264_output_level,
     base::Optional<StorageType> storage_type,
-    ContentType content_type,
-    const std::vector<SpatialLayer>& spatial_layers)
+    ContentType content_type)
     : input_format(input_format),
       input_visible_size(input_visible_size),
       output_profile(output_profile),
@@ -52,8 +51,7 @@ VideoEncodeAccelerator::Config::Config(
       gop_length(gop_length),
       h264_output_level(h264_output_level),
       storage_type(storage_type),
-      content_type(content_type),
-      spatial_layers(spatial_layers) {}
+      content_type(content_type) {}
 
 VideoEncodeAccelerator::Config::~Config() = default;
 
@@ -76,16 +74,6 @@ std::string VideoEncodeAccelerator::Config::AsHumanReadableString() const {
     str += base::StringPrintf(", h264_output_level: %u",
                               h264_output_level.value());
   }
-
-  for (size_t i = 0; i < spatial_layers.size(); ++i) {
-    const auto& sl = spatial_layers[i];
-    str += base::StringPrintf(
-        "\nSL#%zu: width=%d, height=%d, bitrate_bps=%u"
-        ", framerate=%u, max_qp=%u"
-        ", num_of_temporal_layers=%u",
-        i, sl.width, sl.height, sl.bitrate_bps, sl.framerate, sl.max_qp,
-        sl.num_of_temporal_layers);
-  }
   return str;
 }
 

media/video/video_encode_accelerator.h

@@ -110,22 +110,6 @@ class MEDIA_EXPORT VideoEncodeAccelerator {
     // kDmabuf if a video frame is referred by dmabuf.
     enum class StorageType { kShmem, kDmabuf };
 
-    struct MEDIA_EXPORT SpatialLayer {
-      // The encoder dimension of the spatial layer.
-      int32_t width;
-      int32_t height;
-      // The bitrate of encoded output stream of the spatial layer in bits per
-      // second.
-      uint32_t bitrate_bps;
-      uint32_t framerate;
-      // The recommended maximum qp value of the spatial layer. VEA can ignore
-      // this value.
-      uint8_t max_qp;
-      // The number of temporal layers of the spatial layer. The detail of
-      // the temporal layer structure is up to VideoEncodeAccelerator.
-      uint8_t num_of_temporal_layers;
-    };
-
     Config();
     Config(const Config& config);
 
@@ -137,8 +121,7 @@ class MEDIA_EXPORT VideoEncodeAccelerator {
            base::Optional<uint32_t> gop_length = base::nullopt,
            base::Optional<uint8_t> h264_output_level = base::nullopt,
            base::Optional<StorageType> storage_type = base::nullopt,
-           ContentType content_type = ContentType::kCamera,
-           const std::vector<SpatialLayer>& spatial_layers = {});
+           ContentType content_type = ContentType::kCamera);
 
     ~Config();
 
@@ -185,12 +168,6 @@ class MEDIA_EXPORT VideoEncodeAccelerator {
     // bright colors. With this content hint the encoder may choose to optimize
     // for the given use case.
     ContentType content_type;
-
-    // The configuration for spatial layers. This is not empty if and only if
-    // either spatial or temporal layer encoding is configured. When this is not
-    // empty, VideoEncodeAccelerator should refer the width, height, bitrate and
-    // etc. of |spatial_layers|.
-    std::vector<SpatialLayer> spatial_layers;
   };
 
   // Interface for clients that use VideoEncodeAccelerator. These callbacks will