Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Add buffering to video renderers #2249

Closed
wants to merge 24 commits into from

8 participants

@FernetMenta
Collaborator

This is follow up from #870

  • It adds buffering to video and overlay renderers. This way video player can deliver frames and overlays to RenderManager without blocking on every cycle. The player just fills the buffers and leaves scheduling to RenderManager

  • Due to buffering video player needs a more advanced algorithm for dropping in case of player gets late. It has to look at the end of the queue right after decoder and count already dropped frames against lateness.

  • I have analyzed and tested this extensively on Linux where it showed a huge performance gain.

  • In this PR buffering is activated for software rendering and DXVA

  • OMXPlayer is adopted to the changes in order to make it compile. Other players than dvdplayer and OMXPlayer need to be adopted.

  • Needs testing on Darwin and iOS.

@davilla
Collaborator

how should players that use BYPASS work ? Essentially rendering is outside of the scope of gles.

@FernetMenta
Collaborator

Currently there should be no difference using BYPASS because this method does not activate buffering. I was wondering if it makes sense letting RenderManager know the "bypass" buffer size. I tested on RPi and noticed that the recent changes on OMXPlayer makes the GUI drop to below 10fps while the video is running fine.

@bobo1on1
Collaborator

Does this work ok when sync playback to display is turned on?

@FernetMenta
Collaborator

I have been running this for more than a year with sync playback to display on. Calculation of presenttime is moved from DVDPlayer::OutputPicture to RenderManager.

@rbej

If i have Rpi i must apply only last patch Omx:adopt to buffering?.

@davilla
Collaborator

@theuni , need to you to look at this.

xbmc/cores/VideoRenderers/OverlayRenderer.cpp
((5 lines not shown))
}
CRenderer::~CRenderer()
{
- for(int i = 0; i < 2; i++)
+ for(int i = 0; i < 10; i++)
@wsnipex Collaborator
wsnipex added a note

is this hardcoded purposefully?

@FernetMenta Collaborator

probably not. the renderes use a local define NUM_BUFFERS. maybe we should move this up to system.h.

@davilla Collaborator
davilla added a note

xbmc / cores / VideoRenderers / BaseRenderer.h ?

@FernetMenta Collaborator

good idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Memphiz
Owner

What do i need to look for when testing this?

@FernetMenta
Collaborator

Darwin uses dvdplayer and LinuxRenderer, right?

1) does it compile?
2) any regression or improvements for sw decoding
3) check RENDER_FMT_CVBREF, buffering is disables for this format in DVDPlayerVideo::OutputPicture
enabling it may require the codec to provide more video surfaces, see 4a6b1aa07e1283915af214e53f46b04acfbc8af3. the number of render buffers can be passed to the codec in open

@Memphiz
Owner
  1. Compiles - yes
  2. no regressions seen yet (software decoding only at this mashine currently) - not sure if any improvements
  3. @something for davilla ;)
@davilla
Collaborator

The VDA/VTB XBMC codecs have no control over the number of video surfaces used except by not releasing them back to the internal codec's pool, thus forcing it to alloc more. The internal hw codec owns these buffers.

VDA is slightly more complex as there are two ways to handle hw decode, one way uses CVBufs all the way up, the other uses CVBufs but converts to normal dvd picture frames when are then passed up. The choice depends on the physical GPU, ie intel, nvidia or ati.

@FernetMenta FernetMenta referenced this pull request from a commit
@popcornmix popcornmix [rbp] Fix for hang after seeking introduced by ASS fix
#2206 introduced a regression when seeking in some types of SD files where the video stutters and/or stalls.
This is caused by using the wrong clock in FlipPage
8b60327
@Memphiz
Owner

Compiled and tested with ios (atv2). No regressions as far as i can see (tested only soft decoding) - might be that when bringing up the overlay it doesn't hickup that much anymore?

Also tested hw decode with VTB - its still working - not sure if this PR should even influence it.

@FernetMenta
Collaborator

I added a small change to RenderManager and adapted OMXPlayer. It does not block in FlipPage while the GUI is kept alive and responsive.

@elupus
Collaborator

I'm sorry.. But do you think we can take the buffering first then the lateness code. This pull is huge and touches many critical things for rendering. Sorry for making it complicated, but more people than you need to understand the code.

@FernetMenta
Collaborator

sure, I can split the pr into buffering and lateness but those 2 should not be tested separately because buffering frames brakes the old method for dropping frames. At least you would see regression.

@elupus elupus commented on the diff
xbmc/cores/VideoRenderers/OverlayRenderer.cpp
@@ -90,29 +90,32 @@ long COverlayMainThread::Release()
CRenderer::CRenderer()
{
m_render = 0;
- m_decode = (m_render + 1) % 2;
@elupus Collaborator
elupus added a note

constructor should init this, otherwise it's undefined.

@FernetMenta Collaborator

m_decode was a leftover, I removed it completely. It's not needed anymore because this works same way as renderer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
FernetMenta added some commits
@FernetMenta FernetMenta VideoRenerers: add buffering faabff1
@FernetMenta FernetMenta add buffering for GLES 8de4b0d
@FernetMenta FernetMenta WinRenderer: add buffering 5ec9cdd
@FernetMenta FernetMenta drop frame counter in application, ask render manager instead 60ecb26
@FernetMenta FernetMenta vaapi: adopt to buffering in renderer 3d66549
@FernetMenta FernetMenta linuxrenderer: delete all textures on reconfigure cf2ce0c
@FernetMenta FernetMenta DXVA: activate buffering in renderer bbb05a9
@FernetMenta FernetMenta add buffering - some documentation 11c0e31
@FernetMenta FernetMenta move NUM_BUFFERS up to BaseRenderer.h 6e80e5e
@FernetMenta FernetMenta OverlayRenderer: align buffers with index in renderManager a82f645
@FernetMenta FernetMenta add buffering - submit absolute time to render buffers 58b6bc4
@FernetMenta FernetMenta RenderManager: some rework to buffering 25f1561
@FernetMenta FernetMenta dvdplayer: disable buffering unil dropping is improved bfb9177
@FernetMenta FernetMenta RenderManager: skip very late frames in render buffer 8853287
@FernetMenta FernetMenta renderbuffers: drop enable/disable in this iteration 884c5f4
@FernetMenta FernetMenta RenderManager: add method SetSpeed 3086aaf
@FernetMenta FernetMenta RenderManager: return bufferlevel with WaitForBuffer febaae2
@FernetMenta FernetMenta OMXPlayer: adapt to buffering 6a52c0a
@FernetMenta FernetMenta RenderManager: do not drop in renderbuffers, this ruins detection of …
…displayed frame
0cb534b
@FernetMenta FernetMenta videoplayer: adapt lateness detection and dropping to buffering 43906aa
@FernetMenta FernetMenta video player: present correct pts to user for a/v sync (after bufferi…
…ng in renderer)
ddd301e
@FernetMenta FernetMenta videoplayer: some rework and documentation a03f9a1
@FernetMenta FernetMenta Revert "dvdplayer: disable buffering unit dropping is improves"
This reverts commit de1caf5686c1fb53cb7ab11b356e6c22770740db.
4b881bf
@FernetMenta FernetMenta OMXPlayerVideo: adapt to change in FlipPage acd8b3a
@theuni

Are this many really necessary? Does that affect memory consumption as it would imply?

Collaborator

This is just the maximum, the actual number of buffers are requested by CXBMCRenderManager::ResetRenderBuffer(). Currently hardcoded to 5 in case buffering is supported and requested by render method

@theuni

use some define or constant?

Collaborator

6e80e5e
this pr has had some sporadic reviews and I did not squash commits to show progress.

@theuni
Collaborator

Only called from CXBMCRenderManager::FlipPage where the lock is already grabbed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 5, 2013
  1. @FernetMenta
  2. @FernetMenta

    add buffering for GLES

    FernetMenta authored
  3. @FernetMenta

    WinRenderer: add buffering

    FernetMenta authored
  4. @FernetMenta
  5. @FernetMenta
  6. @FernetMenta
  7. @FernetMenta
  8. @FernetMenta
  9. @FernetMenta
  10. @FernetMenta
  11. @FernetMenta
  12. @FernetMenta
  13. @FernetMenta
  14. @FernetMenta
  15. @FernetMenta
  16. @FernetMenta
  17. @FernetMenta
  18. @FernetMenta
  19. @FernetMenta
  20. @FernetMenta
  21. @FernetMenta
  22. @FernetMenta
  23. @FernetMenta

    Revert "dvdplayer: disable buffering unit dropping is improves"

    FernetMenta authored
    This reverts commit de1caf5686c1fb53cb7ab11b356e6c22770740db.
  24. @FernetMenta
Something went wrong with that request. Please try again.