Segfault with smem module

[wetneb]

Yesterday evening on #videolan I talked about a segfault I get with the smem module.

The normal behaviour should be :
1. The prerender callback allocates a buffer with the right size
2. Smem uses memcpy to fill the buffer
3. The postrender callback uses the buffer (and deletes it, if needed)

My code is :

Code: Select all

void StreamPlayer::prepareRender( void* p_audio_data, uint8_t** pp_pcm_buffer , unsigned int size ) { cout << "Prerender" << endl; cout << " pp_pcm_buffer = "; cout << pp_pcm_buffer << endl; cout << " *pp_pcm_buffer = " << flush; cout << *pp_pcm_buffer << endl; // Here I get a segfault. Why ? // This pointer has been passed from another thread, from SendAudio in smem. // I should be able to access to it since I'm supposed to modify it ! (*pp_pcm_buffer) = new uint8_t[size]; // Dummy behaviour : each time, we delete and reallocate, which should take a while cout << "Prerender ended." << endl << endl; } void StreamPlayer::handleStream(void* p_audio_data, uint8_t* p_pcm_buffer, unsigned int channels, unsigned int rate, unsigned int nb_samples, unsigned int bits_per_sample, unsigned int size, int64_t pts ) { cout << "Postrender" << endl; delete p_pcm_buffer; cout << "Postrender ended." << endl << endl; }

Does anybody wrote a code using this module, which runs correctly ?

Thanks

[Jean-Baptiste Kempf]

vlmc does

[Rémi Denis-Courmont]

Your code is patently broken. VLC does not initialize the data pointer, since that's the very job of the prerender function. In other words, pp_pcm_buffer is valid (it should point a few byte up the stack), but *pp_pcm_buffer is garbage.

cout << *pp_pcm_buffer << endl; tries to print a string contained at *pp_pcm_buffer, i.e. it will read **pp_pcm_buffer. Since this is a garbage address, it is likely to segfault.

[wetneb]

*pp_pcm_buffer should have the same value as p_pcm_buffer in the SendAudio function, since pp_pcm_buffer has been initialized with &p_pcm_buffer.
So I should be able to read it.

If I write this code, everything works :

uint16_t *test;
uint16_t **pp = &test;
cout << pp << endl;
cout << *pp << endl;

Or let's remove this line. I still get a segfault when I allocate my memory (in the following line).

[wetneb]

Here is the code used by VLMC :

Code: Select all

void VideoClipWorkflow::lock( VideoClipWorkflow *cw, void **pp_ret, int size ) { Q_UNUSED( size ); LightVideoFrame* lvf = NULL; cw->m_renderLock->lock(); if ( cw->m_availableBuffers.isEmpty() == true ) { lvf = new LightVideoFrame( cw->m_width, cw->m_height ); } else lvf = cw->m_availableBuffers.dequeue(); cw->m_computedBuffers.enqueue( lvf ); *pp_ret = (*(lvf))->frame.octets; } void VideoClipWorkflow::unlock( VideoClipWorkflow *cw, void *buffer, int width, int height, int bpp, int size, qint64 pts ) { Q_UNUSED( buffer ); Q_UNUSED( width ); Q_UNUSED( height ); Q_UNUSED( bpp ); Q_UNUSED( size ); cw->computePtsDiff( pts ); LightVideoFrame *lvf = cw->m_computedBuffers.last(); (*(lvf))->ptsDiff = cw->m_currentPts - cw->m_previousPts; cw->commonUnlock(); cw->m_renderWaitCond->wakeAll(); cw->m_renderLock->unlock(); }

Maybe I need to use mutexes ? Currently, my code doesn't access to all this data, except from the lock and unlock functions, so I don't see why my code would be responsible for this segfault…

[Rémi Denis-Courmont]

I think you're not getting the point. Your code is basically doing this:

Code: Select all

uint8_t *p; // uninitialized uint8_t **pp = &p; // in VLC smem plugin cout << *pp << endl; // in your callback

which is equivalent to this:

Code: Select all

unsigned char *p; // uninitialized cout << p << endl;

but is completely different from this:

Code: Select all

unsigned char *p; // uninitialized cout << (void *)p << endl;

All code snippets are undefined, as they use an undefined value. But the first two snippets try to print a nul-terminated string at the undefined location "p". This expectedly results in a segmentation fault. The third snippet just prints a pointer value, which should result in an hexadecimal number.

[wetneb]

I didn't think that the type of the pointer could be the problem. But now if i add this cast (void*), it doesn't work better.

The first two snippets you gave run well here (actually, the addresses aren't displayed correctly, but there's no segfault at all).

[Rémi Denis-Courmont]

The VLC code is correct, otherwise VLMC could not use it. Whether you need to acquire a mutex is dependent on your application, there is no magic yes/no answer.

[wetneb]

Ok, thanks for your help .