From f9bf197f2b1c5a76a45082fe7dd4db7b0cb6314d Mon Sep 17 00:00:00 2001 From: Jacob Trimble Date: Wed, 22 Jun 2016 17:03:53 -0700 Subject: [PATCH] Fix bug in VPx codec configuration. Before, we converted the WebM configuration to MP4 in the video client, however this lead to a bug when fields were missing. So now don't convert until the cluster parser so the extra info from the stream can be added. This also fixes a bug where the value was not printed in the warning logs. b/29580732 Change-Id: If0a1a4d135f98292cdaae15a11027f42d844e85d --- .../codecs/vp_codec_configuration_record.cc | 132 ++++++++---------- .../media/formats/webm/webm_cluster_parser.cc | 2 +- .../media/formats/webm/webm_video_client.cc | 14 +- 3 files changed, 63 insertions(+), 85 deletions(-) diff --git a/packager/media/codecs/vp_codec_configuration_record.cc b/packager/media/codecs/vp_codec_configuration_record.cc index 93c651aa86..c76f32f851 100644 --- a/packager/media/codecs/vp_codec_configuration_record.cc +++ b/packager/media/codecs/vp_codec_configuration_record.cc @@ -38,6 +38,23 @@ std::string VPCodecAsString(VideoCodec codec) { } } +template +void MergeField(const std::string& name, + T source_value, + bool source_is_set, + T* dest_value, + bool* dest_is_set) { + if (!*dest_is_set || source_is_set) { + if (*dest_is_set && source_value != *dest_value) { + LOG(WARNING) << "VPx " << name << " is inconsistent, " + << static_cast(*dest_value) << " vs " + << static_cast(source_value); + } + *dest_value = source_value; + *dest_is_set = true; + } +} + } // namespace VPCodecConfigurationRecord::VPCodecConfigurationRecord() {} @@ -153,29 +170,36 @@ void VPCodecConfigurationRecord::WriteMP4(std::vector* data) const { void VPCodecConfigurationRecord::WriteWebM(std::vector* data) const { BufferWriter writer; - writer.AppendInt(static_cast(kFeatureProfile)); // ID = 1 - writer.AppendInt(static_cast(1)); // Length = 1 - writer.AppendInt(static_cast(profile_)); + if (profile_is_set_) { + writer.AppendInt(static_cast(kFeatureProfile)); // ID = 1 + writer.AppendInt(static_cast(1)); // Length = 1 + writer.AppendInt(static_cast(profile_)); + } - if (level_ != 0) { + if (level_is_set_ && level_ != 0) { writer.AppendInt(static_cast(kFeatureLevel)); // ID = 2 writer.AppendInt(static_cast(1)); // Length = 1 writer.AppendInt(static_cast(level_)); } - writer.AppendInt(static_cast(kFeatureBitDepth)); // ID = 3 - writer.AppendInt(static_cast(1)); // Length = 1 - writer.AppendInt(static_cast(bit_depth_)); + if (bit_depth_is_set_) { + writer.AppendInt(static_cast(kFeatureBitDepth)); // ID = 3 + writer.AppendInt(static_cast(1)); // Length = 1 + writer.AppendInt(static_cast(bit_depth_)); + } - // WebM doesn't differentiate whether it is vertical or collocated with luma - // for 4:2:0. - const uint8_t subsampling = - chroma_subsampling_ == CHROMA_420_COLLOCATED_WITH_LUMA - ? CHROMA_420_VERTICAL - : chroma_subsampling_; - writer.AppendInt(static_cast(kFeatureChromaSubsampling)); // ID = 4 - writer.AppendInt(static_cast(1)); // Length = 1 - writer.AppendInt(subsampling); + if (chroma_subsampling_is_set_) { + // WebM doesn't differentiate whether it is vertical or collocated with luma + // for 4:2:0. + const uint8_t subsampling = + chroma_subsampling_ == CHROMA_420_COLLOCATED_WITH_LUMA + ? CHROMA_420_VERTICAL + : chroma_subsampling_; + // ID = 4, Length = 1 + writer.AppendInt(static_cast(kFeatureChromaSubsampling)); + writer.AppendInt(static_cast(1)); + writer.AppendInt(subsampling); + } writer.SwapBuffer(data); } @@ -203,64 +227,24 @@ std::string VPCodecConfigurationRecord::GetCodecString(VideoCodec codec) const { void VPCodecConfigurationRecord::MergeFrom( const VPCodecConfigurationRecord& other) { - if (!profile_is_set_ || other.profile_is_set_) { - profile_ = other.profile(); - profile_is_set_ = true; - } - if (!level_is_set_ || other.level_is_set_) { - if (level_is_set_ && other.level() != level_) { - LOG(WARNING) << "VPx level is inconsistent, " << level_ << " vs " - << other.level(); - } - level_ = other.level(); - level_is_set_ = true; - } - if (!bit_depth_is_set_ || other.bit_depth_is_set_) { - if (bit_depth_is_set_ && bit_depth_ != other.bit_depth()) { - LOG(WARNING) << "VPx bit depth is inconsistent, " << bit_depth_ << " vs " - << other.bit_depth(); - } - bit_depth_ = other.bit_depth(); - bit_depth_is_set_ = true; - } - if (!color_space_is_set_ || other.color_space_is_set_) { - if (color_space_is_set_ && color_space_ != other.color_space()) { - LOG(WARNING) << "VPx color space is inconsistent, " << color_space_ - << " vs " << other.color_space(); - } - color_space_ = other.color_space(); - color_space_is_set_ = true; - } - if (!chroma_subsampling_is_set_ || other.chroma_subsampling_is_set_) { - if (chroma_subsampling_is_set_ && - chroma_subsampling_ != other.chroma_subsampling_) { - LOG(WARNING) << "VPx chroma subsampling is inconsistent, " - << chroma_subsampling_ << " vs " - << other.chroma_subsampling(); - } - chroma_subsampling_ = other.chroma_subsampling(); - chroma_subsampling_is_set_ = true; - } - if (!transfer_function_is_set_ || other.transfer_function_is_set_) { - if (transfer_function_is_set_ && - transfer_function_ != other.transfer_function_) { - LOG(WARNING) << "VPx transfer function is inconsistent, " - << transfer_function_ << " vs " - << other.transfer_function(); - } - transfer_function_ = other.transfer_function(); - transfer_function_is_set_ = true; - } - if (!video_full_range_flag_is_set_ || other.video_full_range_flag_is_set_) { - if (video_full_range_flag_is_set_ && - video_full_range_flag_ != other.video_full_range_flag_) { - LOG(WARNING) << "VPx video full-range flag is inconsistent, " - << video_full_range_flag_<< " vs " - << other.video_full_range_flag(); - } - video_full_range_flag_ = other.video_full_range_flag(); - video_full_range_flag_is_set_ = true; - } + MergeField("profile", other.profile_, other.profile_is_set_, &profile_, + &profile_is_set_); + MergeField("level", other.level_, other.level_is_set_, &level_, + &level_is_set_); + MergeField("bit depth", other.bit_depth_, other.bit_depth_is_set_, + &bit_depth_, &bit_depth_is_set_); + MergeField("color space", other.color_space_, other.color_space_is_set_, + &color_space_, &color_space_is_set_); + MergeField("chroma subsampling", other.chroma_subsampling_, + other.chroma_subsampling_is_set_, &chroma_subsampling_, + &chroma_subsampling_is_set_); + MergeField("transfer function", other.transfer_function_, + other.transfer_function_is_set_, &transfer_function_, + &transfer_function_is_set_); + MergeField("video full range flag", other.video_full_range_flag_, + other.video_full_range_flag_is_set_, &video_full_range_flag_, + &video_full_range_flag_is_set_); + if (codec_initialization_data_.empty() || !other.codec_initialization_data_.empty()) { if (!codec_initialization_data_.empty() && diff --git a/packager/media/formats/webm/webm_cluster_parser.cc b/packager/media/formats/webm/webm_cluster_parser.cc index 9d3850e98d..2c78166a1a 100644 --- a/packager/media/formats/webm/webm_cluster_parser.cc +++ b/packager/media/formats/webm/webm_cluster_parser.cc @@ -458,7 +458,7 @@ bool WebMClusterParser::OnBlock(bool is_simple_block, VPCodecConfigurationRecord codec_config; if (!video_stream_info_->codec_config().empty()) - codec_config.ParseMP4(video_stream_info_->codec_config()); + codec_config.ParseWebM(video_stream_info_->codec_config()); codec_config.MergeFrom(vpx_parser->codec_config()); video_stream_info_->set_codec_string( diff --git a/packager/media/formats/webm/webm_video_client.cc b/packager/media/formats/webm/webm_video_client.cc index 200442f90e..e77d9c4286 100644 --- a/packager/media/formats/webm/webm_video_client.cc +++ b/packager/media/formats/webm/webm_video_client.cc @@ -51,22 +51,16 @@ void WebMVideoClient::Reset() { scoped_refptr WebMVideoClient::GetVideoStreamInfo( int64_t track_num, const std::string& codec_id, - const std::vector& codec_private_in, + const std::vector& codec_private, bool is_encrypted) { - std::vector codec_private = codec_private_in; VideoCodec video_codec = kUnknownVideoCodec; if (codec_id == "V_VP8") { video_codec = kCodecVP8; } else if (codec_id == "V_VP9") { video_codec = kCodecVP9; - - // Need to parse and convert the codec private data to MP4 format. - VPCodecConfigurationRecord vp_config; - if (!vp_config.ParseWebM(codec_private)) { - LOG(ERROR) << "Unable to parse VP9 codec configuration"; - return scoped_refptr(); - } - vp_config.WriteMP4(&codec_private); + // The codec private data is in WebM format, but needs to be converted to + // MP4 format. Don't do it yet, it will be handled in + // webm_cluster_parser.cc } else if (codec_id == "V_VP10") { video_codec = kCodecVP10; } else {