Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[rbp] [retroplayer] Add zero copy mmal support to retroplayer #62

Merged
merged 13 commits into from
Aug 5, 2016

Conversation

popcornmix
Copy link

No description provided.

@popcornmix
Copy link
Author

Probably not working, but it does compile. I'm not currently able to build the emulator add-ons - getting "bnes needs RPi build command in CMakeLists.txt!"

@garbear
Copy link
Owner

garbear commented Jul 13, 2016

I'm not sure why this worked before but not now, you can see the error msg is here and results from the "rbpi" platform not having a build command. If it worked before, it was probably because it was using the "linux" core system name. Which of these is it set to currently?

@popcornmix
Copy link
Author

I'm not 100% sure if I used the emulators I built last time, or the ones from LE repo.
However if I ignore the add-ons that report "needs RPi build command", it still fails with:

CMake Error at /usr/local/lib/kodi/addon-helpers.cmake:131 (message):
CMAKE_INSTALL_PREFIX /opt/xbmc-retro/arm-linux-gnueabihf differs from Kodi
prefix /usr/local. Please pass -DOVERRIDE_PATHS=1 to skip this check

@garbear
Copy link
Owner

garbear commented Jul 14, 2016

Hm, I've encountered this problem before. I applied adfc09f, but I can't remember if this fixed the problem or not.

@garbear
Copy link
Owner

garbear commented Jul 20, 2016

Is it fine if I merge this now? I'll keep the commits separate and if there's any changes you can just send another PR to this branch.

@popcornmix
Copy link
Author

A mmal commit has just gone into master which has a minor conflict with this.
You can merge this now, but you will likely hit the conflict when you next rebase.

If you update retroplayer-17alpha2 branch I can rebase this commit now,
or you can give me a ping in future if you get a conflict.

@garbear
Copy link
Owner

garbear commented Jul 20, 2016

which PR? I'll cherry pick it and you can rebase on that, then next time I rebase on a release (alpha 3) this will apply cleanly

@popcornmix
Copy link
Author

xbmc#10120

@popcornmix
Copy link
Author

(although you might need xbmc#10144 to apply that one...)

@garbear
Copy link
Owner

garbear commented Jul 20, 2016

I cherry-picked that PR (and 4 others it required), but this PR still applies cleanly. Are these PRs needed?

@popcornmix
Copy link
Author

I had a conflict here: 4828fc2#diff-c7916b114ea763a60c3ab748710a75d7R279
on my own tree as that line is no longer present on master...

I am in the process of building with adfc09f present to see if that fixes my issue.

With xbmc#10120 in place 18028ae can be dropped (although the next commit will need fixing up).

@garbear
Copy link
Owner

garbear commented Jul 20, 2016

I plan to rebase on 17 alpha 3 when it's released (around 1 August), do you just want to wait until then?

@popcornmix
Copy link
Author

I've rebased. Hit a build error:

xbmc/tools/depends/target/binary-addons/arm-linux-gnueabihf/build/peripheral.joystick/src/addon.cpp:288:31: error: 'class JOYSTICK::CJoystick' has no member named 'SupportsPowerOff'
   if (!joystick || !joystick->SupportsPowerOff())

Any ideas?

@garbear
Copy link
Owner

garbear commented Jul 20, 2016

xbmc#9663 adds power off support to the Peripheral API. My alpha 2 branch is applied on top of this PR. it looks like you're trying to build against master's headers, try building against RetroPlayer's peripheral API.

@popcornmix
Copy link
Author

Are you just saying a need to do a clean build from this branch?

I think the new commit have also broken something:

RetroPlayerAudio.cpp: In member function 'virtual bool GAME::CRetroPlayerAudio::OpenEncodedStream(AVCodecID, unsigned int, const CAEChannelInfo&)':
RetroPlayerAudio.cpp:121:69: error: no matching function for call to 'CDVDFactoryCodec::CreateAudioCodec(CDVDStreamInfo&, bool)'
   m_pAudioCodec.reset(CDVDFactoryCodec::CreateAudioCodec(hint, false));

@garbear
Copy link
Owner

garbear commented Jul 20, 2016

This looks correct, it references [CDVDFactoryCodec::CreateAudioCodec()](https://github.com/garbear/xbmc/blob/retroplayer-17alpha2/xbmc/cores/VideoPlayer/DVDCodecs/DVDFactoryCodec.h#L44]. The third parameter (bool) is optional, so calling with a CDVDStreamInfo& and a bool should compile.

Previously we would always wait an extra vsync (even if one had been missed) in RenderUpdate.
Now if a single vsync is missed, we won't wait.
Pool containing mmal buffers (e.g. decoded pictures) need to be attached
to a mmal components port.

Managing the lifespan of these is tricky, as renderer may consitue to
display an image after the decoder has been closed and a new one opened.

So create a class for handling this. The class creates and destroys the
mmal component associated with the pool, and creates the mmal pool
(which is just headers) as well as handling allocation of the gpu buffers.

Each object from the pool includes a shared pointer to the pool class,
ensuring the pool and mmal component are not destroyed until the last
object is destroyed.

The pool class is used by both MMAL hw decode and the FFMPEG zero copy
decoders which avoids some special casing in the renderer.

It will also handle the pool of deinterlaced frames later.
mmal callbacks all come from a common thread, so blocking in a callback
is inefficient at best and can risk deadlocks.

Creating a dedicated thread for processing mmal buffers
(including the reconfiguration when formats change) avoids this.
@popcornmix
Copy link
Author

I've rebased, but I'm struggling to run anything. Previously I was using this repo:
https://retroplayer.freestylephenoms.com/8.0/RPi2/arm/addons.xml

I get as far as my code;

16:20:45 T:1661989904    INFO: RetroPlayerVideo: Creating video stream with pixel format: 44, 320x240
16:20:45 T:1661989904   DEBUG: CPixelConverter::Open: pixfmt:44(rgb565le) targetfmt:44(rgb565le) 320x240

but get a segfault from the game itself (e.g. 2048 or MAME 2010). Is that repo up to date with this branch's API?

@popcornmix popcornmix force-pushed the retroplayer branch 2 times, most recently from ebde7c3 to 1c6c38e Compare July 22, 2016 14:13
@popcornmix popcornmix force-pushed the retroplayer branch 2 times, most recently from f1da25a to 3cb1e3d Compare July 22, 2016 15:43
@popcornmix
Copy link
Author

Finally got retroplayer working again on Pi.
I've updated this PR to include the upstream MMAL render commits plus updated support for MMAL rendering from retroplayer

I did find the second game I ran always crashes. Looks to be a bug here:
kodi-game/game.libretro@68d6351#commitcomment-18360022
(possibly combined with something adding but not removing input ports).

But this PR should be okay for merging.

@garbear garbear merged commit 285a242 into garbear:retroplayer-17alpha2 Aug 5, 2016
@garbear
Copy link
Owner

garbear commented Aug 5, 2016

Cheers. I'll fix the game.libretro issue

@popcornmix popcornmix deleted the retroplayer branch August 5, 2016 18:55
garbear added a commit that referenced this pull request Aug 6, 2016
Thanks to elpendor for RGB565 support, poisson for RAII improvements,
ChrisMyhre for catching a compile error, notspiff for CMake fixes and
popcornmix for Raspberry Pi support (#62).

The RetroPlayer commit from the retroplayer-15.2 branch (20975ad) contains a
TODO about `g_renderManager.IsStarted()` that may or may not apply still.
garbear added a commit that referenced this pull request Aug 6, 2016
Thanks to elpendor for RGB565 support, poisson for RAII improvements,
ChrisMyhre for catching a compile error, notspiff for CMake fixes and
popcornmix for Raspberry Pi support (#62).

The RetroPlayer commit from the retroplayer-15.2 branch (20975ad) contains a
TODO about `g_renderManager.IsStarted()` that may or may not apply still.
garbear added a commit that referenced this pull request Aug 6, 2016
Thanks to elpendor for RGB565 support, poisson for RAII improvements,
ChrisMyhre for catching a compile error, notspiff for CMake fixes and
popcornmix for Raspberry Pi support (#62).

The RetroPlayer commit from the retroplayer-15.2 branch (20975ad) contains a
TODO about `g_renderManager.IsStarted()` that may or may not apply still.
garbear added a commit that referenced this pull request Aug 6, 2016
Thanks to elpendor for RGB565 support, poisson for RAII improvements,
ChrisMyhre for catching a compile error, notspiff for CMake fixes and
popcornmix for Raspberry Pi support (#62).

The RetroPlayer commit from the retroplayer-15.2 branch (20975ad) contains a
TODO about `g_renderManager.IsStarted()` that may or may not apply still.
garbear added a commit that referenced this pull request Aug 6, 2016
Thanks to elpendor for RGB565 support, poisson for RAII improvements,
ChrisMyhre for catching a compile error, notspiff for CMake fixes and
popcornmix for Raspberry Pi support (#62).

The RetroPlayer commit from the retroplayer-15.2 branch (20975ad) contains a
TODO about `g_renderManager.IsStarted()` that may or may not apply still.
garbear added a commit that referenced this pull request Aug 6, 2016
Thanks to elpendor for RGB565 support, poisson for RAII improvements,
ChrisMyhre for catching a compile error, notspiff for CMake fixes and
popcornmix for Raspberry Pi support (#62).

The RetroPlayer commit from the retroplayer-15.2 branch (20975ad) contains a
TODO about `g_renderManager.IsStarted()` that may or may not apply still.
garbear added a commit that referenced this pull request Aug 6, 2016
Thanks to elpendor for RGB565 support, poisson for RAII improvements,
ChrisMyhre for catching a compile error, notspiff for CMake fixes and
popcornmix for Raspberry Pi support (#62).

The RetroPlayer commit from the retroplayer-15.2 branch (20975ad) contains a
TODO about `g_renderManager.IsStarted()` that may or may not apply still.
garbear added a commit that referenced this pull request Aug 8, 2016
Thanks to elpendor for RGB565 support, poisson for RAII improvements,
ChrisMyhre for catching a compile error, notspiff for CMake fixes and
popcornmix for Raspberry Pi support (#62).

The RetroPlayer commit from the retroplayer-15.2 branch (20975ad) contains a
TODO about `g_renderManager.IsStarted()` that may or may not apply still.
@garbear
Copy link
Owner

garbear commented Aug 9, 2016

The crash should be fixed now. I'm uploading builds for the new branch based on 17alpha3

garbear pushed a commit that referenced this pull request Jan 4, 2017
[addons] cleanup and improve changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants