Add buffering to video renderers #2777

elupus · 2013-05-22T18:42:46Z

This replaces: #2309

It implements a render queue in the renderers allowing main thread to continue rendering more frames while we are decoding. This should reduce the likelihood of stuttering due to high cpu load.

The default queue size is 3. Thus 1 additional frames. This is likely a candidate for advanced settings since it will allocate full pictures frames in memory during playback. This could be increased on systems that have enough memory.

The queue will always select the last decoded frame that has a present time that has passed. This mean it will potentially drop frames at output stage. This has the added benefit of us finally being able to play 50fps video's on 30hz displays and 60fps video's on 50hz displays without causing total audio desync.

elupus · 2013-05-22T18:45:08Z

This need testing on vaap, rbpi and omx. I'm not sure what android uses in playback, but it won't hurt to test it there too.

FernetMenta · 2013-05-22T19:24:21Z

maybe we should drop the OMXPlayer commit: 3e9c2b9785fba9336192b203e26285c53273b5c9 from this pr, only keep the required change to configure. It will correct out of sync subs but regression in performance. The current code in OMXPlayer is not correct because it just skips driving the gui but this requires more work to get this fixed without sacrifice current performance.

wsnipex · 2013-05-23T12:05:43Z

ubuntu packages for testing: https://launchpad.net/~wsnipex/+archive/xbmc-pr-tests

elupus · 2013-05-23T21:52:52Z

There is a performance issue with the queue (should not be worse than old code) causing it to not queue items unless decode is quite a lot faster than display. This is due to our large scale locks in rendermanager.

FlipPage get's blocked on a shared lock during render. The queue and the present info need to be on a separate lock from the shared lock.

elupus · 2013-05-25T14:13:36Z

I'm starting to feel quite happy with this pull now. But again, i need testing on OMX. I think dxva/vaapi/vdpau/vda are fine.

FernetMenta · 2013-05-25T18:03:39Z

@elupus I needed this change for successful compilation on RPi: FernetMenta@1876d2a
I did some quick tests and did not see any (new) problems. As mentioned before, OMXPlayers's interface to renderer is borked anyway and needs rework.

huceke · 2013-05-28T16:29:13Z

Looking ok to me on the PI. Waiting for feedback from popcornmix.

popcornmix · 2013-05-28T16:39:56Z

I'll try to test this tonight with some borderline performance clips and see if things are better or worse.
In principle I like the change.

elupus · 2013-05-28T16:51:22Z

Note. Rbpi is limited to tw buffers at the moment. Change that 0 in
configure to 3 and verify it ends up at that if the Rbpi codes can handle
it.

popcornmix · 2013-05-28T17:02:40Z

@elupus
You are refering to whether GL is double or triple buffered on Pi? It is triple buffered by default.

rbej · 2013-05-28T17:19:09Z

Little out of sync internal subs [1/17] in this video sample on Rpi.

http://www.mediafire.com/download/eo6q1udxdj64ppf/The.Hobbit.An.Unexpected.Journey.2012.1080p.BluRay.10bit.48fps.edition.DTS.x264-HDMaNiAcS-sample.mkv

On FernetMenta version this patch subs sync is perfect.

elupus · 2013-05-28T18:15:35Z

Nope not glx buffering, I'm not sure how the Rbpi renders frames. Is it
using the gl renderer at all? or is it blended in with some external
overlay?

Regarding subs, it aught to be identical to before the pull? But I think I
see an easy way to improve it.

popcornmix · 2013-05-28T18:24:11Z

Video is separate from GL on Pi (an external overlay).
Yes, subs sync was off before this pull. It did get fixed in some versions of FernetMenta's "buffering" pull request, although it seemed tricky to fix without causing a performance loss (by blocking OMXPlayerVideo.cpp thread).

FernetMenta · 2013-05-28T19:30:50Z

Currently OMXplayer just skips driving the gl layer when getting late. This causes out-of-sync subs but a gain in performance. Buffering alone in the gl layer won't cure the loss in performance when keeping video and gl in sync. The gl layer still gets cleared and flipped on every cycle. It has to be treated like a dirty region in order to get both, synced subs and performance.
This is out of scope of this pr, hence we left out the first part as well. There should be no difference between after and before this pr.

elupus · 2013-05-28T21:48:33Z

I just pushed a shot in the dark at an RBPI improvement. It will avoid blocking rbpi thread, and queue up overlays.

popcornmix · 2013-05-28T23:57:03Z

I checked out master and wound the clock frequency down until my test clip just played without buffering.
I then applied this PR without 284f9c3. I think I'm okay with performance. It seemed a little worse, but there's a certain amount of variance in this test. It did play without buffering on a number of runs.

My test clip did not contain subtitles.

284f9c3 was less successful. That clearly seemed to hurt performance (it buffered every time on about 10 runs).
It also makes subtitles appear 6 seconds early. (Probably the diffrerence in timestamps of frames being added to GPU fifo and decoded frames appearing on screen).

OMXMediaTime() is the time of the currently displayed video frame which can be several seconds behind the pts values of packets arriving at OMXPlayerVideo.cpp. You need the pts of the subtitle being rendered to match OMXMediaTime().

popcornmix · 2013-05-29T00:20:31Z

I think I'm seeing a regression. If I play a file through json. Then play it again (or a different one) without quitting back to UI, I get no video. I hear the audio, the home menu is still present (no visualisation). I can select "fullscreen" from menu, but I just get a blank screen.

Once in this state I can't play any video (even if video is quit first). Restarting xbmc allows video to work again.

This required 3 plays to get into this bad state:
http://pastebin.com/0KuMduT4

xbmc/cores/VideoRenderers/RenderManager.cpp

+    if(m_presentstep == PRESENT_IDLE)
+    {
+      if(GetNextRender() >= 0)
+        m_presentstep = PRESENT_READY;


kraqh3d · 2013-05-31T12:02:51Z

I've not any issues with video playlists but some dvd's don't play correctly. The menu seems to render correctly, but for some movies the last frame of the menu gets stuck on screen. The audio for the movie will play behind it and xbmc becomes unresponsive. It won't respond to the remote or to events sent from xbmc-send to localhost. The same problematic videos play in Frodo and in Gotham without the patch applied. (They were the same ones I used when testing the dvd_reader patch.) I just noticed the new commit, 00ad3e6. My build is from before this commit. I will test with it as soon as a I can to see if it fixes the problem.

elupus · 2013-05-31T12:14:15Z

Yea that should fix that issue. Dvds reconfigure all the time.

FernetMenta · 2013-05-31T12:25:56Z

It can still hang due to negative timeouts in WaitForBuffer, it did so during my last tests.

elupus · 2013-05-31T12:28:22Z

That is a bit strange. That should just cause a huge timeout value. Why
don't we get a buffer?

FernetMenta · 2013-05-31T12:40:24Z

It happened on stop and video player thread was waiting with that huge timeout in m_presentevent.wait

iirc main thread was waiting for player to exit which was waiting for video player.

kraqh3d · 2013-05-31T17:41:40Z

I've got an iso that fails consistently and that commit didn't fix it. The same thing happens. Nothing looks anomalous from the log. I'll be away for the weekend but can help debug after I'm back.

elupus · 2013-05-31T17:54:49Z

I'm rebasing and merging within in the hour. @kraqh3d i would think your issue is the negative timestamp problem.

This is how many references to pictures that can be retained when calling Decode the next time around

We don't even need to hold lock here potentially

This allow renderer to queue up frames for display after decode. Original Implementation By: Rainer Hochecker <fernetmenta@online.de>

Allo dropping a frame that is about to be rendered, if it is late and there already exist a replacement frame that should be presented now.

RenderManager can now take care of this in a much more intellegent fashion

This looks rather messy. But is solves an important performance issue where decode thread would be blocked waiting flip by render thread while it was rendering current frame. Without this faster than display rate decode is not possible

Add buffering to video renderers

Voyager1 · 2013-05-31T20:59:36Z

this causes major regression on dvd with menu playback. Tested several dvds, after I go through the dvd menu, select "play", the image freezes and all I hear is sound, but no video starts. The only thing I can do is kill xbmc. Excerpt debug Log:

22:56:38 T:9832   DEBUG: CDVDInputStreamNavigator::ProcessBlock - Cell change: Title 1, Chapter 1
22:56:38 T:9832   DEBUG: CDVDInputStreamNavigator::ProcessBlock - At position 0% inside the feature
22:56:38 T:9832   DEBUG: DVDNAV_CELL_CHANGE
22:56:38 T:9832   DEBUG: DVDNAV_NAV_PACKET - DISCONTINUITY FROM:8287266 TO:375733 DIFF:-7911533
22:56:38 T:5600   DEBUG: CAESinkDirectSound::CheckPlayStatus: Resuming Playback
22:56:38 T:9064 WARNING: CRenderManager::WaitForBuffer - timeout waiting for buffer
22:56:38 T:9064 WARNING: Previous line repeats 3 times.
22:56:38 T:9064   DEBUG: CDVDPlayerVideo - CDVDMsg::GENERAL_SYNCHRONIZE
22:56:38 T:9064 WARNING: CRenderManager::WaitForBuffer - timeout waiting for buffer
22:56:39 T:10920 WARNING: Previous line repeats 5 times.
22:56:39 T:10920   DEBUG: CDVDPlayerAudio:: Discontinuity1 - was:2993715.829705, should be:2824000.000000, error:-169715.829705
22:56:39 T:9064 WARNING: CRenderManager::WaitForBuffer - timeout waiting for buffer
22:56:39 T:10920 WARNING: Previous line repeats 3 times.
22:56:39 T:10920   DEBUG: CDVDPlayerAudio:: Discontinuity1 - was:3205081.026865, should be:3100957.031292, error:-104123.995573
22:56:39 T:9064 WARNING: CRenderManager::WaitForBuffer - timeout waiting for buffer
22:56:43 T:9064 WARNING: Previous line repeats 101 times.
22:56:43 T:9064   DEBUG: CPullupCorrection: detected pattern of length 1: 40000.00, frameduration: 40000.000000
22:56:43 T:9064 WARNING: CRenderManager::WaitForBuffer - timeout waiting for buffer
22:56:58 T:9832 WARNING: Previous line repeats 364 times.
22:56:58 T:9832   DEBUG: CDVDPlayer::ProcessSubData: Got complete SPU packet
22:56:58 T:9064 WARNING: CRenderManager::WaitForBuffer - timeout waiting for buffer
22:57:03 T:8864 WARNING: Previous line repeats 136 times.
22:57:03 T:8864   DEBUG: Thread JobWorker 8864 terminating (autodelete)
22:57:03 T:8312   DEBUG: Thread JobWorker 8312 terminating (autodelete)
22:57:03 T:7356   DEBUG: Thread JobWorker 7356 terminating (autodelete)
22:57:03 T:4528   DEBUG: Thread JobWorker 4528 terminating (autodelete)
22:57:03 T:9064 WARNING: CRenderManager::WaitForBuffer - timeout waiting for buffer
22:57:05 T:9832 WARNING: Previous line repeats 49 times.
22:57:05 T:9832   DEBUG: CDVDPlayer::ProcessSubData: Got complete SPU packet
22:57:05 T:9064 WARNING: CRenderManager::WaitForBuffer - timeout waiting for buffer
22:57:13 T:9832 WARNING: Previous line repeats 197 times.
22:57:13 T:9832   DEBUG: CDVDPlayer::ProcessSubData: Got complete SPU packet
22:57:13 T:9064 WARNING: CRenderManager::WaitForBuffer - timeout waiting for buffer

Voyager1 · 2013-05-31T21:01:13Z

update - this is the same problem @kraqh3d described. Last frame of the menu stays stuck instead of playing the video.

elupus · 2013-05-31T21:59:13Z

yup. seeing it too. just can't figure out what the heck is going on. my xcode keeps crashing.

elupus · 2013-06-01T00:51:32Z

hang should be resolved at df90ddd

Voyager1 · 2013-06-01T06:23:15Z

it is resolved indeed - thanks!

rbej · 2013-06-01T12:03:42Z

When stop playing movie on Rpi, picture freeze and only hard reset helps.

http://pastebin.com/Q0guRnse

FernetMenta · 2013-06-01T12:31:53Z

omxplayer still suffers from negative timeout. I will prepare a pr.

FernetMenta · 2013-06-01T12:41:11Z

@popcornmix could do do this with #2798? dvdplayer clamps timeout for WaitForBuffers to positive values.

popcornmix · 2013-06-01T12:44:55Z

@FernetMenta
I will re-merge with latest dvdplayer when that goes in, although that PR is waiting on a commit from @elupus.

elupus · 2013-06-01T14:50:51Z

@popcornmix those cleanups are already in.

rbej · 2013-06-02T18:55:13Z

This patch fixed frezze picture on Rpi #2798, but cause out of sync subs (subs display some 1.5s too fast)

elupus · 2013-06-02T21:49:41Z

Rbpis output logic need to be looked over.

FernetMenta reviewed May 29, 2013
View reviewed changes

xbmc/cores/VideoRenderers/RenderManager.cpp

if(m_presentstep == PRESENT_IDLE)

{

if(GetNextRender() >= 0)

m_presentstep = PRESENT_READY;

This comment was marked as spam.

Sign in to view

fritsch mentioned this pull request May 31, 2013

Second video from playlist stays black (OE gotham) FernetMenta/xbmc#123

Closed

elupus added 3 commits May 31, 2013 19:59

dvdplayer: factor out render config flag from OutputPicture

ae90cae

dvdplayer: cosmetic alignment of checks

9bcadcf

renderer: split out frame move from render action in video renderers

31e13aa

FernetMenta and others added 14 commits May 31, 2013 19:59

move NUM_BUFFERS up to BaseRenderer.h

805d5ba

dvdplayer: add interface to codec for number of shared references

7de62dd

This is how many references to pictures that can be retained when calling Decode the next time around

renderer: replace BYTE* with uint8_t*

0db1db9

linuxrenderer: delete all textures on reconfigure

ee74126

renderer: calculate present parameters before setting up fields

94baa85

We don't even need to hold lock here potentially

renderer: add buffering to render system

cb67010

This allow renderer to queue up frames for display after decode. Original Implementation By: Rainer Hochecker <fernetmenta@online.de>

renderer: allow dropping at output stage when we have a replacment frame

527dfa5

Allo dropping a frame that is about to be rendered, if it is late and there already exist a replacement frame that should be presented now.

renderer: skip second field of bob deinterlace if late

2699c08

add buffering - some documentation

f18a743

dvdplayer: display pictures skipped on output

00add08

(dvd/omx)player: drop now redundant code for frame drops on output

38debf8

RenderManager can now take care of this in a much more intellegent fashion

renderer: reduce thread ping ponging by using real condition variable

a280c19

renderer: fix hang if we DiscardBuffers and try to start again

9d14c1f

elupus added a commit that referenced this pull request May 31, 2013

Merge pull request #2777 from elupus/renderbuf

3fcde70

Add buffering to video renderers

elupus merged commit 3fcde70 into xbmc:master May 31, 2013

elupus deleted the renderbuf branch May 31, 2013 21:33

ghost mentioned this pull request Dec 1, 2013

Sometimes no picture mkv/mp4 quarnster/boxeebox-xbmc#62

Closed

Add buffering to video renderers #2777

Add buffering to video renderers #2777

Conversation

elupus commented May 22, 2013

elupus commented May 22, 2013

FernetMenta commented May 22, 2013

wsnipex commented May 23, 2013

elupus commented May 23, 2013

elupus commented May 25, 2013

FernetMenta commented May 25, 2013

huceke commented May 28, 2013

popcornmix commented May 28, 2013

elupus commented May 28, 2013

popcornmix commented May 28, 2013

rbej commented May 28, 2013

elupus commented May 28, 2013

popcornmix commented May 28, 2013

FernetMenta commented May 28, 2013

elupus commented May 28, 2013

popcornmix commented May 28, 2013

popcornmix commented May 29, 2013

This comment was marked as spam.

kraqh3d commented May 31, 2013

elupus commented May 31, 2013

FernetMenta commented May 31, 2013

elupus commented May 31, 2013

FernetMenta commented May 31, 2013

kraqh3d commented May 31, 2013

elupus commented May 31, 2013

Voyager1 commented May 31, 2013

Voyager1 commented May 31, 2013

elupus commented May 31, 2013

elupus commented Jun 1, 2013

Voyager1 commented Jun 1, 2013

rbej commented Jun 1, 2013

FernetMenta commented Jun 1, 2013

FernetMenta commented Jun 1, 2013

popcornmix commented Jun 1, 2013

elupus commented Jun 1, 2013

rbej commented Jun 2, 2013

elupus commented Jun 2, 2013