AudioDecoder: Replace Init() with Reset()

AudioDecoder: Replace Init() with Reset()

The Init() method was previously used to initialize and reset
decoders, and returned an error code. The new Reset() method is used
for reset only; the constructor is now responsible for fully
initializing the AudioDecoder.

Reset() doesn't return an error code; it turned out that none of the
functions it ended up calling could actually fail, so this CL removes
their error return codes as well.

R=henrik.lundin@webrtc.org

Committed: https://chromium.googlesource.com/external/webrtc/+/4376648df021fd82f25a38694e33678f802d06ea

[kwiberg-webrtc, 2015-08-26 09:30:37]

kwiberg@webrtc.org changed reviewers:
+ henrik.lundin@webrtc.org

[kwiberg-webrtc, 2015-08-26 09:32:54]

...and I go: la la la la la, please take a look!

[hlundin-webrtc, 2015-08-26 13:07:23]

LG. A few nits and comments.

https://codereview.webrtc.org/1319683002/diff/1/webrtc/modules/audio_coding/c...
File webrtc/modules/audio_coding/codecs/audio_decoder.h (right):

https://codereview.webrtc.org/1319683002/diff/1/webrtc/modules/audio_coding/c...
webrtc/modules/audio_coding/codecs/audio_decoder.h:67: // Reset the decoder
state (empty buffers etc.).
Resets

https://codereview.webrtc.org/1319683002/diff/1/webrtc/modules/audio_coding/c...
File webrtc/modules/audio_coding/codecs/g722/g722_interface.c (right):

https://codereview.webrtc.org/1319683002/diff/1/webrtc/modules/audio_coding/c...
webrtc/modules/audio_coding/codecs/g722/g722_interface.c:72:
WebRtc_g722_decode_init((G722DecoderState*)inst, 64000, 2);
You can argue that the code cannot fail here. But, the old code produced an
error if inst was NULL. The new code won't. I'd like to CHECK(inst), but since
this is C... :(

https://codereview.webrtc.org/1319683002/diff/1/webrtc/modules/audio_coding/c...
File webrtc/modules/audio_coding/codecs/g722/include/g722_interface.h (right):

https://codereview.webrtc.org/1319683002/diff/1/webrtc/modules/audio_coding/c...
webrtc/modules/audio_coding/codecs/g722/include/g722_interface.h:119: * This
function initializes a G722 instance
Haha! Did you search-and-replace the file for more of these?

https://codereview.webrtc.org/1319683002/diff/1/webrtc/modules/audio_coding/c...
File webrtc/modules/audio_coding/codecs/ilbc/ilbc.c (right):

https://codereview.webrtc.org/1319683002/diff/1/webrtc/modules/audio_coding/c...
webrtc/modules/audio_coding/codecs/ilbc/ilbc.c:134: void
WebRtcIlbcfix_DecoderInit20Ms(IlbcDecoderInstance *iLBCdec_inst) {
Join the type and the asterisk while you're poking at this code.

https://codereview.webrtc.org/1319683002/diff/1/webrtc/modules/audio_coding/c...
webrtc/modules/audio_coding/codecs/ilbc/ilbc.c:137: void
WebRtcIlbcfix_Decoderinit30Ms(IlbcDecoderInstance *iLBCdec_inst) {
Here too.

https://codereview.webrtc.org/1319683002/diff/1/webrtc/modules/audio_coding/c...
File webrtc/modules/audio_coding/codecs/opus/opus_interface.c (right):

https://codereview.webrtc.org/1319683002/diff/1/webrtc/modules/audio_coding/c...
webrtc/modules/audio_coding/codecs/opus/opus_interface.c:14: #include <assert.h>
Why? You're not adding any asserts and I couldn't find any in the old code
either.

[kwiberg-webrtc, 2015-08-26 18:54:37]

https://codereview.webrtc.org/1319683002/diff/1/webrtc/modules/audio_coding/c...
File webrtc/modules/audio_coding/codecs/audio_decoder.h (right):

https://codereview.webrtc.org/1319683002/diff/1/webrtc/modules/audio_coding/c...
webrtc/modules/audio_coding/codecs/audio_decoder.h:67: // Reset the decoder
state (empty buffers etc.).
On 2015/08/26 13:07:23, hlundin-webrtc wrote:
> Resets

Done.

https://codereview.webrtc.org/1319683002/diff/1/webrtc/modules/audio_coding/c...
File webrtc/modules/audio_coding/codecs/g722/g722_interface.c (right):

https://codereview.webrtc.org/1319683002/diff/1/webrtc/modules/audio_coding/c...
webrtc/modules/audio_coding/codecs/g722/g722_interface.c:72:
WebRtc_g722_decode_init((G722DecoderState*)inst, 64000, 2);
On 2015/08/26 13:07:23, hlundin-webrtc wrote:
> You can argue that the code cannot fail here. But, the old code produced an
> error if inst was NULL. The new code won't. I'd like to CHECK(inst), but since
> this is C... :(

Well, this CL also changes WebRtc_g722_decode_init so that it will always return
non-NULL, so if you want an assert it should go in there.

https://codereview.webrtc.org/1319683002/diff/1/webrtc/modules/audio_coding/c...
File webrtc/modules/audio_coding/codecs/g722/include/g722_interface.h (right):

https://codereview.webrtc.org/1319683002/diff/1/webrtc/modules/audio_coding/c...
webrtc/modules/audio_coding/codecs/g722/include/g722_interface.h:119: * This
function initializes a G722 instance
On 2015/08/26 13:07:23, hlundin-webrtc wrote:
> Haha! Did you search-and-replace the file for more of these?

No. There's one left in another comment, but I decided not to fix it here.

https://codereview.webrtc.org/1319683002/diff/1/webrtc/modules/audio_coding/c...
File webrtc/modules/audio_coding/codecs/ilbc/ilbc.c (right):

https://codereview.webrtc.org/1319683002/diff/1/webrtc/modules/audio_coding/c...
webrtc/modules/audio_coding/codecs/ilbc/ilbc.c:134: void
WebRtcIlbcfix_DecoderInit20Ms(IlbcDecoderInstance *iLBCdec_inst) {
On 2015/08/26 13:07:23, hlundin-webrtc wrote:
> Join the type and the asterisk while you're poking at this code.

Done. Curiously, git cl format didn't fix this automatically (which is perhaps
why it wasn't already fixed).

https://codereview.webrtc.org/1319683002/diff/1/webrtc/modules/audio_coding/c...
webrtc/modules/audio_coding/codecs/ilbc/ilbc.c:137: void
WebRtcIlbcfix_Decoderinit30Ms(IlbcDecoderInstance *iLBCdec_inst) {
On 2015/08/26 13:07:23, hlundin-webrtc wrote:
> Here too.

Done.

https://codereview.webrtc.org/1319683002/diff/1/webrtc/modules/audio_coding/c...
File webrtc/modules/audio_coding/codecs/opus/opus_interface.c (right):

https://codereview.webrtc.org/1319683002/diff/1/webrtc/modules/audio_coding/c...
webrtc/modules/audio_coding/codecs/opus/opus_interface.c:14: #include <assert.h>
On 2015/08/26 13:07:23, hlundin-webrtc wrote:
> Why? You're not adding any asserts and I couldn't find any in the old code
> either.

No. There was probably some intermediate state where there was an assert.
Removed.

[hlundin-webrtc, 2015-08-27 12:02:56]

lgtm

https://codereview.webrtc.org/1319683002/diff/1/webrtc/modules/audio_coding/c...
File webrtc/modules/audio_coding/codecs/g722/g722_interface.c (right):

https://codereview.webrtc.org/1319683002/diff/1/webrtc/modules/audio_coding/c...
webrtc/modules/audio_coding/codecs/g722/g722_interface.c:72:
WebRtc_g722_decode_init((G722DecoderState*)inst, 64000, 2);
On 2015/08/26 18:54:37, kwiberg-webrtc wrote:
> On 2015/08/26 13:07:23, hlundin-webrtc wrote:
> > You can argue that the code cannot fail here. But, the old code produced an
> > error if inst was NULL. The new code won't. I'd like to CHECK(inst), but
since
> > this is C... :(
> 
> Well, this CL also changes WebRtc_g722_decode_init so that it will always
return
> non-NULL, so if you want an assert it should go in there.

I was referring to what happens if the _input_ to WebRtcG722_DecoderInit is
null. But nothing bad happens, right?

[kwiberg-webrtc, 2015-08-27 12:11:21]

https://codereview.webrtc.org/1319683002/diff/1/webrtc/modules/audio_coding/c...
File webrtc/modules/audio_coding/codecs/g722/g722_interface.c (right):

https://codereview.webrtc.org/1319683002/diff/1/webrtc/modules/audio_coding/c...
webrtc/modules/audio_coding/codecs/g722/g722_interface.c:72:
WebRtc_g722_decode_init((G722DecoderState*)inst, 64000, 2);
On 2015/08/27 12:02:56, hlundin-webrtc wrote:
> On 2015/08/26 18:54:37, kwiberg-webrtc wrote:
> > On 2015/08/26 13:07:23, hlundin-webrtc wrote:
> > > You can argue that the code cannot fail here. But, the old code produced
an
> > > error if inst was NULL. The new code won't. I'd like to CHECK(inst), but
> since
> > > this is C... :(
> > 
> > Well, this CL also changes WebRtc_g722_decode_init so that it will always
> return
> > non-NULL, so if you want an assert it should go in there.
> 
> I was referring to what happens if the _input_ to WebRtcG722_DecoderInit is
> null. But nothing bad happens, right?

No. It allocates memory unless the caller passes it in.

[kwiberg-webrtc, 2015-08-27 12:11:29]

The CQ bit was checked by kwiberg@webrtc.org

[commit-bot: I haz the power, 2015-08-27 12:11:34]

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

[commit-bot: I haz the power, 2015-08-27 12:14:56]

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

[commit-bot: I haz the power, 2015-08-27 12:14:56]

Try jobs failed on following builders:
  presubmit on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/523)

[kwiberg-webrtc, 2015-08-27 13:22:26]

Committed patchset #2 (id:20001) manually as
4376648df021fd82f25a38694e33678f802d06ea (presubmit successful).