ref: b49c07d1c460f13a1292723ca5dc38140a782a90
parent: 60b527aedd2469ef1810f41087fd791196ac9c5e
author: Timothy B. Terriberry <tterribe@xiph.org>
date: Fri Jul 26 02:19:27 EDT 2024
Advance frame number after an L=0 RTE. This avoids the need to code a frame separator in the case that the last repeated extension is a short extension, and the next non-repeated extension is in the following frame instead of the current one (saving one additional byte). Also add tests for both encoding and decoding this. Signed-off-by: Jean-Marc Valin <jeanmarcv@google.com>
--- a/src/extensions.c
+++ b/src/extensions.c
@@ -157,11 +157,6 @@
if (iter->curr_len < 0) {
return OPUS_INVALID_PACKET;
}
- /* Checking this here allows opus_extension_iterator_set_frame_max() to be
- called at any point. */
- if (iter->curr_frame >= iter->frame_max) {
- return 0;
- }
if (iter->repeat_frame > 0) {
/* We are in the process of repeating some extensions. */
for (;iter->repeat_frame < iter->nb_frames; iter->repeat_frame++) {
@@ -195,13 +190,6 @@
/* If we were asked to stop at frame_max, skip extensions for later
frames. */
if (iter->repeat_frame >= iter->frame_max) {
- if (iter->repeat_l == 0) {
- /* If L == 0, there will be no more extensions after these
- repeats, so we can just stop. */
- iter->repeat_frame = 0;
- iter->curr_len = 0;
- return 0;
- }
continue;
}
if (ext != NULL) {
@@ -218,14 +206,22 @@
}
/* We finished repeating extensions. */
iter->repeat_data_end = iter->repeat_data = iter->curr_data;
- /* Even if there is more data (because there was nothing to repeat or
- because the last extension was a short extension and we did not use
- all the data), when L == 0 we are done decoding extensions. */
+ /* If L == 0, advance the frame number to handle the case where we did
+ not consume all of the data with an L == 0 long extension. */
if (iter->repeat_l == 0) {
- iter->curr_len = 0;
+ iter->curr_frame++;
+ /* Ignore additional padding if this was already the last frame. */
+ if (iter->curr_frame >= iter->nb_frames) {
+ iter->curr_len = 0;
+ }
}
iter->repeat_frame = 0;
}
+ /* Checking this here allows opus_extension_iterator_set_frame_max() to be
+ called at any point. */
+ if (iter->curr_frame >= iter->frame_max) {
+ return 0;
+ }
while (iter->curr_len > 0) {
const unsigned char *curr_data0;
int id;
@@ -483,7 +479,7 @@
opus_int32 repeat_data_end_idx;
int repeat_count;
repeat_count = 0;
- repeat_data_end_idx = -1;
+ repeat_data_end_idx = i;
if (f + 1 < nb_frames)
{
for (i=frame_min_idx[f];i<frame_max_idx[f];i++)
@@ -566,6 +562,9 @@
(extensions[frame_repeat_idx[nb_frames-1]].len/255 + 1);
if (savings < 0) break;
}
+ /* N.B., we are only considering the case where the last
+ repeated extension is a long extension, so we do not
+ check if we can save a frame separator by using L=0. */
celt_assert(savings >= 0);
}
#endif
@@ -616,7 +615,9 @@
int g;
/* Add the repeat indicator. */
nb_repeated = repeat_count*(nb_frames - (f + 1));
- last = written + nb_repeated == nb_extensions;
+ last = written + nb_repeated == nb_extensions
+ || (extensions[repeat_data_end_idx].id < 32
+ && i+1 >= frame_max_idx[f]);
if (len-pos < 1)
return OPUS_BUFFER_TOO_SMALL;
if (data) data[pos] = 0x04 + !last;
@@ -636,6 +637,7 @@
}
frame_min_idx[g] = j;
}
+ if (last) curr_frame++;
}
}
}
--- a/tests/test_opus_extensions.c
+++ b/tests/test_opus_extensions.c
@@ -420,7 +420,7 @@
}
}
-#define NB_EXT (12)
+#define NB_EXT (13)
void test_extensions_repeating(void)
{
@@ -435,8 +435,9 @@
{32, 2, (const unsigned char *)"DRED2", 5},
{5, 1, (const unsigned char *)NULL, 0},
{5, 2, (const unsigned char *)NULL, 0},
+ {6, 2, (const unsigned char *)"f", 1},
{6, 1, (const unsigned char *)"e", 1},
- {6, 2, (const unsigned char *)"f", 1}
+ {32, 2, (const unsigned char *)"DREDthree", 9}
};
static const opus_int32 encoded_len[] = {
0,
@@ -463,10 +464,16 @@
/* nb_ext = 10: code the last repeated long extension with L=0 even if it
is followed by repeated short extensions, as long as they have L=0. */
22,
- 25,
+ /* nb_ext = 11: don't use L=0 to skip a frame separator if repeats end
+ on a short L=0 extension if there was a preceding L=0 long
+ extension. */
+ 26,
/* nb_ext = 12: don't code the last repeated long extension with L=0 if
it is followed by repeated short extensions with L=1. */
- 26
+ 26,
+ /* nb_ext = 13: do use L=0 to skip a frame separator if repeats end on a
+ short L=1 extension */
+ 36
};
opus_int32 nb_ext;
for (nb_ext = 0; nb_ext <= NB_EXT; nb_ext++) {
@@ -474,7 +481,7 @@
opus_int32 nb_frame_exts[48];
opus_int32 nb_ext_out;
int len, result;
- unsigned char packet[32];
+ unsigned char packet[64];
len = opus_packet_extensions_generate(packet, sizeof(packet), ext,
nb_ext, 3, 0);
expect_true(len == encoded_len[nb_ext],
@@ -510,6 +517,14 @@
packet[15] = 0x3;
packet[16] = 0;
len += 2;
+ }
+ else if (nb_ext == 13) {
+ /* use L=0 to skip a frame separator if a repeat has no extensions at
+ all */
+ packet[18] = 2<<1|1;
+ memmove(packet+27, packet+26, len-26);
+ packet[26] = 2<<1|0;
+ len++;
}
else continue;
result = opus_packet_extensions_count_ext(packet, len, nb_frame_exts, 3);
--
⑨