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

Upside-down image sent to AI translation service when using HW renderer #16275

Closed
bslenul opened this issue Feb 22, 2024 · 19 comments
Closed

Upside-down image sent to AI translation service when using HW renderer #16275

bslenul opened this issue Feb 22, 2024 · 19 comments

Comments

@bslenul
Copy link
Contributor

bslenul commented Feb 22, 2024

Description

When using the AI translation hotkey with a HW rendered core, the screenshot is sent upside-down to the translation server, making the translation impossible.

For example this is my RetroArch window:

image

and this is what the translation server received:

image

AFAICT it doesn't happen with software rendered cores.

Expected behavior

Send screenshot in the correct position.

Actual behavior

Send screenshot upside-down.

Steps to reproduce the bug

  1. Set AI translation stuff (AI service and a AI hotkey).
  2. Start a game with a HW rendered core (I was able to reproduce with Beetle PSX HW, SwanStation and Flycast).
  3. Press the AI hotkey, translation should be gibberish.
  4. Check your last screenshot sent on the AI translation website you used.

Bisect Results

Bisected to 274d47f

Version/Commit

Environment information

  • OS: Windows 10 but @Sanaki confirmed it happening on Linux as well
@zoltanvb
Copy link
Contributor

zoltanvb commented Mar 2, 2024

Tagging the original PR #15640 in hope of more attention - maybe this will ring a bell to someone.

@hizzlekizzle
Copy link
Contributor

Did the translation service work with HW-rendered cores before this was merged? I was thinking it didn't, though I don't know if this is the reason why.

@bslenul
Copy link
Contributor Author

bslenul commented Mar 2, 2024

Yes it used to work fine, the same Ikaruga screenshot with stable RA 1.16.0 for example:

image

It doesn't work with D3D11 tho, but that's because the driver doesn't support screenshots.

@PCdasala
Copy link

PCdasala commented Mar 4, 2024

orque fechada se não resolveu?

@bslenul
Copy link
Contributor Author

bslenul commented Mar 5, 2024

orque fechada se não resolveu?

What?

@cpod12 @xunkar any idea about the issue?

@xunkar
Copy link
Contributor

xunkar commented Mar 6, 2024

I do actually. I will not be able to look to much into it right away but off the top of my head I remember that the frame was returned upside down by the rendered, so we had to manually invert it before sending it to the AI service. It would seem that this particular renderer configuration causes the frame to be capture in the correct position from the start, if we can detect the renderer being used we can simply ignore this step.

@viachaslavic
Copy link
Contributor

@xunkar here also another case is described for GNU/Linux, when retroarch crashes when using the hw-rendered core.
I tried to debug a little and found that the crash occurs after read_viewport() call at

&& video_st->current_video->read_viewport(
This call executes successfully in the main loop, but may not have access to the viewport resource after being moved to the task context.

X Error of failed request:  BadAccess (attempt to access private resource denied)
  Major opcode of failed request:  152 (GLX)
  Minor opcode of failed request:  26 (X_GLXMakeContextCurrent)
  Serial number of failed request:  3684
  Current serial number in output stream:  3684
 double free or corruption (!prev)
 Aborted (core dumped)

Env

  • Arch Linux
  • X11 (amdgpu)
  • RetroArch: 1.17.0 / ac69414
  • SwanStation : GPU Renderer - Hardware (OpenGL)

@whoiamwhy
Copy link

whoiamwhy commented Mar 20, 2024

the same problem with Citra and any 3ds game, RA 1.17.0
image

@LibretroAdmin
Copy link
Contributor

LibretroAdmin commented Mar 21, 2024

Looks like it can't even be reverted anymore, too much stuff has changed.

So it looks like 1.18.0 will have this regression in it until @xunkar fixes this.

@cpod12
Copy link
Contributor

cpod12 commented Mar 21, 2024

Hey, I'm sorry about this and replying so late on this thread. I can try to take a look at this too since I did some work on that PR

@LibretroAdmin
Copy link
Contributor

That'd be awesome yes if this issue can be fixed.

@xunkar
Copy link
Contributor

xunkar commented Mar 21, 2024

Wait, something's toubling me here. OP showed that Ikaruga was flipped vertically, but with Citra it's flipped horizontally? I'm confused.

I've found the part of the code I mentioned earlier. It's legacy code from Beaker that I left as is. He said as a comment: "Returned output from the png processor is an upside down RGBA image, so we have to change that to RGB first. This should probably be replaced with a scaler call."

As it stands, a simple conditional statement on this block to filter out the problematic renderers should be enough. But the question about the difference in cores is most important, it might require much more in-depth testing.

@LibretroAdmin
Copy link
Contributor

@xunkar Could it be that some cores use 32-bit color and others 16bit color? For 32bit it's XRGB8888 and for 16bit RGB565

@bslenul
Copy link
Contributor Author

bslenul commented Mar 21, 2024

Wait, something's toubling me here. OP showed that Ikaruga was flipped vertically, but with Citra it's flipped horizontally? I'm confused.

Yeah idk, just tried Citra and for me it's upside-down just like the other affected cores:

images

@BarryJRowe
Copy link
Contributor

Firstly, is it verified that screenshots themselves are not flipped upside down in these cases? If that’s the case then we should not fix it at the ai service level.

@xunkar The op stated that the image the server gets is upside down. The code for decoding pngs is used in the callback after getting the response back from the server since pngs are naturally bottom to top encoded and have to be flipped for rendering.

@xunkar
Copy link
Contributor

xunkar commented Mar 22, 2024

Then what we need is a scaler call in translation_frame_encode right after this call, or as a temporary fix, get rid of the ifdef altogether and just send the image as BMP.

buffer = rpng_save_image_bgr24_string(

@hizzlekizzle
Copy link
Contributor

Sending BMPs of hardware-accelerated cores could end up with some very large files, right?

@xunkar
Copy link
Contributor

xunkar commented Mar 22, 2024

The frames are grabbed as native core resolution, so for the most part it makes little difference. Before the refactoring only BMP was supported and there wasn't much complain, I still added PNG support to be safe.

@LibretroAdmin
Copy link
Contributor

LibretroAdmin commented Apr 6, 2024

This is still not fixed isn't it?

I am planning a release of 1.18.0 sometime next week. We will see if by then someone will have submitted a PR to fix this issue or not.

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

No branches or pull requests

10 participants