Skip to content

libobs/graphics: Update libnsgif to 1.0.0#11178

Open
RytoEX wants to merge 1 commit into
obsproject:masterfrom
RytoEX:really-update-libnsgif
Open

libobs/graphics: Update libnsgif to 1.0.0#11178
RytoEX wants to merge 1 commit into
obsproject:masterfrom
RytoEX:really-update-libnsgif

Conversation

@RytoEX

@RytoEX RytoEX commented Aug 23, 2024

Copy link
Copy Markdown
Member

Description

Update libnsgif to from some ancient version to 1.0.0.

Motivation and Context

Wanted to attempt to update this while tackling #10993.

How Has This Been Tested?

Tested on Windows 11 with an animated GIF.

Types of changes

  • Tweak (non-breaking change to improve existing functionality)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

@RytoEX RytoEX added the kind/enhancement Enhancements are not bugs or new features but can improve usability or performance. label Aug 23, 2024
@RytoEX RytoEX requested review from Lain-B and derrod August 23, 2024 18:18
@RytoEX

RytoEX commented Aug 23, 2024

Copy link
Copy Markdown
Member Author

This could use further review, particularly on the struct changes, but generally on everything because I've never had to touch these files before.

This could also use further testing.

Comment thread libobs/cmake/os-windows.cmake Outdated
)

target_compile_options(libobs PRIVATE $<$<COMPILE_LANGUAGE:C,CXX>:/EHc->)
target_compile_options(libobs PRIVATE $<$<COMPILE_LANGUAGE:C,CXX>:/EHc-> /wd4244 /wd4267)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really a fan of disabling these warnings for all of libobs just because of a vendored dependency. Can we perhaps move it into deps/?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can probably try, but it will require a "bit" more work.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be clear, I'm not a fan of it either.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An attempt was made:
https://github.com/RytoEX/obs-studio/tree/really-update-libnsgif-and-move

However, while libobs compiles, other projects that link to libobs that do #include <graphics/image-file.h> (obs-filters, obs-transitions, etc.) complain that they cannot find nsgif.h. I'm not sure what to do about that.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed this by using set_source_files_properties to only add the warning suppression to the libnsgif files instead of all of libobs. I still don't have the time to rework this into moving libnsgif into deps. Hopefully this change addresses the primary concern.

@RytoEX RytoEX force-pushed the really-update-libnsgif branch from 3599ae1 to f42b501 Compare August 23, 2024 19:56
@RytoEX

RytoEX commented Aug 23, 2024

Copy link
Copy Markdown
Member Author

Fixing clang-format complaint, hopefully.

@Lain-B Lain-B left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to get this moved over to the deps dir but I don't think that it's a big deal for now, especially if we want to fix up issues for the next update.

@norihiro

norihiro commented Aug 25, 2024

Copy link
Copy Markdown
Contributor

I'd like to note that this breaks the ABI for 3rd party plugins using struct gs_image_file.

Probably, it's still safe for most cases because these conditions are satisfied.

  • The size of gs_image_file does not grow.
  • The 3rd party plugin does not access the member variables gif or later.
    Majority of 3rd party plugins just access texture, cx, cy, and loaded.

@norihiro

Copy link
Copy Markdown
Contributor

I want to suggest to hide nsgif.h from the exported header file so that the it does not require to disable some warnings and also we can update libnsgif in the future.
I made a commit on my branch. https://github.com/norihiro/obs-studio/tree/really-update-libnsgif-internal

@RytoEX

RytoEX commented Aug 26, 2024

Copy link
Copy Markdown
Member Author

cc @Lain-B and @derrod on the breaking changes concern raised by @norihiro

My initial and primary concern was working out an update to an outdated vendored dependency. If there are additional concerns, such as moving the dependency to deps/ or mitigating breaking changes, those merit attention. Unfortunately, I do not have time to address either of them. If someone is able to build upon the work here and address either of those concerns, feel free.

@RytoEX RytoEX force-pushed the really-update-libnsgif branch from f42b501 to 3ed6734 Compare February 7, 2025 21:25
@RytoEX RytoEX force-pushed the really-update-libnsgif branch from 3ed6734 to 1852a17 Compare February 7, 2025 21:34
@RytoEX

RytoEX commented Feb 7, 2025

Copy link
Copy Markdown
Member Author

Rebased and made the warning suppression more targeted.

Will probably need @Lain-B to comment on the ABI breakage concern.

@Warchamp7 Warchamp7 added this to the OBS Studio 33.0 milestone May 16, 2026
@Warchamp7 Warchamp7 linked an issue Jun 30, 2026 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/enhancement Enhancements are not bugs or new features but can improve usability or performance.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GIFs from EmoteLab freeze every second

5 participants