After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 322254 - avcodec_open()/close() aren't thread-safe
avcodec_open()/close() aren't thread-safe
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-libav
git master
Other Linux
: Normal normal
: 0.10.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2005-11-23 17:13 UTC by Sebastien Cote
Modified: 2006-05-09 16:48 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
proposed fix (5.92 KB, patch)
2005-11-23 19:28 UTC, Sebastien Cote
none Details | Review
test case (3.85 KB, text/x-csrc)
2005-12-08 13:14 UTC, Luca Ognibene
  Details
updated patch (make the mutex static) (5.39 KB, patch)
2005-12-13 13:35 UTC, Luca Ognibene
committed Details | Review

Description Sebastien Cote 2005-11-23 17:13:14 UTC
Run many instances of ffmpeg decoders and/or encoders simultaneously and you get
the following message: "insufficient thread locking around avcodec_open/close()".

It seems a lock must be held before calling avcodec_open/close(). When you get
the warning, ffmpeg doesn't initialize properly and problems start happening. We
probably need a static mutex that both gstffmpegenc and gstffmpegdec can lock.
Comment 1 Sebastien Cote 2005-11-23 19:28:25 UTC
Created attachment 55157 [details] [review]
proposed fix

Use a GMutex around all calls to avcodec_open and avcodec_close. It gets rid of
the issue.
Comment 2 Thomas Vander Stichele 2005-11-27 13:22:30 UTC
sounds like a good idea to fix.  Any chance you could provide a testcase to
verify this ?
Comment 3 Sebastien Cote 2005-11-28 15:36:11 UTC
I will try to make a testcase, but it could take a while. Btw, there is no
"check" directoty in gst-ffmpeg. Should I create one and put the test there?
Comment 4 Luca Ognibene 2005-12-08 13:14:09 UTC
Created attachment 55777 [details]
test case

I've done a test case. It's based on simple_launch_lines.c from
gst-plugins-base. I don't have any autofoo skills so i don't really know how to
integrate it into gst-ffmpeg :/
Comment 5 Tim-Philipp Müller 2005-12-12 16:21:42 UTC
Wouldn't it be nicer to add gst_ffmpeg_avcodec_open()/_close() functions which
do the locking on a static lock instead?
Comment 6 Luca Ognibene 2005-12-13 13:35:36 UTC
Created attachment 55940 [details] [review]
updated patch (make the mutex static)

Updated patch following Tim's comment.
(i'm using "libtool gcc -o ffmpeg-lock `pkg-config --cflags --libs
gstreamer-0.10 gstreamer-base-0.10 gstreamer-check-0.10` ffmpeg-lock.c 
-lcheck" to compile the test case)
Comment 7 Tim-Philipp Müller 2005-12-16 16:30:18 UTC
I've committed the actual patch, as I really want this to go into the upcoming
release.

A separate bug #324279 has been filed to make sure we add the build stuff for
unit tests at some point, with reference to the test case here.


2005-12-16  Tim-Philipp Müller  <tim at centricular dot net>

        * ext/ffmpeg/gstffmpeg.c: (gst_ffmpeg_avcodec_open),
        (gst_ffmpeg_avcodec_close):
        * ext/ffmpeg/gstffmpeg.h:
        * ext/ffmpeg/gstffmpegdec.c: (gst_ffmpegdec_close),
        (gst_ffmpegdec_open):
        * ext/ffmpeg/gstffmpegenc.c: (gst_ffmpegenc_dispose),
        (gst_ffmpegenc_getcaps), (gst_ffmpegenc_setcaps),
        (gst_ffmpegenc_change_state):
          Do proper locking around avcodec_open() and avcodec_close()
          (fixes #322254, patch by: Sebastien Cote and Luca Ognibene).
Comment 8 Tim-Philipp Müller 2006-05-09 16:48:43 UTC
Test case committed:

 2006-05-09  Tim-Philipp Müller  <tim at centricular dot net>

        Patch by: Luca Ognibene  <luogni at tin dot it>

        * tests/check/Makefile.am:
        * tests/check/generic/libavcodec-locking.c: (setup_pipeline),
        (run_pipeline), (GST_START_TEST), (simple_launch_lines_suite),
        (main):
          Add test case for libavcodec locking