user2563661
user2563661

Reputation: 334

jpeg_write_scanlines and glTexImage2D thread safety. Why doesn't this crash?

I am working on a video software and use some existing code. The existing code includes a circular buffer. As a producer I have a video camera and as consumers two different threads. One, GLThread, uses OpenGL to draw the frame and the other one, VideoCompressorThread, compresses the frame to jpeg format to save it to a video file. What is strange is that currently both of these threads work on the same data at the same time, but this doesn't produce a race condition. In the GLThread I have:

while(!shouldStop) {
        mutex_.lock();
        glw_->makeCurrent();

        shaderProgram_.bind();
        shaderProgram_.setUniformValue("texture", 0);
        shaderProgram_.setAttributeArray("vertex", vertices_.constData());
        shaderProgram_.enableAttributeArray("vertex");
        shaderProgram_.setAttributeArray("textureCoordinate", textureCoordinates_.constData());
        shaderProgram_.enableAttributeArray("textureCoordinate");

        qDebug() << "GLThread: " << "data address: " << static_cast<void*>(imBuf_)  << "time: " << QDateTime::currentMSecsSinceEpoch();
        glTexImage2D(GL_TEXTURE_2D, 0, GL_RGB8, VIDEO_WIDTH, VIDEO_HEIGHT, 0, GL_RGB, GL_UNSIGNED_BYTE, (GLubyte*)imBuf_);
        qDebug() << "GLThread finished";

        glClear(GL_COLOR_BUFFER_BIT);
        glDrawArrays(GL_TRIANGLES, 0, 6);
        glw_->swapBuffers();

        shaderProgram_.disableAttributeArray("vertex");
        shaderProgram_.disableAttributeArray("textureCoordinate");
        shaderProgram_.release();

        glw_->doneCurrent();
        mutex_.unlock();
}

and in the VideoCompressorThread:

while(!shouldStop)
{
    // JPEG-related stuff
    struct jpeg_compress_struct cinfo;
    struct jpeg_error_mgr       jerr;
    JSAMPROW                    row_pointer;
    unsigned char*              jpgBuf=NULL;
    unsigned long               jpgBufLen=0;

    unsigned char*              data;
    ChunkAttrib                 chunkAttrib;

    // Get raw image from the input buffer
    data = inpBuf->getChunk(&chunkAttrib);

    // Initialize JPEG
    cinfo.err = jpeg_std_error(&jerr);
    jpeg_create_compress(&cinfo);
    jpeg_mem_dest(&cinfo, &jpgBuf, &jpgBufLen);

    // Set the parameters of the output file
    cinfo.image_width = VIDEO_WIDTH;
    cinfo.image_height = VIDEO_HEIGHT;
    cinfo.input_components = 3;
    cinfo.in_color_space = JCS_RGB;

    // Use default compression parameters
    jpeg_set_defaults(&cinfo);
    jpeg_set_quality(&cinfo, jpgQuality, TRUE);

    // Do the compression
    jpeg_start_compress(&cinfo, TRUE);

    // write one row at a time
    qDebug() << "VideoCompressorThread: " << "data address: " << static_cast<void*>(data) << "time: " << QDateTime::currentMSecsSinceEpoch();
    while(cinfo.next_scanline < cinfo.image_height)
    {
        row_pointer = (data + (cinfo.next_scanline * cinfo.image_width * 3));
        jpeg_write_scanlines(&cinfo, &row_pointer, 1);
    }
    qDebug() << "VideoCompressorThread finished";

    // clean up after we're done compressing
    jpeg_finish_compress(&cinfo);


    // Insert compressed image into the output buffer
    chunkAttrib.chunkSize = jpgBufLen;
    outBuf->insertChunk(jpgBuf, chunkAttrib);

    // The output buffer needs to be explicitly freed by the libjpeg client
    free(jpgBuf);
    jpeg_destroy_compress(&cinfo);
}

As an output I get:

VideoCompressorThread:  data address:  0x7fffbdcd1060 time:  1438594694479 
VideoCompressorThread finished 
GLThread:  data address:  0x7fffbdcd1060 time:  1438594694488 
GLThread finished 
GLThread:  data address:  0x7fffbddb20b0 time:  1438594694497 
GLThread finished 
VideoCompressorThread:  data address:  0x7fffbddb20b0 time:  1438594694498 
VideoCompressorThread finished 
VideoCompressorThread:  data address:  0x7fffbde93100 time:  1438594694521 
GLThread:  data address:  0x7fffbde93100 time:  1438594694521 
GLThread finished 
VideoCompressorThread finished 
VideoCompressorThread:  data address:  0x7fffbdf74150 time:  1438594694538 
GLThread:  data address:  0x7fffbdf74150 time:  1438594694538 
GLThread finished 
VideoCompressorThread finished 
VideoCompressorThread:  data address:  0x7fffbe0551a0 time:  1438594694555 
GLThread:  data address:  0x7fffbe0551a0 time:  1438594694555 
GLThread finished 
VideoCompressorThread finished 
VideoCompressorThread:  data address:  0x7fffbe1361f0 time:  1438594694571 
GLThread:  data address:  0x7fffbe1361f0 time:  1438594694571 
GLThread finished 
VideoCompressorThread finished 
VideoCompressorThread:  data address:  0x7fffbe217240 time:  1438594694588 
GLThread:  data address:  0x7fffbe217240 time:  1438594694588 
GLThread finished 
VideoCompressorThread finished 
VideoCompressorThread:  data address:  0x7fffbe2f8290 time:  1438594694604 
GLThread:  data address:  0x7fffbe2f8290 time:  1438594694604 
GLThread finished 
VideoCompressorThread finished 

As you can see, at times both threads access the same data at the same time but there is no crash. Is this pure luck, or is there something I don't understand here? I am using Xubuntu 14.04 if that makes any difference.

Edit. insertChunk and getChunk() funtions. Notice that only VideoCompressorThread gets the data pointer with getChunk(). GLThread is connected to the chunkReady qt signal. This enables the use of one primary and multiple secondary consumers for the buffer.

void CycDataBuffer::insertChunk(unsigned char* _data, ChunkAttrib &_attrib)
{

    // Check for buffer overflow. CIRC_BUF_MARG is the safety margin against
    // race condition between consumer and producer threads when the buffer
    // is close to full.
    if (buffSemaphore->available() >=  bufSize * (1-CIRC_BUF_MARG))
    {
        cerr << "Circular buffer overflow!" << endl;
        abort();
    }

    // Make sure that the safety margin is at least several (four) times the
    // chunk size. This is necessary to prevent the race condition between
    // consumer and producer threads when the buffer is close to full.
    if(_attrib.chunkSize+sizeof(ChunkAttrib)+MAXLOG > bufSize*MAX_CHUNK_SIZE)
    {
        cerr << "The chunk size is too large!" << endl;
        abort();
    }

    // insert the data into the circular buffer
    _attrib.isRec = isRec;

    memcpy(dataBuf + insertPtr, (unsigned char*)(&_attrib), sizeof(ChunkAttrib));
    insertPtr += sizeof(ChunkAttrib);
    buffSemaphore->release(sizeof(ChunkAttrib));

    memcpy(dataBuf + insertPtr, _data, _attrib.chunkSize);
    buffSemaphore->release(_attrib.chunkSize);

    emit chunkReady(dataBuf + insertPtr);
    insertPtr += _attrib.chunkSize;
    if(insertPtr >= bufSize)
    {
        insertPtr = 0;
    }
}

unsigned char* CycDataBuffer::getChunk(ChunkAttrib* _attrib)
{
    unsigned char* res;

    buffSemaphore->acquire(sizeof(ChunkAttrib));
    memcpy((unsigned char*)_attrib, dataBuf + getPtr, sizeof(ChunkAttrib));
    getPtr += sizeof(ChunkAttrib);

    buffSemaphore->acquire(_attrib->chunkSize);
    res = dataBuf + getPtr;

    getPtr += _attrib->chunkSize;
    if(getPtr >= bufSize)
    {
        getPtr = 0;
    }

    return(res);
}

Upvotes: 1

Views: 382

Answers (2)

derhass
derhass

Reputation: 45352

Besides ratchet freak's fine answer "Just because it doesn't crash doesn't mean it's not a bug" below, I'd like to add that I actually don't see a reason why these two particular pieces of code could not work in parallel. Both access the same image data read only, which is perfectly fine.

Issues with concurrent acces to your buffers will only arise if you have at least two threads (or processes, in case of shared memory) accessing the same buffer and at least one of them is modifying it, i.e. by overwriting the data, or by de-allocating the buffer.

Upvotes: 1

ratchet freak
ratchet freak

Reputation: 48216

Just because it doesn't crash doesn't mean it's not a bug. Writing to a buffer while another thread is reading is will typically result in data corruption as read by the reading thread. With some bytes reading as the new value and some being the old.

One of the things that you would see happening is that portions of the image buffer are overwritten while the other thread is working on it which would result in screen-tearing when watching the video. You can see this best with diagonal stripes moving rapidly across the screen.

2 threads reading the same buffer is perfectly fine, it's when one starts writing to it that the problems begin.

Upvotes: 2

Related Questions