thread safety

[GoogleCodeExporter]

I cannot find any clarification of webp thread safety.
Unfortunately I found some global variables which make it quastionable:

For example WebPInitAlphaProcessing overwrites function pointers first with 
generic functions, then with optimized functions.

Please move these variables to encoder\decoder objects or allow to initialize 
them once before using the library.

Original issue reported on code.google.com by shekh.an...@gmail.com on 19 Dec 2014 at 12:39

[GoogleCodeExporter]

in normal operation mode[*], these functions are always initialized with the 
same values, based solely on the CPU detected.
Hence, thread safety is guarantied. Having a per-object pointer-to-function set 
would mean an extra call indirection (as opposed to using the global 
variables), which we try to avoid.

[*] that is, if you don't override VP8GetCPUInfo, which is handy when testing 
but not advertised otherwise.

Original comment by pascal.m...@gmail.com on 19 Dec 2014 at 4:01

[GoogleCodeExporter]

The code is performed this way:

WebPMultRow = WebPMultRowC;
if (VP8GetCPUInfo(kSSE2)) {
  WebPMultRow = MultRowSSE;
}

This may cause another thread to execute random function: either C or SSE 
version.

I think if the code is changed like below then the problem will be removed:

type MultRow = WebPMultRowC;
if (VP8GetCPUInfo(kSSE2)) {
  MultRow = MultRowSSE;
}

WebPMultRow = MultRow;  // <- writes the same value always 

Original comment by shekh.an...@gmail.com on 19 Dec 2014 at 4:31

[GoogleCodeExporter]

this is true, tables are minimally protected, but these are allowed to 
reinitialize between runs.

Original comment by jz...@google.com on 19 Dec 2014 at 7:33

[GoogleCodeExporter]

#2 : oh, i see what you mean now. As written the functions are transiently 
reset to the default C implement during a short time window, potentially 
impacting other in-flight threads (by making them slower than necessary, since 
the output will still be the same).
Your code solution is correct, but will require quite some massaging of the 
code.

Original comment by pascal.m...@gmail.com on 19 Dec 2014 at 10:54

[GoogleCodeExporter]

patch https://gerrit.chromium.org/gerrit/#/c/73369/ was merged, which should 
address the issue. Now, the platform-dependent implementations are changed only 
if VP8GetCPUInfo was somehow modified in-between calls.

thanks for the report!

Original comment by pascal.m...@gmail.com on 7 Jan 2015 at 12:44

• Changed state: Fixed