Skip to content

Commit

Permalink
media: Remove media::ScalingSettings
Browse files Browse the repository at this point in the history
It is not necessary for chrome hardware encoders to report their
ScalingSettings. We would rather use a default ScalingSetting
value. This CL removes the unnecessary media::ScalingSettings in
chrome.

Bug: 1179020
Test: Build chrome
Change-Id: I90d335a7b82b5f1a1cfa53bb85b6c40b50278eee
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2742256
Commit-Queue: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#861434}
  • Loading branch information
Hirokazu Honda authored and Chromium LUCI CQ committed Mar 10, 2021
1 parent b567f0f commit 83eebf5
Show file tree
Hide file tree
Showing 18 changed files with 24 additions and 123 deletions.
2 changes: 0 additions & 2 deletions media/gpu/vaapi/accelerated_video_encoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,6 @@ class AcceleratedVideoEncoder {
// at least this many frames simultaneously for encode to make progress.
virtual size_t GetMaxNumOfRefFrames() const = 0;

virtual ScalingSettings GetScalingSettings() const = 0;

// Prepares a new |encode_job| to be executed in Accelerator and returns true
// on success. The caller may then call Execute() on the job to run it.
virtual bool PrepareEncodeJob(EncodeJob* encode_job) = 0;
Expand Down
9 changes: 2 additions & 7 deletions media/gpu/vaapi/h264_encoder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ H264Encoder::EncodeParams::EncodeParams()
cpb_window_size_ms(kCPBWindowSizeMs),
cpb_size_bits(0),
initial_qp(kDefaultQP),
scaling_settings(kMinQP, kMaxQP),
min_qp(kMinQP),
max_qp(kMaxQP),
max_num_ref_frames(kMaxNumReferenceFrames),
max_ref_pic_list0_size(kMaxRefIdxL0Size),
max_ref_pic_list1_size(kMaxRefIdxL1Size) {}
Expand Down Expand Up @@ -155,12 +156,6 @@ size_t H264Encoder::GetMaxNumOfRefFrames() const {
return curr_params_.max_num_ref_frames;
}

ScalingSettings H264Encoder::GetScalingSettings() const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

return curr_params_.scaling_settings;
}

bool H264Encoder::PrepareEncodeJob(EncodeJob* encode_job) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

Expand Down
4 changes: 2 additions & 2 deletions media/gpu/vaapi/h264_encoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ class H264Encoder : public AcceleratedVideoEncoder {

// Quantization parameter. Their ranges are 0-51.
int initial_qp;
ScalingSettings scaling_settings;
int min_qp;
int max_qp;

// Maxium Number of Reference frames.
size_t max_num_ref_frames;
Expand Down Expand Up @@ -111,7 +112,6 @@ class H264Encoder : public AcceleratedVideoEncoder {
uint32_t framerate) override;
gfx::Size GetCodedSize() const override;
size_t GetMaxNumOfRefFrames() const override;
ScalingSettings GetScalingSettings() const override;
bool PrepareEncodeJob(EncodeJob* encode_job) override;

private:
Expand Down
20 changes: 8 additions & 12 deletions media/gpu/vaapi/vaapi_video_encode_accelerator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -463,10 +463,6 @@ void VaapiVideoEncodeAccelerator::InitializeTask(const Config& config) {
num_frames_in_flight_, expected_input_coded_size_,
output_buffer_byte_size_));

// TODO(crbug.com/1034686): Set ScalingSettings causes getStats() hangs.
// Investigate and fix the issue.
// encoder_info_.scaling_settings = encoder_->GetScalingSettings();

if (config.HasTemporalLayer()) {
DCHECK(!config.spatial_layers.empty());
encoder_info_.fps_allocation[0] = VP9TemporalLayers::GetFpsAllocation(
Expand Down Expand Up @@ -1187,8 +1183,8 @@ bool VaapiVideoEncodeAccelerator::H264Accelerator::SubmitFrameParameters(
rate_control_param.target_percentage = kTargetBitratePercentage;
rate_control_param.window_size = encode_params.cpb_window_size_ms;
rate_control_param.initial_qp = pic_param.pic_init_qp;
rate_control_param.min_qp = encode_params.scaling_settings.min_qp;
rate_control_param.max_qp = encode_params.scaling_settings.max_qp;
rate_control_param.min_qp = encode_params.min_qp;
rate_control_param.max_qp = encode_params.max_qp;
rate_control_param.rc_flags.bits.disable_frame_skip = true;

VAEncMiscParameterFrameRate framerate_param = {};
Expand Down Expand Up @@ -1375,8 +1371,8 @@ bool VaapiVideoEncodeAccelerator::VP8Accelerator::SubmitFrameParameters(
}

pic_param.sharpness_level = frame_header->loopfilter_hdr.sharpness_level;
pic_param.clamp_qindex_high = encode_params.scaling_settings.max_qp;
pic_param.clamp_qindex_low = encode_params.scaling_settings.min_qp;
pic_param.clamp_qindex_high = encode_params.max_qp;
pic_param.clamp_qindex_low = encode_params.min_qp;

VAQMatrixBufferVP8 qmatrix_buf = {};
for (size_t i = 0; i < base::size(qmatrix_buf.quantization_index); ++i)
Expand All @@ -1399,8 +1395,8 @@ bool VaapiVideoEncodeAccelerator::VP8Accelerator::SubmitFrameParameters(
rate_control_param.target_percentage = kTargetBitratePercentage;
rate_control_param.window_size = encode_params.cpb_window_size_ms;
rate_control_param.initial_qp = encode_params.initial_qp;
rate_control_param.min_qp = encode_params.scaling_settings.min_qp;
rate_control_param.max_qp = encode_params.scaling_settings.max_qp;
rate_control_param.min_qp = encode_params.min_qp;
rate_control_param.max_qp = encode_params.max_qp;
rate_control_param.rc_flags.bits.disable_frame_skip = true;

VAEncMiscParameterFrameRate framerate_param = {};
Expand Down Expand Up @@ -1564,8 +1560,8 @@ bool VaapiVideoEncodeAccelerator::VP9Accelerator::SubmitFrameParameters(
rate_control_param.target_percentage = kTargetBitratePercentage;
rate_control_param.window_size = encode_params.cpb_window_size_ms;
rate_control_param.initial_qp = encode_params.initial_qp;
rate_control_param.min_qp = encode_params.scaling_settings.min_qp;
rate_control_param.max_qp = encode_params.scaling_settings.max_qp;
rate_control_param.min_qp = encode_params.min_qp;
rate_control_param.max_qp = encode_params.max_qp;
rate_control_param.rc_flags.bits.disable_frame_skip = true;

VAEncMiscParameterFrameRate framerate_param = {};
Expand Down
2 changes: 1 addition & 1 deletion media/gpu/vaapi/vaapi_video_encode_accelerator.h
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ class MEDIA_GPU_EXPORT VaapiVideoEncodeAccelerator
static CodecPicture* GetPictureFromJobForTesting(VaapiEncodeJob* job);

// The unchanged values are filled upon the construction. The varied values
// (e.g. ScalingSettings) are filled properly during encoding.
// are filled properly during encoding.
VideoEncoderInfo encoder_info_;

// VaapiWrapper is the owner of all HW resources (surfaces and buffers)
Expand Down
3 changes: 0 additions & 3 deletions media/gpu/vaapi/vaapi_video_encode_accelerator_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,6 @@ class MockAcceleratedVideoEncoder : public AcceleratedVideoEncoder {
bool UpdateRates(const VideoBitrateAllocation&, uint32_t) override {
return false;
}
ScalingSettings GetScalingSettings() const override {
return ScalingSettings();
}
};
} // namespace

Expand Down
9 changes: 2 additions & 7 deletions media/gpu/vaapi/vp8_encoder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ VP8Encoder::EncodeParams::EncodeParams()
cpb_window_size_ms(kCPBWindowSizeMs),
cpb_size_bits(0),
initial_qp(kDefaultQP),
scaling_settings(kMinQP, kMaxQP),
min_qp(kMinQP),
max_qp(kMaxQP),
error_resilient_mode(false) {}

void VP8Encoder::Reset() {
Expand Down Expand Up @@ -89,12 +90,6 @@ size_t VP8Encoder::GetMaxNumOfRefFrames() const {
return kNumVp8ReferenceBuffers;
}

ScalingSettings VP8Encoder::GetScalingSettings() const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

return current_params_.scaling_settings;
}

bool VP8Encoder::PrepareEncodeJob(EncodeJob* encode_job) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

Expand Down
4 changes: 2 additions & 2 deletions media/gpu/vaapi/vp8_encoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ class VP8Encoder : public AcceleratedVideoEncoder {
// Quantization parameter. They are vp8 ac/dc indices and their ranges are
// 0-127.
int initial_qp;
ScalingSettings scaling_settings;
int min_qp;
int max_qp;

bool error_resilient_mode;
};
Expand Down Expand Up @@ -81,7 +82,6 @@ class VP8Encoder : public AcceleratedVideoEncoder {
uint32_t framerate) override;
gfx::Size GetCodedSize() const override;
size_t GetMaxNumOfRefFrames() const override;
ScalingSettings GetScalingSettings() const override;
bool PrepareEncodeJob(EncodeJob* encode_job) override;

private:
Expand Down
17 changes: 5 additions & 12 deletions media/gpu/vaapi/vp9_encoder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,8 @@ libvpx::VP9RateControlRtcConfig CreateRateControlConfig(
libvpx::VP9RateControlRtcConfig rc_cfg{};
rc_cfg.width = encode_size.width();
rc_cfg.height = encode_size.height();
rc_cfg.max_quantizer =
QindexToQuantizer(encode_params.scaling_settings.max_qp);
rc_cfg.min_quantizer =
QindexToQuantizer(encode_params.scaling_settings.min_qp);
rc_cfg.max_quantizer = QindexToQuantizer(encode_params.max_qp);
rc_cfg.min_quantizer = QindexToQuantizer(encode_params.min_qp);
// libvpx::VP9RateControlRtcConfig is kbps.
rc_cfg.target_bandwidth = encode_params.bitrate_allocation.GetSumBps() / 1000;
// These default values come from
Expand Down Expand Up @@ -163,7 +161,8 @@ VP9Encoder::EncodeParams::EncodeParams()
cpb_window_size_ms(kCPBWindowSizeMs),
cpb_size_bits(0),
initial_qp(kDefaultQP),
scaling_settings(kMinQP, kMaxQP),
min_qp(kMinQP),
max_qp(kMaxQP),
error_resilient_mode(false) {}

void VP9Encoder::set_rate_ctrl_for_testing(
Expand Down Expand Up @@ -224,7 +223,7 @@ bool VP9Encoder::Initialize(const VideoEncodeAccelerator::Config& config,
temporal_layers_ =
std::make_unique<VP9TemporalLayers>(num_temporal_layers);
}
current_params_.scaling_settings.max_qp = kMaxQPForSoftwareRateCtrl;
current_params_.max_qp = kMaxQPForSoftwareRateCtrl;

// |rate_ctrl_| might be injected for tests.
if (!rate_ctrl_) {
Expand Down Expand Up @@ -262,12 +261,6 @@ size_t VP9Encoder::GetMaxNumOfRefFrames() const {
return kVp9NumRefFrames;
}

ScalingSettings VP9Encoder::GetScalingSettings() const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

return current_params_.scaling_settings;
}

bool VP9Encoder::PrepareEncodeJob(EncodeJob* encode_job) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

Expand Down
4 changes: 2 additions & 2 deletions media/gpu/vaapi/vp9_encoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ class VP9Encoder : public AcceleratedVideoEncoder {
// Quantization parameter. They are vp9 ac/dc indices and their ranges are
// 0-255.
int initial_qp;
ScalingSettings scaling_settings;
int min_qp;
int max_qp;

bool error_resilient_mode;
};
Expand Down Expand Up @@ -92,7 +93,6 @@ class VP9Encoder : public AcceleratedVideoEncoder {
uint32_t framerate) override;
gfx::Size GetCodedSize() const override;
size_t GetMaxNumOfRefFrames() const override;
ScalingSettings GetScalingSettings() const override;
bool PrepareEncodeJob(EncodeJob* encode_job) override;
void BitrateControlUpdate(uint64_t encoded_chunk_size_bytes) override;
BitstreamBufferMetadata GetMetadata(EncodeJob* encode_job,
Expand Down
4 changes: 0 additions & 4 deletions media/mojo/mojom/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -415,10 +415,6 @@ mojom("mojom") {
},
{
types = [
{
mojom = "media.mojom.ScalingSettings"
cpp = "::media::ScalingSettings"
},
{
mojom = "media.mojom.ResolutionBitrateLimit"
cpp = "::media::ResolutionBitrateLimit"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ namespace media {
TEST(VideoEncoderInfoStructTraitTest, RoundTrip) {
::media::VideoEncoderInfo input;
input.implementation_name = "FakeVideoEncodeAccelerator";
// Scaling settings.
input.scaling_settings = ::media::ScalingSettings(12, 123);
// FPS allocation.
for (size_t i = 0; i < ::media::VideoEncoderInfo::kMaxSpatialLayers; ++i)
input.fps_allocation[i] = {5, 5, 10};
Expand Down
6 changes: 0 additions & 6 deletions media/mojo/mojom/video_encoder_info.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,6 @@ module media.mojom;

import "ui/gfx/geometry/mojom/geometry.mojom";

struct ScalingSettings {
uint8 min_qp;
uint8 max_qp;
};

struct ResolutionBitrateLimit {
gfx.mojom.Size frame_size;
int32 min_start_bitrate_bps;
Expand All @@ -26,7 +21,6 @@ struct VideoEncoderInfo {
bool is_hardware_accelerated;
bool supports_simulcast;

ScalingSettings? scaling_settings;
// This array size is equal to media::VideoEncoderInfo::kMaxSpatialLayers.
array<array<uint8>, 5> fps_allocation;
array<ResolutionBitrateLimit> resolution_bitrate_limits;
Expand Down
13 changes: 0 additions & 13 deletions media/mojo/mojom/video_encoder_info_mojom_traits.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,6 @@

namespace mojo {

// static
bool StructTraits<
media::mojom::ScalingSettingsDataView,
media::ScalingSettings>::Read(media::mojom::ScalingSettingsDataView data,
media::ScalingSettings* out) {
out->min_qp = data.min_qp();
out->max_qp = data.max_qp();
return true;
}

// static
bool StructTraits<media::mojom::ResolutionBitrateLimitDataView,
media::ResolutionBitrateLimit>::
Expand All @@ -42,9 +32,6 @@ bool StructTraits<
if (!data.ReadImplementationName(&out->implementation_name))
return false;

if (!data.ReadScalingSettings(&out->scaling_settings))
return false;

base::span<std::vector<uint8_t>> fps_allocation(out->fps_allocation);
if (!data.ReadFpsAllocation(&fps_allocation))
return false;
Expand Down
19 changes: 0 additions & 19 deletions media/mojo/mojom/video_encoder_info_mojom_traits.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,6 @@

namespace mojo {

template <>
class StructTraits<media::mojom::ScalingSettingsDataView,
media::ScalingSettings> {
public:
static int32_t min_qp(const media::ScalingSettings& scaling_settings) {
return scaling_settings.min_qp;
}
static int32_t max_qp(const media::ScalingSettings& scaling_settings) {
return scaling_settings.max_qp;
}

static bool Read(media::mojom::ScalingSettingsDataView data,
media::ScalingSettings* out);
};

template <>
class StructTraits<media::mojom::ResolutionBitrateLimitDataView,
media::ResolutionBitrateLimit> {
Expand Down Expand Up @@ -75,10 +60,6 @@ class StructTraits<media::mojom::VideoEncoderInfoDataView,
const media::VideoEncoderInfo& video_encoder_info) {
return video_encoder_info.supports_simulcast;
}
static const base::Optional<media::ScalingSettings>& scaling_settings(
const media::VideoEncoderInfo& video_encoder_info) {
return video_encoder_info.scaling_settings;
}
static base::span<const std::vector<uint8_t>,
media::VideoEncoderInfo::kMaxSpatialLayers>
fps_allocation(const media::VideoEncoderInfo& video_encoder_info) {
Expand Down
11 changes: 0 additions & 11 deletions media/video/video_encoder_info.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,6 @@

namespace media {

ScalingSettings::ScalingSettings() = default;
ScalingSettings::ScalingSettings(int min_qp, int max_qp)
: min_qp(min_qp), max_qp(max_qp) {}
ScalingSettings::ScalingSettings(const ScalingSettings&) = default;
ScalingSettings::~ScalingSettings() = default;

ResolutionBitrateLimit::ResolutionBitrateLimit() = default;
ResolutionBitrateLimit::ResolutionBitrateLimit(const ResolutionBitrateLimit&) =
default;
Expand All @@ -29,10 +23,6 @@ VideoEncoderInfo::VideoEncoderInfo() = default;
VideoEncoderInfo::VideoEncoderInfo(const VideoEncoderInfo&) = default;
VideoEncoderInfo::~VideoEncoderInfo() = default;

bool operator==(const ScalingSettings& l, const ScalingSettings& r) {
return l.min_qp == r.min_qp && l.max_qp == r.max_qp;
}

bool operator==(const ResolutionBitrateLimit& l,
const ResolutionBitrateLimit& r) {
return l.frame_size == r.frame_size &&
Expand All @@ -52,7 +42,6 @@ bool operator==(const VideoEncoderInfo& l, const VideoEncoderInfo& r) {
l.has_trusted_rate_controller == r.has_trusted_rate_controller &&
l.is_hardware_accelerated == r.is_hardware_accelerated &&
l.supports_simulcast == r.supports_simulcast &&
l.scaling_settings == r.scaling_settings &&
l.resolution_bitrate_limits == r.resolution_bitrate_limits;
}
} // namespace media
15 changes: 0 additions & 15 deletions media/video/video_encoder_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,6 @@ namespace media {
// These chromium classes are the corresponding classes in webrtc project.
// See third_party/webrtc/api/video_codecs/video_encoder.h for the detail.

struct MEDIA_EXPORT ScalingSettings {
ScalingSettings();
ScalingSettings(int min_qp, int max_qp);
ScalingSettings(const ScalingSettings&);
~ScalingSettings();

// Quantization Parameter in ScalingSettings are codec specific.
// The range of qp is 0-51 (H264), 0-127 (VP8) and 0-255 (VP9 and AV1).
int min_qp = 0;
int max_qp = 255;
};

struct MEDIA_EXPORT ResolutionBitrateLimit {
ResolutionBitrateLimit();
ResolutionBitrateLimit(const ResolutionBitrateLimit&);
Expand Down Expand Up @@ -59,13 +47,10 @@ struct MEDIA_EXPORT VideoEncoderInfo {
bool is_hardware_accelerated = true;
bool supports_simulcast = false;

base::Optional<ScalingSettings> scaling_settings;
std::vector<uint8_t> fps_allocation[kMaxSpatialLayers];
std::vector<ResolutionBitrateLimit> resolution_bitrate_limits;
};

MEDIA_EXPORT bool operator==(const ScalingSettings& l,
const ScalingSettings& r);
MEDIA_EXPORT bool operator==(const ResolutionBitrateLimit& l,
const ResolutionBitrateLimit& r);
MEDIA_EXPORT bool operator==(const VideoEncoderInfo& l,
Expand Down
Loading

0 comments on commit 83eebf5

Please sign in to comment.