[PATCH v4 0/3] MR2080: winegstreamer: GetInputStatus for h264, aac and MF_SA_D3D11_AWARE for h264.
Needed for Gungrave G.O.R.E. (besides some bits in raw AAC handling).
The game is fine with GetInputStatus always returning MFT_INPUT_STATUS_ACCEPT_DATA for both h264 and aac but I think it is better to be correct here.
-- v4: winegstreamer: Set MF_SA_D3D11_AWARE attribute for h264 transform. winegstreamer: Implement _GetInputStatus() for aac decoder transform.
From: Paul Gofman pgofman@codeweavers.com
--- dlls/mf/tests/transform.c | 31 +++++++++++++++++++++++++++++++ dlls/winegstreamer/h264_decoder.c | 11 +++++++++-- 2 files changed, 40 insertions(+), 2 deletions(-)
diff --git a/dlls/mf/tests/transform.c b/dlls/mf/tests/transform.c index 3c068aae743..476bf225b33 100644 --- a/dlls/mf/tests/transform.c +++ b/dlls/mf/tests/transform.c @@ -3502,6 +3502,7 @@ static void test_h264_decoder(void) IMFMediaType *media_type; IMFTransform *transform; ULONG i, ret, ref; + DWORD flags; HRESULT hr;
hr = CoInitialize(NULL); @@ -3538,6 +3539,11 @@ static void test_h264_decoder(void) hr = IMFTransform_GetOutputAvailableType(transform, 0, 0, &media_type); ok(hr == MF_E_TRANSFORM_TYPE_NOT_SET, "GetOutputAvailableType returned %#lx\n", hr);
+ flags = 0xdeadbeef; + hr = IMFTransform_GetInputStatus(transform, 0, &flags); + ok(hr == MF_E_TRANSFORM_TYPE_NOT_SET, "Got %#lx\n", hr); + ok(flags == 0xdeadbeef, "Got flags %#lx.\n", flags); + /* setting output media type first doesn't work */ check_mft_set_output_type(transform, output_type_desc, MF_E_TRANSFORM_TYPE_NOT_SET); check_mft_get_output_current_type(transform, NULL); @@ -3626,12 +3632,20 @@ static void test_h264_decoder(void) ret = IMFSample_Release(output_sample); ok(ret == 0, "Release returned %lu\n", ret);
+ flags = 0; + hr = IMFTransform_GetInputStatus(transform, 0, &flags); + ok(hr == S_OK, "Got %#lx\n", hr); + ok(flags == MFT_INPUT_STATUS_ACCEPT_DATA, "Got flags %#lx.\n", flags); hr = IMFTransform_ProcessInput(transform, 0, input_sample, 0); ok(hr == S_OK, "ProcessInput returned %#lx\n", hr); ret = IMFSample_Release(input_sample); ok(ret <= 1, "Release returned %lu\n", ret); input_sample = next_h264_sample(&h264_encoded_data, &h264_encoded_data_len);
+ flags = 0; + hr = IMFTransform_GetInputStatus(transform, 0, &flags); + ok(hr == S_OK, "Got %#lx\n", hr); + ok(flags == MFT_INPUT_STATUS_ACCEPT_DATA, "Got flags %#lx.\n", flags); hr = IMFTransform_ProcessInput(transform, 0, input_sample, 0); ok(hr == S_OK, "ProcessInput returned %#lx\n", hr); ret = IMFSample_Release(input_sample); @@ -3754,6 +3768,23 @@ static void test_h264_decoder(void) ret = IMFSample_Release(output_sample); ok(ret == 0, "Release returned %lu\n", ret);
+ do + { + flags = 0; + hr = IMFTransform_GetInputStatus(transform, 0, &flags); + ok(hr == S_OK, "Got %#lx\n", hr); + ok(flags == MFT_INPUT_STATUS_ACCEPT_DATA, "Got flags %#lx.\n", flags); + hr = IMFTransform_ProcessInput(transform, 0, input_sample, 0); + ok(hr == S_OK || hr == MF_E_NOTACCEPTING, "Got %#lx\n", hr); + input_sample = next_h264_sample(&h264_encoded_data, &h264_encoded_data_len); + } while (hr == S_OK); + + ok(hr == MF_E_NOTACCEPTING, "Got %#lx\n", hr); + flags = 0; + hr = IMFTransform_GetInputStatus(transform, 0, &flags); + ok(hr == S_OK, "Got %#lx\n", hr); + ok(flags == MFT_INPUT_STATUS_ACCEPT_DATA, "Got flags %#lx.\n", flags); + ret = IMFTransform_Release(transform); ok(ret == 0, "Release returned %lu\n", ret); ret = IMFSample_Release(input_sample); diff --git a/dlls/winegstreamer/h264_decoder.c b/dlls/winegstreamer/h264_decoder.c index ead00e20840..f904c3b9a41 100644 --- a/dlls/winegstreamer/h264_decoder.c +++ b/dlls/winegstreamer/h264_decoder.c @@ -548,8 +548,15 @@ static HRESULT WINAPI transform_GetOutputCurrentType(IMFTransform *iface, DWORD
static HRESULT WINAPI transform_GetInputStatus(IMFTransform *iface, DWORD id, DWORD *flags) { - FIXME("iface %p, id %#lx, flags %p stub!\n", iface, id, flags); - return E_NOTIMPL; + struct h264_decoder *decoder = impl_from_IMFTransform(iface); + + TRACE("iface %p, id %#lx, flags %p.\n", iface, id, flags); + + if (!decoder->wg_transform) + return MF_E_TRANSFORM_TYPE_NOT_SET; + + *flags = MFT_INPUT_STATUS_ACCEPT_DATA; + return S_OK; }
static HRESULT WINAPI transform_GetOutputStatus(IMFTransform *iface, DWORD *flags)
From: Paul Gofman pgofman@codeweavers.com
--- dlls/mf/tests/transform.c | 28 ++++++++++++++++++++++++++++ dlls/winegstreamer/aac_decoder.c | 16 ++++++++++++++-- dlls/winegstreamer/gst_private.h | 1 + dlls/winegstreamer/main.c | 16 ++++++++++++++++ dlls/winegstreamer/unix_private.h | 1 + dlls/winegstreamer/unixlib.h | 7 +++++++ dlls/winegstreamer/wg_parser.c | 1 + dlls/winegstreamer/wg_transform.c | 9 +++++++++ 8 files changed, 77 insertions(+), 2 deletions(-)
diff --git a/dlls/mf/tests/transform.c b/dlls/mf/tests/transform.c index 476bf225b33..143b5ed2b05 100644 --- a/dlls/mf/tests/transform.c +++ b/dlls/mf/tests/transform.c @@ -2370,6 +2370,7 @@ static void test_aac_decoder(void) IMFMediaType *media_type; IMFTransform *transform; const BYTE *aacenc_data; + DWORD flags; HRESULT hr;
hr = CoInitialize(NULL); @@ -2449,10 +2450,23 @@ static void test_aac_decoder(void) ok(aacenc_data_len == 24861, "got length %lu\n", aacenc_data_len);
input_sample = create_sample(aacenc_data + sizeof(DWORD), *(DWORD *)aacenc_data); + + flags = 0; + hr = IMFTransform_GetInputStatus(transform, 0, &flags); + ok(hr == S_OK, "Got %#lx\n", hr); + ok(flags == MFT_INPUT_STATUS_ACCEPT_DATA, "Got flags %#lx.\n", flags); hr = IMFTransform_ProcessInput(transform, 0, input_sample, 0); ok(hr == S_OK, "ProcessInput returned %#lx\n", hr); + flags = 0xdeadbeef; + hr = IMFTransform_GetInputStatus(transform, 0, &flags); + ok(hr == S_OK, "Got %#lx\n", hr); + ok(!flags, "Got flags %#lx.\n", flags); hr = IMFTransform_ProcessInput(transform, 0, input_sample, 0); ok(hr == MF_E_NOTACCEPTING, "ProcessInput returned %#lx\n", hr); + flags = 0xdeadbeef; + hr = IMFTransform_GetInputStatus(transform, 0, &flags); + ok(hr == S_OK, "Got %#lx\n", hr); + ok(!flags, "Got flags %#lx.\n", flags);
/* As output_info.dwFlags doesn't have MFT_OUTPUT_STREAM_CAN_PROVIDE_SAMPLES * IMFTransform_ProcessOutput needs a sample or returns MF_E_TRANSFORM_NEED_MORE_INPUT */ @@ -2460,11 +2474,19 @@ static void test_aac_decoder(void) hr = check_mft_process_output(transform, NULL, &output_status); ok(hr == E_INVALIDARG, "ProcessOutput returned %#lx\n", hr); ok(output_status == 0, "got output[0].dwStatus %#lx\n", output_status); + flags = 0xdeadbeef; + hr = IMFTransform_GetInputStatus(transform, 0, &flags); + ok(hr == S_OK, "Got %#lx\n", hr); + ok(!flags, "Got flags %#lx.\n", flags); hr = IMFTransform_ProcessInput(transform, 0, input_sample, 0); ok(hr == MF_E_NOTACCEPTING, "ProcessInput returned %#lx\n", hr);
hr = IMFTransform_ProcessMessage(transform, MFT_MESSAGE_COMMAND_DRAIN, 0); ok(hr == S_OK, "ProcessMessage returned %#lx\n", hr); + flags = 0xdeadbeef; + hr = IMFTransform_GetInputStatus(transform, 0, &flags); + ok(hr == S_OK, "Got %#lx\n", hr); + ok(!flags, "Got flags %#lx.\n", flags); hr = IMFTransform_ProcessInput(transform, 0, input_sample, 0); ok(hr == MF_E_NOTACCEPTING, "ProcessInput returned %#lx\n", hr);
@@ -2484,6 +2506,12 @@ static void test_aac_decoder(void) winetest_pop_context(); } ok(hr == MF_E_TRANSFORM_NEED_MORE_INPUT, "ProcessOutput returned %#lx\n", hr); + + flags = 0; + hr = IMFTransform_GetInputStatus(transform, 0, &flags); + ok(hr == S_OK, "Got %#lx\n", hr); + ok(flags == MFT_INPUT_STATUS_ACCEPT_DATA, "Got flags %#lx.\n", flags); + ok(output_status == MFT_OUTPUT_DATA_BUFFER_NO_SAMPLE, "got output[0].dwStatus %#lx\n", output_status); ret = IMFSample_Release(output_sample); ok(ret == 0, "Release returned %lu\n", ret); diff --git a/dlls/winegstreamer/aac_decoder.c b/dlls/winegstreamer/aac_decoder.c index a2a6984dd18..8366ac7bb36 100644 --- a/dlls/winegstreamer/aac_decoder.c +++ b/dlls/winegstreamer/aac_decoder.c @@ -500,8 +500,20 @@ static HRESULT WINAPI transform_GetOutputCurrentType(IMFTransform *iface, DWORD
static HRESULT WINAPI transform_GetInputStatus(IMFTransform *iface, DWORD id, DWORD *flags) { - FIXME("iface %p, id %#lx, flags %p stub!\n", iface, id, flags); - return E_NOTIMPL; + struct aac_decoder *decoder = impl_from_IMFTransform(iface); + bool accepts_input; + HRESULT hr; + + TRACE("iface %p, id %#lx, flags %p.\n", iface, id, flags); + + if (!decoder->wg_transform) + return MF_E_TRANSFORM_TYPE_NOT_SET; + + if (FAILED(hr = wg_transform_get_status(decoder->wg_transform, &accepts_input))) + return hr; + + *flags = accepts_input ? MFT_INPUT_STATUS_ACCEPT_DATA : 0; + return S_OK; }
static HRESULT WINAPI transform_GetOutputStatus(IMFTransform *iface, DWORD *flags) diff --git a/dlls/winegstreamer/gst_private.h b/dlls/winegstreamer/gst_private.h index 3890db543ff..1e4aad32f44 100644 --- a/dlls/winegstreamer/gst_private.h +++ b/dlls/winegstreamer/gst_private.h @@ -104,6 +104,7 @@ struct wg_transform *wg_transform_create(const struct wg_format *input_format, const struct wg_format *output_format); void wg_transform_destroy(struct wg_transform *transform); bool wg_transform_set_output_format(struct wg_transform *transform, struct wg_format *format); +bool wg_transform_get_status(struct wg_transform *transform, bool *accepts_input);
unsigned int wg_format_get_max_size(const struct wg_format *format);
diff --git a/dlls/winegstreamer/main.c b/dlls/winegstreamer/main.c index 2675430019f..1afa51ac0aa 100644 --- a/dlls/winegstreamer/main.c +++ b/dlls/winegstreamer/main.c @@ -384,6 +384,22 @@ HRESULT wg_transform_read_data(struct wg_transform *transform, struct wg_sample return params.result; }
+bool wg_transform_get_status(struct wg_transform *transform, bool *accepts_input) +{ + struct wg_transform_get_status_params params = + { + .transform = transform, + }; + + TRACE("transform %p, accepts_input %p.\n", transform, accepts_input); + + if (WINE_UNIX_CALL(unix_wg_transform_get_status, ¶ms)) + return false; + + *accepts_input = params.accepts_input; + return true; +} + bool wg_transform_set_output_format(struct wg_transform *transform, struct wg_format *format) { struct wg_transform_set_output_format_params params = diff --git a/dlls/winegstreamer/unix_private.h b/dlls/winegstreamer/unix_private.h index 617204e97c7..ce67134af16 100644 --- a/dlls/winegstreamer/unix_private.h +++ b/dlls/winegstreamer/unix_private.h @@ -37,6 +37,7 @@ extern NTSTATUS wg_transform_destroy(void *args) DECLSPEC_HIDDEN; extern NTSTATUS wg_transform_set_output_format(void *args) DECLSPEC_HIDDEN; extern NTSTATUS wg_transform_push_data(void *args) DECLSPEC_HIDDEN; extern NTSTATUS wg_transform_read_data(void *args) DECLSPEC_HIDDEN; +extern NTSTATUS wg_transform_get_status(void *args) DECLSPEC_HIDDEN;
/* wg_allocator_release_sample can be used to release any sample that was requested. */ typedef struct wg_sample *(*wg_allocator_request_sample_cb)(gsize size, void *context); diff --git a/dlls/winegstreamer/unixlib.h b/dlls/winegstreamer/unixlib.h index 5424633003e..5c55fbfe229 100644 --- a/dlls/winegstreamer/unixlib.h +++ b/dlls/winegstreamer/unixlib.h @@ -310,6 +310,12 @@ struct wg_transform_set_output_format_params const struct wg_format *format; };
+struct wg_transform_get_status_params +{ + struct wg_transform *transform; + UINT32 accepts_input; +}; + enum unix_funcs { unix_wg_parser_create, @@ -343,6 +349,7 @@ enum unix_funcs
unix_wg_transform_push_data, unix_wg_transform_read_data, + unix_wg_transform_get_status, };
#endif /* __WINE_WINEGSTREAMER_UNIXLIB_H */ diff --git a/dlls/winegstreamer/wg_parser.c b/dlls/winegstreamer/wg_parser.c index f2c59488ef3..32ac4a2830d 100644 --- a/dlls/winegstreamer/wg_parser.c +++ b/dlls/winegstreamer/wg_parser.c @@ -1815,4 +1815,5 @@ const unixlib_entry_t __wine_unix_call_funcs[] =
X(wg_transform_push_data), X(wg_transform_read_data), + X(wg_transform_get_status), }; diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c index 302a64eb57b..f4244708a8f 100644 --- a/dlls/winegstreamer/wg_transform.c +++ b/dlls/winegstreamer/wg_transform.c @@ -931,3 +931,12 @@ NTSTATUS wg_transform_read_data(void *args) wg_allocator_release_sample(transform->allocator, sample, discard_data); return STATUS_SUCCESS; } + +NTSTATUS wg_transform_get_status(void *args) +{ + struct wg_transform_get_status_params *params = args; + struct wg_transform *transform = params->transform; + + params->accepts_input = gst_atomic_queue_length(transform->input_queue) < transform->input_max_length; + return STATUS_SUCCESS; +}
From: Paul Gofman pgofman@codeweavers.com
--- dlls/mf/tests/transform.c | 2 +- dlls/winegstreamer/h264_decoder.c | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/dlls/mf/tests/transform.c b/dlls/mf/tests/transform.c index 143b5ed2b05..38a4333a46a 100644 --- a/dlls/mf/tests/transform.c +++ b/dlls/mf/tests/transform.c @@ -3252,7 +3252,7 @@ static void test_h264_decoder(void) { ATTR_UINT32(MF_LOW_LATENCY, 0), ATTR_UINT32(MF_SA_D3D_AWARE, 1, .todo = TRUE), - ATTR_UINT32(MF_SA_D3D11_AWARE, 1, .todo = TRUE), + ATTR_UINT32(MF_SA_D3D11_AWARE, 1), ATTR_UINT32(MFT_DECODER_EXPOSE_OUTPUT_TYPES_IN_NATIVE_ORDER, 0, .todo = TRUE), /* more H264 decoder specific attributes from CODECAPI */ {0}, diff --git a/dlls/winegstreamer/h264_decoder.c b/dlls/winegstreamer/h264_decoder.c index f904c3b9a41..439701098ad 100644 --- a/dlls/winegstreamer/h264_decoder.c +++ b/dlls/winegstreamer/h264_decoder.c @@ -725,6 +725,8 @@ HRESULT h264_decoder_create(REFIID riid, void **ret) goto failed; if (FAILED(hr = IMFAttributes_SetUINT32(decoder->attributes, &MF_LOW_LATENCY, 0))) goto failed; + if (FAILED(hr = IMFAttributes_SetUINT32(decoder->attributes, &MF_SA_D3D11_AWARE, TRUE))) + goto failed; if (FAILED(hr = MFCreateAttributes(&decoder->output_attributes, 0))) goto failed; if (FAILED(hr = wg_sample_queue_create(&decoder->wg_sample_queue)))
v4: - add trace to wg_transform_get_status(); - return input status in a parameter from Unix function instead of using pointer.
that's not what enums are for.
I disagree. Using enums for flags is pretty idiomatic and certainly the best way to store flags, if they are a proper fit for the constraints imposed by the type (because unfortunately C does not have enums like C++ where you can specify the underlying type, so it won't work if you need a specific bit width to match some ABI or other constraints).
The alternative is using preprocessor macros, which not only pollutes the global namespace (hence the ALL CAPS notation), but far more importantly, it's a lot harder to understand from the type what flags it actually uses. With the enum you simply look up the enum from the variable type, and it might even come with comments describing each of them and what other flags are available.
IMO the benefits far outweight the potential confusion from the "name" of it or the fact it's sequential by default (but by using preprocessor macros you have to specify the value on each flag, anyway…). I mean, it's C, not English. ;)
On Wed Feb 1 20:32:51 2023 +0000, Gabriel Ivăncescu wrote:
that's not what enums are for.
I disagree. Using enums for flags is pretty idiomatic and certainly the best way to store flags, if they are a proper fit for the constraints imposed by the type (because unfortunately C does not have enums like C++ where you can specify the underlying type, so it won't work if you need a specific bit width to match some ABI or other constraints). The alternative is using preprocessor macros, which not only pollutes the global namespace (hence the ALL CAPS notation), but far more importantly, it's a lot harder to understand from the type what flags it actually uses. With the enum you simply look up the enum from the variable type, and it might even come with comments describing each of them and what other flags are available. IMO the benefits far outweight the potential confusion from the "name" of it or the fact it's sequential by default (but by using preprocessor macros you have to specify the value on each flag, anyway…). I mean, it's C, not English. ;)
IMO it is absolutely matter of personal preference whether to prefer enum or #define (minus theoretical issue with 64 bit flags but I don't think we need those here). I always respect the desire of maintainer to see it one way or another in the code they maintain and never argue on that. Of course the things get a bit more complicated when there are two maintainers, but I am sure we can figure something even better, like just return bool flag in this case :)
On Wed Feb 1 20:37:38 2023 +0000, Paul Gofman wrote:
IMO it is absolutely matter of personal preference whether to prefer enum or #define (minus theoretical issue with 64 bit flags but I don't think we need those here). I always respect the desire of maintainer to see it one way or another in the code they maintain and never argue on that. Of course the things get a bit more complicated when there are two maintainers, but I am sure we can figure something even better, like just return bool flag in this case :)
It definitely depends on the surrounding code, in some cases it might be too obvious anyway, even if you see "DWORD flags", I just wanted to mention that I vastly prefer enums sometimes when I read new code full of flags, it makes it a lot easier to find out which is which—but yes in this case bool is even easier.
On Wed Feb 1 20:45:41 2023 +0000, Gabriel Ivăncescu wrote:
It definitely depends on the surrounding code, in some cases it might be too obvious anyway, even if you see "DWORD flags", I just wanted to mention that I vastly prefer enums sometimes when I read new code full of flags, it makes it a lot easier to find out which is which—but yes in this case bool is even easier.
But then you get fun things like, e. g., D3DCULL_FORCE_DWORD in enum to make those flags be 32 bit on some compilers which leaves some room for arguments, but IMO it is still a matter of preference and the local style in a particular module.
This merge request was approved by Rémi Bernon.
This merge request was approved by Zebediah Figura.
This merge request was approved by Nikolay Sivov.
Regarding enabling d3d11-awareness attribute, we'll need to watch out for MFT_MESSAGE_SET_D3D_MANAGER now, that will simply succeed without doing anything.
-
Gabriel Ivăncescu (＠insn) -
Nikolay Sivov (＠nsivov)
-
Paul Gofman
-
Paul Gofman (＠gofman)
-
Rémi Bernon (＠rbernon)
-
Zebediah Figura (＠zfigura)