[x265] Two bugs and one feature request

[Steve Borho]

On Sun, Aug 11, 2013 at 11:07 PM, JMK <three4teen16 at coldmail.nu> wrote:

> On Windows XP, using the MinGW builds:
>
> 1) after commit fabc072, x265.exe crashes at the end of the encodes, the
> last frame of the sequence is not encoded; at the Videohelp forums, someone
> suggested the following workaround, which was tested and works:
>
> {{
> To fix the crash at the end of encoding, open the file
> source\Lib\TLibEncoder\TEncTop.cpp, go to line 99 and change
>  delete [] m_GOPEncoder;
>  to
>  delete m_GOPEncoder;
>

Thanks, fix enqueued


>
>  That's all. While we are at it, there is also another bug that occurs
> when the file size of the input file exceeds the limits of integer
> representation. It manifests itself as negative number of frames to process
> and also negative eta in the information messages. To fix it, open
> source\input\yuv.cpp, go to line 55 and change
>  return (int)(size - cur) / (width * height * pixelbytes * 3 / 2);
>  to
>  return (unsigned)(size - cur) / (width * height * pixelbytes * 3 / 2);
>
>  The same may have to be applied on the file source\input\y4m.cpp, line
> 187 (Y4MInput::guessFrameCount() method)
> }}
>


Hmm.  The function returns the number of frames as a signed integer.  If we
delay the cast to int until after the division, it should do a lot better.
 ifstream::pos_type is probably a 64bit type on most O/S.  The Y4M code
already did the cast after the division, so I think it is safe.  I have
heard of negative frame numbers reported for YUV but not for Y4M, so this
makes sense.


>
> 2) On Windows XP, with various MSVC builds found on the Internet:
>
> a) error message "x265.exe is not a valid Win32 application"
>
> b) error message "The procedure entry point InterlockedCompareExchange64
> could not be located  in the dynamic link library KERNEL32.dll" --- about
> this problem, someone wrote:
>
> {{
> If you are building in Windows 7, then the problem must be the version of
> Windows itself. What I believe happens is this: In wavefront.cpp there is:
>
> Code:
> #define ATOMIC_OR(ptr, mask)            _InterlockedOr64((volatile
> LONG64*)ptr, mask)
> #if defined(__MSC_VER) && !defined(__INTEL_COMPILER)
> #pragma intrinsic(_InterlockedCompareExchange64)
> #endif
> #else // if _WIN32_WINNT <= _WIN32_WINNT_WINXP
> #define ATOMIC_OR(ptr, mask)            InterlockedOr64((volatile
> LONG64*)ptr, mask)
>
> Since you are on Windows 7, _WIN32_WINNT version is _WIN32_WINNT_WIN7
> (0x0601), which is bigger than _WIN32_WINNT_WINXP (0x0501). As a result,
> the non-intrinsic InterlockedCompareExchange64 is called implicitly by the
> non-intrinsic InterlockedOr64 and the consequence is that the exe runs fine
> on your system plus any other system with OS later than WINXP, but not on
> WINXP itself. I do not use MSVC, but what I would recommend as a tentative
> fix is to set the define -D_WIN32_WINNT=0x501 in the cmake gui
> CMAKE_C_FLAGS and CMAKE_CXX_FLAGS (need to click the Advanced checkbox for
> those to appear). Or whatever might be needed to trick the compiler into
> thinking that your _WIN32_WINNT version is less than 0x0601.
> }}
>

Interesting problem.  We don't have an explicit list of supported Windows
operating systems.  Microsoft itself is abandoning support for XP next
April (http://windows.microsoft.com/en-us/windows/end-support-help), so I
hesitate to make this our default build setting.   I can see two options:

1 - Document this on the wiki - tell people to build on Win XP if they want
to run on XP, or set this in their build flag
2 - Add this as a cmake option, similar to the flag which enables
statically linked CRT binaries.

Leaning towards solution 2


> Feature request:
>
> considering the "x265 Mission Statement" document says:
>
> {{
> The x265 project has licensed the rights to utilize and modify x264
> source code in x265
> }}
>
> could you please port the stdin and Avisynth support of x264 to x265?
> Because it's a matter of *usability* ---
> --- raw video files are HUGE, whereas stdin consumes no space and
> Avisynth scripts are tiny.
>

Patches welcome for either of these.  stdin support should be simple,
avisynth less so but not that difficult.

--
Steve Borho
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/private/x265-devel/attachments/20130811/6dfb8395/attachment-0001.html>