[FFmpeg-devel] [PATCH] AAC Decoder - Round 2.

[Robert Swain]

2008/6/23 Michael Niedermayer <michaelni at gmx.at>:
> On Sat, Jun 21, 2008 at 06:30:55PM +0100, Robert Swain wrote:
>> 2008/6/20 Michael Niedermayer <michaelni at gmx.at>:
>> > On Thu, Jun 19, 2008 at 04:22:57PM +0100, Robert Swain wrote:
>> >> +    for (i = 0; i < 128; i++) {
>> >> +        sine_short_128[i] *= 8.;
>> >> +        kbd_short_128[i] *= 8.;
>> >> +    }
>> >
>> > not thread safe, you cannot store wrong values in static tables and then
>> > correct them.
>>
>> OK. I'm not really familiar with thread safety as I haven't written
>> any threaded code myself to date or read about writing threaded code,
>> but I think I understand.
>>
>> To clarify, the issue is that the values of the tables are reassigned
>> with different values meaning that at different times after
>> initialisation, the table values change. Separate threads using these
>> tables then have the potential to use incorrect values. So, such
>> alterations must be done either before assignment to the table within
>> the initialisation functions or when using the tables in the code. Is
>> that correct?
>
> Iam not completely sure what you describe but one possible thread saftey
> issue is that
> Thread1 init sine_table, multiply sine_table by 8, decode frame1
> Thread2                                           init sine_table
>
> Thread2 here sets sine_table to "invalid" values while the table is
> used by another thread leading to wrong output from thread1

That's the kind of thing I was thinking of, yes.

>> As this alteration is specific to the use of eight short windows (I
>> think) I'm inclined to not hack it into the init functions but rather
>> multiply by 8.0 at some other appropriate point in the code. Would you
>> agree?
>
> yes

Fixed now in current SoC. :)

Rob