Add texture support to HardwareVideoDecoder.
HardwareVideoDecoder is now a listener for SurfaceTextureHelper. It takes a
SurfaceTextureHelper on construction. If it is non-null, it operates in texture
mode instead of byte-buffer mode.
When in texture mode, the HardwareVideoDecoder renders output frames to a Surface,
listens for the texture frame to become available, wraps it in a VideoFrame, and
pushes it to the decoder callback.
As in MediaCodecVideoDecoder, it may queue up to three buffers while waiting for
the surface to become available for rendering. If more buffers are queued, it will
drop the oldest.
This change also implements the VideoFrame.TextureBuffer and reorganizes code
for wrapping an existing ByteBuffer into an I420Buffer. This makes it easier
to implement the texture buffer's ToI420() method.
BUG=webrtc:7760R=pthatcher@webrtc.org, sakal@webrtc.org
Review-Url: https://codereview.webrtc.org/2977643002 .
Cr-Commit-Position: refs/heads/master@{#19081}
Committed: https://chromium.googlesource.com/external/webrtc/+/8fb23618d8e3c2a20832006d361f99161b4980bd

Description was changed from
==========
Add texture support to HardwareVideoDecoder.
HardwareVideoDecoder is now a listener for SurfaceTextureHelper. It takes a
SurfaceTextureHelper on construction. If it is non-null, it operates in texture
mode instead of byte-buffer mode.
When in texture mode, the HardwareVideoDecoder renders output frames to a
Surface,
listens for the texture frame to become available, wraps it in a VideoFrame, and
pushes it to the decoder callback.
As in MediaCodecVideoDecoder, it may queue up to three buffers while waiting for
the surface to become available for rendering. If more buffers are queued, it
will
drop the oldest.
This change also implements the VideoFrame.TextureBuffer and reorganizes code
for wrapping an existing ByteBuffer into an I420Buffer. This makes it easier
to implement the texture buffer's ToI420() method.
BUG=webrtc:7760
==========
to
==========
Add texture support to HardwareVideoDecoder.
HardwareVideoDecoder is now a listener for SurfaceTextureHelper. It takes a
SurfaceTextureHelper on construction. If it is non-null, it operates in texture
mode instead of byte-buffer mode.
When in texture mode, the HardwareVideoDecoder renders output frames to a
Surface,
listens for the texture frame to become available, wraps it in a VideoFrame, and
pushes it to the decoder callback.
As in MediaCodecVideoDecoder, it may queue up to three buffers while waiting for
the surface to become available for rendering. If more buffers are queued, it
will
drop the oldest.
This change also implements the VideoFrame.TextureBuffer and reorganizes code
for wrapping an existing ByteBuffer into an I420Buffer. This makes it easier
to implement the texture buffer's ToI420() method.
BUG=webrtc:7760
==========

https://codereview.webrtc.org/2977643002/diff/1/webrtc/sdk/android/api/org/we...
File webrtc/sdk/android/api/org/webrtc/HardwareVideoDecoderFactory.java (right):
https://codereview.webrtc.org/2977643002/diff/1/webrtc/sdk/android/api/org/we...
webrtc/sdk/android/api/org/webrtc/HardwareVideoDecoderFactory.java:32:
@Deprecated // Not removed yet to avoid breaking callers. Remove once Duo is
ready.
On 2017/07/17 13:44:56, sakal wrote:
> I don't think we should reference internal projects directly in open source
> code.
Done.
https://codereview.webrtc.org/2977643002/diff/1/webrtc/sdk/android/instrument...
File
webrtc/sdk/android/instrumentationtests/src/org/webrtc/HardwareVideoDecoderTest.java
(right):
https://codereview.webrtc.org/2977643002/diff/1/webrtc/sdk/android/instrument...
webrtc/sdk/android/instrumentationtests/src/org/webrtc/HardwareVideoDecoderTest.java:183:
decoded.set(frame);
On 2017/07/17 13:44:56, sakal wrote:
> I think we should retain the frame here and release it once we are done.
Done.
https://codereview.webrtc.org/2977643002/diff/1/webrtc/sdk/android/src/java/o...
File webrtc/sdk/android/src/java/org/webrtc/HardwareVideoDecoder.java (right):
https://codereview.webrtc.org/2977643002/diff/1/webrtc/sdk/android/src/java/o...
webrtc/sdk/android/src/java/org/webrtc/HardwareVideoDecoder.java:143:
SurfaceTextureHelper surfaceTextureHelper) {
On 2017/07/17 13:44:56, sakal wrote:
> I don't think we have to pass in the SurfaceTextureHelper. It should be able
to
> live in the decoder. We should just pass in the shared context.
Done.
https://codereview.webrtc.org/2977643002/diff/1/webrtc/sdk/android/src/java/o...
webrtc/sdk/android/src/java/org/webrtc/HardwareVideoDecoder.java:338:
surfaceTextureHelper.returnTextureFrame();
On 2017/07/17 13:44:56, sakal wrote:
> I think we should wait that all frames are released and then release the
surface
> texture helper.
Now that the decoder owns the SurfaceTextureHelper, dispose() does this for us.
It waits until any outstanding frame is returned before releasing the helper.
https://codereview.webrtc.org/2977643002/diff/1/webrtc/sdk/android/src/java/o...
webrtc/sdk/android/src/java/org/webrtc/HardwareVideoDecoder.java:428: while
(!decodedTextureBuffers.isEmpty() && !surfaceTextureHelper.isTextureInUse()) {
On 2017/07/17 13:44:56, sakal wrote:
> I don't think we are going to get more than one loop with this strategy. We
can
> render even if the surface is in use. SurfaceTextureHelper will call
> updateTexImage only after the texture is free. However, if we render two
frames
> in this period, we will only get the last one.
>
> Overall, I don't think SurfaceTextureHelper is a good fit here. If you want to
> implement serializing access to the texture yourself, we should create a
> SurfaceTexture in single buffer mode: https://goo.gl/FuXqV7
>
> Alternatively, we could use SurfaceTextureHelper and just always render the
> frame. However, then we will drop frames if the renderer doesn't keep up.
I need to call updateTexImage() whenever I get a texture frame callback, to
update the texture to the right contents. updateTexImage() must be called on
the thread with the EGL context holding the texture. SurfaceTexture's callbacks
are on an arbitrary thread. SurfaceTextureHelper owns the thread, EGL context,
and texture, and calls updateTexImage() on the right thread.
We can't call updateTexImage() while a frame is in flight, or the texture will
change out from under it. SurfaceTextureHelper's synchronization prevents that.
We can only convert a texture frame to YUV on the thread that owns the texture.
SurfaceTextureHelper performs that operation on the correct thread.
As far as I can tell, I need almost everything SurfaceTextureHelper does.
But I don't need to buffer before rendering to the texture. I can render
everything, as long as I'm careful to keep renderedTextureMetadata in sync with
the last thing I rendered. (Otherwise we could attach the wrong metadata to an
image.)
https://codereview.webrtc.org/2977643002/diff/1/webrtc/sdk/android/src/java/o...
webrtc/sdk/android/src/java/org/webrtc/HardwareVideoDecoder.java:453: //
SurfaceTexture's matrix looks like this:
On 2017/07/17 13:44:56, sakal wrote:
> Please copy the helper method from my other CL here:
>
https://chromium-review.googlesource.com/c/555516/4/webrtc/sdk/android/api/or...
Done.
https://codereview.webrtc.org/2977643002/diff/1/webrtc/sdk/android/src/java/o...
File webrtc/sdk/android/src/java/org/webrtc/I420BufferImpl.java (right):
https://codereview.webrtc.org/2977643002/diff/1/webrtc/sdk/android/src/java/o...
webrtc/sdk/android/src/java/org/webrtc/I420BufferImpl.java:33:
I420BufferImpl(ByteBuffer buffer, int width, int height, int yPos, int strideY,
int uPos,
On 2017/07/17 13:44:56, sakal wrote:
> I would prefer to just have a constructor that is passed in a buffer and the
> parameters and a static method I420BufferImpl.allocate that allocates a
buffer.
> We could also add a method getBuffer to this class and just use
> I420BufferImpl.allocate in OesTextureBuffer.toI420.
Changed the second constructor to an allocate() method. We can't use it in
OesTextureBuffer.toI420 regardless of whether the entire backing buffer is
accessible. The YuvConverter uses a different stride and starts the v data in a
different position. It also needs a bit of extra space to prevent Java
byte-buffer slices from overrunning the buffer when accessing the v channel.
https://codereview.webrtc.org/2977643002/diff/1/webrtc/sdk/android/src/java/o...
File webrtc/sdk/android/src/java/org/webrtc/OesTextureBuffer.java (right):
https://codereview.webrtc.org/2977643002/diff/1/webrtc/sdk/android/src/java/o...
webrtc/sdk/android/src/java/org/webrtc/OesTextureBuffer.java:19: class
OesTextureBuffer implements TextureBuffer {
On 2017/07/17 13:44:56, sakal wrote:
> I think this class is inherently tied to SurfaceTextureHelper and should be a
> subclass that can be created using a member method.
Moved this into SurfaceTextureHelper. I'm not sure I like it from a code
organization perspective, but it does restrict it so it's only usable with the
right id and helper.

lgtm with comments addressed
https://codereview.webrtc.org/2977643002/diff/1/webrtc/sdk/android/src/java/o...
File webrtc/sdk/android/src/java/org/webrtc/HardwareVideoDecoder.java (right):
https://codereview.webrtc.org/2977643002/diff/1/webrtc/sdk/android/src/java/o...
webrtc/sdk/android/src/java/org/webrtc/HardwareVideoDecoder.java:428: while
(!decodedTextureBuffers.isEmpty() && !surfaceTextureHelper.isTextureInUse()) {
On 2017/07/17 21:57:15, mellem wrote:
> On 2017/07/17 13:44:56, sakal wrote:
> > I don't think we are going to get more than one loop with this strategy. We
> can
> > render even if the surface is in use. SurfaceTextureHelper will call
> > updateTexImage only after the texture is free. However, if we render two
> frames
> > in this period, we will only get the last one.
> >
> > Overall, I don't think SurfaceTextureHelper is a good fit here. If you want
to
> > implement serializing access to the texture yourself, we should create a
> > SurfaceTexture in single buffer mode: https://goo.gl/FuXqV7
> >
> > Alternatively, we could use SurfaceTextureHelper and just always render the
> > frame. However, then we will drop frames if the renderer doesn't keep up.
>
> I need to call updateTexImage() whenever I get a texture frame callback, to
> update the texture to the right contents. updateTexImage() must be called on
> the thread with the EGL context holding the texture. SurfaceTexture's
callbacks
> are on an arbitrary thread. SurfaceTextureHelper owns the thread, EGL
context,
> and texture, and calls updateTexImage() on the right thread.
>
> We can't call updateTexImage() while a frame is in flight, or the texture will
> change out from under it. SurfaceTextureHelper's synchronization prevents
that.
>
> We can only convert a texture frame to YUV on the thread that owns the
texture.
> SurfaceTextureHelper performs that operation on the correct thread.
>
> As far as I can tell, I need almost everything SurfaceTextureHelper does.
>
> But I don't need to buffer before rendering to the texture. I can render
> everything, as long as I'm careful to keep renderedTextureMetadata in sync
with
> the last thing I rendered. (Otherwise we could attach the wrong metadata to
an
> image.)
Yeah, we of course would have had to implement drawing the new frame only after
the last frame has been released. SurfaceTexture gives callbacks on a given
handler if you supply it with one.
However, I think the current approach looks good as well.
https://codereview.webrtc.org/2977643002/diff/40001/webrtc/sdk/android/api/or...
File webrtc/sdk/android/api/org/webrtc/SurfaceTextureHelper.java (right):
https://codereview.webrtc.org/2977643002/diff/40001/webrtc/sdk/android/api/or...
webrtc/sdk/android/api/org/webrtc/SurfaceTextureHelper.java:283: public
TextureBuffer createTextureBuffer(int width, int height, float[]
transformMatrix) {
nit: Add JavaDoc for this method.
https://codereview.webrtc.org/2977643002/diff/40001/webrtc/sdk/android/api/or...
webrtc/sdk/android/api/org/webrtc/SurfaceTextureHelper.java:287: /** Android OES
texture buffer. */
nit: Improve the comment to explain that the object calls returnTextureFrame
once all references are freed.
https://codereview.webrtc.org/2977643002/diff/40001/webrtc/sdk/android/src/ja...
File webrtc/sdk/android/src/java/org/webrtc/HardwareVideoDecoder.java (right):
https://codereview.webrtc.org/2977643002/diff/40001/webrtc/sdk/android/src/ja...
webrtc/sdk/android/src/java/org/webrtc/HardwareVideoDecoder.java:57: // Max
number of output buffers queued before starting to drop decoded frames.
Not used anymore?
https://codereview.webrtc.org/2977643002/diff/40001/webrtc/sdk/android/src/ja...
webrtc/sdk/android/src/java/org/webrtc/HardwareVideoDecoder.java:133: private
int droppedFrames = 0;
Not used?

sakal

When working on my CL today I noticed my matrix conversion methods were incorrect. I ...

Description was changed from
==========
Add texture support to HardwareVideoDecoder.
HardwareVideoDecoder is now a listener for SurfaceTextureHelper. It takes a
SurfaceTextureHelper on construction. If it is non-null, it operates in texture
mode instead of byte-buffer mode.
When in texture mode, the HardwareVideoDecoder renders output frames to a
Surface,
listens for the texture frame to become available, wraps it in a VideoFrame, and
pushes it to the decoder callback.
As in MediaCodecVideoDecoder, it may queue up to three buffers while waiting for
the surface to become available for rendering. If more buffers are queued, it
will
drop the oldest.
This change also implements the VideoFrame.TextureBuffer and reorganizes code
for wrapping an existing ByteBuffer into an I420Buffer. This makes it easier
to implement the texture buffer's ToI420() method.
BUG=webrtc:7760
==========
to
==========
Add texture support to HardwareVideoDecoder.
HardwareVideoDecoder is now a listener for SurfaceTextureHelper. It takes a
SurfaceTextureHelper on construction. If it is non-null, it operates in texture
mode instead of byte-buffer mode.
When in texture mode, the HardwareVideoDecoder renders output frames to a
Surface,
listens for the texture frame to become available, wraps it in a VideoFrame, and
pushes it to the decoder callback.
As in MediaCodecVideoDecoder, it may queue up to three buffers while waiting for
the surface to become available for rendering. If more buffers are queued, it
will
drop the oldest.
This change also implements the VideoFrame.TextureBuffer and reorganizes code
for wrapping an existing ByteBuffer into an I420Buffer. This makes it easier
to implement the texture buffer's ToI420() method.
BUG=webrtc:7760
R=pthatcher@webrtc.org, sakal@webrtc.org
Review-Url: https://codereview.webrtc.org/2977643002 .
Cr-Commit-Position: refs/heads/master@{#19081}
Committed:
https://chromium.googlesource.com/external/webrtc/+/8fb23618d8e3c2a20832006d3...
==========