Opened 13 years ago
Closed 12 years ago
#773 closed defect (needs_more_info)
Decoding H.264 gets stuck with Win XP, w32threads, and custom AVCodecContext.get_buffer
Reported by: | Andreas Girgensohn | Owned by: | |
---|---|---|---|
Priority: | normal | Component: | avcodec |
Version: | 0.9 | Keywords: | |
Cc: | Blocked By: | ||
Blocking: | Reproduced by developer: | no | |
Analyzed by developer: | no |
Description
I have an application that decodes H.264 video to in-memory RGB frames. It works fine multi-threaded in Windows 7 or single-threaded in Windows XP. I use FFmpeg 0.9 that includes the patches from Dec. 7 of this functionality in w32threads.h.
FFmpeg is cross-compiled from Fedora 15 with this configuration:
configure --enable-memalign-hack --disable-pthreads --enable-w32threads --arch=x86 --target-os=mingw32 --cross-prefix=i686-pc-mingw32-
I traced the problem to my use of a custom get_buffer, the functions ff_thread_get_buffer and submit_packet, and the new w32threads implementation for Windows XP (not using SleepConditionVariableCS).
After decoding some of the frames, the threads get stuck here:
ff_thread_get_buffer:
while (p->state != STATE_SETTING_UP)
pthread_cond_wait(&p->progress_cond, &p->progress_mutex);
submit_packet:
while (p->state == STATE_SETTING_UP)
pthread_cond_wait(&p->progress_cond, &p->progress_mutex);
As the ffmpeg application does not use a custom get_buffer, it does not have this problem. I tried to figure out what is happening in w32threads.h but unfortunately adding too many print statements makes the problem go away.
I think that the problem can be avoided by using different condition variables for signaling in the two directions between the threads but I haven't tried it.
P.S.: I'm using a custom get_buffer to get a more reliable presentation timestamp. I should check if this is still necessary in the current FFmpeg version.
int64_t global_video_pkt_pts = AV_NOPTS_VALUE;
int getFrameBuffer (AVCodecContext* c, AVFrame* pic) {
int ret = avcodec_default_get_buffer (c, pic);
int64_t* pts = (int64_t*) av_malloc (sizeof (int64_t));
*pts = global_video_pkt_pts;
pic->opaque = pts;
return ret;
}
void releaseFrameBuffer (AVCodecContext* c, AVFrame* pic) {
if (pic)
av_freep (&pic->opaque);
avcodec_default_release_buffer (c, pic);
}
...
global_video_pkt_pts = packet_context.packet.dts;
Change History (6)
comment:1 by , 13 years ago
comment:2 by , 13 years ago
This Stack Overflow question points to resources for implementing pthread condition variables in Windows XP: http://stackoverflow.com/questions/1218716/implementing-condition-variables-for-critical-sections-for-winthreads-for-xp
comment:3 by , 13 years ago
Another option would be to use the implementation of pthread_cond_wait and friends from Pthreads-win32: http://sourceware.org/pthreads-win32/
While that project was last updated five years ago, there is a long discussion on the right way to implement condition variables. Also, the source is under LGPL and thus could be used without problems in FFmpeg.
comment:5 by , 12 years ago
Unfortunately, I don't have the implementation with the custom get_buffer anymore. It was intended to get a more reliable presentation timestamp. The current FFmpeg approach for determining timestamps seems to be robust so that it is fine for me.
comment:6 by , 12 years ago
Resolution: | → needs_more_info |
---|---|
Status: | new → closed |
Please reopen if this is still reproducible.
I don't know who implemented that code and based on what, but it is just broken.
There are gems like this I can't see what purpose they would serve:
However more serious the waiter_count is decremented at the wrong place.
If you look at pthread_cond_wait the WaitForSingleObject is outside of all locks.
This means if somehow all threads leaving that function would be delayed for some time, you could use pthread_cond_signal to arbitrarily increase the value of the semaphore, even though there was never more than one thread waiting.
As a consequence pthread_cond_wait would never be waiting anymore after that and from there it is only downhill.
I can't really (easily at least) explain a hang with that though, there should at least some other things go wrong first.
The method presented here: http://research.microsoft.com/pubs/64242/implementingcvs.pdf looks like it might at least work, even if performance is not exactly good.
Seems their hope of people not repeating mistakes was not exactly justified though.