From 9b57b0c248709eba740d7e768d59ec7251009184 Mon Sep 17 00:00:00 2001 From: "Timothy B. Terriberry" <tterribe@xiph.org> Date: Sat, 22 Sep 2012 13:37:38 -0700 Subject: [PATCH] Fix some issues with trailing junk in files. 1) We were treating EOF in op_get_next_page() as a read error when called from op_get_prev_page_serial(). 2) We also assumed op_get_prev_page_serial() stopped scanning at the end of the page it returned, in order to compute the size of that page. Return the page size explicitly instead. 3) Finally, once we discover where the last page is, there is no reason to ever look at data past it. Update 'end' once we find it, and always pass that to op_get_next_page(). --- src/opusfile.c | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/src/opusfile.c b/src/opusfile.c index b224fc5..7886897 100644 --- a/src/opusfile.c +++ b/src/opusfile.c @@ -183,7 +183,15 @@ static opus_int64 op_get_next_page(OggOpusFile *_of,ogg_page *_og, /*Send more paramedics.*/ if(!_boundary)return OP_FALSE; ret=op_get_data(_of); - if(OP_UNLIKELY(ret<0))return ret; + if(OP_UNLIKELY(ret<0)){ + opus_int64 read_offset; + /*If we read up to the boundary (or EOF in a seekable stream), + including buffered sync data, then treat this as EOF. + Otherwise treat it as a read error.*/ + if(_boundary<0)_boundary=_of->end; + read_offset=_of->offset+_of->oy.fill-_of->oy.returned; + return read_offset>=_boundary?OP_FALSE:ret; + } } else{ /*Got a page. @@ -251,7 +259,7 @@ static int op_lookup_page_serialno(ogg_page *_og, start of the stream.*/ static opus_int64 op_get_prev_page_serial(OggOpusFile *_of, const ogg_uint32_t *_serialnos,int _nserialnos,opus_int32 *_chunk_size, - ogg_uint32_t *_serialno,ogg_int64_t *_gp){ + ogg_uint32_t *_serialno,opus_int32 *_page_size,ogg_int64_t *_gp){ ogg_page og; opus_int64 begin; opus_int64 end; @@ -260,6 +268,7 @@ static opus_int64 op_get_prev_page_serial(OggOpusFile *_of, opus_int64 preferred_offset; ogg_uint32_t preferred_serialno; ogg_int64_t ret_serialno; + ogg_int64_t ret_size; ogg_int64_t ret_gp; opus_int32 chunk_size; original_end=end=begin=_of->offset; @@ -282,8 +291,12 @@ static opus_int64 op_get_prev_page_serial(OggOpusFile *_of, ret_serialno=ogg_page_serialno(&og); ret_gp=ogg_page_granulepos(&og); offset=llret; + OP_ASSERT(_of->offset-offset>=0); + OP_ASSERT(_of->offset-offset<=OP_PAGE_SIZE); + ret_size=(opus_int32)(_of->offset-offset); if(ret_serialno==preferred_serialno){ preferred_offset=offset; + *_page_size=ret_size; *_gp=ret_gp; } if(!op_lookup_serialno(ret_serialno,_serialnos,_nserialnos)){ @@ -306,9 +319,10 @@ static opus_int64 op_get_prev_page_serial(OggOpusFile *_of, while(offset==-1); if(_chunk_size!=NULL)*_chunk_size=chunk_size; /*We're not interested in the page... just the serial number, byte offset, - and granule position.*/ + page size, and granule position.*/ if(preferred_offset>=0)return preferred_offset; *_serialno=ret_serialno; + *_page_size=ret_size; *_gp=ret_gp; return offset; } @@ -672,7 +686,7 @@ static int op_find_initial_pcm_offset(OggOpusFile *_of, do{ /*We should get a page unless the file is truncated or mangled. Otherwise there are no audio data packets in the whole logical stream.*/ - if(OP_UNLIKELY(op_get_next_page(_of,_og,-1)<0)){ + if(OP_UNLIKELY(op_get_next_page(_of,_og,_of->end)<0)){ /*Fail if the pre-skip is non-zero, since it's asking us to skip more samples than exist.*/ if(_link->head.pre_skip>0)return OP_EBADTIMESTAMP; @@ -805,10 +819,11 @@ static int op_find_final_pcm_offset(OggOpusFile *_of, chunk_size=OP_CHUNK_SIZE; offset=_of->offset; while(_end_gp==-1||test_serialno!=cur_serialno){ + opus_int32 page_size; test_serialno=cur_serialno; _of->offset=offset; offset=op_get_prev_page_serial(_of,_serialnos,_nserialnos, - &chunk_size,&test_serialno,&_end_gp); + &chunk_size,&test_serialno,&page_size,&_end_gp); if(OP_UNLIKELY(offset<0))return (int)offset; } /*This implementation requires that difference between the first and last @@ -915,7 +930,7 @@ static int op_bisect_forward_serialno(OggOpusFile *_of, ret=op_seek_helper(_of,bisect); if(OP_UNLIKELY(ret<0))return ret; } - last=op_get_next_page(_of,&og,-1); + last=op_get_next_page(_of,&og,_of->end); /*At the worst we should have hit the page at _sr[sri-1].offset.*/ if(OP_UNLIKELY(last<0))return OP_EBADLINK; OP_ASSERT(nsr<_csr); @@ -1053,12 +1068,11 @@ static int op_open_seekable2(OggOpusFile *_of){ contain a single logical bitstream.*/ sr[0].serialno=_of->links[0].serialno; sr[0].offset=op_get_prev_page_serial(_of,_of->serialnos,_of->nserialnos, - NULL,&sr[0].serialno,&end_gp); + NULL,&sr[0].serialno,&sr[0].size,&end_gp); if(OP_UNLIKELY(sr[0].offset<0))return (int)sr[0].offset; + /*If there's any trailing junk, forget about it.*/ + _of->end=sr[0].offset+sr[0].size; /*Now enumerate the bitstream structure.*/ - OP_ASSERT(_of->offset-sr[0].offset>=0); - OP_ASSERT(_of->offset-sr[0].offset<=OP_PAGE_SIZE); - sr[0].size=(opus_int32)(_of->offset-sr[0].offset); data_offset=_of->links[0].data_offset; ret=op_bisect_forward_serialno(_of,data_offset,end_gp,sr, sizeof(sr)/sizeof(*sr),&_of->serialnos,&_of->nserialnos,&_of->cserialnos); @@ -1110,6 +1124,7 @@ static int op_open1(OggOpusFile *_of, int seekable; int ret; memset(_of,0,sizeof(*_of)); + _of->end=-1; _of->source=_source; *&_of->callbacks=*_cb; /*At a minimum, we need to be able to read data.*/ @@ -1487,7 +1502,7 @@ static int op_fetch_and_process_page(OggOpusFile *_of, bitstream.*/ do{ /*Keep reading until we get a page with the correct serialno.*/ - page_pos=op_get_next_page(_of,&og,-1); + page_pos=op_get_next_page(_of,&og,_of->end); /*EOF: Leave uninitialized.*/ if(page_pos<0)return OP_EOF; if(OP_LIKELY(_of->ready_state>=OP_STREAMSET)){ -- GitLab