[RetroPlayer] Add basic resolution changing

[GTechAlpha]

Description

Add basic resolution changing.

Motivation and Context

RetroPlayer does not render after resolution change.

How Has This Been Tested?

PCSX, GenesisPlus, Nestopia. Example: FFVII on PCSX changes resolution every time the in-game menu is accessed.

Screenshots (if appropriate):

Types of change

• [X] Bug fix (non-breaking change which fixes an issue)
• [x] Improvement (non-breaking change which improves existing functionality)
• [ ] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)

[garbear]

In general this is a good approach. I only found one flaw, which I sent you in https://github.com/GTechAlpha/xbmc/pull/1. Once this gets cleaned up it's good to go.

[garbear]

https://github.com/GTechAlpha/xbmc/pull/3

[garbear]

I see one more change that's needed:

std::map<AVPixelFormat, SwsContext*> m_scalers;

m_scalers needs to be indexed by format, width and height instead of just format.

[GTechAlpha]

This should be good to go, AFAIK.

[garbear]

What about this? we could split configuration and reconfiguration: https://github.com/garbear/xbmc/commit/rp-video

Switch to Fullscreen msg should only be sent once. I imagine your amlogic patch doesn't need to be called multiple times either. "re"configuration of the renderers, by definition, doesn't need to happen in the first configuration. So having separate configuration and reconfiguration states makes sense.

[garbear]

Also this: https://github.com/garbear/xbmc/commit/rp-buffers

Buffers need to be protected by the buffer mutex.

[GTechAlpha]

The buffer is protected by m_bFlush plus this being on the render thread.

[garbear]

The game thread can access the buffers.

Also I looked into the m_scalers comment above and no changes are needed for this.

[garbear]

I see that m_bFlush protects the buffers. Still, should we be defensive?

[GTechAlpha]

It is inefficient to lock when unnecessary though right? Maybe a comment?

[garbear]

Comment is fine. When I see buffer access without a mutex I get worried, so explaining why would be good.

Also, thoughts on https://github.com/garbear/xbmc/pull/98#issuecomment-397124590?

[GTechAlpha]

What about this? we could split configuration and reconfiguration: rp-vide

Switch to Fullscreen msg should only be sent once. I imagine your amlogic patch doesn't need to be called multiple times either. "re"configuration of the renderers, by definition, doesn't need to happen in the first configuration. So having separate configuration and reconfiguration states makes sense.

Yes, the Amlogic code only needs to be called once. I think would be a good addition.

[garbear]

K, Configure() needs a way to know which state to set

[GTechAlpha]

I'll add when I get a chance.

[GTechAlpha]

Anything else on this one?

[garbear]

Almost :) In the buffer lock commit, m_bHasCachedFrame and m_cachedFrame should be reset within the lock. Also I'll send you a PR with a reconfig tweak that I think improves the code.

[garbear]

@GTechAlpha: https://github.com/GTechAlpha/xbmc/pull/4

[garbear]

squash into two commits and i'll merge!

[GTechAlpha]

:thumbsup: