GNOME Bugzilla – Bug 767232
videodecoder: Drain data in more situations
Last modified: 2018-07-09 13:35:42 UTC
While investigating trick mode with key units, there were two situations that would introduce big delays in getting the decoded keyframe (which should be decoded and pushed out straight away).
Created attachment 329108 [details] [review] videodecoder: Drain decoder on DISCONT buffers This ensures the decoder is properly drained out when receiving a DISCONT buffer. The optimal way of doing this would have been to receive a GAP event before hand but it is not always possible. Fixes big delays with some decoders (ex gst-libav) that will not drain out data when only decoding keyframes.
Created attachment 329109 [details] [review] videodecoder: Drain out keyframes in TRICK_MODE_KEY_UNITS When asked to just decoder keyframe, if we got a keyframe drain out the decoder straight away. This avoids having to wait for the next frame and reduces delay even more.
Comment on attachment 329109 [details] [review] videodecoder: Drain out keyframes in TRICK_MODE_KEY_UNITS Typo "to just decoder keyframe" -> decode. Otherwise good :)
commit 183e94b2d3ee830f196d45756563f4572c9954f7 Author: Edward Hervey <edward@centricular.com> Date: Sat Jun 4 09:51:17 2016 +0200 videodecoder: Drain out keyframes in TRICK_MODE_KEY_UNITS When asked to just decode keyframe, if we got a keyframe drain out the decoder straight away. This avoids having to wait for the next frame and reduces delay even more. https://bugzilla.gnome.org/show_bug.cgi?id=767232 commit eb1ebf226ff8c29ea976169f81135219f1ab412b Author: Edward Hervey <edward@centricular.com> Date: Sat Jun 4 09:49:00 2016 +0200 videodecoder: Drain decoder on DISCONT buffers This ensures the decoder is properly drained out when receiving a DISCONT buffer. The optimal way of doing this would have been to receive a GAP event before hand but it is not always possible. Fixes big delays with some decoders (ex gst-libav) that will not drain out data when only decoding keyframes. https://bugzilla.gnome.org/show_bug.cgi?id=767232
Since this commit I observe a bunch of GST_FIXMEs spitting out: gstvideodecoder.c:1057:gst_video_decoder_drain_out:<omxh264dec-omxh264dec0> Sub-class should implement drain() The discont flag is set by baseparse. My use-case is the following, an appsrc pushing H264 byte-stream in playbin on RPi2.
Sounds like you should open/clone a bug against gst-omx then, no? :) Or are you saying the log level should be downgraded?
I'm just wondering why I get this message for every buffer handled by the decoder. Wouldn't draining the OMX decoder for every buffer kill the performance? Well perhaps this is actually a baseparse issue, sorry for the noise here.
I don't know, it depends on the kind of buffer - is this I-frame only trick play / reverse playback, or does it happen for every single frame in normal playback or .. ? I don't think the DISCONT flag should be set on every frame/buffer in normal playback, maybe clone this bug into a new one so we can investigate?
Wasn't it a little bold to add drain() virtual, and not implement it outside of avedec ? Specially that we now seem to use it everywhere. The other side effect, is that not all decoder let you drain without removing the decoder state, which means for GAPS it's pretty hard to do.
If that is a problem in practice, we could add something to the base class to let the subclass specify how expensive drains are. And if they are too expensive, they would only be used when really needed (EOS, caps change).
(In reply to Edward Hervey from comment #1) > Created attachment 329108 [details] [review] [review] > videodecoder: Drain decoder on DISCONT buffers > > This ensures the decoder is properly drained out when receiving a > DISCONT buffer. The optimal way of doing this would have been to > receive a GAP event before hand but it is not always possible. > > Fixes big delays with some decoders (ex gst-libav) that will not > drain out data when only decoding keyframes. We observe that this change increase the amount of distortion on packet lost. Draining imply flushing the DPB (the references), so instead of having the following frame missing one reference, it's missing all references. This leads to gray background instead of couple of blocky spots.