[Vorbis-dev] Cover art

[Ian Malone]

2009/3/28 Conrad Parker <conrad at metadecks.org>:
> 2009/3/28 Ian Malone <ibmalone at gmail.com>:
>>
>> The "size parsing error" actually occurs in oggz's reading of the mode
>> section from the end of the decode setup packet and has nothing to do
>> with the cover art.  Packetno 2 ends like this:
>>
>>    0996: 00010100 00010110 00011000 00011010 00011100 00011110  ......
>>    099c: 00100000 00100010 00100100 00000000 00000000 00000000   "$...
>>    09a2: 00000000 00000000 00000000 00000000 00000000 00000000  ......
>>    09a8: 00000000 00000000 00000000 00000100                     ....
>>
>> It looks like the preceding mapping section ends with a sufficiently
>> long string of zeros that oggz_auto.c gets frustrated in attempting to
>> read back and until it finds non-zeros in the 32bits that would be the
>> window and transform blocks. This is liboggz-0.9.8, I see the warning
>> has been #ifdef-ed out in SVN; the size value is still used to read
>> mode blocks in.
>
> thanks for checking that out. Can you suggest a neater fix? Should
> oggz_auto just check further back in the packet for non-zero values?
>

The following works, if the size for the mode section doesn't match
the number counted back then allow going forward one block and trying
again. oggzinfo with and without this change produce the same results
on about 1500 vorbis files (though they're mostly encoded using the
same quality setting).

--- liboggz-0.9.8a/src/liboggz/oggz_auto.c	2008-06-29 06:38:54.000000000 +0100
+++ liboggz-0.9.8b/src/liboggz/oggz_auto.c	2009-03-28 15:04:57.000000000 +0000
@@ -660,6 +660,7 @@
 auto_calc_vorbis(ogg_int64_t now, oggz_stream_t *stream, ogg_packet *op) {

   auto_calc_vorbis_info_t *info;
+  int ii;

   if (stream->calculate_data == NULL) {
     /*
@@ -668,7 +669,7 @@
      */
     int short_size;
     int long_size;
-
+
     long_size = 1 << (op->packet[28] >> 4);
     short_size = 1 << (op->packet[28] & 0xF);

@@ -792,19 +793,32 @@

       }

-      if (offset > 4) {
-        size_check = (current_pos[0] >> (offset - 5)) & 0x3F;
-      } else {
-        /* mask part of byte from current_pos */
-        size_check = (current_pos[0] & ((1 << (offset + 1)) - 1));
-        /* shift to appropriate position */
-        size_check <<= (5 - offset);
-        /* or in part of byte from current_pos - 1 */
-        size_check |= (current_pos[-1] & ~((1 << (offset + 3)) - 1)) >>
-                (offset + 3);
-      }
+      /* Give ourselves a chance to recover if we went back too far by using
+       * the size check. */
+      for (ii=0; ii < 2; ii++) {
+	if (offset > 4) {
+	  size_check = (current_pos[0] >> (offset - 5)) & 0x3F;
+	} else {
+	  /* mask part of byte from current_pos */
+	  size_check = (current_pos[0] & ((1 << (offset + 1)) - 1));
+	  /* shift to appropriate position */
+	  size_check <<= (5 - offset);
+	  /* or in part of byte from current_pos - 1 */
+	  size_check |= (current_pos[-1] & ~((1 << (offset + 3)) - 1)) >>
+	    (offset + 3);
+	}

-      size_check += 1;
+	size_check += 1;
+	if (size_check == size) {
+	  break;
+	}
+        offset = (offset + 1) % 8;
+        if (offset == 0)
+          current_pos += 1;
+	current_pos += 5;
+	size -= 1;
+      }
+
       if (size_check != size)
       {
         printf("WARNING: size parsing failed for VORBIS mode packets\n");


-- 
imalone