Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 889412 - media-libs/libsdl-1.2.15* -- drop obsolete(?) libsdl-1.2.15-gamma.patch?
Summary: media-libs/libsdl-1.2.15* -- drop obsolete(?) libsdl-1.2.15-gamma.patch?
Status: IN_PROGRESS
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Current packages (show other bugs)
Hardware: All Linux
: Normal normal (vote)
Assignee: Gentoo Games
URL: https://github.com/libsdl-org/SDL-1.2...
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-01-02 20:09 UTC by Sebastian Pipping
Modified: 2023-05-27 15:43 UTC (History)
1 user (show)

See Also:
Package list:
Runtime testing required: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Sebastian Pipping gentoo-dev 2023-01-02 20:09:08 UTC
Hi!

It has come to my attention that all in-tree ebuilds of media-libs/libsdl-1.2.15* apply a patch libsdl-1.2.15-gamma.patch.  So I was wondering if it should be upstreamed, and found that:
- The patch was introduced in Gentoo bug #449692
- Upstream Git repo https://github.com/libsdl-org/SDL-1.2 does not have the patch applied
- Upstream issue tracker has a closed ticket https://github.com/libsdl-org/SDL-1.2/issues/717 about it
- The conversion quoted there links to an Xorg bug https://bugs.freedesktop.org/show_bug.cgi?id=27222
- The Xorg bug has a comment "Fix verified with xorg-server-1.19.0" by Petr Pisar, the author of the patch (also in CC)

Should the patch be kept or dropped or discussed again with upstream?

Thanks and best, Sebastian
Comment 1 Petr Pisar 2023-01-03 20:11:43 UTC
I rested the patch again:

media-libs/libsdl-1.2.15_p20221201
x11-base/xorg-server-21.1.4
x11-drivers/xf86-video-ati-19.1.0-r1

The patch is needed. If I recompile media-libs/libsdl-1.2.15_p20221201 without the patch, test/testgamma.c from libsdl sources stops working.

It's in-line with a comment <https://bugs.freedesktop.org/show_bug.cgi?id=27222#c26> in the Xorg bug: the feature has not yet been implemented in modsetting drivers. Which are almost all modern Xorg driver including my x11-drivers/xf86-video-ati.

I don't think the patch harmful. It simply avoids using a function which is not properly supported.
Comment 2 Sebastian Pipping gentoo-dev 2023-01-03 22:29:22 UTC
Hi Petr,

thanks for the update!  Please excuse my use of the word "harmful" previously, it was a theoretical possibility, I had no intentions of offending you or your work.  Thanks for the patch!

Would you be willing to take this upstream as a new issue at https://github.com/libsdl-org/SDL-1.2/issues ?  I would be able to write the ticket but I would not be able to answer questions about details like you would.  What do you think?

Best, Sebastian
Comment 3 James Le Cuirot gentoo-dev 2023-01-03 22:35:17 UTC
Be prepared for upstream to say they don't care because 1.2 is dead. We're about to switch to sdl12-compat anyway. Worth a go though, they are still merging fixes, despite the deadness.
Comment 4 Petr Pisar 2023-01-04 06:12:05 UTC
Unfortunately, I'm not willing to go to the upstream again. The closed upstream bug was mine. I don't see a point in opening new and new bugs, especially if upstream lost interest in SDL 1.2. They focus on 2.0 and pondering 3.0 line.

From Gentoo point of view, I recommend migrating to sdl12-compat. It's an SDL 1.2 ABI reimplemented on top of SDL 2.0 library. While it's not perfect, it's good enough replacement. When I was involved in SDL in Fedora distribution, we made a special effort to a good replacement. E.g. it passes a pretty extensive perl-SDL testsuite without any changes. You will be relieved from carrying dozens of patches for media-libs/libsdl, especially security ones for handling various sound file formats.
Comment 5 Sebastian Pipping gentoo-dev 2023-01-04 18:09:18 UTC
Thanks for your comments!

I have opened a pull request upstream now, see URL field.  Let's see what we get.

Regarding sdl12-compat, bug #834629 may be of interest.
Comment 6 Sebastian Pipping gentoo-dev 2023-01-05 00:49:58 UTC
Petr, the patch got merged (unmodified) upstream but shortly after the merge
someone voiced concerns about video drivers not supporting SetGamma [1].
I'm considering to help out with a follow-up pull request to address that wish.
Would this version of SDL_SetGamma fix both the old and the new problem?:

  int SDL_SetGamma(float red, float green, float blue)
  {
      int succeeded = -1;
      SDL_VideoDevice *video = current_video;
      SDL_VideoDevice *this  = current_video;    
  
      if ( video->SetGamma ) {
          succeeded = video->SetGamma(this, red, green, blue);
      } else {
          Uint16 ramp[3][256];

          CalculateGammaRamp(red, ramp[0]);
          CalculateGammaRamp(green, ramp[1]);
          CalculateGammaRamp(blue, ramp[2]);

          succeeded = SDL_SetGammaRamp(ramp[0], ramp[1], ramp[2]);
      }
      return succeeded;
  }

Thanks in advance!

--
[1] https://github.com/libsdl-org/SDL-1.2/pull/874#issuecomment-1371299070
Comment 7 Sebastian Pipping gentoo-dev 2023-01-05 03:32:01 UTC
@Petr, quick update: Upstream reverted now and applied this fix instead https://github.com/libsdl-org/SDL-1.2/commit/d4b1811edcbd2cc14b192c0dc7b4d5f25cc8864d .  What do you think?
Comment 8 Petr Pisar 2023-01-05 19:14:52 UTC
That upstream commit works for me either. It's cleaner in disabling the X11 function call. Use that.
Comment 9 Sebastian Pipping gentoo-dev 2023-01-05 21:58:05 UTC
I think we can do 1.2.15_p20230105 or 1.2.15_p20221201-r1 or wait for 1.2.16.  I'm good either way and happy to help realize any of these.  Any thoughts from @games?
Comment 10 Pacho Ramos gentoo-dev 2023-05-27 14:42:34 UTC
I think this is obsolete with newer 1.2.6* versions
Comment 11 Sebastian Pipping gentoo-dev 2023-05-27 15:32:14 UTC
(In reply to Pacho Ramos from comment #10)
> I think this is obsolete with newer 1.2.6* versions

Yes, but we still have 1.2.15_p20221201 with libsdl-1.2.15-gamma.patch in tree and there is no release 1.2.16 upstream (https://github.com/libsdl-org/SDL-1.2/tags).  Personally I think if we want to close this ticket we should ask upstream for a new release, package 1.2.16, and then close.  What do you think?
Comment 12 Sam James archtester Gentoo Infrastructure gentoo-dev Security 2023-05-27 15:43:11 UTC
(In reply to Sebastian Pipping from comment #11)
> (In reply to Pacho Ramos from comment #10)
> > I think this is obsolete with newer 1.2.6* versions
> 
> Yes, but we still have 1.2.15_p20221201 with libsdl-1.2.15-gamma.patch in
> tree and there is no release 1.2.16 upstream
> (https://github.com/libsdl-org/SDL-1.2/tags).  Personally I think if we want
> to close this ticket we should ask upstream for a new release, package
> 1.2.16, and then close.  What do you think?

Feel free to add a newer snapshot, but it's shadowed in ~arch so I don't think it'll get much testing.

Not sure it's a big deal given we have the new shim stuff, but if you're bothered by it, no objection if you want to handle it ofc. Thanks!