Update to opus-HEAD-66611f1.

Update to opus-HEAD-66611f1.

R=henrika@chromium.org, sergeyu@chromium.org
BUG=452984
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=293851

[wtc, 2015-01-27 23:57:20]

wtc@chromium.org changed reviewers:
+ henrika@chromium.org, sergeyu@chromium.org

[wtc, 2015-01-28 00:01:44]

Henrik, Sergey:

I need to update third_party/opus/src to the current HEAD of the Opus repository
so that I can enable the MIPS optimizations.

Although the Opus website says:

  libopus 1.1.1-beta

  This is a beta release for the upcoming 1.1.1 version.
  Compared to 1.1, it mostly includes optimizations for
  x86, MIPS and ARM.

opus-1.1.1-beta.tar.gz does not contain any MIPS optimizations. Therefore I have
to update to the current snapshot.

Please advise how I should update third_party/opus and third_party/opus/src, and
what tests I should run. Thanks.

[wtc, 2015-01-28 00:04:43]

https://codereview.chromium.org/882843002/diff/1/autogen.sh
File autogen.sh (right):

https://codereview.chromium.org/882843002/diff/1/autogen.sh#newcode1
autogen.sh:1: #!/bin/sh
I added autogen.sh to third_party/opus/src. I am wondering if you deliberately
omitted autogen.sh from third_party/opus/src. If so, I'll remove it from this
CL.

[Sergey Ulanov, 2015-01-28 01:39:14]

Please remove autogen.sh and update_version. 
Otherwise lgtm.
Once this CL lands you just need to roll the change in DEPS and update
third_party/opus/opus_srcs.gypi.
opus should be covered by webrtc (unit)tests and there are some tests for it in
remoting_unittests.

https://codereview.chromium.org/882843002/diff/1/autogen.sh
File autogen.sh (right):

https://codereview.chromium.org/882843002/diff/1/autogen.sh#newcode1
autogen.sh:1: #!/bin/sh
On 2015/01/28 00:04:43, wtc wrote:
> I added autogen.sh to third_party/opus/src. I am wondering if you deliberately
> omitted autogen.sh from third_party/opus/src. If so, I'll remove it from this
> CL.

I think it's because it's not included in tar balls. Another problem with this
file is that it doesn't have copyright header, so checklicenses script will not
like it.

https://codereview.chromium.org/882843002/diff/1/update_version
File update_version (right):

https://codereview.chromium.org/882843002/diff/1/update_version#newcode14
update_version:14: # This script should NOT be included in distributed tarballs,
because if a
I guess we don't want to check in this file either. Also it doesn't have
copyright header.

[henrika (OOO until Aug 14), 2015-01-28 07:24:41]

LGTM as a member from the WebRTC team. I rely on the feedback from Sergey here.
Will CC tlegrand just in case.

[henrika (OOO until Aug 14), 2015-01-28 07:25:24]

henrika@chromium.org changed reviewers:
+ tlegrand@chromium.org

[wtc, 2015-01-28 17:50:47]

I removed autogen.sh, doc/release.txt, and update_version in patch set 2. I am
going to commit it.

https://codereview.chromium.org/882843002/diff/1/autogen.sh
File autogen.sh (right):

https://codereview.chromium.org/882843002/diff/1/autogen.sh#newcode1
autogen.sh:1: #!/bin/sh

Done. I removed this file from the CL.

https://codereview.chromium.org/882843002/diff/1/doc/release.txt
File doc/release.txt (right):

https://codereview.chromium.org/882843002/diff/1/doc/release.txt#newcode1
doc/release.txt:1: = Release checklist =

This file isn't in the source tarball opus-1.1.1-beta.tar.gz, so I also removed
it from this CL.

https://codereview.chromium.org/882843002/diff/1/update_version
File update_version (right):

https://codereview.chromium.org/882843002/diff/1/update_version#newcode14
update_version:14: # This script should NOT be included in distributed tarballs,
because if a
On 2015/01/28 01:39:14, Sergey Ulanov wrote:
> I guess we don't want to check in this file either. Also it doesn't have
> copyright header.

Done.

[wtc, 2015-01-28 17:59:01]

The CQ bit was checked by wtc@chromium.org

[commit-bot: I haz the power, 2015-01-28 17:59:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/882843002/10001

[commit-bot: I haz the power, 2015-01-28 18:00:25]

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

[commit-bot: I haz the power, 2015-01-28 18:00:27]

Failed to apply patch for Makefile.mips:
While running svn copy Makefile.unix Makefile.mips --config-dir
/b/infra_internal/commit_queue/subversion_config --non-interactive --username
commit-bot@chromium.org;
  svn: E155010: Path
'/b/infra_internal/commit_queue/workdir/chromium_deps/Makefile.unix' does not
exist

Patch:   NR  Makefile.unix->Makefile.mips
Index: Makefile.mips
diff --git a/Makefile.unix b/Makefile.mips
similarity index 68%
copy from Makefile.unix
copy to Makefile.mips
index
36fd337216aefba3610922459869d3546c071036..464ca6bd237f78931e4ad6eaf1676ad2d41b0735
100644
--- a/Makefile.unix
+++ b/Makefile.mips
@@ -1,7 +1,7 @@
 #################### COMPILE OPTIONS #######################
 
 # Uncomment this for fixed-point build
-#FIXED_POINT=1
+FIXED_POINT=1
 
 # It is strongly recommended to uncomment one of these
 # VAR_ARRAYS: Use C99 variable-length arrays for stack allocation
@@ -45,7 +45,9 @@ ldflags-from-ldlibdirs  = $(addprefix -L,$(1))
 ldlibs-from-libs        = $(addprefix -l,$(1))
 
 WARNINGS = -Wall -W -Wstrict-prototypes -Wextra -Wcast-align -Wnested-externs
-Wshadow
-CFLAGS  += -O2 -g $(WARNINGS) -DOPUS_BUILD
+
+CFLAGS  += -mips32r2 -mno-mips16 -std=gnu99 -O2 -g $(WARNINGS)
-DENABLE_ASSERTIONS -DMIPSr1_ASM -DOPUS_BUILD -mdspr2 -march=74kc -mtune=74kc
-mmt -mgp32
+
 CINCLUDES = include silk celt
 
 ifdef FIXED_POINT
@@ -91,11 +93,23 @@ OBJS := $(patsubst %.c,%$(OBJSUFFIX),$(SRCS_C))
 OPUSDEMO_SRCS_C = src/opus_demo.c
 OPUSDEMO_OBJS := $(patsubst %.c,%$(OBJSUFFIX),$(OPUSDEMO_SRCS_C))
 
+TESTOPUSAPI_SRCS_C = tests/test_opus_api.c
+TESTOPUSAPI_OBJS := $(patsubst %.c,%$(OBJSUFFIX),$(TESTOPUSAPI_SRCS_C))
+
+TESTOPUSDECODE_SRCS_C = tests/test_opus_decode.c
+TESTOPUSDECODE_OBJS := $(patsubst %.c,%$(OBJSUFFIX),$(TESTOPUSDECODE_SRCS_C))
+
+TESTOPUSENCODE_SRCS_C = tests/test_opus_encode.c
+TESTOPUSENCODE_OBJS := $(patsubst %.c,%$(OBJSUFFIX),$(TESTOPUSENCODE_SRCS_C))
+
+TESTOPUSPADDING_SRCS_C = tests/test_opus_padding.c
+TESTOPUSPADDING_OBJS := $(patsubst %.c,%$(OBJSUFFIX),$(TESTOPUSPADDING_SRCS_C))
+
 OPUSCOMPARE_SRCS_C = src/opus_compare.c
 OPUSCOMPARE_OBJS := $(patsubst %.c,%$(OBJSUFFIX),$(OPUSCOMPARE_SRCS_C))
 
 # Rules
-all: lib opus_demo opus_compare
+all: lib opus_demo opus_compare test_opus_api test_opus_decode test_opus_encode
test_opus_padding
 
 lib: $(TARGET)
 
@@ -105,6 +119,18 @@ $(TARGET): $(OBJS)
 opus_demo$(EXESUFFIX): $(OPUSDEMO_OBJS) $(TARGET)
 	$(LINK.o.cmdline)
 
+test_opus_api$(EXESUFFIX): $(TESTOPUSAPI_OBJS) $(TARGET)
+	$(LINK.o.cmdline)
+
+test_opus_decode$(EXESUFFIX): $(TESTOPUSDECODE_OBJS) $(TARGET)
+	$(LINK.o.cmdline)
+
+test_opus_encode$(EXESUFFIX): $(TESTOPUSENCODE_OBJS) $(TARGET)
+	$(LINK.o.cmdline)
+
+test_opus_padding$(EXESUFFIX): $(TESTOPUSPADDING_OBJS) $(TARGET)
+	$(LINK.o.cmdline)
+
 opus_compare$(EXESUFFIX): $(OPUSCOMPARE_OBJS)
 	$(LINK.o.cmdline)
 
@@ -122,6 +148,9 @@ force:
 
 clean:
 	rm -f opus_demo$(EXESUFFIX) opus_compare$(EXESUFFIX) $(TARGET) \
-		$(OBJS) $(OPUSDEMO_OBJS) $(OPUSCOMPARE_OBJS)
+                test_opus_api$(EXESUFFIX) test_opus_decode$(EXESUFFIX) \
+                test_opus_encode$(EXESUFFIX) test_opus_padding$(EXESUFFIX) \
+		$(OBJS) $(OPUSDEMO_OBJS) $(OPUSCOMPARE_OBJS) $(TESTOPUSAPI_OBJS) \
+                $(TESTOPUSDECODE_OBJS) $(TESTOPUSENCODE_OBJS)
$(TESTOPUSPADDING_OBJS)
 
 .PHONY: all lib clean

[wtc, 2015-01-28 21:10:23]

The CQ bit was checked by wtc@chromium.org

[commit-bot: I haz the power, 2015-01-28 21:11:29]

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

[commit-bot: I haz the power, 2015-01-28 21:12:23]

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

[commit-bot: I haz the power, 2015-01-28 21:12:27]

Failed to apply patch for Makefile.mips:
While running svn copy NEWS Makefile.mips --config-dir
/b/infra_internal/commit_queue/subversion_config --non-interactive --username
commit-bot@chromium.org;
  svn: E155010: Path '/b/infra_internal/commit_queue/workdir/chromium_deps/NEWS'
does not exist

Patch:   NR  NEWS->Makefile.mips
Index: Makefile.mips
diff --git a/NEWS b/Makefile.mips
similarity index 100%
copy from NEWS
copy to Makefile.mips

[wtc, 2015-01-28 22:15:55]

Committed patchset #4 (id:30001) manually as 293851 (presubmit successful).

[tlegrand1, 2015-01-29 07:36:58]

On 2015/01/28 22:15:55, wtc wrote:
> Committed patchset #4 (id:30001) manually as 293851 (presubmit successful).

What is the reason for this change? I'm missing a description.

I have to object to this change. We don't want to run Opus ToT from
http://git.xiph.org/?p=opus.git. We only want to run well tested versions of the
codec, and the latest real release is 1.1 from December 2013. That's the version
we already had.

WebRTC runs Opus by default, so this change will eventually affect all the
hundreds of applications running WebRTC.

All settings regarding Opus inside WebRTC assumes the 1.1 version, so updating
to a different version isn't as trivial as this. We need to make sure we get the
behavior we expect, and verify quality.

Can you please revert this CL until we have discussed this?

[hlundin-chromium, 2015-02-03 08:42:06]

On 2015/01/29 07:36:58, tlegrand1 wrote:
> On 2015/01/28 22:15:55, wtc wrote:
> > Committed patchset #4 (id:30001) manually as 293851 (presubmit successful).
> 
> What is the reason for this change? I'm missing a description.
> 
> I have to object to this change. We don't want to run Opus ToT from
> http://git.xiph.org/?p=opus.git. We only want to run well tested versions of
the
> codec, and the latest real release is 1.1 from December 2013. That's the
version
> we already had.
> 
> WebRTC runs Opus by default, so this change will eventually affect all the
> hundreds of applications running WebRTC.
> 
> All settings regarding Opus inside WebRTC assumes the 1.1 version, so updating
> to a different version isn't as trivial as this. We need to make sure we get
the
> behavior we expect, and verify quality.
> 
> Can you please revert this CL until we have discussed this?

I believe this change causes bit-exactness tests to fail. The CL
https://webrtc-codereview.appspot.com/35969004/ tries to update the Chrome
revision that WebRTC build upon and also picks up this new version of Opus. But
the try bots are going red on some Opus tests. I support tlegrand's revert
request.

[henrika (OOO until Aug 14), 2015-02-03 09:04:41]

I will revert this CL since it breaks WebRTC in Chrome.

[henrika (OOO until Aug 14), 2015-02-03 09:06:19]

A revert of this CL (patchset #4 id:30001) has been created in
https://codereview.chromium.org/896583006/ by henrika@chromium.org.

The reason for reverting is: See detailed discussion in original CL on why this
CL breaks WebRTC in Chrome..