[PATCH v2 0/5] MR6442: winedmo: Introduce a new DLL as a MF FFmpeg backend.
This introduces a new alternative FFmpeg-based implementation for the MF byte stream handlers, while keeping the current GStreamer-based as default. The new implementation can be enabled by setting the DWORD value: DisableGstByteStreamHandler = 1 in the HKCU\Software\Wine\MediaFoundation registry key. -- v2: mfsrcsnk: Stub byte stream handlers if demuxing is supported. winedmo: Export a new winedmo_demuxer_check function. winedmo: Link and initialize FFmpeg on load. https://gitlab.winehq.org/wine/wine/-/merge_requests/6442
From: Rémi Bernon <rbernon(a)codeweavers.com> --- configure.ac | 1 + dlls/winedmo/Makefile.in | 4 ++++ dlls/winedmo/main.c | 33 +++++++++++++++++++++++++++++++++ dlls/winedmo/winedmo.spec | 1 + 4 files changed, 39 insertions(+) create mode 100644 dlls/winedmo/Makefile.in create mode 100644 dlls/winedmo/main.c create mode 100644 dlls/winedmo/winedmo.spec diff --git a/configure.ac b/configure.ac index 4dcb780013a..8b44370122f 100644 --- a/configure.ac +++ b/configure.ac @@ -3290,6 +3290,7 @@ WINE_CONFIG_MAKEFILE(dlls/winebus.sys) WINE_CONFIG_MAKEFILE(dlls/winecoreaudio.drv) WINE_CONFIG_MAKEFILE(dlls/winecrt0) WINE_CONFIG_MAKEFILE(dlls/wined3d) +WINE_CONFIG_MAKEFILE(dlls/winedmo) WINE_CONFIG_MAKEFILE(dlls/winegstreamer) WINE_CONFIG_MAKEFILE(dlls/winehid.sys) WINE_CONFIG_MAKEFILE(dlls/winemac.drv) diff --git a/dlls/winedmo/Makefile.in b/dlls/winedmo/Makefile.in new file mode 100644 index 00000000000..e73a82235c6 --- /dev/null +++ b/dlls/winedmo/Makefile.in @@ -0,0 +1,4 @@ +MODULE = winedmo.dll + +SOURCES = \ + main.c diff --git a/dlls/winedmo/main.c b/dlls/winedmo/main.c new file mode 100644 index 00000000000..2d86fe05fd0 --- /dev/null +++ b/dlls/winedmo/main.c @@ -0,0 +1,33 @@ +/* + * Copyright 2024 Rémi Bernon for CodeWeavers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#include <stddef.h> +#include <stdarg.h> + +#include "windef.h" +#include "winbase.h" + +#include "wine/debug.h" + +WINE_DEFAULT_DEBUG_CHANNEL(dmo); + +BOOL WINAPI DllMain( HINSTANCE instance, DWORD reason, void *reserved ) +{ + TRACE( "instance %p, reason %lu, reserved %p\n", instance, reason, reserved ); + return TRUE; +} diff --git a/dlls/winedmo/winedmo.spec b/dlls/winedmo/winedmo.spec new file mode 100644 index 00000000000..76421d7e35b --- /dev/null +++ b/dlls/winedmo/winedmo.spec @@ -0,0 +1 @@ +# nothing to export -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6442
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winedmo/Makefile.in | 4 ++- dlls/winedmo/main.c | 12 ++++++++ dlls/winedmo/unix_private.h | 29 +++++++++++++++++++ dlls/winedmo/unixlib.c | 55 +++++++++++++++++++++++++++++++++++++ dlls/winedmo/unixlib.h | 38 +++++++++++++++++++++++++ 5 files changed, 137 insertions(+), 1 deletion(-) create mode 100644 dlls/winedmo/unix_private.h create mode 100644 dlls/winedmo/unixlib.c create mode 100644 dlls/winedmo/unixlib.h diff --git a/dlls/winedmo/Makefile.in b/dlls/winedmo/Makefile.in index e73a82235c6..bea3b43546a 100644 --- a/dlls/winedmo/Makefile.in +++ b/dlls/winedmo/Makefile.in @@ -1,4 +1,6 @@ MODULE = winedmo.dll +UNIXLIB = winedmo.so SOURCES = \ - main.c + main.c \ + unixlib.c diff --git a/dlls/winedmo/main.c b/dlls/winedmo/main.c index 2d86fe05fd0..1e6c074b068 100644 --- a/dlls/winedmo/main.c +++ b/dlls/winedmo/main.c @@ -23,11 +23,23 @@ #include "winbase.h" #include "wine/debug.h" +#include "unixlib.h" WINE_DEFAULT_DEBUG_CHANNEL(dmo); BOOL WINAPI DllMain( HINSTANCE instance, DWORD reason, void *reserved ) { TRACE( "instance %p, reason %lu, reserved %p\n", instance, reason, reserved ); + + if (reason == DLL_PROCESS_ATTACH) + { + NTSTATUS status; + DisableThreadLibraryCalls( instance ); + + status = __wine_init_unix_call(); + if (!status) status = UNIX_CALL( process_attach, NULL ); + if (status) WARN( "Failed to init unixlib, status %#lx\n", status ); + } + return TRUE; } diff --git a/dlls/winedmo/unix_private.h b/dlls/winedmo/unix_private.h new file mode 100644 index 00000000000..a34021d7339 --- /dev/null +++ b/dlls/winedmo/unix_private.h @@ -0,0 +1,29 @@ +/* + * Copyright 2024 Rémi Bernon for CodeWeavers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#include "config.h" + +#include <stddef.h> +#include <stdarg.h> + +#include "ntstatus.h" +#define WIN32_NO_STATUS +#include "windef.h" +#include "winbase.h" + +#include "unixlib.h" diff --git a/dlls/winedmo/unixlib.c b/dlls/winedmo/unixlib.c new file mode 100644 index 00000000000..7d27e031e37 --- /dev/null +++ b/dlls/winedmo/unixlib.c @@ -0,0 +1,55 @@ +/* + * Copyright 2024 Rémi Bernon for CodeWeavers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#if 0 +#pragma makedep unix +#endif + +#include "config.h" + +#include "unix_private.h" + +#include "wine/debug.h" + +#define MAKE_UNSUPPORTED_ENTRY( name ) \ + static NTSTATUS name( void *arg ) \ + { \ + return STATUS_NOT_SUPPORTED; \ + } +MAKE_UNSUPPORTED_ENTRY( process_attach ) +#undef MAKE_UNSUPPORTED_ENTRY + +const unixlib_entry_t __wine_unix_call_funcs[] = +{ +#define X( name ) [unix_##name] = name + X( process_attach ), +}; + +C_ASSERT(ARRAY_SIZE(__wine_unix_call_funcs) == unix_funcs_count); + +#ifdef _WIN64 + +const unixlib_entry_t __wine_unix_call_wow64_funcs[] = +{ +#define X64( name ) [unix_##name] = wow64_##name + X( process_attach ), +}; + +C_ASSERT(ARRAY_SIZE(__wine_unix_call_wow64_funcs) == unix_funcs_count); + +#endif /* _WIN64 */ diff --git a/dlls/winedmo/unixlib.h b/dlls/winedmo/unixlib.h new file mode 100644 index 00000000000..c1b0542f33b --- /dev/null +++ b/dlls/winedmo/unixlib.h @@ -0,0 +1,38 @@ +/* + * Copyright 2024 Rémi Bernon for CodeWeavers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#ifndef __WINE_WINEDMO_UNIXLIB_H +#define __WINE_WINEDMO_UNIXLIB_H + +#include <stddef.h> +#include <stdarg.h> + +#include "windef.h" +#include "winbase.h" + +#include "wine/unixlib.h" + +enum unix_funcs +{ + unix_process_attach, + unix_funcs_count, +}; + +#define UNIX_CALL( func, params ) WINE_UNIX_CALL( unix_##func, params ) + +#endif /* __WINE_WINEDMO_UNIXLIB_H */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6442
From: Rémi Bernon <rbernon(a)codeweavers.com> --- configure.ac | 16 ++++++++++++++++ dlls/winedmo/Makefile.in | 2 ++ dlls/winedmo/unix_private.h | 6 ++++++ dlls/winedmo/unixlib.c | 31 +++++++++++++++++++++++++++++++ 4 files changed, 55 insertions(+) diff --git a/configure.ac b/configure.ac index 8b44370122f..01949a711ab 100644 --- a/configure.ac +++ b/configure.ac @@ -29,6 +29,7 @@ AC_ARG_WITH(capi, AS_HELP_STRING([--without-capi],[do not use CAPI (ISDN su AC_ARG_WITH(coreaudio, AS_HELP_STRING([--without-coreaudio],[do not use the CoreAudio sound support])) AC_ARG_WITH(cups, AS_HELP_STRING([--without-cups],[do not use CUPS])) AC_ARG_WITH(dbus, AS_HELP_STRING([--without-dbus],[do not use DBus (dynamic device support)])) +AC_ARG_WITH(ffmpeg, AS_HELP_STRING([--without-ffmpeg],[do not use the FFmpeg library])) AC_ARG_WITH(fontconfig,AS_HELP_STRING([--without-fontconfig],[do not use fontconfig])) AC_ARG_WITH(freetype, AS_HELP_STRING([--without-freetype],[do not use the FreeType library])) AC_ARG_WITH(gettext, AS_HELP_STRING([--without-gettext],[do not use gettext])) @@ -1630,6 +1631,21 @@ WINE_NOTICE_WITH(pulse, [test -z "$PULSE_LIBS"], [libpulse ${notice_platform}development files not found or too old, Pulse won't be supported.], [enable_winepulse_drv]) +dnl **** Check for FFmpeg **** +if test "x$with_ffmpeg" != "xno"; +then + WINE_PACKAGE_FLAGS(FFMPEG,[libavutil],,,, + [AC_CHECK_HEADER([libavutil/avutil.h], + [AC_CHECK_LIB(avutil,av_log_set_callback,[:],[FFMPEG_LIBS=""],[$FFMPEG_LIBS])], + [FFMPEG_LIBS=""])]) + if test "x$FFMPEG_LIBS" != "x"; + then + FFMPEG_CFLAGS="-DENABLE_FFMPEG $FFMPEG_CFLAGS" + fi +fi +WINE_NOTICE_WITH(ffmpeg,[test -z "x$FFMPEG_LIBS"], + [FFmpeg ${notice_platform}development files not found.]) + dnl **** Check for gstreamer **** if test "x$with_gstreamer" != "xno" then diff --git a/dlls/winedmo/Makefile.in b/dlls/winedmo/Makefile.in index bea3b43546a..939941884e8 100644 --- a/dlls/winedmo/Makefile.in +++ b/dlls/winedmo/Makefile.in @@ -1,5 +1,7 @@ MODULE = winedmo.dll UNIXLIB = winedmo.so +UNIX_CFLAGS = $(FFMPEG_CFLAGS) +UNIX_LIBS = $(FFMPEG_LIBS) $(PTHREAD_LIBS) SOURCES = \ main.c \ diff --git a/dlls/winedmo/unix_private.h b/dlls/winedmo/unix_private.h index a34021d7339..8e97cede3f8 100644 --- a/dlls/winedmo/unix_private.h +++ b/dlls/winedmo/unix_private.h @@ -27,3 +27,9 @@ #include "winbase.h" #include "unixlib.h" + +#ifdef ENABLE_FFMPEG + +#include <libavutil/avutil.h> + +#endif /* ENABLE_FFMPEG */ diff --git a/dlls/winedmo/unixlib.c b/dlls/winedmo/unixlib.c index 7d27e031e37..0fbae3e3f68 100644 --- a/dlls/winedmo/unixlib.c +++ b/dlls/winedmo/unixlib.c @@ -26,14 +26,45 @@ #include "wine/debug.h" +WINE_DEFAULT_DEBUG_CHANNEL(dmo); + +#ifdef ENABLE_FFMPEG + +static void vlog( void *ctx, int level, const char *fmt, va_list va_args ) +{ + enum __wine_debug_class dbcl = __WINE_DBCL_TRACE; + if (level <= AV_LOG_ERROR) dbcl = __WINE_DBCL_ERR; + if (level <= AV_LOG_WARNING) dbcl = __WINE_DBCL_WARN; + wine_dbg_vlog( dbcl, __wine_dbch___default, __func__, fmt, va_args ); +} + +static const char *debugstr_version( UINT version ) +{ + return wine_dbg_sprintf("%u.%u.%u", AV_VERSION_MAJOR(version), AV_VERSION_MINOR(version), AV_VERSION_MICRO(version)); +} + +static NTSTATUS process_attach( void *arg ) +{ + TRACE( "FFmpeg support:\n" ); + TRACE( " avutil version %s\n", debugstr_version( avutil_version() ) ); + + av_log_set_callback( vlog ); + return STATUS_SUCCESS; +} + +#else /* ENABLE_FFMPEG */ + #define MAKE_UNSUPPORTED_ENTRY( name ) \ static NTSTATUS name( void *arg ) \ { \ + WARN( "FFmpeg support not compiled in\n" ); \ return STATUS_NOT_SUPPORTED; \ } MAKE_UNSUPPORTED_ENTRY( process_attach ) #undef MAKE_UNSUPPORTED_ENTRY +#endif /* ENABLE_FFMPEG */ + const unixlib_entry_t __wine_unix_call_funcs[] = { #define X( name ) [unix_##name] = name -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6442
From: Rémi Bernon <rbernon(a)codeweavers.com> --- configure.ac | 5 +++- dlls/winedmo/Makefile.in | 2 ++ dlls/winedmo/main.c | 13 ++++++++++ dlls/winedmo/unix_demuxer.c | 52 +++++++++++++++++++++++++++++++++++++ dlls/winedmo/unix_private.h | 5 ++++ dlls/winedmo/unixlib.c | 17 ++++++++++++ dlls/winedmo/unixlib.h | 10 +++++++ dlls/winedmo/winedmo.spec | 2 +- include/Makefile.in | 1 + include/wine/winedmo.h | 31 ++++++++++++++++++++++ 10 files changed, 136 insertions(+), 2 deletions(-) create mode 100644 dlls/winedmo/unix_demuxer.c create mode 100644 include/wine/winedmo.h diff --git a/configure.ac b/configure.ac index 01949a711ab..e685aa52034 100644 --- a/configure.ac +++ b/configure.ac @@ -1634,9 +1634,12 @@ WINE_NOTICE_WITH(pulse, [test -z "$PULSE_LIBS"], dnl **** Check for FFmpeg **** if test "x$with_ffmpeg" != "xno"; then - WINE_PACKAGE_FLAGS(FFMPEG,[libavutil],,,, + WINE_PACKAGE_FLAGS(FFMPEG,[libavutil libavformat],,,, [AC_CHECK_HEADER([libavutil/avutil.h], [AC_CHECK_LIB(avutil,av_log_set_callback,[:],[FFMPEG_LIBS=""],[$FFMPEG_LIBS])], + [FFMPEG_LIBS=""]) + AC_CHECK_HEADER([libavformat/avformat.h], + [AC_CHECK_LIB(avformat,av_find_input_format,[:],[FFMPEG_LIBS=""],[$FFMPEG_LIBS])], [FFMPEG_LIBS=""])]) if test "x$FFMPEG_LIBS" != "x"; then diff --git a/dlls/winedmo/Makefile.in b/dlls/winedmo/Makefile.in index 939941884e8..5b816357770 100644 --- a/dlls/winedmo/Makefile.in +++ b/dlls/winedmo/Makefile.in @@ -1,8 +1,10 @@ MODULE = winedmo.dll UNIXLIB = winedmo.so +IMPORTLIB = winedmo UNIX_CFLAGS = $(FFMPEG_CFLAGS) UNIX_LIBS = $(FFMPEG_LIBS) $(PTHREAD_LIBS) SOURCES = \ main.c \ + unix_demuxer.c \ unixlib.c diff --git a/dlls/winedmo/main.c b/dlls/winedmo/main.c index 1e6c074b068..b0913be044e 100644 --- a/dlls/winedmo/main.c +++ b/dlls/winedmo/main.c @@ -43,3 +43,16 @@ BOOL WINAPI DllMain( HINSTANCE instance, DWORD reason, void *reserved ) return TRUE; } + + +NTSTATUS CDECL winedmo_demuxer_check( const char *mime_type ) +{ + struct demuxer_check_params params = {0}; + NTSTATUS status; + + TRACE( "mime_type %s\n", debugstr_a(mime_type) ); + lstrcpynA( params.mime_type, mime_type, sizeof(params.mime_type) ); + + if ((status = UNIX_CALL( demuxer_check, ¶ms ))) WARN( "returning %#lx\n", status ); + return status; +} diff --git a/dlls/winedmo/unix_demuxer.c b/dlls/winedmo/unix_demuxer.c new file mode 100644 index 00000000000..3a4a74b304e --- /dev/null +++ b/dlls/winedmo/unix_demuxer.c @@ -0,0 +1,52 @@ +/* + * Copyright 2024 Rémi Bernon for CodeWeavers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#if 0 +#pragma makedep unix +#endif + +#include "config.h" +#include "unix_private.h" + +#include "wine/debug.h" + +#ifdef ENABLE_FFMPEG + +WINE_DEFAULT_DEBUG_CHANNEL(dmo); + +NTSTATUS demuxer_check( void *arg ) +{ + struct demuxer_check_params *params = arg; + const AVInputFormat *format = NULL; + + if (!strcmp( params->mime_type, "video/mp4" )) format = av_find_input_format( "mp4" ); + else if (!strcmp( params->mime_type, "video/avi" )) format = av_find_input_format( "avi" ); + else if (!strcmp( params->mime_type, "audio/wav" )) format = av_find_input_format( "wav" ); + else if (!strcmp( params->mime_type, "audio/x-ms-wma" )) format = av_find_input_format( "asf" ); + else if (!strcmp( params->mime_type, "video/x-ms-wmv" )) format = av_find_input_format( "asf" ); + else if (!strcmp( params->mime_type, "video/x-ms-asf" )) format = av_find_input_format( "asf" ); + else if (!strcmp( params->mime_type, "video/mpeg" )) format = av_find_input_format( "mpeg" ); + else if (!strcmp( params->mime_type, "audio/mp3" )) format = av_find_input_format( "mp3" ); + + if (format) TRACE( "Found format %s (%s)\n", format->name, format->long_name ); + else FIXME( "Unsupported MIME type %s\n", debugstr_a(params->mime_type) ); + + return format ? STATUS_SUCCESS : STATUS_NOT_SUPPORTED; +} + +#endif /* ENABLE_FFMPEG */ diff --git a/dlls/winedmo/unix_private.h b/dlls/winedmo/unix_private.h index 8e97cede3f8..5e38ecb4a60 100644 --- a/dlls/winedmo/unix_private.h +++ b/dlls/winedmo/unix_private.h @@ -31,5 +31,10 @@ #ifdef ENABLE_FFMPEG #include <libavutil/avutil.h> +#include <libavutil/imgutils.h> +#include <libavformat/avformat.h> + +/* unix_demuxer.c */ +extern NTSTATUS demuxer_check( void * ); #endif /* ENABLE_FFMPEG */ diff --git a/dlls/winedmo/unixlib.c b/dlls/winedmo/unixlib.c index 0fbae3e3f68..e3a4bbdb552 100644 --- a/dlls/winedmo/unixlib.c +++ b/dlls/winedmo/unixlib.c @@ -45,8 +45,20 @@ static const char *debugstr_version( UINT version ) static NTSTATUS process_attach( void *arg ) { + const AVInputFormat *demuxer; + void *opaque; + TRACE( "FFmpeg support:\n" ); TRACE( " avutil version %s\n", debugstr_version( avutil_version() ) ); + TRACE( " avformat version %s\n", debugstr_version( avformat_version() ) ); + + TRACE( "available demuxers:\n" ); + for (opaque = NULL; (demuxer = av_demuxer_iterate( &opaque ));) + { + TRACE( " %s (%s)\n", demuxer->name, demuxer->long_name ); + if (demuxer->extensions) TRACE( " extensions: %s\n", demuxer->extensions ); + if (demuxer->mime_type) TRACE( " mime_types: %s\n", demuxer->mime_type ); + } av_log_set_callback( vlog ); return STATUS_SUCCESS; @@ -61,6 +73,7 @@ static NTSTATUS process_attach( void *arg ) return STATUS_NOT_SUPPORTED; \ } MAKE_UNSUPPORTED_ENTRY( process_attach ) +MAKE_UNSUPPORTED_ENTRY( demuxer_check ) #undef MAKE_UNSUPPORTED_ENTRY #endif /* ENABLE_FFMPEG */ @@ -69,6 +82,8 @@ const unixlib_entry_t __wine_unix_call_funcs[] = { #define X( name ) [unix_##name] = name X( process_attach ), + + X( demuxer_check ), }; C_ASSERT(ARRAY_SIZE(__wine_unix_call_funcs) == unix_funcs_count); @@ -79,6 +94,8 @@ const unixlib_entry_t __wine_unix_call_wow64_funcs[] = { #define X64( name ) [unix_##name] = wow64_##name X( process_attach ), + + X( demuxer_check ), }; C_ASSERT(ARRAY_SIZE(__wine_unix_call_wow64_funcs) == unix_funcs_count); diff --git a/dlls/winedmo/unixlib.h b/dlls/winedmo/unixlib.h index c1b0542f33b..d71d570c335 100644 --- a/dlls/winedmo/unixlib.h +++ b/dlls/winedmo/unixlib.h @@ -27,9 +27,19 @@ #include "wine/unixlib.h" + +struct demuxer_check_params +{ + char mime_type[256]; +}; + + enum unix_funcs { unix_process_attach, + + unix_demuxer_check, + unix_funcs_count, }; diff --git a/dlls/winedmo/winedmo.spec b/dlls/winedmo/winedmo.spec index 76421d7e35b..661703b6c50 100644 --- a/dlls/winedmo/winedmo.spec +++ b/dlls/winedmo/winedmo.spec @@ -1 +1 @@ -# nothing to export +@ cdecl winedmo_demuxer_check(str) diff --git a/include/Makefile.in b/include/Makefile.in index 000214484ed..8d25e40ba05 100644 --- a/include/Makefile.in +++ b/include/Makefile.in @@ -940,6 +940,7 @@ SOURCES = \ wine/windef16.h \ wine/wine_common_ver.rc \ wine/wined3d.h \ + wine/winedmo.h \ wine/winedxgi.idl \ wine/wingdi16.h \ wine/winnet16.h \ diff --git a/include/wine/winedmo.h b/include/wine/winedmo.h new file mode 100644 index 00000000000..06ac1e04261 --- /dev/null +++ b/include/wine/winedmo.h @@ -0,0 +1,31 @@ +/* + * Copyright 2024 Rémi Bernon for CodeWeavers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#ifndef __WINE_WINEDMO_H +#define __WINE_WINEDMO_H + +#include <stddef.h> +#include <stdarg.h> + +#include "windef.h" +#include "winbase.h" +#include "winternl.h" + +NTSTATUS CDECL winedmo_demuxer_check( const char *mime_type ); + +#endif /* __WINE_WINEDMO_H */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6442
From: Rémi Bernon <rbernon(a)codeweavers.com> This introduces a new alternative FFmpeg-based implementation for the MF byte stream handlers, while keeping the current GStreamer-based as default. The new implementation can be enabled by setting the DWORD value: DisableGstByteStreamHandler = 1 in the HKCU\Software\Wine\MediaFoundation registry key. --- dlls/mfasfsrcsnk/Makefile.in | 2 +- dlls/mfmp4srcsnk/Makefile.in | 2 +- dlls/mfsrcsnk/Makefile.in | 2 +- dlls/mfsrcsnk/media_source.c | 71 ++++++++++++++++++++++++++++++++---- 4 files changed, 66 insertions(+), 11 deletions(-) diff --git a/dlls/mfasfsrcsnk/Makefile.in b/dlls/mfasfsrcsnk/Makefile.in index f450c304d3b..e2dbc7afb4d 100644 --- a/dlls/mfasfsrcsnk/Makefile.in +++ b/dlls/mfasfsrcsnk/Makefile.in @@ -1,5 +1,5 @@ MODULE = mfasfsrcsnk.dll -IMPORTS = ole32 mfplat mfuuid uuid +IMPORTS = advapi32 ole32 mfplat mfuuid uuid winedmo PARENTSRC = ../mfsrcsnk EXTRADLLFLAGS = -Wb,--prefer-native diff --git a/dlls/mfmp4srcsnk/Makefile.in b/dlls/mfmp4srcsnk/Makefile.in index cc91ab58df9..5464e1d86e3 100644 --- a/dlls/mfmp4srcsnk/Makefile.in +++ b/dlls/mfmp4srcsnk/Makefile.in @@ -1,5 +1,5 @@ MODULE = mfmp4srcsnk.dll -IMPORTS = ole32 mfplat mfuuid uuid +IMPORTS = advapi32 ole32 mfplat mfuuid uuid winedmo PARENTSRC = ../mfsrcsnk EXTRADLLFLAGS = -Wb,--prefer-native diff --git a/dlls/mfsrcsnk/Makefile.in b/dlls/mfsrcsnk/Makefile.in index d238b43424d..5bef58ca10d 100644 --- a/dlls/mfsrcsnk/Makefile.in +++ b/dlls/mfsrcsnk/Makefile.in @@ -1,6 +1,6 @@ MODULE = mfsrcsnk.dll IMPORTLIB = mfsrcsnk -IMPORTS = ole32 mfplat mfuuid uuid +IMPORTS = advapi32 ole32 mfplat mfuuid uuid winedmo EXTRADLLFLAGS = -Wb,--prefer-native diff --git a/dlls/mfsrcsnk/media_source.c b/dlls/mfsrcsnk/media_source.c index 8c02743e811..fe6f8cea256 100644 --- a/dlls/mfsrcsnk/media_source.c +++ b/dlls/mfsrcsnk/media_source.c @@ -19,12 +19,43 @@ #include "mfsrcsnk_private.h" #include "wine/debug.h" +#include "wine/winedmo.h" + +WINE_DEFAULT_DEBUG_CHANNEL(mfplat); + +static HRESULT byte_stream_plugin_create(IUnknown *outer, REFIID riid, void **out) +{ + FIXME("outer %p, riid %s, out %p stub!.\n", outer, debugstr_guid(riid), out); + *out = NULL; + return E_NOTIMPL; +} + +static BOOL use_gst_byte_stream_handler(void) +{ + BOOL result; + DWORD size = sizeof(result); + + /* @@ Wine registry key: HKCU\Software\Wine\MediaFoundation */ + if (!RegGetValueW( HKEY_CURRENT_USER, L"Software\\Wine\\MediaFoundation", L"DisableGstByteStreamHandler", + RRF_RT_REG_DWORD, NULL, &result, &size )) + return !result; + + return TRUE; +} static HRESULT WINAPI asf_byte_stream_plugin_factory_CreateInstance(IClassFactory *iface, IUnknown *outer, REFIID riid, void **out) { - static const GUID CLSID_GStreamerByteStreamHandler = {0x317df618,0x5e5a,0x468a,{0x9f,0x15,0xd8,0x27,0xa9,0xa0,0x81,0x62}}; - return CoCreateInstance(&CLSID_GStreamerByteStreamHandler, outer, CLSCTX_INPROC_SERVER, riid, out); + NTSTATUS status; + + if ((status = winedmo_demuxer_check("video/x-ms-asf")) || use_gst_byte_stream_handler()) + { + static const GUID CLSID_GStreamerByteStreamHandler = {0x317df618,0x5e5a,0x468a,{0x9f,0x15,0xd8,0x27,0xa9,0xa0,0x81,0x62}}; + if (status) WARN("Unsupported demuxer, status %#lx.\n", status); + return CoCreateInstance(&CLSID_GStreamerByteStreamHandler, outer, CLSCTX_INPROC_SERVER, riid, out); + } + + return byte_stream_plugin_create(outer, riid, out); } static const IClassFactoryVtbl asf_byte_stream_plugin_factory_vtbl = @@ -41,8 +72,16 @@ IClassFactory asf_byte_stream_plugin_factory = {&asf_byte_stream_plugin_factory_ static HRESULT WINAPI avi_byte_stream_plugin_factory_CreateInstance(IClassFactory *iface, IUnknown *outer, REFIID riid, void **out) { - static const GUID CLSID_GStreamerByteStreamHandler = {0x317df618,0x5e5a,0x468a,{0x9f,0x15,0xd8,0x27,0xa9,0xa0,0x81,0x62}}; - return CoCreateInstance(&CLSID_GStreamerByteStreamHandler, outer, CLSCTX_INPROC_SERVER, riid, out); + NTSTATUS status; + + if ((status = winedmo_demuxer_check("video/avi")) || use_gst_byte_stream_handler()) + { + static const GUID CLSID_GStreamerByteStreamHandler = {0x317df618,0x5e5a,0x468a,{0x9f,0x15,0xd8,0x27,0xa9,0xa0,0x81,0x62}}; + if (status) WARN("Unsupported demuxer, status %#lx.\n", status); + return CoCreateInstance(&CLSID_GStreamerByteStreamHandler, outer, CLSCTX_INPROC_SERVER, riid, out); + } + + return byte_stream_plugin_create(outer, riid, out); } static const IClassFactoryVtbl avi_byte_stream_plugin_factory_vtbl = @@ -59,8 +98,16 @@ IClassFactory avi_byte_stream_plugin_factory = {&avi_byte_stream_plugin_factory_ static HRESULT WINAPI mpeg4_byte_stream_plugin_factory_CreateInstance(IClassFactory *iface, IUnknown *outer, REFIID riid, void **out) { - static const GUID CLSID_GStreamerByteStreamHandler = {0x317df618,0x5e5a,0x468a,{0x9f,0x15,0xd8,0x27,0xa9,0xa0,0x81,0x62}}; - return CoCreateInstance(&CLSID_GStreamerByteStreamHandler, outer, CLSCTX_INPROC_SERVER, riid, out); + NTSTATUS status; + + if ((status = winedmo_demuxer_check("video/mp4")) || use_gst_byte_stream_handler()) + { + static const GUID CLSID_GStreamerByteStreamHandler = {0x317df618,0x5e5a,0x468a,{0x9f,0x15,0xd8,0x27,0xa9,0xa0,0x81,0x62}}; + if (status) WARN("Unsupported demuxer, status %#lx.\n", status); + return CoCreateInstance(&CLSID_GStreamerByteStreamHandler, outer, CLSCTX_INPROC_SERVER, riid, out); + } + + return byte_stream_plugin_create(outer, riid, out); } static const IClassFactoryVtbl mpeg4_byte_stream_plugin_factory_vtbl = @@ -77,8 +124,16 @@ IClassFactory mpeg4_byte_stream_plugin_factory = {&mpeg4_byte_stream_plugin_fact static HRESULT WINAPI wav_byte_stream_plugin_factory_CreateInstance(IClassFactory *iface, IUnknown *outer, REFIID riid, void **out) { - static const GUID CLSID_GStreamerByteStreamHandler = {0x317df618,0x5e5a,0x468a,{0x9f,0x15,0xd8,0x27,0xa9,0xa0,0x81,0x62}}; - return CoCreateInstance(&CLSID_GStreamerByteStreamHandler, outer, CLSCTX_INPROC_SERVER, riid, out); + NTSTATUS status; + + if ((status = winedmo_demuxer_check("audio/wav")) || use_gst_byte_stream_handler()) + { + static const GUID CLSID_GStreamerByteStreamHandler = {0x317df618,0x5e5a,0x468a,{0x9f,0x15,0xd8,0x27,0xa9,0xa0,0x81,0x62}}; + if (status) WARN("Unsupported demuxer, status %#lx.\n", status); + return CoCreateInstance(&CLSID_GStreamerByteStreamHandler, outer, CLSCTX_INPROC_SERVER, riid, out); + } + + return byte_stream_plugin_create(outer, riid, out); } static const IClassFactoryVtbl wav_byte_stream_plugin_factory_vtbl = -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6442
5988 achieves deterministic ordering according to my testing.
I don't know, maybe it does now. Who knows what GStreamer does here anyway, it could very well use threads, or not.
Which deadlocks?
Please, come on, people report them from time to time on IRC, and not so long ago you were even asking them for details... Here's one: Spyro deadlocks with 5988.
Which media files are you seeing this with? I'm seeing creation time under 1 ms with 5988, which is already faster than the time given for Windows, and fast enough to fix the application bugs mentioned.
Nothing special just some locally generated .mp4 file. It takes 35-65ms on my computer to resolve with the current code, takes 2.5-5ms with 5988. In a Windows VM on the same computer takes 0.2-0.5ms. Some Unreal Engine versions have a race condition that triggers if this takes more than 200ms, which is large compared to the improved results, but still possible on low-end computers, we want to make this as small as possible. This also makes the demuxer usable to implement some specific native behavior. The WM synchronous reader for instance, performs reads from its caller thread, this is simply impossible to implement with GStreamer. In general, using FFmpeg directly also proved to be faster in all decoding scenarios. This is especially true on 32bit where GStreamer ORC color conversion performs badly. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6442#note_81157
v2: Drop the log thread check, there's not going to be internal threads until we allow them for decoding. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6442#note_81158
On Thu Sep 5 08:16:33 2024 +0000, Chunhao Hung wrote:
I also hope this implement will eventually fix various game video render issue under winegstreamer. I have see various game only has audio render. has no video render. but normal in proton wine implement Probably not many. Most video issues I've seen are missing PE-side functionality.
But it could fix a few, like the deadlock rbernon mentioned. And if we're lucky, it could simplify the code and make the video issues easier to fix. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6442#note_81159
On Thu Sep 5 08:47:54 2024 +0000, Alfred Agrell wrote:
Probably not many. Most video issues I've seen are missing PE-side functionality. But it could fix a few, like the deadlock rbernon mentioned. And if we're lucky, it could simplify the code and make the video issues easier to fix. So the video render is black, audio is correct, is actually an issue on wine. I always wondering that if there are something error on my PC setup.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6442#note_81164
5988 achieves deterministic ordering according to my testing.
I don't know, maybe it does now. Who knows what GStreamer does here anyway, it could very well use threads, or not.
decodebin explicitly exposes pads in a certain order. The only thing preventing that from working in Wine was the appended decoders. So yes, we know what GStreamer does here. GStreamer is making this guarantee just as much as ffmpeg is.
Which deadlocks?
Please, come on, people report them from time to time on IRC, and not so long ago you were even asking them for details...
I'm trying to ask for details because I've seen two reported deadlocks, both of which were application bugs, and according to my analyses, changing the threads we make those requests from isn't actually going to fix anything. Rewriting an entire component is a waste of time if we don't fully understand a deadlock and why Windows avoids it, and why we can be sure we will also avoid it. And, again, we *can* change the threads we make read requests from without needing to rewrite anything. You *can* handle pending read requests from the same thread that you call wg_parser_stream_get_buffer() from.
Here's one: Spyro deadlocks with 5988.
The deadlock with Spyro is already fixed by !6444, but thanks for reporting it. I don't know whether that's the same kind of deadlock you're trying to fix here, but if so, clearly we don't need to rewrite an entire component to do it.
Which media files are you seeing this with? I'm seeing creation time under 1 ms with 5988, which is already faster than the time given for Windows, and fast enough to fix the application bugs mentioned.
Nothing special just some locally generated .mp4 file. It takes 35-65ms on my computer to resolve with the current code, takes 2.5-5ms with 5988. In a Windows VM on the same computer takes 0.2-0.5ms.
Some Unreal Engine versions have a race condition that triggers if this takes more than 200ms, which is large compared to the improved results, but still possible on low-end computers, we want to make this as small as possible.
What kind of video file? I'm not seeing 5988 perform any worse than Windows with any video file I can find; often it performs about 50% better. Even on a slow old laptop (slow enough that it took me hours to compile Wine even with ccache) I'm not seeing any slower than 50 ms. Even if that 200 ms is a constant regardless of processor speed, we're already multiple times faster than we need to be. Even if ffmpeg resolution can be faster—and there's no inherent reason to assume it can—there's no reason we need to rewrite this code.
In general, using FFmpeg directly also proved to be faster in all decoding scenarios. This is especially true on 32bit where GStreamer ORC color conversion performs badly.
A bug in one component of a library we use is not grounds to rewrite our entire stack to use a different library. What happens when we find a comparable problem in ffmpeg? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6442#note_81305
On Fri Sep 6 00:00:49 2024 +0000, Elizabeth Figura wrote:
5988 achieves deterministic ordering according to my testing.
I don't know, maybe it does now. Who knows what GStreamer does here anyway, it could very well use threads, or not. decodebin explicitly exposes pads in a certain order. The only thing preventing that from working in Wine was the appended decoders. So yes, we know what GStreamer does here. GStreamer is making this guarantee just as much as ffmpeg is.
Which deadlocks?
Please, come on, people report them from time to time on IRC, and not so long ago you were even asking them for details... I'm trying to ask for details because I've seen two reported deadlocks, both of which were application bugs, and according to my analyses, changing the threads we make those requests from isn't actually going to fix anything. Rewriting an entire component is a waste of time if we don't fully understand a deadlock and why Windows avoids it, and why we can be sure we will also avoid it. And, again, we *can* change the threads we make read requests from without needing to rewrite anything. You *can* handle pending read requests from the same thread that you call wg_parser_stream_get_buffer() from. Here's one: Spyro deadlocks with 5988. The deadlock with Spyro is already fixed by !6444, but thanks for reporting it. I don't know whether that's the same kind of deadlock you're trying to fix here, but if so, clearly we don't need to rewrite an entire component to do it.
Which media files are you seeing this with? I'm seeing creation time under 1 ms with 5988, which is already faster than the time given for Windows, and fast enough to fix the application bugs mentioned.
Nothing special just some locally generated .mp4 file. It takes 35-65ms on my computer to resolve with the current code, takes 2.5-5ms with 5988. In a Windows VM on the same computer takes 0.2-0.5ms.
Some Unreal Engine versions have a race condition that triggers if this takes more than 200ms, which is large compared to the improved results, but still possible on low-end computers, we want to make this as small as possible. What kind of video file? I'm not seeing 5988 perform any worse than Windows with any video file I can find; often it performs about 50% better. Even on a slow old laptop (slow enough that it took me hours to compile Wine even with ccache) I'm not seeing any slower than 50 ms. Even if that 200 ms is a constant regardless of processor speed, we're already multiple times faster than we need to be. Even if ffmpeg resolution can be faster—and there's no inherent reason to assume it can—there's no reason we need to rewrite this code. In general, using FFmpeg directly also proved to be faster in all decoding scenarios. This is especially true on 32bit where GStreamer ORC color conversion performs badly. A bug in one component of a library we use is not grounds to rewrite our entire stack to use a different library. What happens when we find a comparable problem in ffmpeg? For a user perspective. I support add another back end as long as it will make developer easier or faster to fix the BUG. I still got lots of game has video render issue. Game from Arekara4nen has repeat video, but it only repeat once and stuck. Chilla art game all has video render issue etc. Same as Rune factory 5. I did not want those BUG take another 10 or 20 years to fix ....
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6442#note_81308
I'm trying to ask for details because I've seen two reported deadlocks, both of which were application bugs, and according to my analyses, changing the threads we make those requests from isn't actually going to fix anything.
Doesn't help much to say things are application bugs. if they work on Windows it's a Wine bug, regardless of how theoretically incorrect the application is.
Rewriting an entire component is a waste of time if we don't fully understand a deadlock and why Windows avoids it, and why we can be sure we will also avoid it.
At least it would be frontend API specific problems to solve, which can be solved without worrying about breaking other frontends with some backend threading changes.
And, again, we can change the threads we make read requests from without needing to rewrite anything. You can handle pending read requests from the same thread that you call wg_parser_stream_get_buffer() from.
Sure through very convoluted ways we maybe can. FFmpeg is simply better there because it doesn't force any threading pattern upon us. Anyway, having experimented with FFmpeg up to the point where it mostly passes all our tests, I can say that it's definitely a better fit for us as a backend. 1) It doesn't force any unix side threads upon us, it can still use internal decoding threads but hides them and we don't need to care, 2) it is performing better in every decoding scenario I tried with (20% faster on a given H264 4K video), 3) it is simpler, the whole branch is ~6K LoC shorter than winegstreamer, while passing most of the tests (with some additional successes, and a couple of remaining failures), 4) most of the code is moved to the frontend modules, IMO much better for maintainability, given how hard it has been to get reviews for the MF media source, I'd say that's a huge win. Now if you claim that all the winegtreamer bugs are solved, very good, it's only a strategical choice left. Up to julliard to decide what he thinks is best. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6442#note_81322
On Fri Sep 6 07:34:21 2024 +0000, Rémi Bernon wrote: > > I'm trying to ask for details because I've seen two reported > deadlocks, both of which were application bugs, and according to my > analyses, changing the threads we make those requests from isn't > actually going to fix anything. > Doesn't help much to say things are application bugs. if they work on > Windows it's a Wine bug, regardless of how theoretically incorrect the > application is. > > Rewriting an entire component is a waste of time if we don't fully > understand a deadlock and why Windows avoids it, and why we can be sure > we will also avoid it. > At least it would be frontend API specific problems to solve, which can > be solved without worrying about breaking other frontends with some > backend threading changes. > > And, again, we can change the threads we make read requests from > without needing to rewrite anything. You can handle pending read > requests from the same thread that you call > wg_parser_stream_get_buffer() from. > Sure through very convoluted ways we maybe can. FFmpeg is simply better > there because it doesn't force any threading pattern upon us. > Anyway, having experimented with FFmpeg up to the point where it mostly > passes all our tests, I can say that it's definitely a better fit for us > as a backend. > 1) It doesn't force any unix side threads upon us, it can still use > internal decoding threads but hides them and we don't need to care, > 2) it is performing better in every decoding scenario I tried with (20% > faster on a given H264 4K video), > 3) it is simpler, the whole branch is ~6K LoC shorter than > winegstreamer, while passing most of the tests (with some additional > successes, and a couple of remaining failures), > 4) most of the code is moved to the frontend modules, IMO much better > for maintainability, given how hard it has been to get reviews for the > MF media source, I'd say that's a huge win. > Now if you claim that all the winegtreamer bugs are solved, very good, > it's only a strategical choice left. Up to julliard to decide what he > thinks is best. As a mostly impartial party between you two: zf is right that short-term, rewriting such a big piece involves huge regression risk and unclear benefits, especially with how many apps do bizarre things; it would be easier to hack around whatever issues still exist in winegstreamer. winetest will catch many possible regressions, but some craziness never made its way there. However, long-term, rbernon is right that if we get past the regressions, it will closer match native, simplify future maintenance, get rid of a bunch of hacks, and reduce the amount of cross-thread communication. I don't know which (if any) immediate app compat benefits would manifest, but simplifying the codebase is benefit enough that I'd vote yes. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6442#note_81333
Rewriting an entire component is a waste of time if we don't fully understand a deadlock and why Windows avoids it, and why we can be sure we will also avoid it.
At least it would be frontend API specific problems to solve, which can be solved without worrying about breaking other frontends with some backend threading changes.
I don't understand this, sorry. How does changing the library we use make this any more "frontend API specific problems"?
And, again, we can change the threads we make read requests from without needing to rewrite anything. You can handle pending read requests from the same thread that you call wg_parser_stream_get_buffer() from.
Sure through very convoluted ways we maybe can. FFmpeg is simply better there because it doesn't force any threading pattern upon us.
Queueing requests and consuming them isn't convoluted. That's a ubiquitous pattern.
Anyway, having experimented with FFmpeg up to the point where it mostly passes all our tests, I can say that it's definitely a better fit for us as a backend.
1. It doesn't force any unix side threads upon us, it can still use internal decoding threads but hides them and we don't need to care,
ffmpeg doesn't actually guarantee that in the documentation. If we're willing to rely on internals like that, we can get the same behaviour from gstreamer by overriding the default thread pool.
2. it is performing better in every decoding scenario I tried with (20% faster on a given H264 4K video),
20% speed difference cannot possibly be attributable simply to glue. Was GStreamer using avdec_h264 here? Was software decoding being used in both cases?
3. it is simpler, the whole branch is \~6K LoC shorter than winegstreamer, while passing most of the tests (with some additional successes, and a couple of remaining failures),
That can't possibly be true; there's only 6000 lines of GStreamer glue in the first place. (And about a thousand lines of that would go away if we didn't have two separate paths to translate media types.)
4. most of the code is moved to the frontend modules, IMO much better for maintainability, given how hard it has been to get reviews for the MF media source, I'd say that's a huge win.
I don't even understand this. The code that's on the Unix side of winegstreamer is Unix side glue, I don't even see why any of it would be moved to the PE side without unnecessarily convolution or duplication. And whether someone reviews and points out problems with code has nothing to do with where it is. Since I've been ousted as maintainer it's not like I have any say over the "backend" compared to the "frontend" anyway. Now, with that said, here are the problems I see with ffmpeg and this rewrite: (1) Much worse documentation. Many symbols simply aren't documented. (2) Narrower codec support. Perhaps nobody cares about this except for our users, but broad codec support is a useful feature. By virtue of wrapping ffmpeg, GStreamer necessarily has broader support. (3) In 23 years of existence, GStreamer has broken backwards compatibility once. ffmpeg was started at about the same time and has broken backwards compatibility 61 times. (4) This rewrite throws away 7 years of active development and maintenance, and understanding of the GStreamer libraries, and wastes time reviewing, testing, and debugging. It does this without actually fixing any bugs. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6442#note_81415
For what it's worth, replacing GStreamer with ffmpeg is something I have thought about for some time. The API fit is broadly better. However, those reasons are always what has led me to believe that it's simply not worth it. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6442#note_81420
On Fri Sep 6 18:19:03 2024 +0000, Elizabeth Figura wrote:
Rewriting an entire component is a waste of time if we don't fully understand a deadlock and why Windows avoids it, and why we can be sure we will also avoid it.
At least it would be frontend API specific problems to solve, which can be solved without worrying about breaking other frontends with some backend threading changes. I don't understand this, sorry. How does changing the library we use make this any more "frontend API specific problems"?
And, again, we can change the threads we make read requests from without needing to rewrite anything. You can handle pending read requests from the same thread that you call wg_parser_stream_get_buffer() from.
Sure through very convoluted ways we maybe can. FFmpeg is simply better there because it doesn't force any threading pattern upon us. Queueing requests and consuming them isn't convoluted. That's a ubiquitous pattern. Anyway, having experimented with FFmpeg up to the point where it mostly passes all our tests, I can say that it's definitely a better fit for us as a backend.
1. It doesn't force any unix side threads upon us, it can still use internal decoding threads but hides them and we don't need to care, ffmpeg doesn't actually guarantee that in the documentation. If we're willing to rely on internals like that, we can get the same behaviour from gstreamer by overriding the default thread pool. 2. it is performing better in every decoding scenario I tried with (20% faster on a given H264 4K video), 20% speed difference cannot possibly be attributable simply to glue. Was GStreamer using avdec_h264 here? Was software decoding being used in both cases? 3. it is simpler, the whole branch is \~6K LoC shorter than winegstreamer, while passing most of the tests (with some additional successes, and a couple of remaining failures), That can't possibly be true; there's only 6000 lines of GStreamer glue in the first place. (And about a thousand lines of that would go away if we didn't have two separate paths to translate media types.) 4. most of the code is moved to the frontend modules, IMO much better for maintainability, given how hard it has been to get reviews for the MF media source, I'd say that's a huge win. I don't even understand this. The code that's on the Unix side of winegstreamer is Unix side glue, I don't even see why any of it would be moved to the PE side without unnecessarily convolution or duplication. And whether someone reviews and points out problems with code has nothing to do with where it is. Since I've been ousted as maintainer it's not like I have any say over the "backend" compared to the "frontend" anyway. Now, with that said, here are the problems I see with ffmpeg and this rewrite: (1) Much worse documentation. Many symbols simply aren't documented. (2) Narrower codec support. Perhaps nobody cares about this except for our users, but broad codec support is a useful feature. By virtue of wrapping ffmpeg, GStreamer necessarily has broader support. (3) In 23 years of existence, GStreamer has broken backwards compatibility once. ffmpeg was started at about the same time and has broken backwards compatibility 61 times. (4) This rewrite throws away 7 years of active development and maintenance, and understanding of the GStreamer libraries, and wastes time reviewing, testing, and debugging. It does this without actually fixing any bugs. To 4: I think the fact that after all those years something as simple as passing through media data to an api is still being wrangled with and stretched to fit our purposes this much, which has led most of the developers working on it to come to the conclusion that it needs a change, is enough to let go of the sink cost feeling.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6442#note_81435
On Fri Sep 6 18:19:03 2024 +0000, Derek Lesho wrote:
To 4: I think the fact that after all those years something as simple as passing through media data to an api is still being wrangled with and stretched to fit our purposes this much, which has led most of the developers working on it to come to the conclusion that it needs a change, is enough to let go of the sink cost feeling. But... it's not. The problem is solved; it's been solved for quite some time now.
There have been two changes to the area of buffer passing: once when the PE/Unix conversion was introduced, and once to add zero-copy support. That hardly seems like a constant wrangling. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6442#note_81442
have you ever build with 32bit. I have build fail without on 32bit show mfasfsrcsnk did not link winedmo -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6442#note_81562
I don't understand this, sorry. How does changing the library we use make this any more "frontend API specific problems"?
Because being synchronous and thread-free the backend is agnostic to any use case and frontends can decide to organize their calls the way they like, letting us implement the same threading pattern as native in each frontend.
Queueing requests and consuming them isn't convoluted. That's a ubiquitous pattern.
It is definitely more convoluted than not having to do anything.
20% speed difference cannot possibly be attributable simply to glue. Was GStreamer using avdec_h264 here? Was software decoding being used in both cases?
Both are using the same FFmpeg libraries, so any overhead is due to either GStreamer or winegstreamer. GStreamer-based H264 decoder takes 4.6ms per output frame on average to decode the full "Vampire: The Masquerade - Bloodhunt" intro video to I420 (so even without any kind of color conversion), the FFmpeg-based decoder code takes 3.5ms per frame on average.
That can't possibly be true; there's only 6000 lines of GStreamer glue in the first place. (And about a thousand lines of that would go away if we didn't have two separate paths to translate media types.)
With a change to drop winegstreamer entirely to the wip/winedmo branch in my tree: $ git diff origin/master..wip/winedmo --shortstat 90 files changed, 19455 insertions(+), 25703 deletions(-) There's some things left unimplemented here and there, and most importantly I didn't reimplement any kind of generic parser, but it still passes most of the tests (and fixes multiple todo_wine) and 6K LoC leaves a lot of room for the missing bits.
(1) Much worse documentation. Many symbols simply aren't documented.
To the contraty to GStreamer, we don't actually need to use much of the API, and to understand much about it either, everything is completely straightforward. The headers are well documented, proof is that I've been able to rewrite pretty much everything in a couple of weeks, and didn't feel at loss at any point.
(2) Narrower codec support. Perhaps nobody cares about this except for our users, but broad codec support is a useful feature. By virtue of wrapping ffmpeg, GStreamer necessarily has broader support.
GStreamer does not necessarily have a broader support, and the main reason is that it disables several of the FFmpeg-based formats because they have considered a long time ago that the gst-libav code was not good enough. They instead have re-implemented several of them differently and never reconsidered that decision. If you have some examples of formats supported by GStreamer and not by FFmpeg, I am interested to know because I'm confident that FFmpeg is more widely used in the industry.
(4) This rewrite throws away 7 years of active development and maintenance, and understanding of the GStreamer libraries, and wastes time reviewing, testing, and debugging. It does this without actually fixing any bugs.
Of which I've written a large amount. I don't think it throws away anything, the code is still there in mostly the same form and the actual useful knowledge we acquired is about the Win32 side of things, this isn't lost. Tests aren't lost. Sure, this introduces potential regressions, and it throws away a lot of the testing that has been done on a different version of the code. I've been trying hard enough not to lose that before when I was still trying to use GStreamer, but that didn't seem to matter so much at the time. Then, many of the problems that the testing has found were in the frontend, and that isn't lost either. Regardless, it's not because we've put a lot of effort into something that we have to be stuck with it forever if it's not a good fit. I think that GStreamer complexity have forced to waste a huge amount of time and effort in the past reviews (say how hard I had to argue to get wg_transform, zero copy accepted), whereas if we had been using FFmpeg instead at that time, it would have been a no-brainer to implement (I did, it is absolutely plain simple and even still completely out-performing our current code).
For what it's worth, replacing GStreamer with ffmpeg is something I have thought about for some time. The API fit is broadly better. However, those reasons are always what has led me to believe that it's simply not worth it.
Then lets try it. Fwiw this is *not* replacing anything at this point or even in the near or far future, not until we decide it does, and instead it offers an alternative backend to users. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6442#note_81629
On Mon Sep 9 22:39:45 2024 +0000, Rémi Bernon wrote:
I don't understand this, sorry. How does changing the library we use make this any more "frontend API specific problems"? Because being synchronous and thread-free the backend is agnostic to any use case and frontends can decide to organize their calls the way they like, letting us implement the same threading pattern as native in each frontend. Queueing requests and consuming them isn't convoluted. That's a ubiquitous pattern. It is definitely more convoluted than not having to do anything. 20% speed difference cannot possibly be attributable simply to glue. Was GStreamer using avdec_h264 here? Was software decoding being used in both cases? Both are using the same FFmpeg libraries, so any overhead is due to either GStreamer or winegstreamer. GStreamer-based H264 decoder takes 4.6ms per output frame on average to decode the full "Vampire: The Masquerade - Bloodhunt" intro video to I420 (so even without any kind of color conversion), the FFmpeg-based decoder code takes 3.5ms per frame on average. That can't possibly be true; there's only 6000 lines of GStreamer glue in the first place. (And about a thousand lines of that would go away if we didn't have two separate paths to translate media types.) With a change to drop winegstreamer entirely to the wip/winedmo branch in my tree: $ git diff origin/master..wip/winedmo --shortstat 90 files changed, 19455 insertions(+), 25703 deletions(-) There's some things left unimplemented here and there, and most importantly I didn't reimplement any kind of generic decoding parser, but it still passes most of the tests (and fixes multiple todo_wine) and 6K LoC leaves a lot of room for the missing bits. (1) Much worse documentation. Many symbols simply aren't documented. To the contraty to GStreamer, we don't actually need to use much of the API, and to understand much about it either, everything is completely straightforward. The headers are well documented, proof is that I've been able to rewrite pretty much everything in a couple of weeks, and didn't feel at loss at any point. (2) Narrower codec support. Perhaps nobody cares about this except for our users, but broad codec support is a useful feature. By virtue of wrapping ffmpeg, GStreamer necessarily has broader support. GStreamer does not necessarily have a broader support, and the main reason is that it disables several of the FFmpeg-based formats because they have considered a long time ago that the gst-libav code was not good enough. They instead have re-implemented several of them differently and never reconsidered that decision. If you have some examples of formats supported by GStreamer and not by FFmpeg, I am interested to know because I'm confident that FFmpeg is more widely used in the industry. (4) This rewrite throws away 7 years of active development and maintenance, and understanding of the GStreamer libraries, and wastes time reviewing, testing, and debugging. It does this without actually fixing any bugs. Of which I've written a large amount. I don't think it throws away anything, the code is still there in mostly the same form and the actual useful knowledge we acquired is about the Win32 side of things, this isn't lost. Tests aren't lost. Sure, this introduces potential regressions, and it throws away a lot of the testing that has been done on a different version of the code. I've been trying hard enough not to lose that before when I was still trying to use GStreamer, but that didn't seem to matter so much at the time. Then, many of the problems that the testing has found were in the frontend, and that isn't lost either. Regardless, it's not because we've put a lot of effort into something that we have to be stuck with it forever if it's not a good fit. I think that GStreamer complexity have forced to waste a huge amount of time and effort in the past reviews (say how hard I had to argue to get wg_transform, zero copy accepted), whereas if we had been using FFmpeg instead at that time, it would have been a no-brainer to implement (I did, it is absolutely plain simple and even still completely out-performing our current code). For what it's worth, replacing GStreamer with ffmpeg is something I have thought about for some time. The API fit is broadly better. However, those reasons are always what has led me to believe that it's simply not worth it. Then lets try it. Fwiw this is *not* replacing anything at this point or even in the near or far future, not until we decide it does, and instead it offers an alternative backend to users. hi as user how can test your winedmo branch (which dependencies are required?, need some 32bit dependencies?), and work for 32bit apps?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6442#note_81702
On Mon Sep 9 22:39:45 2024 +0000, Carlos Rodriguez wrote:
hi as user how can test your winedmo branch (which dependencies are required?, need some 32bit dependencies?), and work for 32bit apps? I have tested that branch before. somehow it stuck on audio decode for some reason, I do not know If that is my merge method incorrect or a BUG
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6442#note_81705
participants (6)
-
Alfred Agrell (＠Alcaro) -
Carlos Rodriguez (＠mrdeathjr28)
-
Chunhao Hung (＠otakuxtom)
-
Derek Lesho (＠dlesho)
-
Elizabeth Figura (＠zfigura)
-
Rémi Bernon