6

I am working on a multithreaded median function as part of a larger project. I have little C++ experience. The median function below should take a vector of 3 dimensional int vectors, and return a 3 dimensional vector of ints where each entry is the median value of all the entries in that index in the input vectors. So if the input is <<3,2,1>,<1,2,3>,<2,2,2>>, the return in <2,2,2>. This code will be used in the implementation of a median blur for use on real-time video, hence the desire to multithread it.

#include <thread>
#include <iostream>
#include <mutex>
#include <vector>
#include <algorithm>
#include "median.h"

// mutex to protect bgrPixel (possibly not needed)
std::mutex mtx;


std::vector<int> median(const std::vector<std::vector<int> >& input)
{
    std::vector<int> bgrPixel;              // Vector to store median BGR value
    std::thread first(thread_function, bgrPixel, input, 0); // thread for each colour channel
    std::thread second(thread_function, bgrPixel, input, 1);
    std::thread third(thread_function, bgrPixel, input, 2);
    first.join();
    second.join();
    third.join(); 
    return bgrPixel;
}

void thread_function(std::vector<int>& bgrPixel, const std::vector<std::vector<int> >&                 input1, int channel)
{

    std::vector<int> input = input1[channel];  // copy the colour channel
    std::sort(input.begin(), input.end());
    int size = input.size();
    if (size %2 == 0)   // get the median
    {
        mtx.lock();
        bgrPixel[channel] = (input[size/2] + input[size/2 + 1])/2;
        mtx.unlock();
    } else
    {
        mtx.lock();
        bgrPixel[channel] = input[(size-1)/2];
        mtx.unlock();
    }
}

The problem I am having is, at compile time, g++ (and clang also) gives a fairly unintelligible error:

 g++ -std=c++11 -pthread -o median median.cpp

 In file included from /usr/include/c++/4.8.2/thread:39:0,
                  from median.cpp:1:
 /usr/include/c++/4.8.2/functional: In instantiation of ‘struct std::_Bind_simple<void           (*(std::vector<int>, std::vector<std::vector<int> >, int))(std::vector<int>&, const      std::vector<std::vector<int> >&, int)>’:
 /usr/include/c++/4.8.2/thread:137:47:   required from ‘std::thread::thread(_Callable&&,           _Args&& ...) [with _Callable = void (&)(std::vector<int>&, const      std::vector<std::vector<int> >&, int); _Args = {std::vector<int, std::allocator<int> >&,      const std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int,      std::allocator<int> > > >&, int}]’
 median.cpp:15:58:   required from here
 /usr/include/c++/4.8.2/functional:1697:61: error: no type named ‘type’ in ‘class                std::result_of<void (*(std::vector<int>, std::vector<std::vector<int> >, int))     (std::vector<int>&, const std::vector<std::vector<int> >&, int)>’
        typedef typename result_of<_Callable(_Args...)>::type result_type;
                                                         ^
 /usr/include/c++/4.8.2/functional:1727:9: error: no type named ‘type’ in ‘class      std::result_of<void (*(std::vector<int>, std::vector<std::vector<int> >, int))     (std::vector<int>&, const std::vector<std::vector<int> >&, int)>’
          _M_invoke(_Index_tuple<_Indices...>)
          ^

I have found a similar error message c++11 Thread class how to use a class member function, but it does not deal specifically with my problem. Any help would be much appreciated, I fully expect this is because I don't know what I am doing :P

EDIT: Prototypes for thread_function and median are included from the header file median.h.

Community
  • 1
  • 1
chaffdog
  • 163
  • 1
  • 1
  • 5
  • 1
    When you get this to compile you have a more devious problem: Your `bgrPixel` vector does not contain three items, so when you do `bgrPixel[channel] = ...` you have [*undefined behavior*](http://en.wikipedia.org/wiki/Undefined_behavior). This can easily be solved by saying the vector have three items at the declaration: `std::vector bgrPixel(3);` – Some programmer dude Apr 24 '14 at 11:18
  • 3
    Since your vectors have the size 3, you can replace `std::vector` with `std::array`. That would be less error prone. I don't have a g++ compiler, but it works well with VC++. Probably something wrong with your compiler. – Marius Bancila Apr 24 '14 at 11:20
  • You don't need the mutex to protect against simultaneous access to `bgrPixel`: each of the threads accesses a distinct element - `channel` is distinct per-thread - and accesses to distinct container elements are non-conflicting. – Casey Apr 24 '14 at 18:14

1 Answers1

16

Replace

std::thread first(thread_function, bgrPixel, input, 0);

by

std::thread first(thread_function, std::ref(bgrPixel), std::ref(input), 0);

Live example: http://coliru.stacked-crooked.com/a/630775aafc3d4642

Jarod42
  • 203,559
  • 14
  • 181
  • 302
  • 1
    Thanks! That works perfectly! I also discovered that it will compile if I don't use references at all. I need to read up on references. – chaffdog Apr 24 '14 at 12:09
  • Tested and it seems like when the variable is considered to be unnecessary by the main thread it might get deleted and you might read some "random" info instead of what you expect. So I guess it's safer to not pass by reference if you don't really need to. – Íhor Mé Mar 04 '20 at 15:32