Re: Cover art

2009/3/29 Ian Malone <[email protected]>:
> 2009/3/28 Conrad Parker <[email protected]>:
>> 2009/3/28 Ian Malone <[email protected]>:
>>>
>>> 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");
>

thanks, applied in r3904

Conrad.