Your fix seems incomplete.
It doesn't prevent writing in the unused surface area when doing pixel copies and pixel runs and I am not sure they can't cross lines.

@lephilousophe can you be more specific? I don't understand the code that much but output_end (which is terminated by width, not by pitch) should handle all overflows? (the example I posted in PR description is from another class where such problem can occur, yes).

output_end is the end of the image.
If you are at the end of a line and start copying n bytes, I think it should wrap to the beginning of the next line.
In the case where pitch != w the data will continue beyond w but not wrap to the next line: it will end up in the unused area.

I must say that I don't know RLE and that this cross line coding may be forbidden by the specification.

Ah, right, now I see what you mean. Judging from the value == 0 which explicitly marks end of line I'd say this scenario shouldn't be supported but I'm also not 100% sure. The description isn't very explicit about this either but since the deltas are just byte values, I'd say there's a pretty good chance they did not consider having pictures with width less then 256 pixels filled over to the next line.

I'd argue that if this assumption turns out to be false, I'd be the first to notice as it seems that Atari is the only backend having w != pitch for 8bpp graphics.

Btw do you know how to quickly test the 4-bit variant? I could fix & test that one as well.

Btw do you know how to quickly test the 4-bit variant? I could fix & test that one as well.

I don't. :(

Maybe @sev- does, he added it in d35cdf5, only 7 years ago. :-) Looking at https://docs.scummvm.org/en/latest/help/release.html#myst-ery-u-f-o-s-release-2016-10-17 doesn't show me anything concrete to identify the codec...

IIRC, that was for the GNAP engine.

Tried to run U.F.O.s on my Linux and while it has quite amusing animations and gameplay, I wasn't able to hit the decode4 function.

Since I know better than fixing code which I can't test afterwards, I propose to take this PR as is since it fixes something which I can verify and doesn't seem to break anything else.

The testbed engine has a generic video player that can be used for testing codec changes. It's currently hardcoded to use the QuickTime decoder, but that can be swapped with a different one with relative ease. The testbed changes in PR #4736 improve this feature further.

Okay, thank you!