Add VideoCodec::PreferDecodeLate

Add VideoCodec::PreferDecodeLate
The purpose is so that a decoder (Android) that only have a limited number of output buffers can make sure that decoding is done just before the frame is needed.

Removed unused iSupportsRenderTiming and the settings structs since it was not used.
Added VCMReceiver::FrameForDecoding unit test for the case when PreferDecodeLate is set.

Note that this does not change the current behaviour. We actually currently always decode frames late. This cl is to make sure the behaviour is kept for Android, if the default behaviour is changed.

Committed: https://crrev.com/796cfaf7f76aa740cc7f4bb2c94f88637e475324
Cr-Commit-Position: refs/heads/master@{#10974}

[perkj_webrtc, 2015-11-05 11:56:36]

perkj@webrtc.org changed reviewers:
+ pbos@webrtc.org, stefan@webrtc.org

[perkj_webrtc, 2015-11-05 11:56:37]

Hi, Can you please take a look? 

For Android, both this, and something like
https://codereview.webrtc.org/1414693006/ will be needed.

https://codereview.webrtc.org/1428293003/diff/1/webrtc/video_decoder.h
File webrtc/video_decoder.h (right):

https://codereview.webrtc.org/1428293003/diff/1/webrtc/video_decoder.h#newcode67
webrtc/video_decoder.h:67: // Returns true if the currently active decoder only
have one output buffer.
has

[pbos-webrtc, 2015-11-05 17:47:31]

Would esp. like stefan@ to take a look here :)

https://codereview.webrtc.org/1428293003/diff/1/webrtc/video_decoder.h
File webrtc/video_decoder.h (right):

https://codereview.webrtc.org/1428293003/diff/1/webrtc/video_decoder.h#newcode70
webrtc/video_decoder.h:70: virtual bool HasOnlyOneOutputBuffer() const {
I would like us to decode the extra output buffer as soon as it's available, was
that impossible to wire up?

https://codereview.webrtc.org/1428293003/diff/20001/webrtc/video/video_decode...
File webrtc/video/video_decoder.cc (right):

https://codereview.webrtc.org/1428293003/diff/20001/webrtc/video/video_decode...
webrtc/video/video_decoder.cc:135: return decoder_->HasOnlyOneOutputBuffer();
Not true for fallback decoders right? How should this function then? Same as
above?

https://codereview.webrtc.org/1428293003/diff/20001/webrtc/video/video_receiv...
File webrtc/video/video_receive_stream.cc (right):

https://codereview.webrtc.org/1428293003/diff/20001/webrtc/video/video_receiv...
webrtc/video/video_receive_stream.cc:273:
vie_channel_->SetExpectedRenderDelay(config.render_delay_ms);
Can you put this one step down to split up incoming_video_stream and
vie_channel_ calls?

https://codereview.webrtc.org/1428293003/diff/20001/webrtc/video_engine/vie_c...
File webrtc/video_engine/vie_channel.cc (right):

https://codereview.webrtc.org/1428293003/diff/20001/webrtc/video_engine/vie_c...
webrtc/video_engine/vie_channel.cc:431: int32_t
ViEChannel::RegisterExternalDecoder(const uint8_t pl_type,
I think you can make this void and CHECK that this returns ok.

https://codereview.webrtc.org/1428293003/diff/20001/webrtc/video_engine/vie_c...
File webrtc/video_engine/vie_channel.h (right):

https://codereview.webrtc.org/1428293003/diff/20001/webrtc/video_engine/vie_c...
webrtc/video_engine/vie_channel.h:90: VideoDecoder* decoder);
git cl format, and remove the const please :)

[perkj_webrtc, 2015-11-09 15:23:20]

perkj@webrtc.org changed reviewers:
+ mflodman@webrtc.org

[perkj_webrtc, 2015-11-09 15:23:20]

Adding Magnus. 
Here is the cl I was talking about.

[perkj_webrtc, 2015-12-04 12:22:59]

Description was changed from

==========
Add VideoCodec::HasOnlyOneOutputBuffer
The purpose is so that a decoder (Android) that only have one real output buffer
can make sure that decoding is done just before the frame is needed.

Removed unused iSupportsRenderTiming and the settings structs since it was not
used.
Added VCMReceiver::FrameForDecoding unit test for the case when
HasOnlyOneOutPutBuffer is set.
==========

to

==========
Add VideoCodec::PreferDecodeLate
The purpose is so that a decoder (Android) that only have a limited number of
output buffers can make sure that decoding is done just before the frame is
needed.

Removed unused iSupportsRenderTiming and the settings structs since it was not
used.
Added VCMReceiver::FrameForDecoding unit test for the case when PreferDecodeLate
is set.

Note that this does not change the current behaviour. We actually currently
always decode frames late. This cl is to make sure the behaviour is kept for
Android, if the default behaviour is changed.
==========

[perkj_webrtc, 2015-12-04 12:24:20]

stefan, please take a look. 
This cl now don't change any behaviour at all. It is your call later if you want
to change the default behaviour, but it would then not break Android.

[stefan-webrtc, 2015-12-07 09:02:19]

https://codereview.webrtc.org/1428293003/diff/120001/webrtc/modules/video_cod...
File webrtc/modules/video_coding/codec_database.h (right):

https://codereview.webrtc.org/1428293003/diff/120001/webrtc/modules/video_cod...
webrtc/modules/video_coding/codec_database.h:123: bool PreferDecodeLate() const;
Do we need this method? Isn't it enough to get the decoder and ask it directly
if it prefers late decoding?

https://codereview.webrtc.org/1428293003/diff/120001/webrtc/modules/video_cod...
File webrtc/modules/video_coding/video_coding_impl.h (right):

https://codereview.webrtc.org/1428293003/diff/120001/webrtc/modules/video_cod...
webrtc/modules/video_coding/video_coding_impl.h:154: uint8_t payloadType);
git cl format

https://codereview.webrtc.org/1428293003/diff/120001/webrtc/modules/video_cod...
File webrtc/modules/video_coding/video_receiver.cc (right):

https://codereview.webrtc.org/1428293003/diff/120001/webrtc/modules/video_cod...
webrtc/modules/video_coding/video_receiver.cc:249: return;
Remove

https://codereview.webrtc.org/1428293003/diff/120001/webrtc/modules/video_cod...
webrtc/modules/video_coding/video_receiver.cc:282: bool
has_only_one_output_buffer = false;
prefers_decode_late?

https://codereview.webrtc.org/1428293003/diff/120001/webrtc/video_decoder.h
File webrtc/video_decoder.h (right):

https://codereview.webrtc.org/1428293003/diff/120001/webrtc/video_decoder.h#n...
webrtc/video_decoder.h:81: return true;
Shouldn't the default be false? Or do you want to preserve current behavior with
this CL?

[perkj_webrtc, 2015-12-07 12:06:34]

PTAL

https://codereview.webrtc.org/1428293003/diff/120001/webrtc/modules/video_cod...
File webrtc/modules/video_coding/codec_database.h (right):

https://codereview.webrtc.org/1428293003/diff/120001/webrtc/modules/video_cod...
webrtc/modules/video_coding/codec_database.h:123: bool PreferDecodeLate() const;
On 2015/12/07 09:02:19, stefan-webrtc (holmer) wrote:
> Do we need this method? Isn't it enough to get the decoder and ask it directly
> if it prefers late decoding?

Well, video_receiver uses GetDecoder once it actually have a frame to decode. I
need to query even before the decoder has been set so I think this is needed.

https://codereview.webrtc.org/1428293003/diff/120001/webrtc/modules/video_cod...
File webrtc/modules/video_coding/video_coding_impl.h (right):

https://codereview.webrtc.org/1428293003/diff/120001/webrtc/modules/video_cod...
webrtc/modules/video_coding/video_coding_impl.h:154: uint8_t payloadType);
On 2015/12/07 09:02:19, stefan-webrtc (holmer) wrote:
> git cl format

Done.

https://codereview.webrtc.org/1428293003/diff/120001/webrtc/modules/video_cod...
File webrtc/modules/video_coding/video_receiver.cc (right):

https://codereview.webrtc.org/1428293003/diff/120001/webrtc/modules/video_cod...
webrtc/modules/video_coding/video_receiver.cc:249: return;
On 2015/12/07 09:02:19, stefan-webrtc (holmer) wrote:
> Remove

Done.

https://codereview.webrtc.org/1428293003/diff/120001/webrtc/modules/video_cod...
webrtc/modules/video_coding/video_receiver.cc:282: bool
has_only_one_output_buffer = false;
On 2015/12/07 09:02:19, stefan-webrtc (holmer) wrote:
> prefers_decode_late?

Done.

https://codereview.webrtc.org/1428293003/diff/120001/webrtc/video_decoder.h
File webrtc/video_decoder.h (right):

https://codereview.webrtc.org/1428293003/diff/120001/webrtc/video_decoder.h#n...
webrtc/video_decoder.h:81: return true;
On 2015/12/07 09:02:19, stefan-webrtc (holmer) wrote:
> Shouldn't the default be false? Or do you want to preserve current behavior
with
> this CL?

I would prefer to not change the current behavior. If you guys want to, you can
do that whenever you want after this cl.

[stefan-webrtc, 2015-12-09 15:52:25]

This works for me. lgtm

pbos, mflodman, any comments?

https://codereview.webrtc.org/1428293003/diff/200001/webrtc/video_engine/vie_...
File webrtc/video_engine/vie_channel.h (right):

https://codereview.webrtc.org/1428293003/diff/200001/webrtc/video_engine/vie_...
webrtc/video_engine/vie_channel.h:98: void SetExpectedRenderDelay(int32_t
delay_ms);
nit: Make this input int.

[mflodman, 2015-12-10 07:39:16]

LGTM

[pbos-webrtc, 2015-12-10 11:27:20]

lgtm w/ nits

https://codereview.webrtc.org/1428293003/diff/200001/talk/app/webrtc/java/jni...
File talk/app/webrtc/java/jni/androidmediadecoder_jni.cc (right):

https://codereview.webrtc.org/1428293003/diff/200001/talk/app/webrtc/java/jni...
talk/app/webrtc/java/jni/androidmediadecoder_jni.cc:93: bool PreferDecodeLate()
const override { return true; }
PreferLateDecoding imo.

https://codereview.webrtc.org/1428293003/diff/200001/webrtc/modules/video_cod...
File webrtc/modules/video_coding/video_coding_robustness_unittest.cc (right):

https://codereview.webrtc.org/1428293003/diff/200001/webrtc/modules/video_cod...
webrtc/modules/video_coding/video_coding_robustness_unittest.cc:207:
EXPECT_EQ(VCM_OK, vcm_->Decode(33));  // Decode timestamp 0.
git cl format please

[pbos-webrtc, 2015-12-10 11:27:45]

https://codereview.webrtc.org/1428293003/diff/200001/talk/app/webrtc/java/jni...
File talk/app/webrtc/java/jni/androidmediadecoder_jni.cc (right):

https://codereview.webrtc.org/1428293003/diff/200001/talk/app/webrtc/java/jni...
talk/app/webrtc/java/jni/androidmediadecoder_jni.cc:93: bool PreferDecodeLate()
const override { return true; }
On 2015/12/10 11:27:20, pbos-webrtc wrote:
> PreferLateDecoding imo.

Make that PrefersLateDecoding().

[perkj_webrtc, 2015-12-10 13:11:08]

Patchset #12 (id:220001) has been deleted

[perkj_webrtc, 2015-12-10 13:11:28]

Patchset #12 (id:240001) has been deleted

[perkj_webrtc, 2015-12-10 13:14:27]

The CQ bit was checked by perkj@webrtc.org

[perkj_webrtc, 2015-12-10 13:14:27]

The patchset sent to the CQ was uploaded after l-g-t-m from pbos@webrtc.org,
stefan@webrtc.org, mflodman@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1428293003/#ps280001 (title:
"Rebased")

[commit-bot: I haz the power, 2015-12-10 13:14:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1428293003/280001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1428293003/280001

[commit-bot: I haz the power, 2015-12-10 15:14:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-12-10 15:14:52]

Try jobs failed on following builders:
  android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  mac_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  win_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[perkj_webrtc, 2015-12-10 17:25:04]

The CQ bit was checked by perkj@webrtc.org

[commit-bot: I haz the power, 2015-12-10 17:25:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1428293003/280001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1428293003/280001

[commit-bot: I haz the power, 2015-12-10 17:27:43]

Description was changed from

==========
Add VideoCodec::PreferDecodeLate
The purpose is so that a decoder (Android) that only have a limited number of
output buffers can make sure that decoding is done just before the frame is
needed.

Removed unused iSupportsRenderTiming and the settings structs since it was not
used.
Added VCMReceiver::FrameForDecoding unit test for the case when PreferDecodeLate
is set.

Note that this does not change the current behaviour. We actually currently
always decode frames late. This cl is to make sure the behaviour is kept for
Android, if the default behaviour is changed.
==========

to

==========
Add VideoCodec::PreferDecodeLate
The purpose is so that a decoder (Android) that only have a limited number of
output buffers can make sure that decoding is done just before the frame is
needed.

Removed unused iSupportsRenderTiming and the settings structs since it was not
used.
Added VCMReceiver::FrameForDecoding unit test for the case when PreferDecodeLate
is set.

Note that this does not change the current behaviour. We actually currently
always decode frames late. This cl is to make sure the behaviour is kept for
Android, if the default behaviour is changed.
==========

[commit-bot: I haz the power, 2015-12-10 17:27:44]

Committed patchset #13 (id:280001)

[commit-bot: I haz the power, 2015-12-10 17:27:51]

Description was changed from

==========
Add VideoCodec::PreferDecodeLate
The purpose is so that a decoder (Android) that only have a limited number of
output buffers can make sure that decoding is done just before the frame is
needed.

Removed unused iSupportsRenderTiming and the settings structs since it was not
used.
Added VCMReceiver::FrameForDecoding unit test for the case when PreferDecodeLate
is set.

Note that this does not change the current behaviour. We actually currently
always decode frames late. This cl is to make sure the behaviour is kept for
Android, if the default behaviour is changed.
==========

to

==========
Add VideoCodec::PreferDecodeLate
The purpose is so that a decoder (Android) that only have a limited number of
output buffers can make sure that decoding is done just before the frame is
needed.

Removed unused iSupportsRenderTiming and the settings structs since it was not
used.
Added VCMReceiver::FrameForDecoding unit test for the case when PreferDecodeLate
is set.

Note that this does not change the current behaviour. We actually currently
always decode frames late. This cl is to make sure the behaviour is kept for
Android, if the default behaviour is changed.

Committed: https://crrev.com/796cfaf7f76aa740cc7f4bb2c94f88637e475324
Cr-Commit-Position: refs/heads/master@{#10974}
==========

[commit-bot: I haz the power, 2015-12-10 17:27:53]

Patchset 13 (id:??) landed as
https://crrev.com/796cfaf7f76aa740cc7f4bb2c94f88637e475324
Cr-Commit-Position: refs/heads/master@{#10974}