From: Ziqing Hui zhui@codeweavers.com
MF_MT_INTERLACE_MODE, MF_MT_VIDEO_NOMINAL_RANGE and MF_MT_PIXEL_ASPECT_RATIO are not required in input type. However, if they appears, they should match output type. --- dlls/winegstreamer/video_encoder.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/dlls/winegstreamer/video_encoder.c b/dlls/winegstreamer/video_encoder.c index ee27175ee4f..1bfe26261af 100644 --- a/dlls/winegstreamer/video_encoder.c +++ b/dlls/winegstreamer/video_encoder.c @@ -280,6 +280,7 @@ static HRESULT WINAPI transform_SetInputType(IMFTransform *iface, DWORD id, IMFM struct video_encoder *encoder = impl_from_IMFTransform(iface); IMFMediaType *good_input_type; GUID major, subtype; + UINT64 ratio; BOOL result; HRESULT hr; ULONG i; @@ -317,10 +318,14 @@ static HRESULT WINAPI transform_SetInputType(IMFTransform *iface, DWORD id, IMFM if (i == encoder->input_type_count) return MF_E_INVALIDMEDIATYPE;
+ if (FAILED(IMFMediaType_GetUINT64(type, &MF_MT_FRAME_SIZE, &ratio)) + || FAILED(IMFMediaType_GetUINT64(type, &MF_MT_FRAME_RATE, &ratio))) + return MF_E_INVALIDMEDIATYPE; + if (FAILED(hr = video_encoder_create_input_type(encoder, &subtype, &good_input_type))) return hr; hr = IMFMediaType_Compare(good_input_type, (IMFAttributes *)type, - MF_ATTRIBUTES_MATCH_OUR_ITEMS, &result); + MF_ATTRIBUTES_MATCH_INTERSECTION, &result); IMFMediaType_Release(good_input_type); if (FAILED(hr) || !result) return MF_E_INVALIDMEDIATYPE;
From: Ziqing Hui zhui@codeweavers.com
--- dlls/winegstreamer/video_encoder.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/dlls/winegstreamer/video_encoder.c b/dlls/winegstreamer/video_encoder.c index 1bfe26261af..e15c799d81f 100644 --- a/dlls/winegstreamer/video_encoder.c +++ b/dlls/winegstreamer/video_encoder.c @@ -367,6 +367,7 @@ static HRESULT WINAPI transform_SetOutputType(IMFTransform *iface, DWORD id, IMF { IMFMediaType_Release(encoder->output_type); encoder->output_type = NULL; + memset(&encoder->output_info, 0, sizeof(encoder->output_info)); } if (encoder->wg_transform) { @@ -412,6 +413,11 @@ static HRESULT WINAPI transform_SetOutputType(IMFTransform *iface, DWORD id, IMF
/* FIXME: Add MF_MT_MPEG_SEQUENCE_HEADER attribute. */
+ /* FIXME: Hardcode a size that native uses for 1920 * 1080. + * And hope it's large enough to make things work for now. + * The right way is to calculate it based on frame width and height. */ + encoder->output_info.cbSize = 0x3bc400; + if (encoder->wg_transform) { wg_transform_destroy(encoder->wg_transform); @@ -627,10 +633,6 @@ HRESULT h264_encoder_create(REFIID riid, void **out) h264_encoder_output_types, ARRAY_SIZE(h264_encoder_output_types), &encoder))) return hr;
- /* FIXME: Hardcode a size that is large enough to make things work for now. - * The right way is to calculate the size based on output frame size. */ - encoder->output_info.cbSize = 0x3bc400; - TRACE("Created h264 encoder transform %p.\n", &encoder->IMFTransform_iface);
hr = IMFTransform_QueryInterface(&encoder->IMFTransform_iface, riid, out);
From: Ziqing Hui zhui@codeweavers.com
--- dlls/winegstreamer/wg_transform.c | 78 +++++++++++++++++-------------- 1 file changed, 44 insertions(+), 34 deletions(-)
diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c index 76dbd347064..d331b210337 100644 --- a/dlls/winegstreamer/wg_transform.c +++ b/dlls/winegstreamer/wg_transform.c @@ -468,11 +468,52 @@ static GstCaps *transform_get_parsed_caps(GstCaps *caps, const char *media_type) return parsed_caps; }
+static bool transform_create_decoder_elements(struct wg_transform *transform, + const gchar *input_mime, const gchar *output_mime, GstElement **first, GstElement **last) +{ + GstCaps *parsed_caps = NULL, *sink_caps = NULL; + GstElement *element; + bool ret = false; + + if (!(parsed_caps = transform_get_parsed_caps(transform->input_caps, input_mime))) + return false; + + /* Since we append conversion elements, we don't want to filter decoders + * based on the actual output caps now. Matching decoders with the + * raw output media type should be enough. + */ + if (!(sink_caps = gst_caps_new_empty_simple(output_mime))) + goto done; + + if ((element = find_element(GST_ELEMENT_FACTORY_TYPE_PARSER, transform->input_caps, parsed_caps)) + && !append_element(transform->container, element, first, last)) + goto done; + else if (!element) + { + gst_caps_unref(parsed_caps); + parsed_caps = gst_caps_ref(transform->input_caps); + } + + if (!(element = find_element(GST_ELEMENT_FACTORY_TYPE_DECODER, parsed_caps, sink_caps)) + || !append_element(transform->container, element, first, last)) + goto done; + + set_max_threads(element); + + ret = true; + +done: + if (sink_caps) + gst_caps_unref(sink_caps); + if (parsed_caps) + gst_caps_unref(parsed_caps); + return ret; +} + NTSTATUS wg_transform_create(void *args) { struct wg_transform_create_params *params = args; GstElement *first = NULL, *last = NULL, *element; - GstCaps *sink_caps = NULL, *parsed_caps = NULL; NTSTATUS status = STATUS_UNSUCCESSFUL; const gchar *input_mime, *output_mime; GstPadTemplate *template = NULL; @@ -531,34 +572,10 @@ NTSTATUS wg_transform_create(void *args) gst_pad_set_query_function(transform->my_sink, transform_sink_query_cb); gst_pad_set_chain_function(transform->my_sink, transform_sink_chain_cb);
- if (!(parsed_caps = transform_get_parsed_caps(transform->input_caps, input_mime))) - goto out; - - /* Since we append conversion elements, we don't want to filter decoders - * based on the actual output caps now. Matching decoders with the - * raw output media type should be enough. - */ - if (!(sink_caps = gst_caps_new_empty_simple(output_mime))) + if (strcmp(input_mime, "audio/x-raw") && strcmp(input_mime, "video/x-raw") + && !transform_create_decoder_elements(transform, input_mime, output_mime, &first, &last)) goto out;
- if (strcmp(input_mime, "audio/x-raw") && strcmp(input_mime, "video/x-raw")) - { - if ((element = find_element(GST_ELEMENT_FACTORY_TYPE_PARSER, transform->input_caps, parsed_caps)) - && !append_element(transform->container, element, &first, &last)) - goto out; - else if (!element) - { - gst_caps_unref(parsed_caps); - parsed_caps = gst_caps_ref(transform->input_caps); - } - - if (!(element = find_element(GST_ELEMENT_FACTORY_TYPE_DECODER, parsed_caps, sink_caps)) - || !append_element(transform->container, element, &first, &last)) - goto out; - - set_max_threads(element); - } - if (g_str_has_prefix(output_mime, "audio/")) { if (strcmp(output_mime, "audio/x-raw")) @@ -632,9 +649,6 @@ NTSTATUS wg_transform_create(void *args) || !push_event(transform->my_src, event)) goto out;
- gst_caps_unref(parsed_caps); - gst_caps_unref(sink_caps); - GST_INFO("Created winegstreamer transform %p.", transform); params->transform = (wg_transform_t)(ULONG_PTR)transform; return STATUS_SUCCESS; @@ -650,10 +664,6 @@ out: gst_object_unref(transform->my_src); if (transform->input_caps) gst_caps_unref(transform->input_caps); - if (parsed_caps) - gst_caps_unref(parsed_caps); - if (sink_caps) - gst_caps_unref(sink_caps); if (transform->allocator) wg_allocator_destroy(transform->allocator); if (transform->drain_query)
From: Ziqing Hui zhui@codeweavers.com
--- dlls/winegstreamer/wg_transform.c | 99 +++++++++++++++++-------------- 1 file changed, 55 insertions(+), 44 deletions(-)
diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c index d331b210337..75f58fb5a9b 100644 --- a/dlls/winegstreamer/wg_transform.c +++ b/dlls/winegstreamer/wg_transform.c @@ -510,10 +510,62 @@ done: return ret; }
+static bool transform_create_converter_elements(struct wg_transform *transform, + const gchar *mime, GstElement **first, GstElement **last) +{ + GstElement *element; + + if (g_str_has_prefix(mime, "audio/")) + { + if (strcmp(mime, "audio/x-raw")) + { + GST_FIXME("output caps %"GST_PTR_FORMAT" not implemented!", transform->output_caps); + return false; + } + else + { + /* The MF audio decoder transforms allow decoding to various formats + * as well as resampling the audio at the same time, whereas + * GStreamer decoder plugins usually only support decoding to a + * single format and at the original rate. + * + * The WMA decoder transform also has output samples interleaved on + * Windows, whereas GStreamer avdec_wmav2 output uses + * non-interleaved format. + */ + if (!(element = create_element("audioconvert", "base")) + || !append_element(transform->container, element, first, last)) + return false; + if (!(element = create_element("audioresample", "base")) + || !append_element(transform->container, element, first, last)) + return false; + } + } + + if (g_str_has_prefix(mime, "video/")) + { + if (strcmp(mime, "video/x-raw")) + { + GST_FIXME("output caps %"GST_PTR_FORMAT" not implemented!", transform->output_caps); + return false; + } + else + { + if (!(element = create_element("videoconvert", "base")) + || !append_element(transform->container, element, first, last)) + return false; + /* Let GStreamer choose a default number of threads. */ + gst_util_set_object_arg(G_OBJECT(element), "n-threads", "0"); + } + } + + return true; +} + NTSTATUS wg_transform_create(void *args) { struct wg_transform_create_params *params = args; - GstElement *first = NULL, *last = NULL, *element; + GstElement *first = NULL, *last = NULL; NTSTATUS status = STATUS_UNSUCCESSFUL; const gchar *input_mime, *output_mime; GstPadTemplate *template = NULL; @@ -576,49 +628,8 @@ NTSTATUS wg_transform_create(void *args) && !transform_create_decoder_elements(transform, input_mime, output_mime, &first, &last)) goto out;
- if (g_str_has_prefix(output_mime, "audio/")) - { - if (strcmp(output_mime, "audio/x-raw")) - { - GST_FIXME("output caps %"GST_PTR_FORMAT" not implemented!", transform->output_caps); - goto out; - } - else - { - /* The MF audio decoder transforms allow decoding to various formats - * as well as resampling the audio at the same time, whereas - * GStreamer decoder plugins usually only support decoding to a - * single format and at the original rate. - * - * The WMA decoder transform also has output samples interleaved on - * Windows, whereas GStreamer avdec_wmav2 output uses - * non-interleaved format. - */ - if (!(element = create_element("audioconvert", "base")) - || !append_element(transform->container, element, &first, &last)) - goto out; - if (!(element = create_element("audioresample", "base")) - || !append_element(transform->container, element, &first, &last)) - goto out; - } - } - - if (g_str_has_prefix(output_mime, "video/")) - { - if (strcmp(output_mime, "video/x-raw")) - { - GST_FIXME("output caps %"GST_PTR_FORMAT" not implemented!", transform->output_caps); - goto out; - } - else - { - if (!(element = create_element("videoconvert", "base")) - || !append_element(transform->container, element, &first, &last)) - goto out; - /* Let GStreamer choose a default number of threads. */ - gst_util_set_object_arg(G_OBJECT(element), "n-threads", "0"); - } - } + if (!transform_create_converter_elements(transform, output_mime, &first, &last)) + goto out;
if (!link_src_to_element(transform->my_src, first)) goto out;
From: Ziqing Hui zhui@codeweavers.com
--- dlls/mf/tests/transform.c | 25 +++++++++----- dlls/winegstreamer/wg_transform.c | 55 ++++++++++++++++++++++++++++--- 2 files changed, 67 insertions(+), 13 deletions(-)
diff --git a/dlls/mf/tests/transform.c b/dlls/mf/tests/transform.c index ef0bec60ed5..3e902aa67f3 100644 --- a/dlls/mf/tests/transform.c +++ b/dlls/mf/tests/transform.c @@ -3910,12 +3910,12 @@ static void test_h264_encoder(void) ATTR_RATIO(MF_MT_FRAME_SIZE, actual_width, actual_height), ATTR_RATIO(MF_MT_FRAME_RATE, 30000, 1001), ATTR_UINT32(MF_MT_AVG_BITRATE, 193540), - ATTR_BLOB(MF_MT_MPEG_SEQUENCE_HEADER, test_h264_sequence_header, sizeof(test_h264_sequence_header)), + ATTR_BLOB(MF_MT_MPEG_SEQUENCE_HEADER, test_h264_sequence_header, sizeof(test_h264_sequence_header), .todo = TRUE), ATTR_UINT32(MF_MT_INTERLACE_MODE, MFVideoInterlace_Progressive), ATTR_UINT32(test_attr_guid, 0), {0}, }; - static const MFT_OUTPUT_STREAM_INFO expect_output_info[] = {{.cbSize = 0x8000}, {.cbSize = 0x3bc400}}; + MFT_OUTPUT_STREAM_INFO output_info, expect_output_info[] = {{.cbSize = 0x8000}, {.cbSize = 0x3bc400}}; MFT_REGISTER_TYPE_INFO output_type = {MFMediaType_Video, MFVideoFormat_H264}; MFT_REGISTER_TYPE_INFO input_type = {MFMediaType_Video, MFVideoFormat_NV12}; IMFMediaType *media_type; @@ -3940,10 +3940,7 @@ static void test_h264_encoder(void) check_mft_get_info(class_id, &expect_mft_info);
hr = CoCreateInstance(class_id, NULL, CLSCTX_INPROC_SERVER, &IID_IMFTransform, (void **)&transform); - todo_wine ok(hr == S_OK, "CoCreateInstance returned %#lx.\n", hr); - if (hr != S_OK) - goto failed;
check_interface(transform, &IID_IMFTransform, TRUE); check_interface(transform, &IID_IMediaObject, FALSE); @@ -3977,8 +3974,13 @@ static void test_h264_encoder(void)
check_mft_set_output_type_required(transform, output_type_desc); check_mft_set_output_type(transform, output_type_desc, S_OK); - check_mft_get_output_current_type(transform, expect_output_type_desc); - check_mft_get_output_stream_info(transform, S_OK, &expect_output_info[0]); + check_mft_get_output_current_type_(__LINE__, transform, expect_output_type_desc, FALSE, TRUE); + hr = IMFTransform_GetOutputStreamInfo(transform, 0, &output_info); + ok(hr == S_OK, "GetOutputStreamInfo returned %#lx\n", hr); + check_member(output_info, expect_output_info[0], "%#lx", dwFlags); + todo_wine + check_member(output_info, expect_output_info[0], "%#lx", cbSize); + check_member(output_info, expect_output_info[0], "%#lx", cbAlignment);
/* Input types can now be enumerated. */ i = -1; @@ -4032,7 +4034,14 @@ static void test_h264_encoder(void) if (IsEqualGUID(test_attributes[i].key, &MF_MT_FRAME_SIZE)) check_mft_get_output_stream_info(transform, S_OK, &expect_output_info[1]); else - check_mft_get_output_stream_info(transform, S_OK, &expect_output_info[0]); + { + hr = IMFTransform_GetOutputStreamInfo(transform, 0, &output_info); + ok(hr == S_OK, "GetOutputStreamInfo returned %#lx\n", hr); + check_member(output_info, expect_output_info[0], "%#lx", dwFlags); + todo_wine + check_member(output_info, expect_output_info[0], "%#lx", cbSize); + check_member(output_info, expect_output_info[0], "%#lx", cbAlignment); + }
winetest_pop_context(); } diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c index 75f58fb5a9b..a5a03c72040 100644 --- a/dlls/winegstreamer/wg_transform.c +++ b/dlls/winegstreamer/wg_transform.c @@ -69,6 +69,11 @@ static struct wg_transform *get_transform(wg_transform_t trans) return (struct wg_transform *)(ULONG_PTR)trans; }
+static bool mime_is_raw(const gchar *mime) +{ + return !strcmp(mime, "audio/x-raw") || !strcmp(mime, "video/x-raw"); +} + static void align_video_info_planes(MFVideoInfo *video_info, gsize plane_align, GstVideoInfo *info, GstVideoAlignment *align) { @@ -562,6 +567,24 @@ static bool transform_create_converter_elements(struct wg_transform *transform, return true; }
+static bool transform_create_encoder_elements(struct wg_transform *transform, + const gchar *output_mime, GstElement **first, GstElement **last) +{ + GstElement *element; + GstCaps *src_caps; + bool ret; + + if (!(src_caps = gst_caps_new_empty_simple(output_mime))) + return false; + + ret = (element = find_element(GST_ELEMENT_FACTORY_TYPE_ENCODER, NULL, src_caps)) + && append_element(transform->container, element, first, last); + + gst_caps_unref(src_caps); + + return ret; +} + NTSTATUS wg_transform_create(void *args) { struct wg_transform_create_params *params = args; @@ -624,12 +647,34 @@ NTSTATUS wg_transform_create(void *args) gst_pad_set_query_function(transform->my_sink, transform_sink_query_cb); gst_pad_set_chain_function(transform->my_sink, transform_sink_chain_cb);
- if (strcmp(input_mime, "audio/x-raw") && strcmp(input_mime, "video/x-raw") - && !transform_create_decoder_elements(transform, input_mime, output_mime, &first, &last)) - goto out; - - if (!transform_create_converter_elements(transform, output_mime, &first, &last)) + /* Create gstreamer elements. */ + if (mime_is_raw(input_mime) && mime_is_raw(output_mime)) + { + /* Converter transform. */ + if (!transform_create_converter_elements(transform, output_mime, &first, &last)) + goto out; + } + else if (!mime_is_raw(input_mime) && mime_is_raw(output_mime)) + { + /* Decoder transform. */ + if (!transform_create_decoder_elements(transform, input_mime, output_mime, &first, &last)) + goto out; + if (!transform_create_converter_elements(transform, output_mime, &first, &last)) + goto out; + } + else if (mime_is_raw(input_mime) && !mime_is_raw(output_mime)) + { + /* Encoder transform. */ + if (!transform_create_converter_elements(transform, input_mime, &first, &last)) + goto out; + if (!transform_create_encoder_elements(transform, output_mime, &first, &last)) + goto out; + } + else + { + GST_ERROR("Unsupported transform from %s to %s.", input_mime, output_mime); goto out; + }
if (!link_src_to_element(transform->my_src, first)) goto out;
Hi,
It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated.
The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details:
The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=146896
Your paranoid android.
=== debian11b (64 bit WoW report) ===
ddraw: ddraw1.c:3645: Test failed: Expected (0,0)-(640,480), got (-32000,-32000)-(-31840,-31969).
Rémi Bernon (@rbernon) commented about dlls/winegstreamer/wg_transform.c:
goto out;if (!transform_create_encoder_elements(transform, output_mime, &first, &last)) goto out;
if (!(element = create_element("videoconvert", "base"))|| !append_element(transform->container, element, &first, &last))
GST_ERROR("Unsupported transform from %s to %s.", input_mime, output_mime); goto out;
/* Let GStreamer choose a default number of threads. */gst_util_set_object_arg(G_OBJECT(element), "n-threads", "0");}}
if (!link_src_to_element(transform->my_src, first))
What about just removing the raw checks and adding the encoder elements inline here?
```diff diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c index 76dbd347064..3de478e893e 100644 --- a/dlls/winegstreamer/wg_transform.c +++ b/dlls/winegstreamer/wg_transform.c @@ -561,46 +561,37 @@ NTSTATUS wg_transform_create(void *args)
if (g_str_has_prefix(output_mime, "audio/")) { - if (strcmp(output_mime, "audio/x-raw")) - { - GST_FIXME("output caps %"GST_PTR_FORMAT" not implemented!", transform->output_caps); + /* The MF audio decoder transforms allow decoding to various formats + * as well as resampling the audio at the same time, whereas + * GStreamer decoder plugins usually only support decoding to a + * single format and at the original rate. + * + * The WMA decoder transform also has output samples interleaved on + * Windows, whereas GStreamer avdec_wmav2 output uses + * non-interleaved format. + */ + if (!(element = create_element("audioconvert", "base")) + || !append_element(transform->container, element, &first, &last)) + goto out; + if (!(element = create_element("audioresample", "base")) + || !append_element(transform->container, element, &first, &last)) goto out; - } - else - { - /* The MF audio decoder transforms allow decoding to various formats - * as well as resampling the audio at the same time, whereas - * GStreamer decoder plugins usually only support decoding to a - * single format and at the original rate. - * - * The WMA decoder transform also has output samples interleaved on - * Windows, whereas GStreamer avdec_wmav2 output uses - * non-interleaved format. - */ - if (!(element = create_element("audioconvert", "base")) - || !append_element(transform->container, element, &first, &last)) - goto out; - if (!(element = create_element("audioresample", "base")) - || !append_element(transform->container, element, &first, &last)) - goto out; - } }
if (g_str_has_prefix(output_mime, "video/")) { - if (strcmp(output_mime, "video/x-raw")) - { - GST_FIXME("output caps %"GST_PTR_FORMAT" not implemented!", transform->output_caps); + if (!(element = create_element("videoconvert", "base")) + || !append_element(transform->container, element, &first, &last)) + goto out; + /* Let GStreamer choose a default number of threads. */ + gst_util_set_object_arg(G_OBJECT(element), "n-threads", "0"); + } + + if (strcmp(output_mime, "audio/x-raw") && strcmp(output_mime, "video/x-raw")) + { + if (!(element = find_element(GST_ELEMENT_FACTORY_TYPE_ENCODER, NULL, transform->output_caps)) + || !append_element(transform->container, element, &first, &last)) goto out; - } - else - { - if (!(element = create_element("videoconvert", "base")) - || !append_element(transform->container, element, &first, &last)) - goto out; - /* Let GStreamer choose a default number of threads. */ - gst_util_set_object_arg(G_OBJECT(element), "n-threads", "0"); - } }
if (!link_src_to_element(transform->my_src, first)) ```
On Wed Jul 10 06:58:14 2024 +0000, Ziqing Hui wrote:
I'm good with both ways. Your suggestion make less code, the original way in this MR make the code looks clearer. It's up to you now.
But I still want to keep the helpers(transform_create_{decoder,converter}_elements) I introduced, they make wg_transform_create() looks clearer than before.
-
Marvin -
Rémi Bernon (＠rbernon) -
Ziqing Hui
-
Ziqing Hui (＠zhui)