The frontend UI assumes transitions are always available and crashes if
none are present. Resolving the crashes resulted in a completely broken
UI, so it's safe to assume that transitions are a core requirement for a
working OBS.
Crash reports indicate that obs-transitions is failing to load for some
users, so we should handle the case where no transitions load and abort
with an error instead of crashing.
This process mitigation flag is inherited by child processes and applies
to load-time DLL resolution, pushing the application's own directory
below the system directories. If there are system-installed copies of
dependent DLLs such as FFmpeg, those get loaded when OBS is updated as
the updater and subsequent relaunch of OBS inherit the mitigation.
As we use SetDefaultDllDirectories, this mitigation is less important,
the proper fix would be to use /DEPENDENTLOADFLAG to protect load-time
import resolution.
We suspect users hit errors updating the system-wide hook files even if
the file is not in use, but without logging we have no way to know for
sure. This commit adds additional logs. Not logging source paths
anywhere is intentional to avoid wide char to utf-8 issues making this
more complicated than it needs to be - the source will always be the OBS
install dir regardless.
More useless "security" software to try to prevent screenshots that
installs global hooks and kernel drivers. Crash reports indicate that
their hook attaches to and subsequently crashes OBS.
We already started using SetProcessMitigationPolicy before #5164 got
merged and we dropped Windows 7 support, so we can do away with the
runtime detection too.
DEP: Enable DEP as it's still opt-in even on Windows 10. No OBS code or
plugins should be executing data as code and this is an important
mitigation against stack-based buffer overflows.
ASLR: Enable and force ASLR. This makes it harder for any potential
exploits to use fixed offsets into OBS or Windows DLLs to run gadgets.
Extension Points: Disable extension points. These are typically used for
system-wide code injection, and we have generally had trouble with
various things injecting into OBS and causing issues.
Handle Check: Enable strict handle checks when running debug builds.
This will raise an exception if we operate on an invalid handle,
something that should hopefully not ever happen in the current code.
The main race was possible concurrent access to unordered_map which
could corrupt internal state. Our global variables were also not
atomically updated. DownloadWorkerThread also returned true on error,
which could cause the updater to proceed without all updates being
downloaded which would then fail at the install stage.
The default of 2500 kbps was chosen 10 years ago and times have changed.
Logs and forums posts show that many users of OBS for recording don't
change their bitrate and end up with a 2.5 mbps recording which looks
terrible.
Now that service bitrate enforcement exists, this will be automatically
capped to the maximum bitrate for streaming services, so the only time
this should result in a problem is if the user's upload speed is the
limiting factor, hopefully rarer these days with increasing internet
speeds.
On Windows, shutdown() will not interrupt a blocking connect() call, so
happy_eyeballs_destroy could block until the remaining candidates timed
out. As happy_eyeballs_destroy is called in the RTMP connect path, this
would stall the RTMP connection and cause the winning candidate's socket
to be disconnected due to a timeout.
The code currently assumes that the array does not change during
execution, this assumption is violated when the darray resizes causing
previous pointers to point to invalid memory and cause undefined
behavior and crashes.
This may need refactoring in the future, this commit simply fixes the
issue for now.
While adding or updating files locked this mutex, the graphics thread
did not. As the update operation is not atomic, the graphics thread
might access the darray in the middle of an update, resulting in access
to freed memory (crash) if the files were updated at the same time.
A few reports came in of cache files with the correct size but full of
null bytes, presumably from a system crash and an SSD lying about buffer
flushes. This commit adds a checksum at the end of the compiled bytecode
so we don't try to use invalid data.
Co-Authored-By: derrod <dennis@obsproject.com>
Some errors include HTML links directing users to e.g. driver updates
or further information. Using a raw newline instead of <br> causes Qt to
skip parsing the HTML, resulting in an ugly mess of HTML displayed to
the user instead of the intended links.
Moving the cleanup to OBS_FRONTEND_EVENT_EXIT in #8735 only handled the
cleanup from the dockable window, as the regular stats window is deleted
on close when the UI is shut down. This caused an event handler leak
each time the window is manually closed, resulting in crashes. This code
looks a bit wrong since we delete the same handler in multiple places,
but this is due to the code being used by both the dock (non-closable)
and the window (closable). The OBS_FRONTEND_EVENT_EXIT handler handles
cleanup from the dockable stats window, and the window close handler now
handles cleanup from the non-dockable stats window.
The code assumed MFXCreateSession would always succeed, but it fails on
systems with no QSV implementations, causing a crash when we call
MFXClose later. Additional success checks for the other API calls were
also added.
The frontend API was never explicitly cleaned up which has lead to
several crashes from plugins continuing to call it even after OBS itself
has been destroyed. We now destroy the API after the exit event, so
further calls by plugins will be no-ops instead of potentially accessing
destroyed OBS state.
This also required some changes to our own use of the API, as we relied
on the OBSBasic destructor to clean up some callbacks, by which point
the API should have already been destroyed.
When #5169 implemented deferred encoder updates, it did not account for
the case when the encoder hadn't started yet. This means the encoder would
start and then immediately call update with the same settings it was
started with, which in the case of some hardware encoders would trigger
a reconfiguration request to the driver.
This software seems to be pre-installed on many Lenovo systems and has a
"Network Boost" feature that restricts data usage of background apps,
causing throughput issues when streaming with OBS.
Meta have filed a patent application on the Go Away feature. This goes
against the spirit of free software and puts the use of this feature in
questionable legal status, so let's remove this code until the patent
situation is resolved.
This reverts commits a593fe6755 and
dada82fec1.
This is actually a MainConcept redistributable and not related to Adobe.
Unfortunately Elgato Game Capture HD software relies on this dependency
when presenting the DirectShow device to OBS, so we unintentionally
blocked it from loading.
Instead of outright blocking, we now block only older versions than the
version shipped by Elgato, which has hopefully been patched to fix the
random crashes.
It's possible that frame_ready is false when this function is called,
which implies that the mp_decode frame field is not valid. However we
dereference the frame by counting the number of audio channels before
checking the value of frame_ready, causing a crash.
We were calling avio_close on a field that might have held an
AVIOContext. This commit checks which type was allocated so we
can call the appropriate cleanup function.
Telling a user to "Check your driver is up to date" is too open to
interpretation, many users will use Windows Update or Device Manager to
check for updates which doesn't help. Instead, let's explicitly tell
them what is most likely to fix the problem.
Some send() errors are not treated as fatal but the connection gets shut
down regardless. When this happens, librtmp may send an FCunpublish
message which various services interpret as an "end of stream" message
and disable features like "disconnect protection". Instead, let's
explicitly close the socket so that the remote end is aware that this is
an unclean disconnect.
Adds a NtMapViewOfSection hook which we can use before DLLs are loaded
to decide whether to abort the process. This is useful for blocking
known problematic DLLs that cause crashes. In addition, the PE timestamp
can be checked to block only older DLLs such as older versions of Vtuber
virtual cameras with known deadlock bugs.
A simple counter of each DLL is used for logging purposes. Since DLLs
can be blocked at any time, the output is only logged on exit of OBS.
fstream is not thread-safe and the act of writing the string and the
newline are two separate operations which could execute concurrently in
multiple threads, resulting in lines joining together followed by two
newlines. Due to the presence of a static mutex, this also removes
inline on the function.
It was possible to set the encoder to scale the video to the same size
as the output, resulting in unnecessary passes through swscale and
skipping hardware encoders. This was a common user error in Advanced
Output mode where they would tick the rescale box but leave it at the
same resolution as their main output.
FFmpeg wipes priv_data if the encoder fails to start, so we need some
way of knowing if the GPU index was non-zero. This is a bit ugly but the
best way to do it for now.
This writes the result of nvEncGetLastErrorString to the last encoder
error instead of just the log file. This makes it user-visible, which
should help users diagnose and solve the problem.
Also strips off leading colons from NVENC error strings as these can
look odd given our format string. An example of such an error:
::NV_ENC_TWO_PASS_FULL_RESOLUTION is only supported multipass encoding
on this architecture.
When adding to the queue, this triggers rowCountChanged which then
calls canClearFinished, reading undefined memory for state if this is
not set. The correct state is set immediately after by checkInputPath.
Detected by UBSan.
The gain should be positive for an upward compressor. Initially, the
gain would be zeroe'd below -60 dB to ensure noise is not amplified by
the upward compressor. This created a discontinuity at -60 dB since
just above -60 dB, the audio is boosted by +20 dB (at default
settings). This was fixed in commit 50db097 which decreased smoothly
the gain so that it's 0 dB at -60 dB. However that commit forgot to
limit the gain decrease which was negative below -60 dB.
This is fixed by the current commit.
Additionally initialization allowed -inf gain. We hard limit to positive
gains only as they should be for an upward compressor.
The bugfix was found by R1ch and put in form by pkv.
Co-authored-by: pkv <pkv@obsproject.com>
Signed-off-by: pkv <pkv@obsproject.com>
- Don't repeatedly query version at runtime in case a user installs the
SDK while OBS is running
- Restore default DLL search directory
- Don't show outdated SDK message if the SDK is not found
- Protect minimum version macro with brackets
Now uses GetIfEntry2 which supports 64-bit values for reporting speed, so
10+ gbps adapters are now reported correctly in the log. Also added an
additional log line if the interface error counters are non-zero to possibly
help identify physical faults. Finally the transmit and receive speeds are
logged independently so that asynchronous mediums such as Wi-Fi that might
have good RX but poor TX can be better diagnosed.
If the output is paused the average bitrate will be zero, resulting in
infinite time until disk is full and int overflows. Similarly, if no data
has been collected yet, the result will be NaN and undefined behavior.
If the display wasn't found, the timer was not reset, causing the code
to execute on every single tick, stalling the graphics thread and using
excessive CPU.
Another call to getenv, as well as a call to the POSIX functions
setenv(), unsetenv(), and putenv() may invalidate the pointer
returned by a previous call or modify the string obtained from a
previous call.
https://en.cppreference.com/w/c/program/getenv