Deprecate VideoDecoder::Reset() and remove calls.

Deprecate VideoDecoder::Reset() and remove calls.

Removes calls to decoder reset and instead drops delta frames and
requests keyframes until one arrives.

BUG=webrtc:5475
R=stefan@webrtc.org
TBR=mflodman@webrtc.org

Committed: https://crrev.com/ed3277bf140d2250f2f735c89d3a2c628454f8fa
Cr-Commit-Position: refs/heads/master@{#11460}

[pbos-webrtc, 2016-01-29 15:13:02]

PTAL, woo!

[pbos-webrtc, 2016-01-29 15:13:10]

The CQ bit was checked by pbos@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-01-29 15:13:21]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1647163002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1647163002/1

[commit-bot: I haz the power, 2016-01-29 17:03:07]

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

[commit-bot: I haz the power, 2016-01-29 17:03:08]

Dry run: This issue passed the CQ dry run.

[stefan-webrtc, 2016-02-01 16:23:09]

https://codereview.webrtc.org/1647163002/diff/1/webrtc/modules/video_coding/v...
File webrtc/modules/video_coding/video_receiver.cc (right):

https://codereview.webrtc.org/1647163002/diff/1/webrtc/modules/video_coding/v...
webrtc/modules/video_coding/video_receiver.cc:298: }
Should we instead make sure the jitter buffer does the right thing (by not
returning delta frames after a flush) and DCHECK here?

https://codereview.webrtc.org/1647163002/diff/1/webrtc/video/vie_channel.cc
File webrtc/video/vie_channel.cc (left):

https://codereview.webrtc.org/1647163002/diff/1/webrtc/video/vie_channel.cc#o...
webrtc/video/vie_channel.cc:919: vcm_->ResetDecoder();
Is this safe to remove? I assume it is, but it would be good if you explained
why you think it is.

https://codereview.webrtc.org/1647163002/diff/1/webrtc/video/vie_channel.cc
File webrtc/video/vie_channel.cc (right):

https://codereview.webrtc.org/1647163002/diff/1/webrtc/video/vie_channel.cc#n...
webrtc/video/vie_channel.cc:1177: return 0;
This change doesn't seem trivial to me. Should we instead do an InitEncode()?
Are we guaranteed to do that InitEncode/Reset once a frame gets to the decoder?
For instance, I'm not sure we have checks on payload type change in the VCM, so
if we switch payload type maybe we don't reset the decoder now?

[pbos-webrtc, 2016-02-02 13:25:12]

https://codereview.webrtc.org/1647163002/diff/1/webrtc/modules/video_coding/v...
File webrtc/modules/video_coding/video_receiver.cc (right):

https://codereview.webrtc.org/1647163002/diff/1/webrtc/modules/video_coding/v...
webrtc/modules/video_coding/video_receiver.cc:298: }
On 2016/02/01 16:23:08, stefan-webrtc (holmer) wrote:
> Should we instead make sure the jitter buffer does the right thing (by not
> returning delta frames after a flush) and DCHECK here?

Not obvious to me how to make the jitter buffer continuously request keyframes
due to additional incoming delta frames, the keyframe request may be dropped, so
we need to keep sending them.

https://codereview.webrtc.org/1647163002/diff/1/webrtc/video/vie_channel.cc
File webrtc/video/vie_channel.cc (left):

https://codereview.webrtc.org/1647163002/diff/1/webrtc/video/vie_channel.cc#o...
webrtc/video/vie_channel.cc:919: vcm_->ResetDecoder();
On 2016/02/01 16:23:08, stefan-webrtc (holmer) wrote:
> Is this safe to remove? I assume it is, but it would be good if you explained
> why you think it is. 

Yeah, I don't think there's any point in killing the decoder here, if we don't
then it doesn't have to reinitialize if it starts receiving again.

https://codereview.webrtc.org/1647163002/diff/1/webrtc/video/vie_channel.cc
File webrtc/video/vie_channel.cc (right):

https://codereview.webrtc.org/1647163002/diff/1/webrtc/video/vie_channel.cc#n...
webrtc/video/vie_channel.cc:1177: return 0;
On 2016/02/01 16:23:08, stefan-webrtc (holmer) wrote:
> This change doesn't seem trivial to me. Should we instead do an InitEncode()?
> Are we guaranteed to do that InitEncode/Reset once a frame gets to the
decoder?
> For instance, I'm not sure we have checks on payload type change in the VCM,
so
> if we switch payload type maybe we don't reset the decoder now?

Decode in VideoReceiver calls VCMCodecDataBase::GetDecoder which reinitializes
if payload_type has changed, the only difference here is that it will do a
decoder release on the network(?) thread.

[pbos-webrtc, 2016-02-02 13:31:48]

rebase

[pbos-webrtc, 2016-02-02 13:55:01]

Remove OnInitializeDecoder for video

[stefan-webrtc, 2016-02-02 14:23:27]

lgtm

[pbos-webrtc, 2016-02-02 14:39:44]

Description was changed from

==========
Deprecate VideoDecoder::Reset() and remove calls.

Removes calls to decoder reset and instead drops delta frames and
requests keyframes until one arrives.

BUG=webrtc:5475
R=stefan@webrtc.org
==========

to

==========
Deprecate VideoDecoder::Reset() and remove calls.

Removes calls to decoder reset and instead drops delta frames and
requests keyframes until one arrives.

BUG=webrtc:5475
R=stefan@webrtc.org
TBR=mflodman@webrtc.org
==========

[pbos-webrtc, 2016-02-02 14:39:44]

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

[pbos-webrtc, 2016-02-02 14:39:52]

TBR=mflodman@ for:

Missing LGTM from an OWNER for these files:
    talk/app/webrtc/java/jni/androidmediadecoder_jni.cc
    webrtc/video_decoder.h

[pbos-webrtc, 2016-02-02 14:40:24]

Description was changed from

==========
Deprecate VideoDecoder::Reset() and remove calls.

Removes calls to decoder reset and instead drops delta frames and
requests keyframes until one arrives.

BUG=webrtc:5475
R=stefan@webrtc.org
TBR=mflodman@webrtc.org
==========

to

==========
Deprecate VideoDecoder::Reset() and remove calls.

Removes calls to decoder reset and instead drops delta frames and
requests keyframes until one arrives.

BUG=webrtc:5475
R=stefan@webrtc.org
TBR=mflodman@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/ed3277bf140d2250f2f735c89...
==========

[pbos-webrtc, 2016-02-02 14:40:26]

Committed patchset #3 (id:40001) manually as
ed3277bf140d2250f2f735c89d3a2c628454f8fa (presubmit successful).

[commit-bot: I haz the power, 2016-02-02 14:40:26]

Description was changed from

==========
Deprecate VideoDecoder::Reset() and remove calls.

Removes calls to decoder reset and instead drops delta frames and
requests keyframes until one arrives.

BUG=webrtc:5475
R=stefan@webrtc.org
TBR=mflodman@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/ed3277bf140d2250f2f735c89...
==========

to

==========
Deprecate VideoDecoder::Reset() and remove calls.

Removes calls to decoder reset and instead drops delta frames and
requests keyframes until one arrives.

BUG=webrtc:5475
R=stefan@webrtc.org
TBR=mflodman@webrtc.org

Committed: https://crrev.com/ed3277bf140d2250f2f735c89d3a2c628454f8fa
Cr-Commit-Position: refs/heads/master@{#11460}
==========

[commit-bot: I haz the power, 2016-02-02 14:40:27]

Patchset 3 (id:??) landed as
https://crrev.com/ed3277bf140d2250f2f735c89d3a2c628454f8fa
Cr-Commit-Position: refs/heads/master@{#11460}