obs-studio
obs-studio copied to clipboard
vlc-video: Fix display_width overflow
Description
Current behavior leads to overflow in computing source width with Sample Aspect Ratio. Changing to uint64_t fixed this problem.
Motivation and Context
Resolve #7666.
How Has This Been Tested?
Playing testing videos described in #7666.
Types of changes
Changed precision of computing display_width with SAR.
Checklist:
- [X] My code has been run through clang-format.
- [X] I have read the contributing document.
- [X] My code is not on the master branch.
- [X] The code has been tested.
- [X] All commit messages are properly formatted and commits squashed where appropriate.
- [X] I have included updates to all appropriate documentation.
Wouldn't it make more sense to put the devision into brackets to yield the ratio of those two big values (which usually should yield "1" for digital displays anyway) and then multiply with that comparatively small ratio?
Doing any kind of division with arbitrary values without using a fractional type is also not great to begin with, but not a fault of the PR per se.
If we put division into brackets, we must compute it as floating-point value, because ratio can be fraction (for example 4/3 = 1 in integer division). With integer division, first multiplication then division must be done and because we multiplying two 32bit values (in scenario with normal compiler), it cannot overflow with 64bit number type.
I try change as little code as possible in this fix, so I did it this way. Computing as floating-point values will be probably more elegant way.
We have the util_mul_div64() utility function, which may be of use here.
Sorry for late response. util_mul_div64 is definitely better option. I created new commit with this change.
Please fix the following warning:
D:\a\obs-studio\obs-studio\obs-studio\plugins\vlc-video\vlc-video-source.c(413,21): error C2220: the following warning is treated as an error [D:\a\obs-studio\obs-studio\obs-studio\build32\plugins\vlc-video\vlc-video.vcxproj]
D:\a\obs-studio\obs-studio\obs-studio\plugins\vlc-video\vlc-video-source.c(413,21): warning C4244: '=': conversion from 'uint64_t' to 'int', possible loss of data [D:\a\obs-studio\obs-studio\obs-studio\build32\plugins\vlc-video\vlc-video.vcxproj]
I guess you should simplify the PR and only keep the latest commit.
just git reset --soft f065f20 to the basic version, commit the changed again and then git push -f to your own repository.
@CoalZombik CoalZombik
@tuduweb Thank you for the recommendation.
It is squashed to one commit now, but I don't understand why Flatpak build failed.
@CoalZombik looks like the Flatpak failure was due to something outside this Pull Request. I restarted it and would expect it to complete. Edit: Actually, it probably won't complete. Looks like something upstream is down.