Android GlUtil: Add helper functions generateTexture/deleteTexture

Android GlUtil: Add helper functions generateTexture/deleteTexture

The purpose with this CL is to remove some code bloat. A subtle change is that GL_TEXTURE_MIN_FILTER in MediaCodecVideoDecoder is changed from GL_NEAREST to GL_LINEAR. This may lead to slightly worse performance when the decoded video is rendered minified, but with better visual quality. After reading https://crbug.com/351458 and the fix https://codereview.chromium.org/713603002 I think this is the right choice.

BUG=webrtc:4742
R=hbos@webrtc.org, tommi@webrtc.org

Committed: https://chromium.googlesource.com/external/webrtc/+/1a591ddc7e6529e57a27f4a8f133ddd14a7ead16

[magjed_webrtc, 2015-09-01 12:32:47]

Patchset #1 (id:1) has been deleted

[magjed_webrtc, 2015-09-01 12:39:56]

magjed@webrtc.org changed reviewers:
+ tommi@webrtc.org

[magjed_webrtc, 2015-09-01 13:58:41]

tommi - Please take a look.

[magjed_webrtc, 2015-09-02 07:18:46]

magjed@webrtc.org changed reviewers:
+ hbos@webrtc.org

[magjed_webrtc, 2015-09-02 07:18:57]

hbos - Can you take a look instead?

[hbos, 2015-09-02 08:57:55]

https://codereview.webrtc.org/1303373005/diff/20001/talk/app/webrtc/java/andr...
File talk/app/webrtc/java/android/org/webrtc/GlUtil.java (right):

https://codereview.webrtc.org/1303373005/diff/20001/talk/app/webrtc/java/andr...
talk/app/webrtc/java/android/org/webrtc/GlUtil.java:82:
GLES20.glDeleteTextures(1, textureArray, 0);
If you want this to be a one-liner you can do "new int[] { textureId }".

https://codereview.webrtc.org/1303373005/diff/20001/talk/app/webrtc/java/andr...
File talk/app/webrtc/java/android/org/webrtc/VideoCapturerAndroid.java (right):

https://codereview.webrtc.org/1303373005/diff/20001/talk/app/webrtc/java/andr...
talk/app/webrtc/java/android/org/webrtc/VideoCapturerAndroid.java:84: private
int cameraGlTexture = -1;
I believe 0 is the value to use for "no texture". Make sure -1 cannot in theory
be used as the ID or switch to 0 (thought it would probably never be used in
practice).

https://codereview.webrtc.org/1303373005/diff/20001/talk/app/webrtc/java/andr...
talk/app/webrtc/java/android/org/webrtc/VideoCapturerAndroid.java:462:
cameraGlTexture = -1;
0

https://codereview.webrtc.org/1303373005/diff/20001/talk/app/webrtc/java/andr...
File talk/app/webrtc/java/android/org/webrtc/VideoRendererGui.java (right):

https://codereview.webrtc.org/1303373005/diff/20001/talk/app/webrtc/java/andr...
talk/app/webrtc/java/android/org/webrtc/VideoRendererGui.java:102: private int
oesTexture = -1;
Oh, more -1 instead of 0.

These are created and used for drawing. But the textures are never deleted?
Confirm and fix or add a todo.

[magjed_webrtc, 2015-09-02 10:14:23]

Patchset #2 (id:40001) has been deleted

[magjed_webrtc, 2015-09-02 10:15:39]

hbos - Please take another look.

https://codereview.webrtc.org/1303373005/diff/20001/talk/app/webrtc/java/andr...
File talk/app/webrtc/java/android/org/webrtc/GlUtil.java (right):

https://codereview.webrtc.org/1303373005/diff/20001/talk/app/webrtc/java/andr...
talk/app/webrtc/java/android/org/webrtc/GlUtil.java:82:
GLES20.glDeleteTextures(1, textureArray, 0);
On 2015/09/02 08:57:54, hbos wrote:
> If you want this to be a one-liner you can do "new int[] { textureId }".

Cool, thanks. This defeats the purpose of this function. I will just inline it
instead to make it more transparent.

https://codereview.webrtc.org/1303373005/diff/20001/talk/app/webrtc/java/andr...
File talk/app/webrtc/java/android/org/webrtc/VideoCapturerAndroid.java (right):

https://codereview.webrtc.org/1303373005/diff/20001/talk/app/webrtc/java/andr...
talk/app/webrtc/java/android/org/webrtc/VideoCapturerAndroid.java:84: private
int cameraGlTexture = -1;
On 2015/09/02 08:57:54, hbos wrote:
> I believe 0 is the value to use for "no texture". Make sure -1 cannot in
theory
> be used as the ID or switch to 0 (thought it would probably never be used in
> practice).

True, I think 0 is the only value that is guaranteed to not be returned from
glGenTextures(). I couldn't find anything about -1 in the spec. It's a GLUint so
it's 0xFFFFFFFF. I guess it will never be returned in practice.

https://codereview.webrtc.org/1303373005/diff/20001/talk/app/webrtc/java/andr...
File talk/app/webrtc/java/android/org/webrtc/VideoRendererGui.java (right):

https://codereview.webrtc.org/1303373005/diff/20001/talk/app/webrtc/java/andr...
talk/app/webrtc/java/android/org/webrtc/VideoRendererGui.java:102: private int
oesTexture = -1;
On 2015/09/02 08:57:55, hbos wrote:
> Oh, more -1 instead of 0.
> 
> These are created and used for drawing. But the textures are never deleted?
> Confirm and fix or add a todo.

Done.

[hbos, 2015-09-02 11:10:39]

lgtm

https://codereview.webrtc.org/1303373005/diff/20001/talk/app/webrtc/java/andr...
File talk/app/webrtc/java/android/org/webrtc/VideoCapturerAndroid.java (right):

https://codereview.webrtc.org/1303373005/diff/20001/talk/app/webrtc/java/andr...
talk/app/webrtc/java/android/org/webrtc/VideoCapturerAndroid.java:84: private
int cameraGlTexture = -1;
On 2015/09/02 10:15:39, magjed_webrtc wrote:
> On 2015/09/02 08:57:54, hbos wrote:
> > I believe 0 is the value to use for "no texture". Make sure -1 cannot in
> theory
> > be used as the ID or switch to 0 (thought it would probably never be used in
> > practice).
> 
> True, I think 0 is the only value that is guaranteed to not be returned from
> glGenTextures(). I couldn't find anything about -1 in the spec. It's a GLUint
so
> it's 0xFFFFFFFF. I guess it will never be returned in practice.

Acknowledged.

https://codereview.webrtc.org/1303373005/diff/60001/talk/app/webrtc/java/andr...
File talk/app/webrtc/java/android/org/webrtc/VideoCapturerAndroid.java (right):

https://codereview.webrtc.org/1303373005/diff/60001/talk/app/webrtc/java/andr...
talk/app/webrtc/java/android/org/webrtc/VideoCapturerAndroid.java:462:
GLES20.glDeleteTextures(1, new int[] {cameraGlTexture}, 0);
Optional: the hbos@ style guide is "new int[] { cameraGlTexture }". Check what
the google style guide says.

https://codereview.webrtc.org/1303373005/diff/60001/talk/app/webrtc/java/src/...
File talk/app/webrtc/java/src/org/webrtc/MediaCodecVideoDecoder.java (right):

https://codereview.webrtc.org/1303373005/diff/60001/talk/app/webrtc/java/src/...
talk/app/webrtc/java/src/org/webrtc/MediaCodecVideoDecoder.java:274:
GLES20.glDeleteTextures(1, new int[] {textureID}, 0);
To "{ " or not to "{ "...?

[magjed_webrtc, 2015-09-02 11:50:14]

tommi@ - I need a stamp for:
talk/app/webrtc/androidtests/src/org/webrtc/GlRectDrawerTest.java
talk/app/webrtc/java/src/org/webrtc/MediaCodecVideoDecoder.java

I guess we should move MediaCodecVideoDecoder.java to the android folder at some
point.

https://codereview.webrtc.org/1303373005/diff/60001/talk/app/webrtc/java/andr...
File talk/app/webrtc/java/android/org/webrtc/VideoCapturerAndroid.java (right):

https://codereview.webrtc.org/1303373005/diff/60001/talk/app/webrtc/java/andr...
talk/app/webrtc/java/android/org/webrtc/VideoCapturerAndroid.java:462:
GLES20.glDeleteTextures(1, new int[] {cameraGlTexture}, 0);
On 2015/09/02 11:10:38, hbos wrote:
> Optional: the hbos@ style guide is "new int[] { cameraGlTexture }". Check what
> the google style guide says.

4.6.2 Horizontal whitespace 
...
8. Optional just inside both braces of an array initializer.
    new int[] {5, 6} and new int[] { 5, 6 } are both valid

https://codereview.webrtc.org/1303373005/diff/60001/talk/app/webrtc/java/src/...
File talk/app/webrtc/java/src/org/webrtc/MediaCodecVideoDecoder.java (right):

https://codereview.webrtc.org/1303373005/diff/60001/talk/app/webrtc/java/src/...
talk/app/webrtc/java/src/org/webrtc/MediaCodecVideoDecoder.java:274:
GLES20.glDeleteTextures(1, new int[] {textureID}, 0);
On 2015/09/02 11:10:38, hbos wrote:
> To "{ " or not to "{ "...?

magjed@ style guide is "{x}"

[tommi, 2015-09-02 12:08:04]

rs lgtm. agree that moving to android/ makes sense.

[magjed_webrtc, 2015-09-02 12:43:23]

Committed patchset #2 (id:60001) manually as
1a591ddc7e6529e57a27f4a8f133ddd14a7ead16 (presubmit successful).