New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 709099 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug



Sign in to add a comment

CHECK failed in GLRenderer about color spaces

Project Member Reported by enne@chromium.org, Apr 6 2017

Issue description

See: https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Ftryserver.chromium.mac%2Fmac_chromium_10.10_rel_ng%2F151382%2F%2B%2Frecipes%2Fsteps%2Ftelemetry_perf_unittests__without_patch__on_Mac-10.10%2F0%2Fstdout

[71666:1299:0405/204729.184463:FATAL:gl_renderer.cc(3018)] Check failed: src_color_space == src_color_space.GetAsFullRangeRGB() ({primaries:1, transfer:1, matrix:2, range:1, icc_profile_id:0} vs. {primaries:1, transfer:1, matrix:1, range:2, icc_profile_id:0})
  0   Chromium Framework                  0x000000010bd1ff1c base::debug::StackTrace::StackTrace(unsigned long) + 28
  1   Chromium Framework                  0x000000010bd45ba3 logging::LogMessage::~LogMessage() + 67
  2   Chromium Framework                  0x000000010d4a2cea cc::GLRenderer::SetUseProgram(cc::ProgramKey const&, gfx::ColorSpace const&) + 266
  3   Chromium Framework                  0x000000010d49f720 cc::GLRenderer::FlushTextureQuadCache(cc::GLRenderer::BoundGeometry) + 224
  4   Chromium Framework                  0x000000010d49f4da cc::GLRenderer::DoDrawQuad(cc::DrawQuad const*, gfx::QuadF const*) + 154
  5   Chromium Framework                  0x000000010d499655 cc::DirectRenderer::DrawRenderPass(cc::RenderPass const*) + 1317
  6   Chromium Framework                  0x000000010d498840 cc::DirectRenderer::DrawRenderPassAndExecuteCopyRequests(cc::RenderPass*) + 192
  7   Chromium Framework                  0x000000010d497e00 cc::DirectRenderer::DrawFrame(std::__1::vector<std::__1::unique_ptr<cc::RenderPass, std::__1::default_delete<cc::RenderPass> >, std::__1::allocator<std::__1::unique_ptr<cc::RenderPass, std::__1::default_delete<cc::RenderPass> > > >*, float, gfx::Size const&) + 2480
  8   Chromium Framework                  0x000000010d66c10b cc::Display::DrawAndSwap() + 1483
  9   Chromium Framework                  0x000000010d66e14a cc::DisplayScheduler::DrawAndSwap() + 234
  10  Chromium Framework                  0x000000010d66d963 cc::DisplayScheduler::AttemptDrawAndSwap() + 403
  11  Chromium Framework                  0x000000010d66d0a8 cc::DisplayScheduler::OnBeginFrameDeadline() + 136

This happened in a "without patch" run on one of my try jobs, so is likely flaky.

ccameron, can you investigate?
 
That assert is saying "make sure that we aren't told to draw a YUV resource as RGB". And I noticed the following explosion before the CHECK, namely,

  [20:46:40.595] VTSelectAndCreateVideoEncoderInstanceInternal signalled err=-12908 (err) (Video encoder not available) at /SourceCache/CoreMedia_frameworks/CoreMedia-1562.240/Sources/VideoToolbox/VTVideoEncoderSelection.c line 1245
  [20:46:40.595] VTCompressionSessionCreate signalled err=-12908 (err) (Could not select and open encoder instance) at /SourceCache/CoreMedia_frameworks/CoreMedia-1562.240/Sources/VideoToolbox/VTCompressionSession.c line 946
  [71607:779:0405/204640.594581:ERROR:vt_video_encode_accelerator_mac.cc(520)]  VTCompressionSessionCreate failed: -12908
  [71607:779:0405/204646.926213:ERROR:gpu_video_decode_accelerator.cc(375)] HW video decode not available for profile h264 main
  [71608:28215:0405/204646.926435:ERROR:gpu_video_decode_accelerator_host.cc(99)] Send(GpuCommandBufferMsg_CreateVideoDecoder()) failed
  [71607:779:0405/204647.343886:ERROR:gpu_video_decode_accelerator.cc(375)] HW video decode not available for profile h264 main
  [71608:28215:0405/204647.344110:ERROR:gpu_video_decode_accelerator_host.cc(99)] Send(GpuCommandBufferMsg_CreateVideoDecoder()) failed

So, it appears that HW video decode exploded, and then the browser was confused about how to render the exploded frames. I wonder if this would have passed without this CHECK, or if it would have flaked in a different way.
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8d25b83269837337206aede5522dea9a90daa093

commit 8d25b83269837337206aede5522dea9a90daa093
Author: ccameron <ccameron@chromium.org>
Date: Fri Apr 07 02:06:50 2017

cc: Fix VideoResourceUpdater color conversion

This was assuming that YUV to RGB conversion had been done if the output
format was YUV, which is the opposite of what should be happening.

Make the CHECK for extra YUV to RGB conversion happen always, by adding
a "can ignore for testing" flag for the pixel tests that violate this
assumption.

BUG= 709099 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2799603006
Cr-Commit-Position: refs/heads/master@{#462742}

[modify] https://crrev.com/8d25b83269837337206aede5522dea9a90daa093/cc/output/direct_renderer.h
[modify] https://crrev.com/8d25b83269837337206aede5522dea9a90daa093/cc/output/gl_renderer.cc
[modify] https://crrev.com/8d25b83269837337206aede5522dea9a90daa093/cc/output/renderer_pixeltest.cc
[modify] https://crrev.com/8d25b83269837337206aede5522dea9a90daa093/cc/resources/video_resource_updater.cc

Status: Fixed (was: Assigned)
I don't understand how that ever worked.
Labels: Merge-Request-58
Status: Started (was: Fixed)
Adding M58 merge request.
Project Member

Comment 5 by sheriffbot@chromium.org, Apr 12 2017

Labels: -Merge-Request-58 Merge-Review-58 Hotlist-Merge-Review
This bug requires manual review: We are only 12 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Please add applicable OSs.  Thanks!
Labels: OS-All
Has this been well tested in Canary? And is there enough automated testing/unit tests for this? And is this crucial to get in for M58, or can it wait until M59?
This is safe.
It is a regression in M58 and should not go out to stable.
Labels: -Merge-Review-58 Merge-Approved-58
Approving M58 merge based on comment #9 and #10. Thanks!
Cc: sureshkumari@chromium.org kbr@chromium.org ccameron@chromium.org qiankun....@intel.com yunchao...@intel.com yang...@intel.com hubbe@chromium.org
 Issue 707871  has been merged into this issue.
Labels: Merge-Merged
Status: Fixed (was: Started)
Actually, the original fix was merged in https://codereview.chromium.org/2746863002, so this should be okay without any merges.
Labels: -Merge-Merged -Merge-Approved-58 merge-merged-3029
Removing "Merge-Approved-58" and applying "Merge-Merged-3029" label per comment #13. 

Sign in to add a comment