Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2872223002:
media: Set ColorSpace on decoded frames in DecryptingVideoDecoder (Closed)
Descriptionmedia: Set ColorSpace on decoded frames in DecryptingVideoDecoder
If ColorSpace is not set in the decoded VideoFrame, and if the
VideoDecoderConfig has a valid ColorSpace, use the ColorSpace in the
config to set the ColorSpace of the VideoFrame.
BUG=707128
Review-Url: https://codereview.chromium.org/2872223002
Cr-Commit-Position: refs/heads/master@{#470826}
Committed: https://chromium.googlesource.com/chromium/src/+/443ea5e342701a3b20212c655c7816cbd28b53c8
Patch Set 1 #
Total comments: 2
Patch Set 2 : more fix and test #
Messages
Total messages: 26 (18 generated)
The CQ bit was checked by xhwang@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by xhwang@chromium.org to run a CQ dry run
xhwang@chromium.org changed reviewers: + hubbe@chromium.org
hubbe: This is my first attempt of setting color space in DecryptingVideoDecoder. I am not sure whether this is the right way of doing it, but would like to upload it first for discussion. PTAL and let's talk.
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm https://codereview.chromium.org/2872223002/diff/1/media/filters/decrypting_vi... File media/filters/decrypting_video_decoder.cc (right): https://codereview.chromium.org/2872223002/diff/1/media/filters/decrypting_vi... media/filters/decrypting_video_decoder.cc:282: if (frame->ColorSpace() == gfx::ColorSpace() && It's probably better to use frame->ColorSpace().IsValid() now.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by xhwang@chromium.org to run a CQ dry run
hubbe: Updated as discussed. Also I added a test to cover this. PTAL again! https://codereview.chromium.org/2872223002/diff/1/media/filters/decrypting_vi... File media/filters/decrypting_video_decoder.cc (right): https://codereview.chromium.org/2872223002/diff/1/media/filters/decrypting_vi... media/filters/decrypting_video_decoder.cc:282: if (frame->ColorSpace() == gfx::ColorSpace() && On 2017/05/10 17:23:11, hubbe wrote: > It's probably better to use frame->ColorSpace().IsValid() now. Done and improved this block.
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm
The CQ bit was unchecked by xhwang@chromium.org
The CQ bit was checked by xhwang@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was unchecked by xhwang@chromium.org
The CQ bit was checked by xhwang@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1494461053081990,
"parent_rev": "97c0d877b1aff4ac84138c3eab71d994ead93b14", "commit_rev":
"443ea5e342701a3b20212c655c7816cbd28b53c8"}
Message was sent while issue was closed.
Description was changed from ========== media: Set ColorSpace on decoded frames in DecryptingVideoDecoder If ColorSpace is not set in the decoded VideoFrame, and if the VideoDecoderConfig has a valid ColorSpace, use the ColorSpace in the config to set the ColorSpace of the VideoFrame. BUG=707128 ========== to ========== media: Set ColorSpace on decoded frames in DecryptingVideoDecoder If ColorSpace is not set in the decoded VideoFrame, and if the VideoDecoderConfig has a valid ColorSpace, use the ColorSpace in the config to set the ColorSpace of the VideoFrame. BUG=707128 Review-Url: https://codereview.chromium.org/2872223002 Cr-Commit-Position: refs/heads/master@{#470826} Committed: https://chromium.googlesource.com/chromium/src/+/443ea5e342701a3b20212c655c78... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/443ea5e342701a3b20212c655c78... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
