This bug was detected via Coverity static analysis:
Variable tempfile going out of scope leaks the storage it points to.
The leak happens when invalid keyboard layout file is being loaded:
$ touch xx.kl
$ dosbox .
C:\>keyb xx
Issue reported by Dagar and Pr3tty F1y, and confirmed as a bug by ripsaw8080.
Thank you!
This fixes the GoG release of Betrayal at Krondor which (either due to CD mastering
issues or a faulty rip), requests playback of a given track at the tail end
of the prior track.
In debugging and performing this fix, many debug messages were improved as well
as making some small small code adjustments, such as using iterators to point to
individual tracks (track->attribute) instead of using the tracks array
(tracks[track -1].attribute).
Clang static analyzer returned a false-positive issue in line 671:
The right operand of '<' is a garbage value
Variables needed to be moved up, because otherwise initialization
crosses to the next case.
Static analysis indicated an issue, when line was being passed as 2nd
argument to strcat:
Null pointer passed as an argument to a 'nonnull' parameter
Replace repeated strcat with creating a formatted string and use
the opportunity to do a small format cleanup.
These files assume, that WIN32 is defined by compiler or MSVC project
file, when in MSYS2 and MinGW environments, it is defined in config.h
and appears after dosbox.h is included, which in turn is included by
associated header (cdrom.h in this case).
Fixes: #42
Use C++11 static_assert to assure, that code behaves correctly; if
Chip or Channel structs will be changed to non-std layout or a different
first fields will be introduced, that will invalidate the offset
calculation.
Additionally, add asserts to prevent possibility of introducing a bug
when offset stored in one the tables is 0.
Silence compiler warnings:
enumeration value 'sm2Percussion' not handled in switch [-Wswitch]
enumeration value 'sm3Percussion' not handled in switch [-Wswitch]
Thanks to @ripsaw8080 for insight into CD-DA channel mapping,
@hail-to-the-ryzen for testing and flagging a position-tracking bug,
and @dreamer_ for guidance and code review.
The CD-DA volume and channel mapping loops were moved to generic mixer
calls and no longer require a pre-processing loop:
- Application-controlled CD-DA volume adjustment is now applied using
an existing mixer volume scalar that was previously unused by the
CD-DA code.
- Mapping of CD-DA left and right channels is now applied at the tail
end of the mixer's sample ingest sequence.
The following have been removed:
- The CD-DA callback chunk-wise circular buffer
- The decode buffers in the Opus and MP3 decoders
- The decode buffer and conversion buffers in SDL_Sound
These removals and API changes allow the image player's buffer
to be passed-through ultimately to the audio codec, skipping multiple
intermediate buffers.
Runtime improvements:
- Replaces the existing audio callback routine with an efficient chunked
circular-buffer audio reader
- Replaces assumptions that all audio tracks are 44.1 kHz & stereo.
The mixer is now fed data at the actual compressed track's rate and
channel count
- Eliminates all SDL locks in the audio code in favour of mixer state
control
- Queries the codec for track-length instead of using hundreds of
iterative decimating seeks to determine track length (loading
a 99-track CUE now takes 0.1 user-seconds versus 3+ seconds
previously)
- Seeks are performed within the already-decoded buffer (for all
codecs) instead of discarding and re-running the decode sequence
- SDL_Sound's buffer-size is now set once and never resized, which
eliminates repeated re-malloc'ing in the library
- Only one seek is performed per-playback sequence followed by
sequential decodes, similar to a physical CDROM (The baseline dosbox
performs a seek for every 2352-bytes of uncompressed audio)
- The DOSBox mixer is now only active during playback sequences and
fully dormant otherwise (baseline dosbox instead performs hundreds of
calls/second with empty data)
- When using Opus audio tracks, and if your dosbox.conf [mixer]
rate=48000, then you will (very likely) achieve sample-exact
unadulterated pass-through along your entire audio chain from the
decoder, to DOSBox, to your operating system's software mixer,
to your sound card driver, to your sound card, to your speakers,
or to your digital receiver / USB speakers/headphones / or HDMI
TV/screen. This is because almost all modern hardware DACs use
a native sample rate of 48000
Source-level maintenance improvements:
- It strips all pre-processor #ifdef branching for SDL_Sound from
the code
- Fixes all codec compiler warnings (in the modified files); builds
have been tested with GCC 4 to 10, Clang 6 to 10, and MSVC 14
- Tested on Linux, macOS (Xcode), and Windows (MinGW MSYS 1.0)
operating systems
- Tested on i386, x86_64, ARM, and PowerPC (big-endian) architectures
So far, a splash screen exists only in raster format, encoded with RLE
for easy inclusion in .cpp files (src/gui/dosbox_splash.h).
Vector format is easier to modify and can be used as basis for more
artwork, to provide splash screen in better resolutions, etc.
This vector was obtained by resizing raster image to high resolution,
using a set of Gimp filters to sharpen it, then converted in Inkscape
using Path Bitmap functionality and finally optimized by hand to e.g.
remove excessive path nodes.
To convert it to a format easily usable from C/C++: open the file in
Gimp, edit to fit your requirements and export as C header.
This null-check resolves a potential issue detected by static analysis.
It might be the case, that crash never happens due to the way this
static function is used at the moment, and because DYNFLG_CHANGED is being
cleared in dnew->genreg->Clear() few lines before, but the crash might
still happen if initial state of flags is inconsistent or surrounding
code will be changed even a little bit.
This nullcheck makes the code more robust at no performance penalty.
