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

Fix getting stuck using VideoPlayer and breaking game launching #14478

Merged
merged 1 commit into from
Oct 2, 2018

Conversation

garbear
Copy link
Member

@garbear garbear commented Sep 26, 2018

When a game fails to launch, Kodi gets "stuck" using VideoPlayer. After enabling emulators, Kodi should choose RetroPlayer, but continues to use VideoPlayer, which continues to silently fail.

Description

When I launched a game with no emulators enabled, VideoPlayer tried to play the ROM and failed. However, the player core in app player was never reset, so after enabling emulators, Kodi continues trying to use the existing player to play games, which breaks all game launching.

To solve this, we reset the app player core, which allows Kodi to successfully create a new RetroPlayer core.

Motivation and Context

Discovered after deleting AddonDB and having all emulators disabled.

How Has This Been Tested?

Log when launching a game with emulators disabled. Kodi chooses VideoPlayer (fixed by #14480). VideoPlayer tries to open the game and silently fails:

Debug Print: CPlayerCoreFactory::GetPlayers: adding videodefaultplayer (VideoPlayer)
Debug Print: VideoPlayer::OpenFile: /Users/garrett/Library/Application Support/Kodi/userdata/addon_data/plugin.program.iagl/temp_iagl/Super Metroid (Japan, USA) (En,Ja).sfc
Debug Print: Open - error probing input format, /Users/garrett/Library/Application Support/Kodi/userdata/addon_data/plugin.program.iagl/temp_iagl/Super Metroid (Japan, USA) (En,Ja).sfc
Debug Print: OpenDemuxStream - Error creating demuxer
Debug Print: CVideoPlayer::OnExit()
Debug Print: Thread VideoPlayer 123145463148544 terminating
Debug Print: OnPlayBackStopped: CApplication::OnPlayBackStopped

Next, log when enabling emulators and launching a game without this patch. Kodi chooses RetroPlayer, however the existing VideoPlayer tries to open the game and silently fails:

Debug Print: CPlayerCoreFactory::GetPlayers: adding retroplayer
Debug Print: CPlayerCoreFactory::GetPlayers: added 1 players
Debug Print: VideoPlayer::OpenFile: /Users/garrett/Library/Application Support/Kodi/userdata/addon_data/plugin.program.iagl/temp_iagl/Super Metroid (Japan, USA) (En,Ja).sfc
Debug Print: Open - error probing input format, /Users/garrett/Library/Application Support/Kodi/userdata/addon_data/plugin.program.iagl/temp_iagl/Super Metroid (Japan, USA) (En,Ja).sfc
Debug Print: OpenDemuxStream - Error creating demuxer
Debug Print: CVideoPlayer::OnExit()
Debug Print: Thread VideoPlayer 123145463685120 terminating
Debug Print: OnPlayBackStopped: CApplication::OnPlayBackStopped

Next, log and screenshot when enabling emulators and launching a game with this patch. RetroPlayer successfully opens the game:

Debug Print: CPlayerCoreFactory::GetPlayers: adding retroplayer
Debug Print: CPlayerCoreFactory::GetPlayers: added 1 players
Debug Print: Select game client dialog: Found 3 candidates
Debug Print: Adding game.libretro.snes9x2002 as a candidate
Debug Print: Adding game.libretro.snes9x2010 as a candidate
Debug Print: Adding game.libretro.snes9x as a candidate

screen shot 2018-09-26 at 12 07 27 pm

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • None of the above (please explain below)

@DaveTBlake
Copy link
Member

Thanks for this @garbear, I think it has also helped with some of the issues users reported with playback of mixed playlists. On beta2 once a music video has played, then any subsequent music plays sound only without viz just a black screen. With no viz installed the last frame of the music video remained on the screen instead. Similar to your experience the log shows that the videoplayer had taken over playback and not cleared.

@DaveTBlake
Copy link
Member

@garbear could you take a look at playing a mxied playlist - a .m3u file with both music and music video files in it - with this PR, particularly look at letting playback flow naturally from one item to the next (so you want short files).

I'm hoping you can make sense of what happens. If the first items are songs they play fine, but then the first music video and subsequent items get skipped through. Use next/prev and they play correctly showing video or song art as appropriate, but natural flow to next item they don't.

@garbear
Copy link
Member Author

garbear commented Oct 2, 2018

@DaveTBlake I'm gonna merge to get this in Beta 3. We can look into the playlist issue for Beta 4.

@garbear garbear merged commit 8777af5 into xbmc:master Oct 2, 2018
@garbear garbear deleted the fix-vp-stuck branch October 2, 2018 14:24
@Rechi Rechi added this to the Leia 18.0-beta3 milestone Oct 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Games Type: Fix non-breaking change which fixes an issue v18 Leia
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants