[PATCH 1/2] mshtmlmedia: Add dll.
Chromium-based browsers and CEF will load this file instead of msmpeg2vdec on Windows versions 7 and lower. Signed-off-by: Mohamad Al-Jaf <mohamadaljaf(a)gmail.com> --- configure.ac | 1 + dlls/mshtmlmedia/Makefile.in | 7 +++++++ dlls/mshtmlmedia/main.c | 32 +++++++++++++++++++++++++++++++ dlls/mshtmlmedia/mshtmlmedia.spec | 7 +++++++ dlls/mshtmlmedia/version.rc | 26 +++++++++++++++++++++++++ 5 files changed, 73 insertions(+) create mode 100644 dlls/mshtmlmedia/Makefile.in create mode 100644 dlls/mshtmlmedia/main.c create mode 100644 dlls/mshtmlmedia/mshtmlmedia.spec create mode 100644 dlls/mshtmlmedia/version.rc diff --git a/configure.ac b/configure.ac index d2e0127f262..1c20ce7b56a 100644 --- a/configure.ac +++ b/configure.ac @@ -2741,6 +2741,7 @@ WINE_CONFIG_MAKEFILE(dlls/msgsm32.acm) WINE_CONFIG_MAKEFILE(dlls/mshtml.tlb) WINE_CONFIG_MAKEFILE(dlls/mshtml) WINE_CONFIG_MAKEFILE(dlls/mshtml/tests) +WINE_CONFIG_MAKEFILE(dlls/mshtmlmedia) WINE_CONFIG_MAKEFILE(dlls/msi) WINE_CONFIG_MAKEFILE(dlls/msi/tests) WINE_CONFIG_MAKEFILE(dlls/msident) diff --git a/dlls/mshtmlmedia/Makefile.in b/dlls/mshtmlmedia/Makefile.in new file mode 100644 index 00000000000..35295737e51 --- /dev/null +++ b/dlls/mshtmlmedia/Makefile.in @@ -0,0 +1,7 @@ +MODULE = mshtmlmedia.dll + +C_SRCS = \ + main.c + +RC_SRCS = \ + version.rc diff --git a/dlls/mshtmlmedia/main.c b/dlls/mshtmlmedia/main.c new file mode 100644 index 00000000000..86580f7994e --- /dev/null +++ b/dlls/mshtmlmedia/main.c @@ -0,0 +1,32 @@ +/* + * Copyright 2022 Mohamad Al-Jaf + * + * 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 <stdarg.h> + +#include "windef.h" +#include "winbase.h" + +#include "wine/debug.h" + +WINE_DEFAULT_DEBUG_CHANNEL(mshtmlmedia); + +HRESULT WINAPI DllGetClassObject(REFCLSID clsid, REFIID riid, LPVOID *ppv) +{ + FIXME("(%s %s %p)\n", debugstr_guid(clsid), debugstr_guid(riid), ppv); + return CLASS_E_CLASSNOTAVAILABLE; +} diff --git a/dlls/mshtmlmedia/mshtmlmedia.spec b/dlls/mshtmlmedia/mshtmlmedia.spec new file mode 100644 index 00000000000..8303bf2c0f8 --- /dev/null +++ b/dlls/mshtmlmedia/mshtmlmedia.spec @@ -0,0 +1,7 @@ +@ stub DllAllThreadsDetach +@ stdcall -private DllCanUnloadNow() +@ stdcall -private DllGetClassObject(ptr ptr ptr) +@ stub MFCreateDXGIDeviceManager +@ stub MFCreateMFByteStreamOnStreamEx +@ stub MFLockDXGIDeviceManager +@ stub MFUnlockDXGIDeviceManager diff --git a/dlls/mshtmlmedia/version.rc b/dlls/mshtmlmedia/version.rc new file mode 100644 index 00000000000..d96c4c20f6e --- /dev/null +++ b/dlls/mshtmlmedia/version.rc @@ -0,0 +1,26 @@ +/* + * Copyright 2022 Mohamad Al-Jaf + * + * 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 + */ + +#define WINE_FILEDESCRIPTION_STR "Wine mshtmlmedia" +#define WINE_FILENAME_STR "mshtmlmedia.dll" +#define WINE_FILEVERSION 11,0,9600,17840 +#define WINE_FILEVERSION_STR "11.0.9600.17840" +#define WINE_PRODUCTVERSION 11,00,9600,17840 +#define WINE_PRODUCTVERSION_STR "11.00.9600.17840" + +#include "wine/wine_common_ver.rc" -- 2.36.0
On Windows 7, these functions are implemented directly in this dll. Signed-off-by: Mohamad Al-Jaf <mohamadaljaf(a)gmail.com> --- dlls/mshtmlmedia/Makefile.in | 1 + dlls/mshtmlmedia/mshtmlmedia.spec | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/dlls/mshtmlmedia/Makefile.in b/dlls/mshtmlmedia/Makefile.in index 35295737e51..bedeedd0a88 100644 --- a/dlls/mshtmlmedia/Makefile.in +++ b/dlls/mshtmlmedia/Makefile.in @@ -1,4 +1,5 @@ MODULE = mshtmlmedia.dll +IMPORTS = mfplat C_SRCS = \ main.c diff --git a/dlls/mshtmlmedia/mshtmlmedia.spec b/dlls/mshtmlmedia/mshtmlmedia.spec index 8303bf2c0f8..bf907573014 100644 --- a/dlls/mshtmlmedia/mshtmlmedia.spec +++ b/dlls/mshtmlmedia/mshtmlmedia.spec @@ -1,7 +1,7 @@ @ stub DllAllThreadsDetach @ stdcall -private DllCanUnloadNow() @ stdcall -private DllGetClassObject(ptr ptr ptr) -@ stub MFCreateDXGIDeviceManager -@ stub MFCreateMFByteStreamOnStreamEx -@ stub MFLockDXGIDeviceManager -@ stub MFUnlockDXGIDeviceManager +@ stdcall -import MFCreateDXGIDeviceManager(ptr ptr) +@ stdcall -import MFCreateMFByteStreamOnStreamEx(ptr ptr) +@ stdcall -import MFLockDXGIDeviceManager(ptr ptr) +@ stdcall -import MFUnlockDXGIDeviceManager() -- 2.36.0
On Fri, Apr 22, 2022 at 12:30 AM Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
I don't see this module on win10+, it's probably something internal to IE, that later got removed.
Right, it was superseded by mfplat on Windows 8+. I don't think it's just internal to IE, that would make Chromium on Windows 7 a frankenstein of IE+Chromium.
From the Chromium source code[1]:
// On Windows 8+ mfplat.dll provides the MFCreateDXGIDeviceManager API. // On Windows 7 mshtmlmedia.dll provides it.
The functions are already implemented in mfplat so there's no need to implement this dll any further. [1] https://source.chromium.org/chromium/chromium/src/+/main:media/gpu/windows/d... -- Kind regards, Mohamad
On 4/22/22 07:46, Mohamad Al-Jaf wrote:
On Fri, Apr 22, 2022 at 12:30 AM Nikolay Sivov<nsivov(a)codeweavers.com> wrote:
I don't see this module on win10+, it's probably something internal to IE, that later got removed. Right, it was superseded by mfplat on Windows 8+. I don't think it's just internal to IE, that would make Chromium on Windows 7 a frankenstein of IE+Chromium.
From the Chromium source code[1]:
// On Windows 8+ mfplat.dll provides the MFCreateDXGIDeviceManager API. // On Windows 7 mshtmlmedia.dll provides it. The functions are already implemented in mfplat so there's no need to implement this dll any further.
Do you see it being loaded at all? It's not always compiled in, and I don't immediately see if ENABLE_DX11_FOR_WIN7 is set anywhere.
[1]https://source.chromium.org/chromium/chromium/src/+/main:media/gpu/windows/d...
-- Kind regards, Mohamad
On Fri, Apr 22, 2022 at 12:55 AM Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
Do you see it being loaded at all? It's not always compiled in, and I don't immediately see if ENABLE_DX11_FOR_WIN7 is set anywhere.
Not on the latest versions but it's possible it was used in previous versions. I don't know where ENABLE_DX11_FOR_WIN7 is set, perhaps it's done internally at runtime after it checks the system? Seems odd for deadcode to exist in a project that's heavily reviewed and I really doubt this is the case. I don't see a problem with adding this dll regardless. It's unlikely to need work in the future given it's just a smaller version of mfplat and that the functions are already implemented in mfplat. -- Kind regards, Mohamad
In any case, this dll is also needed by the game NieR Replicant ver.1.22474487139 to prevent a crash when it attempts to load mshtmlmedia.dll to play a cutscene. It looks like it calls MFCreateDXGIDeviceManager in this dll. [1] https://steamcommunity.com/app/1113560/discussions/0/3055112065897151937/ [2] https://steamcommunity.com/app/1113560/discussions/0/3153060451660242023/ [3] https://www.reddit.com/r/nier/comments/od7zks/nier_replicant_petrify_kaine_c... -- Kind regards, Mohamad
On 4/23/22 06:11, Mohamad Al-Jaf wrote:
In any case, this dll is also needed by the game NieR Replicant ver.1.22474487139 to prevent a crash when it attempts to load mshtmlmedia.dll to play a cutscene. It looks like it calls MFCreateDXGIDeviceManager in this dll.
[1] https://steamcommunity.com/app/1113560/discussions/0/3055112065897151937/
[2] https://steamcommunity.com/app/1113560/discussions/0/3153060451660242023/
[3] https://www.reddit.com/r/nier/comments/od7zks/nier_replicant_petrify_kaine_c... -- Kind regards, Mohamad Is it needed when you run it on wine?
On 4/23/22 10:16, Mohamad Al-Jaf wrote:
On Sat, Apr 23, 2022 at 2:10 AM Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
Is it needed when you run it on wine? It depends on whether Wine is running in Windows 7 mode or not. If it is, then yes it is needed.
What I mean is, have you checked that?
-- Kind regards, Mohamad
On Sat, Apr 23, 2022 at 4:01 AM Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
What I mean is, have you checked that?
Why would that be necessary? It's clear that the game tries to load mshtmlmedia under Windows 7 so why wouldn't this be the case in Wine running in Windows 7 mode? -- Kind regards, Mohamad
On 4/23/22 11:57, Mohamad Al-Jaf wrote:
On Sat, Apr 23, 2022 at 4:01 AM Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
What I mean is, have you checked that? Why would that be necessary? It's clear that the game tries to load mshtmlmedia under Windows 7 so why wouldn't this be the case in Wine running in Windows 7 mode?
Testing on wine would be necessary to see if it's needed. It's not clear at all that game loads it. Wine offers more functionality in certain modules than Windows 7, simply setting prefix version does not change that. Even if it does, at this point it makes more sense to bump default prefix version to Win10.
-- Kind regards, Mohamad
On Sat, Apr 23, 2022 at 8:17 AM Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
It's not clear at all that game loads it. Wine offers more functionality in certain modules than Windows 7, simply setting prefix version does not change that.
This doesn't matter. If the game detects it's running under Windows 7 it will try to load mshtmlmedia to play cutscenes, not mfplat. Under Windows 7, mfplat does not have the function MFCreateDXGIDeviceManager, which the game calls. It's in mshtmlmedia. Changing the version changes application behavior. This is evident in many applications, like Microsoft Edge, which significantly changes the libraries it attempts to load for example. I think it's very clear that it tries to load it. Perhaps you didn't read the comments in the links I sent, here's one that should make it clear[1]:
Originally posted by Bingus: You're correct in that I should have been more specific. The function does not appear in the same library in Windows 7, meaning that it doesn't exist in the way it's being called. MFCreateDXGIDeviceManager is in MshtmlMedia.dll on Windows 7, and in the MFPlat.dll on Windows 8 and higher.
Playing on Windows 7 here. I have had the game crash at the title screen when waiting for the intro trailers to appear while idling. I've also had the game crash at the end of Act 1 when the cutscene would appear.
I decided to look up and grab the MshtmlMedia.dll file to see if that would possibly change anything. I am now able to watch the intro trailers and have been able to progress past Act 2.
I'm not sure if this will help anyone else but I can at least say it helped fix my crashing issue for the cutscenes.
Even if it does, at this point it makes more sense to bump default prefix version to Win10.
As stated earlier, changing the Windows version changes the behavior of the application. For instance, Windows 7 lacks DX12, with only some exceptions like World of Warcraft. The game could default to using DX11 or lower, which could yield improved performance and fewer glitches, for example. Wine in general offers better compatibility with older versions of Windows. It makes sense to support them. In fact, this is one area in which Wine is objectively superior to Windows. Compatibility with older applications is often far better under Wine than in newer Windows versions.
Testing on wine would be necessary to see if it's needed.
Well, I've provided more than enough evidence to backup my reasoning, but I'm open to testing the game. Please send me a copy of the game. CodeWeavers is partnered with Valve so this shouldn't be a problem. Also, I'll have to play the entire game to confirm for sure. This will take a significant amount of time to thoroughly test as it can happen either in the main menu or way later in the game at a specific cutscene. A job would be nice. [1] https://steamcommunity.com/app/1113560/discussions/0/3153060451660242023/#c3... -- Kind regards, Mohamad
On Sat, Apr 23, 2022, 07:17 Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
On 4/23/22 11:57, Mohamad Al-Jaf wrote:
On Sat, Apr 23, 2022 at 4:01 AM Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
What I mean is, have you checked that? Why would that be necessary? It's clear that the game tries to load mshtmlmedia under Windows 7 so why wouldn't this be the case in Wine running in Windows 7 mode?
Testing on wine would be necessary to see if it's needed.
I have to agree with Mohamad here. AFAIK, it's never been a requirement that a patch actually be verified before committing it: 1) I don't see it mentioned on https://wiki.winehq.org/Submitting_Patches 2) There are over a thousand bugs that have been closed fixed that contain the comment 'Should be fixed by': https://bugs.winehq.org/buglist.cgi?bug_status=CLOSED&limit=0&list_id=762726... (I realize that isn't an accurate count, but demonstrates my broader point). If there's constructive feedback that hasn't been given, by all means, give it, but "it hasn't been explicitly tested" isn't a reason to block in standard wine development. Maybe we should revisit that, but until we do, I don't think it should block this patch.
On 4/24/22 11:48, Austin English wrote:
On Sat, Apr 23, 2022, 07:17 Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
On 4/23/22 11:57, Mohamad Al-Jaf wrote:
On Sat, Apr 23, 2022 at 4:01 AM Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
What I mean is, have you checked that? Why would that be necessary? It's clear that the game tries to load mshtmlmedia under Windows 7 so why wouldn't this be the case in Wine running in Windows 7 mode? Testing on wine would be necessary to see if it's needed.
I have to agree with Mohamad here. AFAIK, it's never been a requirement that a patch actually be verified before committing it:
1) I don't see it mentioned on https://wiki.winehq.org/Submitting_Patches
2) There are over a thousand bugs that have been closed fixed that contain the comment 'Should be fixed by': https://bugs.winehq.org/buglist.cgi?bug_status=CLOSED&limit=0&list_id=762726...
(I realize that isn't an accurate count, but demonstrates my broader point).
Your point is that there are more CLOSED/FIXED reports that don't have "should be fixed by" ? One way to improve this situation was to link bug urls in commit messages. We should definitely continue improving it, for better traceability. If you mean that "should be fixed" translates to "I think it might be fixed, but I'm not sure", in my experience it usually means the opposite, when commenter actually tested it one or another to make sure.
If there's constructive feedback that hasn't been given, by all means, give it, but "it hasn't been explicitly tested" isn't a reason to block in standard wine development. Maybe we should revisit that, but until we do, I don't think it should block this patch.
It depends really. In this case it's some internal IE11 (or maybe a bit older) module, with only justification that some users, running actual Windows 7 installation, found it useful. Second example is that it was mentioned in chromium sources somewhere. Is it unreasonable to discuss if it's useful for Wine? At the same time, I agree of course that just the contributor's desire to work on something is enough to consider submitted work. For this patch what kind of feedback would you expect? P.S. needless to say, my replies are my own, I don't have the power to reject submitted work, nor would I like to have it.
On Sun, Apr 24, 2022 at 4:38 AM Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
On 4/24/22 11:48, Austin English wrote:
On Sat, Apr 23, 2022, 07:17 Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
On 4/23/22 11:57, Mohamad Al-Jaf wrote:
On Sat, Apr 23, 2022 at 4:01 AM Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
What I mean is, have you checked that? Why would that be necessary? It's clear that the game tries to load mshtmlmedia under Windows 7 so why wouldn't this be the case in Wine running in Windows 7 mode? Testing on wine would be necessary to see if it's needed.
I have to agree with Mohamad here. AFAIK, it's never been a requirement that a patch actually be verified before committing it:
1) I don't see it mentioned on https://wiki.winehq.org/Submitting_Patches
2) There are over a thousand bugs that have been closed fixed that contain the comment 'Should be fixed by':
https://bugs.winehq.org/buglist.cgi?bug_status=CLOSED&limit=0&list_id=762726...
(I realize that isn't an accurate count, but demonstrates my broader
point).
Your point is that there are more CLOSED/FIXED reports that don't have "should be fixed by" ? One way to improve this situation was to link bug urls in commit messages. We should definitely continue improving it, for better traceability.
If you mean that "should be fixed" translates to "I think it might be
fixed, but I'm not sure", in my experience it usually means the opposite, when commenter actually tested it one or another to make sure.
If there's constructive feedback that hasn't been given, by all means,
give
it, but "it hasn't been explicitly tested" isn't a reason to block in standard wine development. Maybe we should revisit that, but until we do, I don't think it should block this patch.
It depends really. In this case it's some internal IE11 (or maybe a bit older) module, with only justification that some users, running actual Windows 7 installation, found it useful. Second example is that it was mentioned in chromium sources somewhere. Is it unreasonable to discuss if it's useful for Wine?
At the same time, I agree of course that just the contributor's desire to work on something is enough to consider submitted work. For this patch what kind of feedback would you expect?
P.S. needless to say, my replies are my own, I don't have the power to reject submitted work, nor would I like to have it.
-- -Austin GPG: 267B CC1F 053F 0749 (expires 2024/02/17)
On Apr 24, 2022 at 4:38 AM Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
On 4/24/22 11:48, Austin English wrote:
On Sat, Apr 23, 2022, 07:17 Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
On 4/23/22 11:57, Mohamad Al-Jaf wrote:
On Sat, Apr 23, 2022 at 4:01 AM Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
What I mean is, have you checked that? Why would that be necessary? It's clear that the game tries to load mshtmlmedia under Windows 7 so why wouldn't this be the case in Wine running in Windows 7 mode? Testing on wine would be necessary to see if it's needed.
I have to agree with Mohamad here. AFAIK, it's never been a requirement that a patch actually be verified before committing it:
1) I don't see it mentioned on https://wiki.winehq.org/Submitting_Patches
2) There are over a thousand bugs that have been closed fixed that contain the comment 'Should be fixed by':
https://bugs.winehq.org/buglist.cgi?bug_status=CLOSED&limit=0&list_id=762726...
(I realize that isn't an accurate count, but demonstrates my broader
point).
---Sorry for preemptive send--- FYI, I was trying to comment constructively, not aggressively.
Your point is that there are more CLOSED/FIXED reports that don't have "should be fixed by" ? One way to improve this situation was to link bug urls in commit messages. We should definitely continue improving it, for better traceability.
I agree, we should absolutely try to improve commit messages/link bug reports appropriately. If you mean that "should be fixed" translates to "I think it might be
fixed, but I'm not sure", in my experience it usually means the opposite, when commenter actually tested it one or another to make sure.
Sometimes it does (which is why the overall number isn't accurate), but a lot of developers also use more definitive phrases like 'this is fixed by XXXXX'. I also recall several bugs that were commented/closed as 'should be fixed by ..' and then reopened after testing confirmed it hadn't been fixed..and for reasons that the author could/should have realized. Generally for bugs that have phrases like 'this is fixed by XXXXX' don't have that mistake.
If there's constructive feedback that hasn't been given, by all means,
give
it, but "it hasn't been explicitly tested" isn't a reason to block in standard wine development. Maybe we should revisit that, but until we do, I don't think it should block this patch.
It depends really. In this case it's some internal IE11 (or maybe a bit older) module, with only justification that some users, running actual Windows 7 installation, found it useful. Second example is that it was mentioned in chromium sources somewhere. Is it unreasonable to discuss if it's useful for Wine?
No, I think it's absolutely reasonable to question it, to a point. But given that the codebase shows it's possible, and that there are public reports showing that it happens is reasonable evidence. At the same time, I agree of course that just the contributor's desire
to work on something is enough to consider submitted work. For this patch what kind of feedback would you expect?
I think the feedback/review, in general, has been fair/valid. I am disagreeing in how explicit Wine is being, as a project, for testing patches before submitting upstream.
P.S. needless to say, my replies are my own, I don't have the power to reject submitted work, nor would I like to have it.
I agree that _actually_ testing would be ideal, but I don't think it should be a _blocker_. That said, I'm in the same position, and my position is my own. I raise it because I feel like few others were giving feedback :). -- -Austin GPG: 267B CC1F 053F 0749 (expires 2024/02/17)
Austin English <austinenglish(a)gmail.com> writes:
At the same time, I agree of course that just the contributor's desire to work on something is enough to consider submitted work. For this patch what kind of feedback would you expect?
I think the feedback/review, in general, has been fair/valid. I am disagreeing in how explicit Wine is being, as a project, for testing patches before submitting upstream.
The issue is not testing the patch, it's demonstrating that there's a need for it in the first place. That has been the policy forever, we only implement things that are actually used by a real application. There are tons of things in the Windows API that are used only internally, or no longer relevant, or simply never used at all. Adding them only increases our workload for no benefit. -- Alexandre Julliard julliard(a)winehq.org
On Sun, Apr 24, 2022 at 5:26 AM Alexandre Julliard <julliard(a)winehq.org> wrote:
Austin English <austinenglish(a)gmail.com> writes:
At the same time, I agree of course that just the contributor's desire to work on something is enough to consider submitted work. For this patch what kind of feedback would you expect?
I think the feedback/review, in general, has been fair/valid. I am disagreeing in how explicit Wine is being, as a project, for testing patches before submitting upstream.
The issue is not testing the patch, it's demonstrating that there's a need for it in the first place.
Can we clarify what is needed to demonstrate that this patch is needed? I.e., it would be helpful, particularly to new/infrequent developers, to be able to note when it's necessary for a developer to explicitly test a patch. I.e., if it involves IE internals rather than win32, if the API is not on MSDN, etc.
That has been the policy forever, we only implement things that are actually used by a real application. There are tons of things in the Windows API that are used only internally, or no longer relevant, or simply never used at all. Adding them only increases our workload for no benefit.
Yes, I'm aware, having been bit by it a few times myself, but thanks for the reminder ;). --
Alexandre Julliard julliard(a)winehq.org
-- -Austin GPG: 267B CC1F 053F 0749 (expires 2024/02/17)
Austin English <austinenglish(a)gmail.com> writes:
On Sun, Apr 24, 2022 at 5:26 AM Alexandre Julliard <julliard(a)winehq.org> wrote:
Austin English <austinenglish(a)gmail.com> writes:
At the same time, I agree of course that just the contributor's desire to work on something is enough to consider submitted work. For this patch what kind of feedback would you expect?
I think the feedback/review, in general, has been fair/valid. I am disagreeing in how explicit Wine is being, as a project, for testing patches before submitting upstream.
The issue is not testing the patch, it's demonstrating that there's a need for it in the first place.
Can we clarify what is needed to demonstrate that this patch is needed?
In the general case, pointing to a bug report showing an app running into the problem on standard Wine is a good first step. In this specific case, since it's not a Windows DLL but some internal IE thing, it would additionally need a convincing explanation of why we need the dll on Wine even though it's not present on most Windows installs. -- Alexandre Julliard julliard(a)winehq.org
On Sun, Apr 24, 2022 at 6:05 AM Austin English <austinenglish(a)gmail.com> wrote:
I agree that _actually_ testing would be ideal, but I don't think it should be a _blocker_. That said, I'm in the same position, and my position is my own. I raise it because I feel like few others were giving feedback :).
Thank you, Austin, I appreciate your input on the matter. On Sun, Apr 24, 2022 at 6:26 AM Alexandre Julliard <julliard(a)winehq.org> wrote:
The issue is not testing the patch, it's demonstrating that there's a need for it in the first place.
In this case, the only way to demonstrate there's a need for it is to test the game. And like I said earlier, this can only happen at specific cutscenes so it'll take a significant amount of time to test. I'm open to doing this, but I'll need the game itself to proceed. If you're serious about it please send me a CD-Key of the game. And while we're at it, please hire me as a tester. I can test patches for other games to confirm they fix issues. On Sun, Apr 24, 2022 at 8:44 AM Alexandre Julliard <julliard(a)winehq.org> wrote:
In this specific case, since it's not a Windows DLL but some internal IE thing, it would additionally need a convincing explanation of why we need the dll on Wine even though it's not present on most Windows installs.
I disagree that it's just an internal IE dll, it doesn't make sense to me. I think the name is misleading. It seems more like a part of the Media Foundation that was later merged into mfplat. This is backed by the exported mf functions. Furthermore, I don't think Chromium would use an internal IE component, that seems bizarre. Since the functions are already implemented, this dll would serve as a viable alternative to msmpeg2vdec in Chromium-based browsers. -- Kind regards, Mohamad
On Apr 21, 2022, at 10:40 PM, Mohamad Al-Jaf <mohamadaljaf(a)gmail.com> wrote:
On Fri, Apr 22, 2022 at 12:55 AM Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
Do you see it being loaded at all? It's not always compiled in, and I don't immediately see if ENABLE_DX11_FOR_WIN7 is set anywhere.
Not on the latest versions but it's possible it was used in previous versions. I don't know where ENABLE_DX11_FOR_WIN7 is set, perhaps it's done internally at runtime after it checks the system? Seems odd for deadcode to exist in a project that's heavily reviewed and I really doubt this is the case.
It actually does look like that’s the case and it’s never enabled, it was left in the Chromium source as a TODO 7 years ago (with no action since and likely none coming): “ Sadly the hack to get DX11 decoding to work on Windows 7 does not work. We can get a pointer to the MFCreateDXGIDeviceManager function and the device creation works. However the IMFTransform we use for color space conversion does not exist on Windows 7. There is a color converter DMO which technically could work. However it needs the video samples to expose functioning IMF2DBuffer interfaces which is not the case sadly. I left the code to get the pointer to the MFCreateDXGIDeviceManager function under an ifdef for Windows 7. I will look into this at a later point in time. " https://codereview.chromium.org/922003002/#msg12 Brendan
On Mon, Apr 25, 2022 at 11:47 AM Brendan Shanks <bshanks(a)codeweavers.com> wrote:
It actually does look like that’s the case and it’s never enabled, it was left in the Chromium source as a TODO 7 years ago (with no action since and likely none coming):
“ Sadly the hack to get DX11 decoding to work on Windows 7 does not work. We can get a pointer to the MFCreateDXGIDeviceManager function and the device creation works. However the IMFTransform we use for color space conversion does not exist on Windows 7. There is a color converter DMO which technically could work. However it needs the video samples to expose functioning IMF2DBuffer interfaces which is not the case sadly.
I left the code to get the pointer to the MFCreateDXGIDeviceManager function under an ifdef for Windows 7. I will look into this at a later point in time. "
https://codereview.chromium.org/922003002/#msg12
Brendan
I see, this makes sense then. Thanks for the constructive and thorough feedback, much appreciated. :) As for Nier Replicant, I tested the game in Windows 7 mode and it does try to load mshtmlmedia.dll. This, along with the comments from the Windows 7 users, should be reasonable evidence that it's needed. -- Kind regards, Mohamad
participants (5)
-
Alexandre Julliard -
Austin English -
Brendan Shanks -
Mohamad Al-Jaf -
Nikolay Sivov