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 726020 - avdec_h265: freezes when max-threads != 1
avdec_h265: freezes when max-threads != 1
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-libav
git master
Other All
: Normal normal
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
: 750400 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2014-03-10 09:16 UTC by Thijs Vermeir
Modified: 2015-06-05 10:00 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
H265 test sequence (16.16 KB, application/octet-stream)
2014-03-10 09:17 UTC, Thijs Vermeir
Details

Description Thijs Vermeir 2014-03-10 09:16:48 UTC
The decoder freezes for the h265 codec freezes when the max-threads property is not 1. This can be reproduced with the following pipelines.

non-working pipeline:
gst-launch-1.0 filesrc location=videotestsrc.265 ! h265parse ! avdec_h265 ! fakesink

working pipeline:
gst-launch-1.0 filesrc location=videotestsrc.265 ! h265parse ! avdec_h265 max-threads=1 ! fakesink
Comment 1 Thijs Vermeir 2014-03-10 09:17:24 UTC
Created attachment 271403 [details]
H265 test sequence
Comment 2 Thiago Sousa Santos 2014-03-11 04:19:21 UTC
The relevant threads:

Thread 4 (Thread 0x7ffff2b9f700 (LWP 18830))

  • #0 __lll_lock_wait
    at ../nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S line 135
  • #1 _L_lock_957
    from /lib/x86_64-linux-gnu/libpthread.so.0
  • #2 __GI___pthread_mutex_lock
    at ../nptl/pthread_mutex_lock.c line 114
  • #3 gst_video_decoder_get_frame
    at gstvideodecoder.c line 3068
  • #4 gst_ffmpegviddec_get_buffer
    at gstavviddec.c line 615
  • #5 ff_get_buffer
    at libavcodec/utils.c line 646
  • #6 set_sps
    at libavcodec/hevc.c line 428
  • #7 hls_slice_header
    at libavcodec/hevc.c line 490
  • #8 decode_nal_unit
    at libavcodec/hevc.c line 2568
  • #9 decode_nal_units
    at libavcodec/hevc.c line 2819
  • #10 hevc_decode_frame
    at libavcodec/hevc.c line 2924
  • #11 frame_worker_thread
    at libavcodec/pthread_frame.c line 145
  • #12 start_thread
    at pthread_create.c line 312
  • #13 clone
    at ../sysdeps/unix/sysv/linux/x86_64/clone.S line 111


Video decoder handle_frame is called with the STREAM_LOCK while libav is calling get_buffer that also needs the STREAM_LOCK
Comment 3 Sebastian Dröge (slomo) 2014-03-11 08:39:17 UTC
So we should release the stream lock while calling avcodec_decode_video2(). Very similar to what we do in androidmedia or gst-omx.

Just needs checking after we get the lock again if anything relevant has changed.
Comment 4 Thiago Sousa Santos 2014-03-11 21:45:56 UTC
(In reply to comment #3)
> So we should release the stream lock while calling avcodec_decode_video2().
> Very similar to what we do in androidmedia or gst-omx.
> 
> Just needs checking after we get the lock again if anything relevant has
> changed.

It seems all of the public API of videodecoder uses the stream lock, isn't it better than to solve this at videodecoder itself?
Comment 5 Nicolas Dufresne (ndufresne) 2014-03-11 22:43:29 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > So we should release the stream lock while calling avcodec_decode_video2().
> > Very similar to what we do in androidmedia or gst-omx.
> > 
> > Just needs checking after we get the lock again if anything relevant has
> > changed.
> 
> It seems all of the public API of videodecoder uses the stream lock, isn't it
> better than to solve this at videodecoder itself?

Yes, and this lock currently prevent buffers to come in while pushing. I had in mind that the solution would be to reduce the scope of this lock, and protect the internal list of the decoder with object lock. Though it seemed tricky due to backward compatibility requirement.
Comment 6 Sebastian Dröge (slomo) 2014-03-12 07:30:57 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > So we should release the stream lock while calling avcodec_decode_video2().
> > Very similar to what we do in androidmedia or gst-omx.
> > 
> > Just needs checking after we get the lock again if anything relevant has
> > changed.
> 
> It seems all of the public API of videodecoder uses the stream lock, isn't it
> better than to solve this at videodecoder itself?

All the videodecoder code that runs from the streaming thread takes this lock, yes... but you have to unlock it yourself if you do a blocking operation in the streaming thread, and this causes another thread to call into your code and taking the stream lock. Exactly the situation you have here :)


Also all Nicolas said is true too, I started looking into it but it seems rather annoying to fix this in the base class now.
Comment 7 Thijs Vermeir 2014-03-12 08:06:10 UTC
Note that also thread_safe_callbacks is not set on the libav context. This causes the streaming thread to wait on the (libav) decoding thread. It should be safe to set the thread_safe_callbacks to true and not block. However for frame based threading libav will then block in the streaming thread till the previous frame is decoded, so there is only little gain.
Comment 8 Thiago Sousa Santos 2014-03-14 18:33:18 UTC
FWIW a related bug: https://bugzilla.gnome.org/show_bug.cgi?id=715192
Comment 9 Sebastian Dröge (slomo) 2015-06-05 09:53:13 UTC
*** Bug 750400 has been marked as a duplicate of this bug. ***
Comment 10 Sebastian Dröge (slomo) 2015-06-05 10:00:44 UTC
This fixes it, nonetheless we should look into improving the situation at the videodecoder level too. That's bug #715192, but it wouldn't have helped in this specific case.

commit b81cb99d9fea5dbe0c1d74008186ef328c7f1a13
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Fri Jun 5 11:57:37 2015 +0200

    avviddec: Release stream lock while calling avcodec_decode_video2()
    
    It might call back into us from another thread and try to take the stream lock
    again, e.g. to allocate a buffer.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=726020