Wrong argument order in nativeOnRGBAFrameAvailable |
||||
Issue description
Here is the method definition:
private native void nativeOnRGBAFrameAvailable(long nativeScreenCaptureMachineAndroid,
ByteBuffer buf, int left, int top, int width, int height, int rowStride,
long timestamp);
Source: https://cs.chromium.org/chromium/src/media/capture/content/android/java/src/org/chromium/media/ScreenCapture.java?rcl=8738071e484555f67be9c1ea8b04d399e62a4999&l=418
Here is the call:
nativeOnRGBAFrameAvailable(mNativeScreenCaptureMachineAndroid,
image.getPlanes()[0].getBuffer(),
image.getPlanes()[0].getRowStride(), image.getCropRect().left,
image.getCropRect().top, image.getCropRect().width(),
image.getCropRect().height(), image.getTimestamp());
Source:
https://cs.chromium.org/chromium/src/media/capture/content/android/java/src/org/chromium/media/ScreenCapture.java?rcl=8738071e484555f67be9c1ea8b04d399e62a4999&l=126
Found by errorprone:
../../media/capture/content/android/java/src/org/chromium/media/ScreenCapture.java:126: warning: [ArgumentSelectionDefectChecker] Arguments are in the wrong order or could be commented for cl
arity.
nativeOnRGBAFrameAvailable(mNativeScreenCaptureMachineAndroid,
^
(see http://errorprone.info/bugpattern/ArgumentSelectionDefectChecker)
Did you mean 'image.getCropRect().left, image.getCropRect().top,' or '/* left= */image.getPlanes()[0].getRowStride(), /* top= */image.getCropRect().left,'?
,
Jan 15 2018
Raising priority and setting milestone for this to be triaged. Please do so before the next milestone (M66).
,
Jan 16 2018
Thanks wnwen@ for finding out this issue. Fortunately this will do no real harm, since all int arguments are involved and they are called and processed in same order. So lower prior to 2. And cl has been uploaded for review.
,
Jan 17 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/24ef5b4438b894245f73e77ff8f95bc6c15ee083 commit 24ef5b4438b894245f73e77ff8f95bc6c15ee083 Author: Weiyong Yao <braveyao@chromium.org> Date: Wed Jan 17 00:15:48 2018 [Android ScreenCapture] fix wrong argument order of nativeOnRGBAFrameAvailable The argument 'int rowStride' should be after 'ByteBuffer buf' in the method definition [1]. See where it's called at [2] and where it's processed in native code at [3]. PS: fortunately this will cause no real harm, since all int arguments are involved and they are called and processed in same order. [1]: https://cs.chromium.org/chromium/src/media/capture/content/android/java/src/org/chromium/media/ScreenCapture.java?l=418 [2]: https://cs.chromium.org/chromium/src/media/capture/content/android/java/src/org/chromium/media/ScreenCapture.java?l=126 [3]: https://cs.chromium.org/chromium/src/media/capture/content/android/screen_capture_machine_android.cc?l=31 Bug: 801259 Change-Id: Ic7363f2b148c9e17820f3b15a3a82691f35a4abe Reviewed-on: https://chromium-review.googlesource.com/867941 Commit-Queue: Yuri Wiitala <miu@chromium.org> Reviewed-by: Yuri Wiitala <miu@chromium.org> Cr-Commit-Position: refs/heads/master@{#529524} [modify] https://crrev.com/24ef5b4438b894245f73e77ff8f95bc6c15ee083/media/capture/content/android/java/src/org/chromium/media/ScreenCapture.java
,
Jan 17 2018
,
Jan 17 2018
Thank you for fixing it! Glad to know it wasn't causing production errors. |
||||
►
Sign in to add a comment |
||||
Comment 1 by wnwen@chromium.org
, Jan 11 2018