[xiph-commits] r3880 - liboggz/trunk/src/tools/oggz-chop

[conrad at svn.annodex.net]

Author: conrad
Date: 2009-03-16 21:19:03 -0700 (Mon, 16 Mar 2009)
New Revision: 3880

Modified:
   liboggz/trunk/src/tools/oggz-chop/oggz-chop.c
Log:
oggz-chop: verify that the earliest_new page written from a
page accumulator actually contains packets from the new GOP.
Previously we always included the last page with granulepos
from the previous GOP, in case it also contained the start of a
new packet. We now explicitly check if we can rule out this
possibility by remembering whether the successive page was
continued: we only include the earlier page if so.
This avoids adding preceding garbage where possible, ie. if
the keyframe starts a new page. This is uncommon with current
Theora encoders, but it is the normal (advised) case for Dirac.

Modified: liboggz/trunk/src/tools/oggz-chop/oggz-chop.c
===================================================================
--- liboggz/trunk/src/tools/oggz-chop/oggz-chop.c	2009-03-17 04:18:55 UTC (rev 3879)
+++ liboggz/trunk/src/tools/oggz-chop/oggz-chop.c	2009-03-17 04:19:03 UTC (rev 3880)
@@ -262,10 +262,17 @@
   int i, accum_size;
   int e, earliest_new; /* Index into page accumulator of the earliest page that
                         * contains a packet from the new GOP */
+  int succ_continued; /* Successor page is continued */
   OCPageAccum * pa;
 
   if (ts == NULL || ts->page_accum == NULL) return 0;
 
+  /* Upon entry, the next page is the one most recently read; we only get here
+   * if that page is continued, otherwise the accumulator is simply cleared,
+   * in read_gs() and read_dirac() below.
+   */
+  succ_continued = 1;
+
   earliest_new = accum_size = oggz_table_size (ts->page_accum);
 
   /* Working backwards through the page accumulator ... */
@@ -273,15 +280,30 @@
     pa = (OCPageAccum *) oggz_table_lookup (ts->page_accum, i);
 
     /* If we have a page with granulepos, it necessarily contains the end
-     * of a packet from an earlier GOP, and may contain the start of
-     * a packet from the new GOP. Thus it is the earliest page that we need
-     * to recover.
-     * We are working backwards, so we can break out when we have found it.
+     * of a packet from an earlier GOP, and may contain the start of a packet
+     * from the new GOP. If so, it is the earliest page to recover.
      */
     if (ogg_page_granulepos (pa->og) != -1) {
       earliest_new = i;
+
+      /* If the successor page was not continued, ie. it began a packet,
+       * then this page only contains packets from the previous GOP or earlier.
+       * In this case, we can be sure that the successor is the earliest_new
+       * page to use.
+       */
+      if (!succ_continued)
+        earliest_new++;
+
+      /* We are working backwards, so we can break out when we have found 
+       * the earliest page to recover.
+       */
       break;
     }
+
+    /* Update succ_continued flag; we are working backwards, so this page
+     * is the successor to the one we will consider on the next iteration.
+     */
+    succ_continued = ogg_page_continued(OGG_PAGE_CONST(pa->og));
   }
 
   /* If all accumulated pages have no granulepos, keep them,