[MPlayer-dev-eng] BUG: flac fails to decode under low memory conditions

[Michael Niedermayer]

Hi

On Thursday 03 February 2005 23:41, elupus wrote:
> Hi,
>
> To begin with, sorry for my poor excuse for mailing client. Wasn't too keen
> on switching to something else, just for this message :).
>
> Anyway found a bug in flac.c. Had been digging around qoute a couple of
> times to figure out why flac files would play fine in mplayer on win32,
> however not in our special version of it on the xbox. Sometimes it'd work
> but most of the time it would fail. Seems it's related to how much free
> memmory is available, and as we have very limited memory on the xbox it
> shows up. It works fine aslong as realloc returns the same pointer as it
> got, but if the memloacation moved, it will fail (which only happens when
> we have less memmory available.)
>
> In flac.c, function flac_decode_frame this code exists
>
>    if(1 && s->max_framesize){//FIXME truncated
>             buf_size= FFMIN(buf_size, s->max_framesize -
> s->bitstream_size); input_buf_size= buf_size;
>  .....
>             memcpy(&s->bitstream[s->bitstream_index + s->bitstream_size],
> buf, buf_size);
>             buf= &s->bitstream[s->bitstream_index];
>             buf_size += s->bitstream_size;
>             s->bitstream_size= buf_size;
>
>             if(buf_size < s->max_framesize){
>                 return input_buf_size;
>             }
>     }
>
>     init_get_bits(&s->gb, buf, buf_size*8);
>
> This is where s->gb is inited to have it's buffer point to the same mem
> area as buf, ie part of s->bitstream. Somewhat later in that function there
> is a loop which finds all header data.
>
>         do {
>             metadata_last = get_bits(&s->gb, 1);
>             metadata_type = get_bits(&s->gb, 7);
>             metadata_size = get_bits_long(&s->gb, 24);
>   .....
>             if(metadata_size){
>                 switch(metadata_type)
>                 {
>                 case METADATA_TYPE_STREAMINFO:
>                     metadata_streaminfo(s);
>                     dump_headers(s);
>                     break;
>                 default:
>                     for(i=0; i<metadata_size; i++)
>                         skip_bits(&s->gb, 8);
>                 }
>             }
>         } while(!metadata_last);
>
> When a stream info is found it calls the metadata_streaminfo function,
> which in the end calls allocate_buffers(s). This function will make this
> call.
>
>     s->bitstream= av_fast_realloc(s->bitstream,
> &s->allocated_bitstream_size, s->max_framesize);
>
> Which incidently happens to be the same memory area as s->gb points to. So
> aslong as realloc manages to get the same memory area back, all is fine,
> however if not, when we get back to the loop looking for header, the call
> metadata_last = get_bits(&s->gb, 1), will fail as s->gb has been
> deallocated be realloc. Not entirly sure how to fix this as I don't entirly
> understand the code, but i suppose it probably just be to reinit s->gb to
> the new memlocation.

fixed, please test

[...]
-- 
Michael

"nothing is evil in the beginning. Even Sauron was not so." -- Elrond