Реализация RGB-алгоритма изменения контраста изображения

[int_index]

Простите, не удержался.

[sergei-grigorev]

8 сек. это слишком много. Мне кажется, у вас потоки борятся за ресурсы. Во время работы алгоритма, какая нагрузка на процессоры, случайно не 100%?

[sergei-grigorev]

Упс, действительно (:

[AxisPod]

125 кадров в секунду, т.е. не более 5 потоков осилит. GPU бы к такому привлечь.

[stmuxa]

8 мс — это очень долго.
мы за это время успеваем FullHD кадр прогнать через несколько намного более тяжелых алгоритмов. (GPU)

[Imp5]

Эта статья — отличный пример неправильного использования многопоточности.
Вычислений почти нет, в основном работа с памтятью. Вот код, который на на 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]];
}

[iago]

Ничего себе вы уместили. А можете показать ошибки автора? У меня недостаточно опыта, чтобы самому их найти, но очень интересно. И ещё вопрос — почему и у вас и у автора коэффициенты

midBright += imageData[i] * 77 + imageData[i + 1] * 150 + imageData[i + 2] * 29;

разные? Потому что глаз воспринимает по-разному? И последнее — в одном случае вы умножаете на 256

256 * dataSize

, а в другом делаете так

>> 8

. Не ухудшает ли это читаемость кода?

[Imp5]

Основная ошибка автора — использование распараллеливания там, где это не нужно. Вычислений почти нет, обращений к памяти много. Основное время процессор будет ждать передачи дынных, на сколько потоков это не раскидывай — лучше не будет.

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

По поводу умножения и сдвига на 8 (деления на 256): да, константы для операций с фиксированной точкой могут показаться странными.

[iago]

Да, так и есть. Я обычно пишу x / 4, а не x >> 2, т.к. всё же деление воспринимается более интуитивно, а компилятор сам решит как поступить. Просто о том, что значит / 4, мой мозг узнал в 1 классе, а про >> 2 — лет 6 назад, так что ему всё же быстрее понять / 4 — не выходит из кэша, а за >> 2 надо лезть в оперативку :)

[Imp5]

ещё по поводу ошибок:

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

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

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

(у поста уже 17 плюсов, кто все эти люди? :) )

[stmuxa]

меня еще смущает вот эта строка:
int correction = _correction;
зачем?

[Maratyszcza]

Алгоритм распаралелен неправильно, но проблема здесь не с забиванием шины памяти. Даже на Sandy Bridge в идеально оптимизированном коде на одном потоке можно получить максимум 75% пропускной способности. А невекторизованным кодом забить шину памяти ещё сложнее. Проблема с кодом у автора поста в том, что разные потоки пишут в одни и те же строки кэша. Когда один из потоков хочет записать что-то в память, он проверяет, не закэширована ли эта строка в кэше другого процессора. Т.к. все потоки пишут в одни и те же строки кэша, скорее всего обнаружится, что эту строку кэша уже изменил другой поток. В результате будет отправлен запрос RFO (Request for ownership) к другому ядру процессора, чтобы оно прислало изменённую строку кэша. Попутно контроллер памяти отследит RFO запрос, и запишет изменённую строку кэша обратно в память (это нужно чтобы правильно работал MESI алгоритм). Всё это, естественно, очень не быстро, и главное, превращает код в по сути однопоточный.

[B08AH]

я правильный вывод сделал, что если параллелить, то не каждый третий пиксель в другой поток, а лучше разбить кадр горизонтальными линиями на n частей/потоков?

[Maratyszcza]

Правильный. Только линии должны быть кратны размеру кэш-строки (можно считать, что 128 байт)

[B08AH]

еще одна очевидная оптимизация первого цикла. +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);

[B08AH]

кстати можно даже вернуть вычисления с двойной точностью без потерь производительности

[Imp5]

Только сначала лучше померить, действительно ли это скомпилируется в более быстрый код. У меня было несколько идей по поводу второго цикла: пробовал производить вычисления вместо обращения к палитре, пробовал считать по 4 байта и писать сразу dword, пробовал разносить чтение и запись на 1024 байта, пробовал вручную разворачивать цикл, в итоге оставил самый простой вариант, как самый быстрый.

[B08AH]

кстати в имеющемся виде 255 максимальное значение субпикселя. в кадре fullHD их 2млн штук каждого цвета. итого после финального умножения 32хбитный инт переполняется.

то бишь надо либо сначала делить, а потом умножать, но так погрешность выше.
либо long int надо брать, ну либо возвращаться к double, как у автора.

[Imp5]

Не переполнится, я посчитал, перед тем как делать.
1920 * 1080 * 256 = 0x 1FA4 0000

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

[B08AH]

это число потом еще умножается на 150!

ЗЫ я имел ввиду int64

[Imp5]

а да, точно, спасибо :)
тогда uint64_t

[Imp5]

А да, не заметил сразу, что умножения вынесены за цикл. Да, так будет быстрее.

[SiGMan]

Данный алгоритм не учитывает нелинейность исходного цветового пространства. Так делать нельзя.