This simplifies forward source compatibility when new fields are
added to SEncParamExt - when new fields are added to SEncParamExt,
this method makes sure those fields are initialized to the
default value - otherwise all API users would have to manually check
SEncParamExt every time it is updated to make sure there's no new fields
that should be set to a nonzero value by default (e.g. like
bEnableFrameSkip).
On processors without HTT, WelsCPUFeatureDetect can't return
a number of cores but might still return a nonzero set of
CPU feature flags. Previously the nonzero cpu feature flag
indicated that cpuid worked and the encoder wouldn't use the
higher level API for getting the number of cores, even though the
number of cores was left at 1.
Previously the loop filter was unconditionally enabled
regardless of what encoder parameter was set. If using
SEncParamBase instead, the loop filter was always disabled.
Previously, these fields kept whatever value was set by
FillDefault. The corresponding fields were set properly within
sSpatialLayers, but the fields within the main struct were left
with the default values.
This doesn't change the hashes in the unit test, since these
fields don't seem to be used in the produced bitstream at all.
Instead just duplicate the common fields. These fields had to
be duplicated for the C interface compatibility anyway - but
this way there is no risk to accidentally introduce an ABI
break since there is no need for the layout of SEncParamBase to
actually match the start of SEncParamExt.
Remove the useless iInitType parameter, make the method
private within CWelsH264SVCEncoder class, give the pointer
parameter the correct type, avoiding needless casting.
TRUE/FALSE has intentionally been left in use for the few
platform specific APIs that define these constants themselves
and expect them to be used, for consistency.
Instead of byteswapping a 32 bit word and writing it out as a
whole (which could even possibly lead to crashes due to
incorrect alignment on some platforms), write it out explicitly
in the intended byte order.
This avoids having to set a define indicating the endianness.
The code interprets an array of 4 uint8_t values as one uint32_t
and does shifts on the value. The same optimization can be
kept in big endian as well, but the shift has to be done in the
other direction.
This code could be made truly independent of endianness, but
that could cause some minimal performance degradaion, at least
in theory.
This makes "make test" pass on big endian, assuming that
WORDS_BIGENDIAN is defined while building.
This makes the code work properly on big endian.
The MC case is similar to how it's done in the encoder.
Neither of these should have any significant performance
impact.
This simplifies the code and makes the buffer size checks
more consistent. Additionally, the previous version wrote
the extra space character without checking if it actually fit
into the buffer.
strlen is not dangerous if the string is known to be null
terminated (and MSVC does not warn about its use either).
For the cases in the decoder welsCodecTrace.cpp, the string
passed to all WriteString instances is produced by WelsVsnprintf
which always null terminates the buffer nowadays.
Additionally, as the string was passed to OutputDebugStringA
without any length specifier before, it was already assumed to
be null terminated.
The file name parameter passed to DumpDependencyRec and
DumpRecFrame in encoder.cpp is always null terminated,
which was already assumed as it is passed to WelsFopen as is.
As for the encoder utils.cpp, the strings returned by GetLogPath
are string constants that are null terminated.
As long as WelsFileHandle* is equal to FILE* this doesn't matter,
but for consistency use the WelsF* functions for all handles
opened by WelsFopen, and use WelsFileHandle* as type for it
instead of FILE*.
Both encoder and decoder versions were functionally equivalent,
but I picked the decoder version (but added the static inline
keywords to it) since the encoder one was quite messy with a lot
of commented out code.
Instead of using "defined(MSC_VER) || defined(__MINGW32__)" to
indicate the windows platform, just check for the _WIN32 define
instead.
Also remove an unused codepath - the removed codepath would
only be used under the condition
"(defined(MSC_VER) || defined(__MINGW32__)) && !defined(_WIN32)",
and I'm not aware of any environment with MSVC or MinGW that
doesn't define _WIN32, thus this codepath never was used.
This fixes two separate issues.
First, with the MSVC _snprintf implementations, the return value
is negative if the buffer wasn't large enough - this would in
the worst case lead to making iBufferUsed negative, writing before
the start of the buffer.
Secondly, when both iBufferUsed and iBufferLeft are accumulated,
one can't do "iBufferLeft -= iBufferUsed;". As an example,
say the buffer is 100 bytes in total and iBufferLeft is 40 and
iBufferUsed is 60. If SNPRINTF then writes 5 more bytes to the
buffer, iBufferUsed would be 65, but if we now do
"iBufferLeft -= iBufferUsed;" then iBufferLeft would end up as
-25 even though there's 35 bytes left in the buffer to use.
Therefore, we use a separate variable to store the return value
from the latest SNPRINTF call. This is checked to make sure it
wasn't negative, and only this amount is added to iBufferUsed
and subtracted from iBufferLeft.
This is the same pattern used in codec/encoder/core/src/utils.cpp.
strftime never returns negative numbers, so those calls don't
need as much checking.
Checking iBufferLeft > iBufferUsed does not make sense, since
this would stop writing into the buffer alredy after the buffer
is half full, when there is less space left than has been used.
The right check is iBufferLeft > 0.
The following pattern is unsafe on all platforms:
n = SNPRINTF(buf, ...);
buf[n] = '\0';
On windows, the _snprintf variants return a negative number
if the buffer was too small, thus buf[n] would be outside
of (before the start of) the buffer.
On other platforms, the C99 snprintf function returns the
total number of characters which would have been written if
the buffer had been large enough, which can be larger than
the buffer size itself, and thus buf[n] would be beyond the
end of the buffer.
The C99 snprintf function always null terminate the buffer.
These invocations of SNPRINTF are within !WIN32, so we can
be sure that the SNPRINTF call itself already null terminated
the buffer.
The decoder used WelsMedian while the encoder used WELS_MEDIAN.
The former has two different implementations, WELS_MEDIAN was
identical to the disabled version of WelsMedian.
Settle on using the same implementation for both decoder and
encoder - whichever version of the implementations is faster
should be used for both.
