/xbmc

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

i.MX6 rework which decouples GUI (fb0) and video rendering (fb1), framebuffers are composed with DP in hw #6351

Merged
merged 8 commits into from Mar 1, 2015

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
  Isengard 15.0-alpha2
7 participants
@smallint @FernetMenta @fritsch @wolfgar @koying @MartijnKaijser @jxgauthier
@smallint
Member

smallint commented Feb 5, 2015

This is another rework of the i.IMX6 decoding and rendering path that supersedes #5805. More details in http://forum.kodi.tv/showthread.php?tid=211289&pid=1892440#pid1892440.

Excerpt of architecture:

  • Instead of render VPU decoded buffers with GPU (which involves expensive memory copy operations in combination with de-interlacing) we now render exclusively to another hardware framebuffer (fb1)
  • Rendering directly to this new framebuffer can be combined with de-interlacing in one step
  • DP combines both framebuffers in hardware with alpha blending (if 32bpp) or colour keying (if 16bpp)
  • Together with the optimizations brought in by @FernetMenta regarding video rendering we can now even achieve double rate de-interlacing of 1080i streams
  • In a nutshell: video rendering with GPU is bad, IPU is good

What does not work yet:

  • RenderCapture

TODOs:

  • Find a better place for class CIMXContext which is currently part of the decoder implementation but required on some other places as well

Recommendations for client setup:

  • enable VPU@352Mhz in kernel
  • setup 32 bit framebuffer in uEnv.txt (bpp=32)
  • disable console blanking in uEnv.txt (consoleblank=0)

Have fun!

@smallint smallint referenced this pull request Feb 5, 2015

Closed

[imx] Deinterlacing rework #5805

@smallint
Member

smallint commented Feb 5, 2015

One additional note: algorithmdirtyregions must be set to either 0 or 3. 1 and 2 do not work for the time being. I suppose that is a side effect of the hardware clipping work in progress.

@FernetMenta FernetMenta and 1 other commented on an outdated diff Feb 6, 2015

View full outdated diff
xbmc/cores/VideoRenderers/LinuxRendererGLES.cpp
@@ -618,7 +663,6 @@ void CLinuxRendererGLES::FlipPage(int source)
m_iYV12RenderBuffer = NextYV12Texture();
m_buffers[m_iYV12RenderBuffer].flipindex = ++m_flipindex;
-
@FernetMenta

FernetMenta Feb 6, 2015

Member

what editor do you use? please drop those unwanted changes.

@smallint

smallint Feb 6, 2015

Member

qtcreator, it stripes unnecessary spaces when saving (but does not remove lines). If that is a complete line that has been removed then it was my work. I will add it again ;)

@FernetMenta FernetMenta commented on the diff Feb 6, 2015

View full changes
xbmc/cores/VideoRenderers/LinuxRendererGLES.cpp
@@ -1253,7 +1291,7 @@ void CLinuxRendererGLES::RenderMultiPass(int index, int field)
// imgwidth *= planes[0].pixpertex_x;
// imgheight *= planes[0].pixpertex_y;
// }
-//
+//
@FernetMenta

FernetMenta Feb 6, 2015

Member

same here

EDIT: this one is fine :)
@FernetMenta
Member

FernetMenta commented Feb 6, 2015

this is mostly code exclusively used by MX6 which you know best. the other code looks ok to me.
@fritsch
Member

fritsch commented Feb 6, 2015

On the raspberry PI the gui update is slowed down, when in video mode. I tried this afternoon to apply the specific patch also to the imx work, but it works a bit different.

Is there a chance that we e.g. throttle the OSD rendering, codec screen, when playing fullscreen video?

That would help imx, wetek and all those a lot. But is slightly off topic here.
@smallint
Member

smallint commented Feb 6, 2015

@koying, it would be interesting if that PR works on Android as well. If not it is most likely just a matter of device names which are different there. If it won't work at all I could still activate the old path and your Bob deinterlacer for Android. I would like to avoid to break Android support with that PR.
@wolfgar
Contributor

wolfgar commented Feb 7, 2015

Thanks a lot smallint for this PR.
I am definitively convinced that this approach is the one which gets the best out of the imx6 architecture...
@koying
Owner

koying commented Feb 7, 2015

@smallint Would you mind adding a high level description of the new architecture in the PR header, please.
Both for my comprehension and for future reference ;)

[EDIT] in tl;dr style, I mean ;)

smallint changed the title from i.MX6 rework to i.MX6 rework which decouples GUI (fb0) and video rendering (fb1), framebuffers are composed with DP in hw Feb 7, 2015

@smallint
Member

smallint commented Feb 7, 2015

@koying, I tried ... OK?
@koying
Owner

koying commented Feb 7, 2015

Perfect thanks. Re your initial question, it works the same way with amlogic, so if the permissions are set right in the FW, we should be able to do the same thing on android.

This was referenced Feb 13, 2015

Open
Closed
@smallint
Member

smallint commented Feb 20, 2015

Latest changes and impressive improvements from @wolfgar merged (thanks) and tested. RenderCapture was added and this PR is now complete.
@fritsch
Member

fritsch commented Feb 20, 2015

Then let's build it. Thank you very much for bringing IMX to a new level.

jenkins build this please
@wolfgar
Contributor

wolfgar commented Feb 21, 2015

Thanks a lot for your tremendous work on this rendering refactoring @smallint
I have just tested that the current rebase works fine
To me it is all good for merge...
@fritsch
Member

fritsch commented Feb 21, 2015

Oki - final build after laster build and that's wait for @FernetMenta final ack.

jenkins build this please

Edit: jenkins build to only show, we did not regress other platforms.
@wolfgar
Contributor

wolfgar commented Feb 21, 2015

To fix little remaining build issue : wolfgar/xbmc@e9b6dfe
@fritsch
Member

fritsch commented Feb 21, 2015

Little compiler warning: https://github.com/smallint/xbmc/blob/1597ceda6e8c9ca8355e970d8e261d61ee7afcad/xbmc/cores/dvdplayer/DVDCodecs/Video/DVDVideoCodecIMX.cpp#L1483

this should be void or return some bool?
@FernetMenta
Member

FernetMenta commented Feb 22, 2015

+1
@fritsch
Member

fritsch commented Feb 22, 2015

jenkins build this please
@MartijnKaijser
Owner

MartijnKaijser commented Feb 22, 2015

feature merge window is closed so this can go in 1 March

MartijnKaijser added this to the I******* 15.0-alpha2 milestone Feb 22, 2015

@fritsch
Member

fritsch commented Feb 22, 2015

Sorry for yet another build, did not see that: wolfgar/xbmc@e9b6dfe was not yet picked.
@FernetMenta
Member

FernetMenta commented Feb 22, 2015

we agreed that we start Match 1st with new process. currently we don't have a merge window, means this can go in when ready. I also want the pvr binary addon PR merged soon because it blocks pvr development.
@smallint
Member

smallint commented Feb 22, 2015

I picked the fix from @wolfgar and removed the compiler warning. Hope it builds now.
@fritsch
Member

fritsch commented Feb 22, 2015

jenkins build this please
@wolfgar
Contributor

wolfgar commented Feb 22, 2015

seems the build issue is unrelated to this commit so we may be fine ?
@MartijnKaijser
Owner

MartijnKaijser commented Feb 28, 2015

jenkins build this please

@fritsch fritsch added a commit that referenced this pull request Mar 1, 2015

@fritsch fritsch Merge pull request #6351 from smallint/imx-rework-pr 
i.MX6 rework which decouples GUI (fb0) and video rendering (fb1), framebuffers are composed with DP in hw
 645b1c6

@fritsch fritsch merged commit 645b1c6 into xbmc:master Mar 1, 2015
1 check passed

1 check passed

default Merged build finished.
Details
@fritsch
Member

fritsch commented Mar 1, 2015

Thanks to everyone involved!
@jxgauthier

jxgauthier commented on xbmc/cores/dvdplayer/DVDCodecs/Video/DVDVideoCodecIMX.cpp in 0418c2b Mar 5, 2015

simple suggestion (unless the compiler is abel to do the same optimization)

cy_X_y0 = cy * y0
r = (cy_X_y0 + cb1 * u + cr1 * v) >> 16;
g = (cy_X_y0 + cb2 * u + cr2 * v) >> 16;
b = (cy_X_y0 + cb3 * u + cr3 * v) >> 16;

how ever, if this code is not called often, this is useless
@jxgauthier

jxgauthier commented on xbmc/cores/dvdplayer/DVDCodecs/Video/DVDVideoCodecIMX.cpp in 0418c2b Mar 5, 2015

and same here
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment