Skip to content

Commit

Permalink
Add color space to VideoEncoderOutput
Browse files Browse the repository at this point in the history
In follow up CLs, individual encoders will use this
to signal color space for WebCodecs encoding.

Bug: 1241448
Change-Id: I2190940dda93220a6b6ff1eba221d6bc488e445b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3224589
Auto-Submit: Chrome Cunningham <chcunningham@chromium.org>
Reviewed-by: Eugene Zemtsov <eugene@chromium.org>
Commit-Queue: Chrome Cunningham <chcunningham@chromium.org>
Cr-Commit-Position: refs/heads/main@{#936537}
  • Loading branch information
chcunningham authored and Chromium LUCI CQ committed Oct 29, 2021
1 parent 7b745f4 commit aba2dc1
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 9 deletions.
2 changes: 2 additions & 0 deletions media/base/video_encoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "media/base/svc_scalability_mode.h"
#include "media/base/video_codecs.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "ui/gfx/color_space.h"
#include "ui/gfx/geometry/size.h"

namespace media {
Expand All @@ -33,6 +34,7 @@ struct MEDIA_EXPORT VideoEncoderOutput {
base::TimeDelta timestamp;
bool key_frame = false;
int temporal_id = 0;
gfx::ColorSpace color_space;
};

class MEDIA_EXPORT VideoEncoder {
Expand Down
31 changes: 23 additions & 8 deletions third_party/blink/renderer/modules/webcodecs/video_encoder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -247,20 +247,21 @@ VideoEncoderTraits::ParsedConfig* ParseConfigStatic(
result->level = 0;
result->codec_string = config->codec();

// Some codec strings provide color space info, but for WebCodecs this is
// ignored. Instead, the VideoFrames given to encode() are the source of truth
// for input color space. Note also that the output color space is up to the
// underlying codec impl. See https://github.com/w3c/webcodecs/issues/345.
media::VideoColorSpace codec_string_color_space;

bool parse_succeeded = media::ParseVideoCodecString(
"", config->codec().Utf8(), &is_codec_ambiguous, &result->codec,
&result->profile, &result->level, &result->color_space);
&result->profile, &result->level, &codec_string_color_space);

if (!parse_succeeded || is_codec_ambiguous) {
exception_state.ThrowTypeError("Unknown codec.");
return nullptr;
}

// For now spec allows UA to always choose the output color space, so ignore
// whatever we parsed from the codec string. Hard-code to 601 since that is
// correct most often. See https://crbug.com/1241448.
result->color_space = media::VideoColorSpace::REC601();

// We are done with the parsing.
if (!config->hasAvc())
return result;
Expand Down Expand Up @@ -858,8 +859,22 @@ void VideoEncoder::CallOutputCallback(
if (active_config->options.scalability_mode.has_value())
metadata->setTemporalLayerId(output.temporal_id);

if (first_output_after_configure_ || codec_desc.has_value()) {
// TODO(https://crbug.com/1241448): All encoders should output color space.
// For now, fallback to 601 since that is correct most often.
gfx::ColorSpace output_color_space = output.color_space.IsValid()
? output.color_space
: gfx::ColorSpace::CreateREC601();

if (first_output_after_configure_ || codec_desc.has_value() ||
output_color_space != last_output_color_space_) {
first_output_after_configure_ = false;

if (output_color_space != last_output_color_space_) {
DCHECK(output.key_frame) << "Encoders should generate a keyframe when "
<< "changing color space";
last_output_color_space_ = output_color_space;
}

auto* decoder_config = VideoDecoderConfig::Create();
decoder_config->setCodec(active_config->codec_string);
decoder_config->setCodedHeight(active_config->options.frame_size.height());
Expand All @@ -873,7 +888,7 @@ void VideoEncoder::CallOutputCallback(
}

VideoColorSpace* color_space =
MakeGarbageCollected<VideoColorSpace>(active_config->color_space);
MakeGarbageCollected<VideoColorSpace>(output_color_space);
decoder_config->setColorSpace(color_space->toJSON());

if (codec_desc.has_value()) {
Expand Down
6 changes: 5 additions & 1 deletion third_party/blink/renderer/modules/webcodecs/video_encoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "third_party/blink/renderer/modules/webcodecs/encoder_base.h"
#include "third_party/blink/renderer/modules/webcodecs/hardware_preference.h"
#include "third_party/blink/renderer/modules/webcodecs/video_frame.h"
#include "ui/gfx/color_space.h"

namespace media {
class GpuVideoAcceleratorFactories;
Expand All @@ -36,7 +37,6 @@ class MODULES_EXPORT VideoEncoderTraits {
media::VideoCodec codec;
media::VideoCodecProfile profile;
uint8_t level;
media::VideoColorSpace color_space;

HardwarePreference hw_pref;

Expand Down Expand Up @@ -119,6 +119,10 @@ class MODULES_EXPORT VideoEncoder final
// The number of encoding requests currently handled by |media_encoder_|
// Should not exceed |kMaxActiveEncodes|.
int active_encodes_ = 0;

// The color space corresponding to the last emitted output. Used to update
// emitted VideoDecoderConfig when necessary.
gfx::ColorSpace last_output_color_space_;
};

} // namespace blink
Expand Down

0 comments on commit aba2dc1

Please sign in to comment.