0

Remove has_side_data() as DecoderBufferSideData is not optional

The `side_data()` method recently was changed to return a pointer,
which means we can check the pointer itself rather than using a
`has` method to check for side data. This change removes this `has`
method and replaces all usage with checking `side_data()` instead.

Fixed: 380087644
Change-Id: Ica3477b10edd67be01a6f8ccc319163462b22539
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6011506
Reviewed-by: Matthew Denton <mpdenton@chromium.org>
Reviewed-by: Thomas Guilbert <tguilbert@chromium.org>
Commit-Queue: Syed AbuTalib <lowkey@google.com>
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Steve Anton <steveanton@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1397611}
This commit is contained in:
Syed AbuTalib
2024-12-17 15:00:42 -08:00
committed by Chromium LUCI CQ
parent 3863639ec1
commit aa6aeafa72
31 changed files with 51 additions and 64 deletions

@ -356,7 +356,7 @@ void ContentDecryptionModuleAdapter::Decrypt(
std::vector<uint8_t>(encrypted->data(),
encrypted->data() + encrypted->size()),
nullptr, true,
encrypted->has_side_data() ? encrypted->side_data()->secure_handle : 0,
encrypted->side_data() ? encrypted->side_data()->secure_handle : 0,
base::BindOnce(&ContentDecryptionModuleAdapter::OnDecrypt,
base::Unretained(this), stream_type, encrypted,
std::move(decrypt_cb)));
@ -380,7 +380,7 @@ void ContentDecryptionModuleAdapter::Decrypt(
std::vector<uint8_t>(encrypted->data(),
encrypted->data() + encrypted->size()),
decrypt_config->Clone(), stream_type == Decryptor::kVideo,
encrypted->has_side_data() ? encrypted->side_data()->secure_handle : 0,
encrypted->side_data() ? encrypted->side_data()->secure_handle : 0,
base::BindOnce(&ContentDecryptionModuleAdapter::OnDecrypt,
base::Unretained(this), stream_type, encrypted,
std::move(decrypt_cb)));
@ -524,7 +524,7 @@ void ContentDecryptionModuleAdapter::OnDecrypt(
// If we decrypted to secure memory, then just send the original buffer back
// because the result is stored in the secure world.
if (encrypted->has_side_data() && encrypted->side_data()->secure_handle) {
if (encrypted->side_data() && encrypted->side_data()->secure_handle) {
std::move(decrypt_cb).Run(media::Decryptor::kSuccess, std::move(encrypted));
return;
}
@ -535,7 +535,7 @@ void ContentDecryptionModuleAdapter::OnDecrypt(
decrypted->set_timestamp(encrypted->timestamp());
decrypted->set_duration(encrypted->duration());
decrypted->set_is_key_frame(encrypted->is_key_frame());
if (encrypted->has_side_data()) {
if (encrypted->side_data()) {
decrypted->set_side_data(encrypted->side_data()->Clone());
}

@ -158,7 +158,7 @@ void DecoderBuffer::set_discard_padding(const DiscardPadding& discard_padding) {
DecoderBufferSideData& DecoderBuffer::WritableSideData() {
DCHECK(!end_of_stream());
if (!has_side_data()) {
if (!side_data()) {
side_data_ = std::make_unique<DecoderBufferSideData>();
}
return *side_data_;
@ -176,12 +176,8 @@ bool DecoderBuffer::MatchesMetadataForTesting(
return false;
}
if (has_side_data() != buffer.has_side_data()) {
return false;
}
// Note: We use `side_data_` directly to avoid DCHECKs for EOS buffers.
if (has_side_data() && !side_data_->Matches(*buffer.side_data_)) {
if (side_data_ && !side_data_->Matches(*buffer.side_data_)) {
return false;
}
@ -239,8 +235,8 @@ std::string DecoderBuffer::AsHumanReadableString(bool verbose) const {
<< " encrypted=" << (decrypt_config_ != nullptr);
if (verbose) {
s << " has_side_data=" << has_side_data();
if (has_side_data()) {
s << " side_data=" << !!side_data();
if (side_data()) {
s << " discard_padding (us)=("
<< side_data_->discard_padding.first.InMicroseconds() << ", "
<< side_data_->discard_padding.second.InMicroseconds() << ")";
@ -270,7 +266,7 @@ size_t DecoderBuffer::GetMemoryUsage() const {
memory_usage += size();
// Side data and decrypt config would not change after construction.
if (has_side_data()) {
if (side_data()) {
memory_usage += sizeof(decltype(side_data_->spatial_layers)::value_type) *
side_data_->spatial_layers.capacity();
memory_usage += side_data_->alpha_data.size();

@ -246,8 +246,6 @@ class MEDIA_EXPORT DecoderBuffer
is_key_frame_ = is_key_frame;
}
bool has_side_data() const { return !!side_data_; }
// Returns DecoderBufferSideData associated with `this`. Check if `side_data_`
// exists using `has_side_data()` before calling this function.
const DecoderBufferSideData* side_data() const {

@ -256,7 +256,7 @@ TEST(DecoderBufferTest, IsKeyFrame) {
TEST(DecoderBufferTest, SideData) {
scoped_refptr<DecoderBuffer> buffer(new DecoderBuffer(0));
EXPECT_FALSE(buffer->has_side_data());
EXPECT_FALSE(buffer->side_data());
constexpr uint64_t kSecureHandle = 42;
const std::vector<uint32_t> kSpatialLayers = {1, 2, 3};
@ -266,7 +266,7 @@ TEST(DecoderBufferTest, SideData) {
buffer->WritableSideData().spatial_layers = kSpatialLayers;
buffer->WritableSideData().alpha_data =
base::HeapArray<uint8_t>::CopiedFrom(kAlphaData);
EXPECT_TRUE(buffer->has_side_data());
EXPECT_TRUE(buffer->side_data());
EXPECT_EQ(buffer->side_data()->secure_handle, kSecureHandle);
EXPECT_EQ(buffer->side_data()->spatial_layers, kSpatialLayers);
EXPECT_EQ(buffer->side_data()->alpha_data.as_span(), base::span(kAlphaData));
@ -281,7 +281,7 @@ TEST(DecoderBufferTest, SideData) {
cloned_side_data->alpha_data.as_span());
buffer->set_side_data(nullptr);
EXPECT_FALSE(buffer->has_side_data());
EXPECT_FALSE(buffer->side_data());
}
TEST(DecoderBufferTest, IsEncrypted) {

@ -92,7 +92,7 @@ void ConvertDecoderBufferToProto(
buffer_message->set_back_discard_usec(
decoder_buffer.discard_padding().second.InMicroseconds());
if (decoder_buffer.has_side_data() &&
if (decoder_buffer.side_data() &&
!decoder_buffer.side_data()->alpha_data.empty()) {
buffer_message->set_side_data(
decoder_buffer.side_data()->alpha_data.data(),

@ -84,7 +84,7 @@ TEST_F(ProtoUtilsTest, PassValidDecoderBuffer) {
ASSERT_TRUE(output_buffer->is_key_frame());
ASSERT_EQ(output_buffer->timestamp(), pts);
EXPECT_EQ(base::span(*output_buffer), base::span(buffer));
ASSERT_TRUE(output_buffer->has_side_data());
ASSERT_TRUE(output_buffer->side_data());
EXPECT_EQ(output_buffer->side_data()->alpha_data.as_span(),
base::span(side_buffer));
}

@ -148,7 +148,7 @@ scoped_refptr<DecoderBuffer> DecryptCbcsBuffer(const DecoderBuffer& input,
buffer->set_timestamp(input.timestamp());
buffer->set_duration(input.duration());
buffer->set_is_key_frame(input.is_key_frame());
if (input.has_side_data()) {
if (input.side_data()) {
buffer->set_side_data(input.side_data()->Clone());
}

@ -147,7 +147,7 @@ TEST(CbcsDecryptorTest, AdditionalData) {
EXPECT_EQ(encrypted_buffer->end_of_stream(),
decrypted_buffer->end_of_stream());
EXPECT_EQ(encrypted_buffer->is_key_frame(), decrypted_buffer->is_key_frame());
EXPECT_TRUE(decrypted_buffer->has_side_data());
EXPECT_TRUE(decrypted_buffer->side_data());
EXPECT_TRUE(
encrypted_buffer->side_data()->Matches(*decrypted_buffer->side_data()));
}

@ -62,7 +62,7 @@ void CopyExtraSettings(const DecoderBuffer& input, DecoderBuffer* output) {
output->set_timestamp(input.timestamp());
output->set_duration(input.duration());
output->set_is_key_frame(input.is_key_frame());
if (input.has_side_data()) {
if (input.side_data()) {
output->set_side_data(input.side_data()->Clone());
}
}

@ -108,7 +108,7 @@ TEST(CencDecryptorTest, ExtraData) {
EXPECT_EQ(encrypted_buffer->end_of_stream(),
decrypted_buffer->end_of_stream());
EXPECT_EQ(encrypted_buffer->is_key_frame(), decrypted_buffer->is_key_frame());
EXPECT_TRUE(decrypted_buffer->has_side_data());
EXPECT_TRUE(decrypted_buffer->side_data());
EXPECT_TRUE(
encrypted_buffer->side_data()->Matches(*decrypted_buffer->side_data()));
}

@ -420,7 +420,7 @@ VpxVideoDecoder::AlphaDecodeStatus VpxVideoDecoder::DecodeAlphaPlane(
const struct vpx_image** vpx_image_alpha,
const DecoderBuffer* buffer) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (!vpx_codec_alpha_ || !buffer->has_side_data() ||
if (!vpx_codec_alpha_ || !buffer->side_data() ||
buffer->side_data()->alpha_data.size() < 8) {
return kAlphaPlaneProcessed;
}

@ -216,8 +216,7 @@ void AV1Decoder::SetStream(int32_t id, const DecoderBuffer& decoder_buffer) {
decrypt_config_ = decoder_buffer.decrypt_config()->Clone();
else
decrypt_config_.reset();
if (decoder_buffer.has_side_data() &&
decoder_buffer.side_data()->secure_handle) {
if (decoder_buffer.side_data() && decoder_buffer.side_data()->secure_handle) {
secure_handle_ = decoder_buffer.side_data()->secure_handle;
} else {
secure_handle_ = 0;

@ -144,7 +144,7 @@ void DecoderBufferTranscryptor::DecryptPendingBuffer() {
current_transcrypt_task_->buffer->set_duration(superframe->duration());
current_transcrypt_task_->buffer->set_is_key_frame(
superframe->is_key_frame());
if (superframe->has_side_data()) {
if (superframe->side_data()) {
current_transcrypt_task_->buffer->set_side_data(
superframe->side_data()->Clone());
}
@ -182,8 +182,7 @@ void DecoderBufferTranscryptor::DecryptPendingBuffer() {
}
// If we've already attached a secure buffer, don't do it again.
if (!curr_buffer->has_side_data() ||
!curr_buffer->side_data()->secure_handle) {
if (!curr_buffer->side_data() || !curr_buffer->side_data()->secure_handle) {
auto status =
decoder_->AttachSecureBuffer(current_transcrypt_task_->buffer);
if (status == CroStatus::Codes::kSecureBufferPoolEmpty) {
@ -196,8 +195,7 @@ void DecoderBufferTranscryptor::DecryptPendingBuffer() {
return;
}
if (curr_buffer->has_side_data() &&
curr_buffer->side_data()->secure_handle) {
if (curr_buffer->side_data() && curr_buffer->side_data()->secure_handle) {
// Wrap the callback so we can release the secure buffer when decoding is
// done.
current_transcrypt_task_->decode_done_cb =

@ -1478,8 +1478,7 @@ void H264Decoder::SetStream(int32_t id, const DecoderBuffer& decoder_buffer) {
parser_.SetStream(ptr, size);
current_decrypt_config_ = nullptr;
}
if (decoder_buffer.has_side_data() &&
decoder_buffer.side_data()->secure_handle) {
if (decoder_buffer.side_data() && decoder_buffer.side_data()->secure_handle) {
secure_handle_ = decoder_buffer.side_data()->secure_handle;
} else {
secure_handle_ = 0;

@ -168,8 +168,7 @@ void H265Decoder::SetStream(int32_t id, const DecoderBuffer& decoder_buffer) {
parser_.SetStream(ptr, size);
current_decrypt_config_ = nullptr;
}
if (decoder_buffer.has_side_data() &&
decoder_buffer.side_data()->secure_handle) {
if (decoder_buffer.side_data() && decoder_buffer.side_data()->secure_handle) {
secure_handle_ = decoder_buffer.side_data()->secure_handle;
} else {
secure_handle_ = 0;

@ -78,7 +78,7 @@ namespace {
bool IsVp9KSVCStream(uint32_t input_format_fourcc,
const DecoderBuffer& decoder_buffer) {
return input_format_fourcc == V4L2_PIX_FMT_VP9 &&
decoder_buffer.has_side_data() &&
decoder_buffer.side_data() &&
!decoder_buffer.side_data()->spatial_layers.empty();
}

@ -35,7 +35,7 @@ namespace {
bool IsVp9KSVCStream(VideoCodecProfile profile,
const DecoderBuffer& decoder_buffer) {
return VideoCodecProfileToVideoCodec(profile) == VideoCodec::kVP9 &&
decoder_buffer.has_side_data() &&
decoder_buffer.side_data() &&
!decoder_buffer.side_data()->spatial_layers.empty();
}

@ -103,7 +103,7 @@ bool OverwriteShowFrame(base::span<uint8_t> frame_data,
} // namespace
bool AppendVP9SuperFrameIndex(scoped_refptr<DecoderBuffer>& buffer) {
DCHECK(buffer->has_side_data());
DCHECK(buffer->side_data());
std::vector<uint32_t> frame_sizes = buffer->side_data()->spatial_layers;
DCHECK(!frame_sizes.empty());

@ -23,7 +23,7 @@ bool GetSpatialLayerFrameSize(const DecoderBuffer& decoder_buffer,
std::vector<uint32_t>& frame_sizes) {
frame_sizes.clear();
if (!decoder_buffer.has_side_data() ||
if (!decoder_buffer.side_data() ||
decoder_buffer.side_data()->spatial_layers.empty()) {
return true;
}
@ -134,8 +134,7 @@ void VP9Decoder::SetStream(int32_t id, const DecoderBuffer& decoder_buffer) {
SetError();
return;
}
if (decoder_buffer.has_side_data() &&
decoder_buffer.side_data()->secure_handle) {
if (decoder_buffer.side_data() && decoder_buffer.side_data()->secure_handle) {
secure_handle_ = decoder_buffer.side_data()->secure_handle;
} else {
secure_handle_ = 0;

@ -500,7 +500,7 @@ void D3D11VideoDecoder::Decode(scoped_refptr<DecoderBuffer> buffer,
}
const bool is_spatial_layer_buffer =
!buffer->end_of_stream() && buffer->has_side_data() &&
!buffer->end_of_stream() && buffer->side_data() &&
!buffer->side_data()->spatial_layers.empty();
input_buffer_queue_.push_back(

@ -855,7 +855,7 @@ TEST_F(MojoStableVideoDecoderTest, Decode) {
EXPECT_EQ(decoder_buffer_to_send->is_key_frame(), kIsKeyFrame);
ASSERT_EQ(decoder_buffer_to_send->size(), std::size(kEncodedData));
EXPECT_EQ(base::span(*decoder_buffer_to_send), base::span(kEncodedData));
ASSERT_TRUE(decoder_buffer_to_send->has_side_data());
ASSERT_TRUE(decoder_buffer_to_send->side_data());
EXPECT_EQ(decoder_buffer_to_send->side_data()->secure_handle,
kSecureHandle);
}

@ -124,7 +124,7 @@ TypeConverter<media::mojom::DecoderBufferPtr, media::DecoderBuffer>::Convert(
data_buffer->duration = input.duration();
data_buffer->is_key_frame = input.is_key_frame();
data_buffer->data_size = base::checked_cast<uint32_t>(input.size());
if (input.has_side_data()) {
if (input.side_data()) {
data_buffer->side_data =
media::mojom::DecoderBufferSideData::From(*input.side_data());
}

@ -91,7 +91,7 @@ TEST(MediaTypeConvertersTest, ConvertDecoderBuffer_Normal) {
// Note: We intentionally do not serialize the data section of the
// DecoderBuffer; no need to check the data here.
EXPECT_EQ(kDataSize, result->size());
EXPECT_TRUE(result->has_side_data());
EXPECT_TRUE(result->side_data());
EXPECT_TRUE(buffer->side_data()->Matches(*result->side_data()));
EXPECT_EQ(buffer->timestamp(), result->timestamp());
EXPECT_EQ(buffer->duration(), result->duration());

@ -333,7 +333,7 @@ std::vector<uint8_t> StructTraits<media::stable::mojom::DecoderBufferDataView,
// need to convert the side data to a raw format. We only care about spatial
// layers since alpha data isn't used by HW decoders and the secure handle is
// only going to used in new code going forward.
if (!input->has_side_data() || input->side_data()->spatial_layers.empty()) {
if (!input->side_data() || input->side_data()->spatial_layers.empty()) {
return {};
}
std::vector<uint8_t> raw_data;
@ -419,7 +419,7 @@ StructTraits<media::stable::mojom::DecoderBufferDataView,
"Unexpected type for input->side_data(). If you need to change this "
"assertion, please contact chromeos-gfx-video@google.com.");
if (input->end_of_stream() || !input->has_side_data()) {
if (input->end_of_stream() || !input->side_data()) {
return nullptr;
}
return input->side_data()->Clone();
@ -496,7 +496,7 @@ bool StructTraits<media::stable::mojom::DecoderBufferDataView,
// TODO(b/269383891): Remove this in M120.
// If the input is an older version than us, then it may have |raw_side_data|
// set and we need to copy that into the potential values in |side_data|.
if (!raw_side_data.empty() && !decoder_buffer->has_side_data()) {
if (!raw_side_data.empty() && !decoder_buffer->side_data()) {
// Spatial layers is always a multiple of 4 with a max size of 12.
// HW decoders don't use alpha data, so we can ignore that case.
if (raw_side_data.size() % sizeof(uint32_t) != 0 ||

@ -55,7 +55,7 @@ TEST(StableVideoDecoderTypesMojomTraitsTest, ValidNonEOSDecoderBuffer) {
base::strict_cast<size_t>(mojom_decoder_buffer->data_size));
EXPECT_EQ(deserialized_decoder_buffer->is_key_frame(),
mojom_decoder_buffer->is_key_frame);
EXPECT_FALSE(deserialized_decoder_buffer->has_side_data());
EXPECT_FALSE(deserialized_decoder_buffer->side_data());
}
TEST(StableVideoDecoderTypesMojomTraitsTest, InfiniteDecoderBufferDuration) {
@ -111,7 +111,7 @@ TEST(StableVideoDecoderTypesMojomTraitsTest, RawSideDataToValidSpatialLayers) {
ASSERT_TRUE(deserialized_decoder_buffer);
ASSERT_FALSE(deserialized_decoder_buffer->end_of_stream());
ASSERT_TRUE(deserialized_decoder_buffer->has_side_data());
ASSERT_TRUE(deserialized_decoder_buffer->side_data());
EXPECT_EQ(deserialized_decoder_buffer->side_data()->spatial_layers,
std::vector<uint32_t>(kValidSpatialLayers,
kValidSpatialLayers + kLayersSize));

@ -435,8 +435,7 @@ bool WebmMuxer::WriteWebmFrame(EncodedFrame frame,
uint8_t track_index = absl::get_if<AudioParameters>(&frame.params)
? audio_track_index_
: video_track_index_;
return frame.data->has_side_data() &&
!frame.data->side_data()->alpha_data.empty()
return frame.data->side_data() && !frame.data->side_data()->alpha_data.empty()
? segment_.AddFrameWithAdditional(
frame.data->data(), frame.data->size(),
frame.data->side_data()->alpha_data.data(),

@ -584,9 +584,9 @@ TEST_P(VideoTrackRecorderTestParam, VideoEncoding) {
EXPECT_GE(third_frame_encoded_data->side_data()->alpha_data.size(),
kEncodedSizeThreshold);
} else {
EXPECT_FALSE(first_frame_encoded_data->has_side_data());
EXPECT_FALSE(second_frame_encoded_data->has_side_data());
EXPECT_FALSE(third_frame_encoded_data->has_side_data());
EXPECT_FALSE(first_frame_encoded_data->side_data());
EXPECT_FALSE(second_frame_encoded_data->side_data());
EXPECT_FALSE(third_frame_encoded_data->side_data());
}
// The encoder is configured non screen content by default.
@ -1040,11 +1040,11 @@ TEST_P(VideoTrackRecorderTestMediaVideoEncoderParam,
run_loop.Run();
const size_t kEmptySize = 0;
EXPECT_FALSE(first_frame_encoded_alpha->has_side_data());
EXPECT_TRUE(second_frame_encoded_alpha->has_side_data());
EXPECT_FALSE(first_frame_encoded_alpha->side_data());
EXPECT_TRUE(second_frame_encoded_alpha->side_data());
EXPECT_GT(second_frame_encoded_alpha->side_data()->alpha_data.size(),
kEmptySize);
EXPECT_TRUE(third_frame_encoded_alpha->has_side_data());
EXPECT_TRUE(third_frame_encoded_alpha->side_data());
EXPECT_GT(third_frame_encoded_alpha->side_data()->alpha_data.size(),
kEmptySize);

@ -155,7 +155,7 @@ scoped_refptr<media::StreamParserBuffer> MakeAudioStreamParserBuffer(
kWebCodecsAudioTrackId);
// Currently, we do not populate any side_data in these converters.
DCHECK(!stream_parser_buffer->has_side_data());
DCHECK(!stream_parser_buffer->side_data());
stream_parser_buffer->set_timestamp(audio_chunk.buffer()->timestamp());
// TODO(crbug.com/1144908): Get EncodedAudioChunk to have an optional duration
@ -180,7 +180,7 @@ scoped_refptr<media::StreamParserBuffer> MakeVideoStreamParserBuffer(
kWebCodecsVideoTrackId);
// Currently, we do not populate any side_data in these converters.
DCHECK(!stream_parser_buffer->has_side_data());
DCHECK(!stream_parser_buffer->side_data());
stream_parser_buffer->set_timestamp(video_chunk.buffer()->timestamp());
// TODO(crbug.com/1144908): Get EncodedVideoChunk to have an optional decode

@ -68,7 +68,7 @@ class Vp9ResolutionMonitor : public ResolutionMonitor {
std::optional<gfx::Size> GetResolution(
const media::DecoderBuffer& buffer) override {
std::vector<uint32_t> frame_sizes;
if (buffer.has_side_data()) {
if (buffer.side_data()) {
frame_sizes = buffer.side_data()->spatial_layers;
}
parser_.SetStream(buffer.data(), base::checked_cast<off_t>(buffer.size()),

@ -187,7 +187,7 @@ std::optional<RTCVideoDecoderFallbackReason> NeedSoftwareFallback(
// Fall back to software decoding if there's no support for VP9 spatial
// layers. See https://crbug.com/webrtc/9304.
const bool is_spatial_layer_buffer =
buffer.has_side_data() && !buffer.side_data()->spatial_layers.empty();
buffer.side_data() && !buffer.side_data()->spatial_layers.empty();
if (codec == media::VideoCodec::kVP9 && is_spatial_layer_buffer &&
!media::IsVp9kSVCHWDecodingEnabled()) {
return RTCVideoDecoderFallbackReason::kSpatialLayers;

@ -793,7 +793,7 @@ TEST_F(RTCVideoDecoderAdapterTest, DecodesImageWithSingleSpatialLayer) {
// Check the side data was not set as there was only 1 spatial layer.
ASSERT_TRUE(decoder_buffer);
if (decoder_buffer->has_side_data()) {
if (decoder_buffer->side_data()) {
EXPECT_TRUE(decoder_buffer->side_data()->spatial_layers.empty());
}
}