similar to: Patches for adding 120 ms encoding

Hi Felicia, I still don't quite understand why you need to make 120 ms a special case, rather than extend the code that already handles 40 ms and 60 ms. Cheers, Jean-Marc On 06/01/2016 12:58 PM, Felicia Lim wrote: > Hi all, > > I've just realized that there's a better and simpler way of doing this > which ensures that analysis and selection of the mode/bandwidth etc

On 06/01/2016 02:06 PM, Felicia Lim wrote: > That was my intention with refactoring out the subframe encoding and > repacketizing bit. Or do you mean I should merge the explicit check for > 120 ms frame and the existing checks for 40/60 ms wideband? What I mean is that this line in opus_encoder.c: if (frame_size > st->Fs/50 && (st->mode == MODE_CELT_ONLY ||

Attached is the amended second patch. It now extends the multistream API as well to 80/100/120 ms and incorporates changes based on Mark's comments. Thanks, Felicia On Mon, Jun 13, 2016 at 4:21 PM Felicia Lim <flim at google.com> wrote: > Hi Mark, Jean-Marc, > > Thanks for your comments. > > On Sun, Jun 12, 2016 at 6:34 AM Mark Harris <mark.hsj at gmail.com>

Hi, I wondered if are there any further thoughts on these patches? Thanks, Felicia On Thu, Jun 2, 2016 at 2:13 PM Felicia Lim <flim at google.com> wrote: > OK, I've amended the second patch and also added 80 and 100 ms. > > Thanks, > Felicia > > > On Thu, Jun 2, 2016 at 7:20 AM Jean-Marc Valin <jmvalin at jmvalin.ca> wrote: > >> On 06/01/2016 02:06

Hi Felicia, A few comments: > - /* CELT can only support up to 20 ms */ > subframe_size = st->Fs/50; > - nb_subframes = frame_size > st->Fs/25 ? 3 : 2; > + nb_subframes = frame_size/subframe_size; This will use six 20ms frames to make a 120ms packet, even for SILK-only mode where frames can be up to 60ms. For SILK, two 60ms frames would be a more

Hi Ulrich, thanks for the suggestion. My concern is that one of the valid inputs is "2.5", which would require conversion to an int, e.g. x10, but doing something like this would start to affect the code readability. On Mon, Jun 27, 2016 at 3:02 PM Ulrich Windl < Ulrich.Windl at rz.uni-regensburg.de> wrote: > Hi! > > A note on style: Looking at this chunk of the patch

Hi all, I've just realized that there's a better and simpler way of doing this which ensures that analysis and selection of the mode/bandwidth etc is done on the whole 120 ms frame. I believe that the logic and threshold values for making these decisions are still valid for 120 ms, but I might be missing something? Please find my updated patches attached. Thanks, Felicia On Tue, May

Hi all, The attached patch adds/fixes a few null argument checks in the multistream API. Do these changes make sense? Thanks, Felicia -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.xiph.org/pipermail/opus/attachments/20170324/1afee1e1/attachment.html> -------------- next part -------------- A non-text attachment was scrubbed... Name:

Hi! A note on style: Looking at this chunk of the patch -- @@ -382,9 +382,15 @@ int main(int argc, char *argv[]) frame_size = sampling_rate/25; else if (strcmp(argv[ args + 1 ], "60")==0) frame_size = 3*sampling_rate/50; + else if (strcmp(argv[ args + 1 ], "80")==0) + frame_size = 4*sampling_rate/50; +

Hi Felica, yes, this is the only reason to have larger frames is on bandwidth limited links having a gross (physical) bit rates of 10kbps per channel. Decades ago I calculated with figure (for AMR-NB but the same holds for Opus). https://www.tkn.tu-berlin.de/fileadmin/fg112/Hard_Software_Components/Software/hoene_07_2004_paper.pdf Von: Felicia Lim [mailto:flim at google.com]

Hi Felicia, I still need to look very carefully, which will take some time. That being said, some comments I can already make: 1) You need to also update the multi-stream API. 2) You might want to check that CBR works for >60 ms encoding Cheers, Jean-Marc On 06/10/2016 10:19 AM, Felicia Lim wrote: > Hi, I wondered if are there any further thoughts on these patches? > > Thanks,

Hi Mark, Jean-Marc, Thanks for your comments. On Sun, Jun 12, 2016 at 6:34 AM Mark Harris <mark.hsj at gmail.com> wrote: > Hi Felicia, > > A few comments: > > > - /* CELT can only support up to 20 ms */ > > subframe_size = st->Fs/50; > > - nb_subframes = frame_size > st->Fs/25 ? 3 : 2; > > + nb_subframes =

We see the MOVQ instruction but this patch deliberately uses it rather than MOVQDA (load 128-bits aligned). We were seeing that with the trace below, the final invocation is not 128-bit aligned but MOVQDA insists on it (the calling function was pitch_sse4_1.c:90, in the 4-way N - i >= 4 loop). 07-31 11:00:13.469 210 2540 <(469)%20210-2540> D opus_sse1: RBE celt_inner_prod_sse4_1: x

Hi Jean-Marc, I looked at that but importantly these streams need to remain absolutely independent, Further they may have been encoded at some previous time. So my question stands. Thanks, Marc On Jul 26, 2013, at 9:10 PMEDT, Jean-Marc Valin wrote: > Hi Marc, > > I recommend you have a look at the multistream API and how we use it for > surround in the Ogg Opus draft. Sounds

Hi, Please find attached a patch to fix alignment exceptions. Without this change, we were seeing occasional alignment faults when using this with clang. Thanks, Felicia -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.xiph.org/pipermail/opus/attachments/20170818/e0d6bb06/attachment.html> -------------- next part -------------- A non-text

I can't quite figure this out from looking at the repacketizer code. Let's say I have 4 separate stereo streams (say from an 8 channel converter) and want to transmit them somewhere in one stream e.g. RTP or the like. (assuming, custom format if necessary) So could I merge 4 packets with the repacketizer, TX the merged packets, on the other side RX them then split them with the

OK, I've amended the second patch and also added 80 and 100 ms. Thanks, Felicia On Thu, Jun 2, 2016 at 7:20 AM Jean-Marc Valin <jmvalin at jmvalin.ca> wrote: > On 06/01/2016 02:06 PM, Felicia Lim wrote: > > That was my intention with refactoring out the subframe encoding and > > repacketizing bit. Or do you mean I should merge the explicit check for > > 120 ms

Jonathan, Here's the code difference we see with the recent change -- what amounts to reverting your change from a couple years back. It doesn't look like we're getting superfluous instructions from clang now. the bad behavior for us was the alignment exception on the movdqa instructions when the input data wasn't 128-bit aligned. We had to change something because the code as-is

|frame_size_enum| in tests/test_opus_encode.cl:117 is flagged as potentially uninitialized but get_frame_size_enum() will fail anyway if a valid value is not found. --- tests/test_opus_common.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_opus_common.h b/tests/test_opus_common.h index ff7f0142..8b878607 100644 --- a/tests/test_opus_common.h +++ b/tests/test_opus_common.h @@

r+ with James' fix. On 12/07/17 02:01 PM, James Zern wrote: > On Wed, Jul 12, 2017 at 10:58 AM, Felicia Lim <flim at google.com> wrote: >> |frame_size_enum| in tests/test_opus_encode.cl:117 is flagged as potentially >> uninitialized but get_frame_size_enum() will fail anyway if a valid value is >> not found. >> --- >> tests/test_opus_common.h | 2 ++