shithub: opus

Download patch

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);
--