Commit 3d18154f authored by Dale Curtis's avatar Dale Curtis Committed by Commit Bot

Remove mutually exclusive comments and code from VpxVideoDecoder.

Code and comments say opposite things about which color space info
to prefer. Unify on the config only after checking with hubbe@ and
confirming that webm vp9.2 encodes do need to use the config color
space instead of bitstream.

BUG=none
TEST=hubbe@ && vp9.2 playback.

Change-Id: If990c79481eab3e6925c39b1457d182a7d1cbdbb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1670122
Auto-Submit: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: default avatarChrome Cunningham <chcunningham@chromium.org>
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#672723}
parent aa7c8455
...@@ -300,7 +300,7 @@ bool VpxVideoDecoder::VpxDecode(const DecoderBuffer* buffer, ...@@ -300,7 +300,7 @@ bool VpxVideoDecoder::VpxDecode(const DecoderBuffer* buffer,
} }
const vpx_image_t* vpx_image_alpha = nullptr; const vpx_image_t* vpx_image_alpha = nullptr;
AlphaDecodeStatus alpha_decode_status = const auto alpha_decode_status =
DecodeAlphaPlane(vpx_image, &vpx_image_alpha, buffer); DecodeAlphaPlane(vpx_image, &vpx_image_alpha, buffer);
if (alpha_decode_status == kAlphaPlaneError) { if (alpha_decode_status == kAlphaPlaneError) {
return false; return false;
...@@ -308,9 +308,10 @@ bool VpxVideoDecoder::VpxDecode(const DecoderBuffer* buffer, ...@@ -308,9 +308,10 @@ bool VpxVideoDecoder::VpxDecode(const DecoderBuffer* buffer,
*video_frame = nullptr; *video_frame = nullptr;
return true; return true;
} }
if (!CopyVpxImageToVideoFrame(vpx_image, vpx_image_alpha, video_frame)) {
if (!CopyVpxImageToVideoFrame(vpx_image, vpx_image_alpha, video_frame))
return false; return false;
}
if (vpx_image_alpha && config_.codec() == kCodecVP8) { if (vpx_image_alpha && config_.codec() == kCodecVP8) {
libyuv::CopyPlane(vpx_image_alpha->planes[VPX_PLANE_Y], libyuv::CopyPlane(vpx_image_alpha->planes[VPX_PLANE_Y],
vpx_image_alpha->stride[VPX_PLANE_Y], vpx_image_alpha->stride[VPX_PLANE_Y],
...@@ -322,71 +323,61 @@ bool VpxVideoDecoder::VpxDecode(const DecoderBuffer* buffer, ...@@ -322,71 +323,61 @@ bool VpxVideoDecoder::VpxDecode(const DecoderBuffer* buffer,
(*video_frame)->set_timestamp(buffer->timestamp()); (*video_frame)->set_timestamp(buffer->timestamp());
// Default to the color space from the config, but if the bistream specifies // Prefer the color space from the config if available. It generally comes
// one, prefer that instead. // from the color tag which is more expressive than the vp8 and vp9 bitstream.
if (config_.color_space_info().IsSpecified()) { if (config_.color_space_info().IsSpecified()) {
(*video_frame) (*video_frame)
->set_color_space(config_.color_space_info().ToGfxColorSpace()); ->set_color_space(config_.color_space_info().ToGfxColorSpace());
return true;
} }
if (config_.color_space_info().IsSpecified()) { auto primaries = gfx::ColorSpace::PrimaryID::INVALID;
// config_.color_space_info() comes from the color tag which is auto transfer = gfx::ColorSpace::TransferID::INVALID;
// more expressive than the bitstream, so prefer it over the auto matrix = gfx::ColorSpace::MatrixID::INVALID;
// bitstream data below. auto range = vpx_image->range == VPX_CR_FULL_RANGE
(*video_frame) ? gfx::ColorSpace::RangeID::FULL
->set_color_space(config_.color_space_info().ToGfxColorSpace()); : gfx::ColorSpace::RangeID::LIMITED;
} else {
gfx::ColorSpace::PrimaryID primaries = gfx::ColorSpace::PrimaryID::INVALID; switch (vpx_image->cs) {
gfx::ColorSpace::TransferID transfer = gfx::ColorSpace::TransferID::INVALID; case VPX_CS_BT_601:
gfx::ColorSpace::MatrixID matrix = gfx::ColorSpace::MatrixID::INVALID; case VPX_CS_SMPTE_170:
gfx::ColorSpace::RangeID range = vpx_image->range == VPX_CR_FULL_RANGE primaries = gfx::ColorSpace::PrimaryID::SMPTE170M;
? gfx::ColorSpace::RangeID::FULL transfer = gfx::ColorSpace::TransferID::SMPTE170M;
: gfx::ColorSpace::RangeID::LIMITED; matrix = gfx::ColorSpace::MatrixID::SMPTE170M;
break;
switch (vpx_image->cs) { case VPX_CS_SMPTE_240:
case VPX_CS_BT_601: primaries = gfx::ColorSpace::PrimaryID::SMPTE240M;
case VPX_CS_SMPTE_170: transfer = gfx::ColorSpace::TransferID::SMPTE240M;
primaries = gfx::ColorSpace::PrimaryID::SMPTE170M; matrix = gfx::ColorSpace::MatrixID::SMPTE240M;
transfer = gfx::ColorSpace::TransferID::SMPTE170M; break;
matrix = gfx::ColorSpace::MatrixID::SMPTE170M; case VPX_CS_BT_709:
break; primaries = gfx::ColorSpace::PrimaryID::BT709;
case VPX_CS_SMPTE_240: transfer = gfx::ColorSpace::TransferID::BT709;
primaries = gfx::ColorSpace::PrimaryID::SMPTE240M; matrix = gfx::ColorSpace::MatrixID::BT709;
transfer = gfx::ColorSpace::TransferID::SMPTE240M; break;
matrix = gfx::ColorSpace::MatrixID::SMPTE240M; case VPX_CS_BT_2020:
break; primaries = gfx::ColorSpace::PrimaryID::BT2020;
case VPX_CS_BT_709: if (vpx_image->bit_depth >= 12)
primaries = gfx::ColorSpace::PrimaryID::BT709; transfer = gfx::ColorSpace::TransferID::BT2020_12;
else if (vpx_image->bit_depth >= 10)
transfer = gfx::ColorSpace::TransferID::BT2020_10;
else
transfer = gfx::ColorSpace::TransferID::BT709; transfer = gfx::ColorSpace::TransferID::BT709;
matrix = gfx::ColorSpace::MatrixID::BT709; matrix = gfx::ColorSpace::MatrixID::BT2020_NCL; // is this right?
break; break;
case VPX_CS_BT_2020: case VPX_CS_SRGB:
primaries = gfx::ColorSpace::PrimaryID::BT2020; primaries = gfx::ColorSpace::PrimaryID::BT709;
if (vpx_image->bit_depth >= 12) { transfer = gfx::ColorSpace::TransferID::IEC61966_2_1;
transfer = gfx::ColorSpace::TransferID::BT2020_12; matrix = gfx::ColorSpace::MatrixID::BT709;
} else if (vpx_image->bit_depth >= 10) { break;
transfer = gfx::ColorSpace::TransferID::BT2020_10; default:
} else { break;
transfer = gfx::ColorSpace::TransferID::BT709; }
}
matrix = gfx::ColorSpace::MatrixID::BT2020_NCL; // is this right?
break;
case VPX_CS_SRGB:
primaries = gfx::ColorSpace::PrimaryID::BT709;
transfer = gfx::ColorSpace::TransferID::IEC61966_2_1;
matrix = gfx::ColorSpace::MatrixID::BT709;
break;
default:
break;
}
// TODO(ccameron): Set a color space even for unspecified values. // TODO(ccameron): Set a color space even for unspecified values.
if (primaries != gfx::ColorSpace::PrimaryID::INVALID) { if (primaries != gfx::ColorSpace::PrimaryID::INVALID) {
(*video_frame) (*video_frame)
->set_color_space( ->set_color_space(gfx::ColorSpace(primaries, transfer, matrix, range));
gfx::ColorSpace(primaries, transfer, matrix, range));
}
} }
return true; return true;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment