RetroArch icon indicating copy to clipboard operation
RetroArch copied to clipboard

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

Open bslenul opened this issue 1 year ago • 7 comments

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 274d47f957c917b2139d7248cd0589c25725f656

Version/Commit

  • RetroArch: 1.17.0 / 0f36082

Environment information

  • OS: Windows 10 but @Sanaki confirmed it happening on Linux as well

bslenul avatar Feb 22 '24 11:02 bslenul

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

zoltanvb avatar Mar 02 '24 14:03 zoltanvb

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.

hizzlekizzle avatar Mar 02 '24 16:03 hizzlekizzle

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.

bslenul avatar Mar 02 '24 17:03 bslenul

orque fechada se não resolveu?

PCdasala avatar Mar 04 '24 14:03 PCdasala

orque fechada se não resolveu?

What?

@Cpod12 @xunkar any idea about the issue?

bslenul avatar Mar 05 '24 19:03 bslenul

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.

xunkar avatar Mar 06 '24 12:03 xunkar

@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 https://github.com/libretro/RetroArch/blob/ac694145f03c7c87edf4942075f3a8d593d4ffdf/tasks/task_translation.c#L1250 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 / https://github.com/libretro/RetroArch/commit/ac694145f03c7c87edf4942075f3a8d593d4ffdf
  • SwanStation : GPU Renderer - Hardware (OpenGL)

viachaslavic avatar Mar 06 '24 22:03 viachaslavic

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

whoiamwhy avatar Mar 20 '24 22:03 whoiamwhy

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.

LibretroAdmin avatar Mar 21 '24 21:03 LibretroAdmin

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

cpod12 avatar Mar 21 '24 21:03 cpod12

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

LibretroAdmin avatar Mar 21 '24 22:03 LibretroAdmin

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." https://github.com/libretro/RetroArch/blob/06fa5325f8b3cd42e6fba3d57835d5924c9ea2e7/tasks/task_translation.c#L768

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.

xunkar avatar Mar 21 '24 22:03 xunkar

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

LibretroAdmin avatar Mar 21 '24 22:03 LibretroAdmin

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

bslenul avatar Mar 21 '24 23:03 bslenul

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.

BarryJRowe avatar Mar 22 '24 02:03 BarryJRowe

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.

https://github.com/libretro/RetroArch/blob/06fa5325f8b3cd42e6fba3d57835d5924c9ea2e7/tasks/task_translation.c#L1363

xunkar avatar Mar 22 '24 09:03 xunkar

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

hizzlekizzle avatar Mar 22 '24 13:03 hizzlekizzle

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.

xunkar avatar Mar 22 '24 15:03 xunkar

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.

LibretroAdmin avatar Apr 06 '24 17:04 LibretroAdmin