shithub: opusfile

Info  •  Files  •  Log  •  Branches

Fix short-circuit test when seeking in short files

When a file is very, very short (i.e., only one packet) and uses
 end-trimming, the apparent granule position preceding the first
 sample in the first packet can underflow.
We were computing this value by subtracting the packet duration
 from the computed per-packet granule position and expecting this
 computation to always succeed.
Because it could fail in the presence of end-trimming on the first
 packet (ironically, exactly the situation where the short-circuit
 is helpful), it would leave the value uninitialized, and then use
 it in a comparison, which is undefined behavior.
The correct solution is to check for failure and force the previous
 page's granule position to 0 in this case.

--- a/src/opusfile.c

+++ b/src/opusfile.c

@@ -2358,8 +2358,19 @@

               For very small files (with all of the data in a single page,

                generally 1 second or less), we can loop them continuously

                without seeking at all.*/

-            OP_ALWAYS_TRUE(!op_granpos_add(&prev_page_gp,_of->op[0].granulepos,

-             -op_get_packet_duration(_of->op[0].packet,_of->op[0].bytes)));

+            if(op_granpos_add(&prev_page_gp,_of->op[0].granulepos,

+             -op_get_packet_duration(_of->op[0].packet,_of->op[0].bytes))<0) {

+              /*We validate/sanitize the per-packet timestamps, so the only way

+                 we should fail to calculate a granule position for the

+                 previous page is if the first page with completed packets in

+                 the stream is also the last, and end-trimming causes the

+                 apparent granule position preceding the first sample in the

+                 first packet to underflow.

+                The starting PCM offset is then 0 by spec mandate (see also:

+                 op_find_initial_pcm_offset()).*/

+              OP_ASSERT(_of->op[0].e_o_s);

+              prev_page_gp=0;

+            }

             if(op_granpos_cmp(prev_page_gp,_target_gp)<=0){

               /*Don't call op_decode_clear(), because it will dump our

                  packets.*/