Pull to refresh

Comments 27

8 сек. это слишком много. Мне кажется, у вас потоки борятся за ресурсы. Во время работы алгоритма, какая нагрузка на процессоры, случайно не 100%?
UFO landed and left these words here
125 кадров в секунду, т.е. не более 5 потоков осилит. GPU бы к такому привлечь.
UFO landed and left these words here
8 мс — это очень долго.
мы за это время успеваем FullHD кадр прогнать через несколько намного более тяжелых алгоритмов. (GPU)
UFO landed and left these words here
Эта статья — отличный пример неправильного использования многопоточности.
Вычислений почти нет, в основном работа с памтятью. Вот код, который на на 15% быстрее и выполняется в одном потоке:
void contrastFilter(unsigned char * imageData, size_t dataSize, int contrast) // contrast (256 - normal)
{
  unsigned char buf[256];
  unsigned int midBright = 0;
  for (size_t i = 0; i < dataSize; i += 3)
    midBright += imageData[i] * 77 + imageData[i + 1] * 150 + imageData[i + 2] * 29;
  midBright /= (256 * dataSize / 3);
  for (size_t i = 0; i < 256; i++)
  {
    int a = (((i - midBright) * contrast) >> 8) + midBright;
    if (a < 0) buf[i] = 0;
      else if (a > 255) buf[i] = 255;
        else buf[i] = a;
  }
  for (size_t i = 0; i < dataSize; i++)
    imageData[i] = buf[imageData[i]];
}
Ничего себе вы уместили. А можете показать ошибки автора? У меня недостаточно опыта, чтобы самому их найти, но очень интересно. И ещё вопрос — почему и у вас и у автора коэффициенты
midBright += imageData[i] * 77 + imageData[i + 1] * 150 + imageData[i + 2] * 29;
разные? Потому что глаз воспринимает по-разному? И последнее — в одном случае вы умножаете на 256
256 * dataSize
, а в другом делаете так
>> 8
. Не ухудшает ли это читаемость кода?
Основная ошибка автора — использование распараллеливания там, где это не нужно. Вычислений почти нет, обращений к памяти много. Основное время процессор будет ждать передачи дынных, на сколько потоков это не раскидывай — лучше не будет.

Вместо 1.0 максимальная яркость принята за 256, поэтому константы почти такие же как у автора.
77 / 256 ~= 0.3
150 / 256 ~= 0.586
29 / 256 ~= 0.113

По поводу умножения и сдвига на 8 (деления на 256): да, константы для операций с фиксированной точкой могут показаться странными.
UFO landed and left these words here
Да, так и есть. Я обычно пишу x / 4, а не x >> 2, т.к. всё же деление воспринимается более интуитивно, а компилятор сам решит как поступить. Просто о том, что значит / 4, мой мозг узнал в 1 классе, а про >> 2 — лет 6 назад, так что ему всё же быстрее понять / 4 — не выходит из кэша, а за >> 2 надо лезть в оперативку :)
ещё по поводу ошибок:

int b[L];
палитру лучше было сделать из (unsigned char)

lAB += (int)(valueR * 0.299 + valueG * 0.587 + valueB * 0.114);
при округлении в этом месте будет теряться много информации, например, если весь буфер будет заполнен синим цветом с яркостью 6, то переменная lAB, которая средняя яркость всех пикселей, будет равна 0.

ну и ещё много мелочей, вроде «return 0;» в функции которая возвращает void, кучи лишних приведений типов и ни о чём не говорящих названий переменных.

(у поста уже 17 плюсов, кто все эти люди? :) )
меня еще смущает вот эта строка:
int correction = _correction;
зачем?
Алгоритм распаралелен неправильно, но проблема здесь не с забиванием шины памяти. Даже на Sandy Bridge в идеально оптимизированном коде на одном потоке можно получить максимум 75% пропускной способности. А невекторизованным кодом забить шину памяти ещё сложнее. Проблема с кодом у автора поста в том, что разные потоки пишут в одни и те же строки кэша. Когда один из потоков хочет записать что-то в память, он проверяет, не закэширована ли эта строка в кэше другого процессора. Т.к. все потоки пишут в одни и те же строки кэша, скорее всего обнаружится, что эту строку кэша уже изменил другой поток. В результате будет отправлен запрос RFO (Request for ownership) к другому ядру процессора, чтобы оно прислало изменённую строку кэша. Попутно контроллер памяти отследит RFO запрос, и запишет изменённую строку кэша обратно в память (это нужно чтобы правильно работал MESI алгоритм). Всё это, естественно, очень не быстро, и главное, превращает код в по сути однопоточный.
я правильный вывод сделал, что если параллелить, то не каждый третий пиксель в другой поток, а лучше разбить кадр горизонтальными линиями на n частей/потоков?
Правильный. Только линии должны быть кратны размеру кэш-строки (можно считать, что 128 байт)
еще одна очевидная оптимизация первого цикла. +5% буст

	unsigned int midBright = 0, midBright1 = 0, midBright2 = 0;
	for (size_t i = 0; i < dataSize; )
	{
		midBright += imageData[i++];
		midBright1 += imageData[i++];
		midBright2 += imageData[i++];
	}
		midBright = (midBright * 77 + midBright1 * 150 + midBright2 * 29) / (256 * dataSize / 3);
кстати можно даже вернуть вычисления с двойной точностью без потерь производительности
Только сначала лучше померить, действительно ли это скомпилируется в более быстрый код. У меня было несколько идей по поводу второго цикла: пробовал производить вычисления вместо обращения к палитре, пробовал считать по 4 байта и писать сразу dword, пробовал разносить чтение и запись на 1024 байта, пробовал вручную разворачивать цикл, в итоге оставил самый простой вариант, как самый быстрый.
кстати в имеющемся виде 255 максимальное значение субпикселя. в кадре fullHD их 2млн штук каждого цвета. итого после финального умножения 32хбитный инт переполняется.

то бишь надо либо сначала делить, а потом умножать, но так погрешность выше.
либо long int надо брать, ну либо возвращаться к double, как у автора.
Не переполнится, я посчитал, перед тем как делать.
1920 * 1080 * 256 = 0x 1FA4 0000

Размер long int сейчас обычно такой же как и int, возможно, вы имели в виду long long int.
это число потом еще умножается на 150!

ЗЫ я имел ввиду int64
А да, не заметил сразу, что умножения вынесены за цикл. Да, так будет быстрее.
Данный алгоритм не учитывает нелинейность исходного цветового пространства. Так делать нельзя.
Only those users with full accounts are able to leave comments. Log in, please.