Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2709923005:
Android HW decoder: Support odd heights for non-texture output (Closed)
|
Created:
3 years, 10 months ago by magjed_webrtc Modified:
3 years, 10 months ago Reviewers:
sakal CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com Target Ref:
refs/heads/master Project:
webrtc Visibility:
Public. |
DescriptionAndroid HW decoder: Support odd heights for non-texture output
When textures are not enabled and we are using byte buffer outputs, the
decoder is currently crashing for odd heights because of an RTC_CHECK.
This CL removes the check and handles the pointer offset to the chroma
planes for the odd height case instead.
This has been verified to work correctly on a Pixel device.
BUG=webrtc:6651
Review-Url: https://codereview.webrtc.org/2709923005
Cr-Commit-Position: refs/heads/master@{#16805}
Committed: https://chromium.googlesource.com/external/webrtc/+/0e22a4cfd3790d80ad1ae699891341fe322cb418
Patch Set 1 #
Total comments: 2
Patch Set 2 : Don't read outside buffers. #Messages
Total messages: 20 (14 generated)
The CQ bit was checked by magjed@webrtc.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.webrtc.org/...
Description was changed from ========== Android HW decoder: Support odd heights for non-texture output When textures are not enabled and we are using byte buffer outputs, the decoder is currently crashing for odd heights because of an RTC_CHECK. This CL removes the check and handles the pointer offset to the chroma planes for the odd height case instead. BUG=webrtc:6651 ========== to ========== Android HW decoder: Support odd heights for non-texture output When textures are not enabled and we are using byte buffer outputs, the decoder is currently crashing for odd heights because of an RTC_CHECK. This CL removes the check and handles the pointer offset to the chroma planes for the odd height case instead. This has been verified to work correctly on a Pixel device. BUG=webrtc:6651 ==========
magjed@webrtc.org changed reviewers: + sakal@webrtc.org
Sami - please take a look.
lgtm
https://codereview.webrtc.org/2709923005/diff/1/webrtc/sdk/android/src/jni/an... File webrtc/sdk/android/src/jni/androidmediadecoder_jni.cc (right): https://codereview.webrtc.org/2709923005/diff/1/webrtc/sdk/android/src/jni/an... webrtc/sdk/android/src/jni/androidmediadecoder_jni.cc:820: libyuv::I420Copy(y_ptr, stride, u_ptr, uv_stride, v_ptr, uv_stride, Hmm, it seems I420Copy is reading uv_stride * ((height + 1) / 2) amount of data from both U- and V-planes. Is this safe?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by magjed@webrtc.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.webrtc.org/...
https://codereview.webrtc.org/2709923005/diff/1/webrtc/sdk/android/src/jni/an... File webrtc/sdk/android/src/jni/androidmediadecoder_jni.cc (right): https://codereview.webrtc.org/2709923005/diff/1/webrtc/sdk/android/src/jni/an... webrtc/sdk/android/src/jni/androidmediadecoder_jni.cc:820: libyuv::I420Copy(y_ptr, stride, u_ptr, uv_stride, v_ptr, uv_stride, On 2017/02/23 12:59:07, sakal wrote: > Hmm, it seems I420Copy is reading uv_stride * ((height + 1) / 2) amount of data > from both U- and V-planes. Is this safe? No. I made a new patch set that is more complicated but should be safe.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by magjed@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from sakal@webrtc.org Link to the patchset: https://codereview.webrtc.org/2709923005/#ps20001 (title: "Don't read outside buffers.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1487862525923390,
"parent_rev": "88b82b59f185e9cdc8425be228e3879ba152a83b", "commit_rev":
"0e22a4cfd3790d80ad1ae699891341fe322cb418"}
Message was sent while issue was closed.
Description was changed from ========== Android HW decoder: Support odd heights for non-texture output When textures are not enabled and we are using byte buffer outputs, the decoder is currently crashing for odd heights because of an RTC_CHECK. This CL removes the check and handles the pointer offset to the chroma planes for the odd height case instead. This has been verified to work correctly on a Pixel device. BUG=webrtc:6651 ========== to ========== Android HW decoder: Support odd heights for non-texture output When textures are not enabled and we are using byte buffer outputs, the decoder is currently crashing for odd heights because of an RTC_CHECK. This CL removes the check and handles the pointer offset to the chroma planes for the odd height case instead. This has been verified to work correctly on a Pixel device. BUG=webrtc:6651 Review-Url: https://codereview.webrtc.org/2709923005 Cr-Commit-Position: refs/heads/master@{#16805} Committed: https://chromium.googlesource.com/external/webrtc/+/0e22a4cfd3790d80ad1ae6998... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/external/webrtc/+/0e22a4cfd3790d80ad1ae6998... |
