[PATCH v2 0/5] MR139: winegstreamer: Implement video plane alignment for the H264 decoder.
-- v2: winegstreamer: Enforce frame size in H264 decoder SetOutputType. winegstreamer: Support mapping of MF_MT_MINIMUM_DISPLAY_APERTURE. winegstreamer: Request native buffer alignment using video pool meta. winegstreamer: Reply with a video buffer pool to ALLOCATION queries. winegstreamer: Use a custom flag instead of sample info for caps change. https://gitlab.winehq.org/wine/wine/-/merge_requests/139
From: Rémi Bernon <rbernon(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=45988 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=47084 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=49715 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52183 Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winegstreamer/wg_transform.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c index 946268ee353..42188672209 100644 --- a/dlls/winegstreamer/wg_transform.c +++ b/dlls/winegstreamer/wg_transform.c @@ -42,6 +42,12 @@ GST_DEBUG_CATEGORY_EXTERN(wine); #define GST_CAT_DEFAULT wine +#ifndef GST_SAMPLE_FLAG_LAST +#define GST_SAMPLE_FLAG_WG_CAPS_CHANGED (GST_MINI_OBJECT_FLAG_LAST << 0) +#else +#define GST_SAMPLE_FLAG_WG_CAPS_CHANGED (GST_SAMPLE_FLAG_LAST << 0) +#endif + struct wg_transform { GstElement *container; @@ -59,26 +65,21 @@ struct wg_transform static GstFlowReturn transform_sink_chain_cb(GstPad *pad, GstObject *parent, GstBuffer *buffer) { struct wg_transform *transform = gst_pad_get_element_private(pad); - GstStructure *info = NULL; GstSample *sample; GST_LOG("transform %p, buffer %p.", transform, buffer); - if (transform->output_caps_changed && !(info = gst_structure_new_empty("format-changed"))) - { - GST_ERROR("Failed to allocate transform %p output sample info.", transform); - gst_buffer_unref(buffer); - return GST_FLOW_ERROR; - } - transform->output_caps_changed = false; - - if (!(sample = gst_sample_new(buffer, transform->output_caps, NULL, info))) + if (!(sample = gst_sample_new(buffer, transform->output_caps, NULL, NULL))) { GST_ERROR("Failed to allocate transform %p output sample.", transform); gst_buffer_unref(buffer); return GST_FLOW_ERROR; } + if (transform->output_caps_changed) + GST_MINI_OBJECT_FLAG_SET(sample, GST_SAMPLE_FLAG_WG_CAPS_CHANGED); + transform->output_caps_changed = false; + gst_atomic_queue_push(transform->output_queue, sample); gst_buffer_unref(buffer); return GST_FLOW_OK; @@ -521,9 +522,9 @@ NTSTATUS wg_transform_read_data(void *args) output_buffer = gst_sample_get_buffer(transform->output_sample); output_caps = gst_sample_get_caps(transform->output_sample); - if (gst_sample_get_info(transform->output_sample)) + if (GST_MINI_OBJECT_FLAG_IS_SET(transform->output_sample, GST_SAMPLE_FLAG_WG_CAPS_CHANGED)) { - gst_sample_set_info(transform->output_sample, NULL); + GST_MINI_OBJECT_FLAG_UNSET(transform->output_sample, GST_SAMPLE_FLAG_WG_CAPS_CHANGED); if (format) wg_format_from_caps(format, output_caps); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/139
On 5/30/22 03:20, Rémi Bernon wrote:
+#ifndef GST_SAMPLE_FLAG_LAST +#define GST_SAMPLE_FLAG_WG_CAPS_CHANGED (GST_MINI_OBJECT_FLAG_LAST << 0) +#else +#define GST_SAMPLE_FLAG_WG_CAPS_CHANGED (GST_SAMPLE_FLAG_LAST << 0) +#endif
This define doesn't currently exist in GStreamer, and from my searching never has. I get the idea here, but it seems pointless, since they can't actually add it with a different value without breaking ABI.
From: Rémi Bernon <rbernon(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=45988 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=47084 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=49715 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52183 Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winegstreamer/wg_transform.c | 65 +++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c index 42188672209..f98f3bdd14b 100644 --- a/dlls/winegstreamer/wg_transform.c +++ b/dlls/winegstreamer/wg_transform.c @@ -62,6 +62,17 @@ struct wg_transform GstCaps *output_caps; }; +static bool is_caps_video(GstCaps *caps) +{ + const gchar *media_type; + + if (!caps || !gst_caps_get_size(caps)) + return false; + + media_type = gst_structure_get_name(gst_caps_get_structure(caps, 0)); + return g_str_has_prefix(media_type, "video/"); +} + static GstFlowReturn transform_sink_chain_cb(GstPad *pad, GstObject *parent, GstBuffer *buffer) { struct wg_transform *transform = gst_pad_get_element_private(pad); @@ -85,6 +96,59 @@ static GstFlowReturn transform_sink_chain_cb(GstPad *pad, GstObject *parent, Gst return GST_FLOW_OK; } +static gboolean transform_sink_query_cb(GstPad *pad, GstObject *parent, GstQuery *query) +{ + struct wg_transform *transform = gst_pad_get_element_private(pad); + + GST_LOG("transform %p, type \"%s\".", transform, gst_query_type_get_name(query->type)); + + switch (query->type) + { + case GST_QUERY_ALLOCATION: + { + GstStructure *config; + gboolean needs_pool; + GstBufferPool *pool; + GstVideoInfo info; + GstCaps *caps; + + gst_query_parse_allocation(query, &caps, &needs_pool); + if (!is_caps_video(caps) || !needs_pool) + break; + + if (!gst_video_info_from_caps(&info, caps) + || !(pool = gst_video_buffer_pool_new())) + break; + + if (!(config = gst_buffer_pool_get_config(pool))) + GST_ERROR("Failed to get pool %p config.", pool); + else + { + gst_buffer_pool_config_set_params(config, caps, + info.size, 0, 0); + if (!gst_buffer_pool_set_config(pool, config)) + GST_ERROR("Failed to set pool %p config.", pool); + } + + if (gst_query_get_n_allocation_pools(query) > 0) + gst_query_set_nth_allocation_pool(query, 0, pool, info.size, 0, 0); + else + gst_query_add_allocation_pool(query, pool, info.size, 0, 0); + + GST_INFO("Proposing pool %p, buffer size %#zx, for query %p.", + pool, info.size, query); + + g_object_unref(pool); + return true; + } + default: + GST_WARNING("Ignoring \"%s\" query.", gst_query_type_get_name(query->type)); + break; + } + + return gst_pad_query_default(pad, parent, query); +} + static gboolean transform_sink_event_cb(GstPad *pad, GstObject *parent, GstEvent *event) { struct wg_transform *transform = gst_pad_get_element_private(pad); @@ -256,6 +320,7 @@ NTSTATUS wg_transform_create(void *args) gst_pad_set_element_private(transform->my_sink, transform); gst_pad_set_event_function(transform->my_sink, transform_sink_event_cb); + gst_pad_set_query_function(transform->my_sink, transform_sink_query_cb); gst_pad_set_chain_function(transform->my_sink, transform_sink_chain_cb); /* Since we append conversion elements, we don't want to filter decoders -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/139
On 5/30/22 03:20, Rémi Bernon wrote:
+ if (gst_query_get_n_allocation_pools(query) > 0) + gst_query_set_nth_allocation_pool(query, 0, pool, info.size, 0, 0); + else + gst_query_add_allocation_pool(query, pool, info.size, 0, 0);
GStreamer documentation isn't exactly clear about this, but my understanding is that, as the sink, we are supposed to be the first element to add things to this query, and as such there shouldn't be any pools already. Have you encountered cases where that wasn't true? Or were you modeling this after some GStreamer code I didn't find? FWIW, the above seems to be a common pattern for filters or source elements *after* receiving a query response from the sink, but that's not the case here.
From: Rémi Bernon <rbernon(a)codeweavers.com> To support VA-API plugins we need to implement video frame copy as well, as the plugins simply ignore downstream buffer alignment requirements. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=45988 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=47084 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=49715 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52183 Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/mf/tests/mf.c | 3 - dlls/winegstreamer/wg_transform.c | 123 +++++++++++++++++++++++++++--- 2 files changed, 114 insertions(+), 12 deletions(-) diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c index 3fe819ba23d..81bfafbda04 100644 --- a/dlls/mf/tests/mf.c +++ b/dlls/mf/tests/mf.c @@ -7284,9 +7284,7 @@ static void test_h264_decoder(void) ok(hr == S_OK, "ConvertToContiguousBuffer returned %#lx\n", hr); hr = IMFMediaBuffer_Lock(media_buffer, &data, NULL, &length); ok(hr == S_OK, "Lock returned %#lx\n", hr); - todo_wine ok(length == nv12_frame_len, "got length %lu\n", length); - if (length != nv12_frame_len) goto skip_nv12_tests; for (i = 0; i < actual_aperture.Area.cy; ++i) { @@ -7302,7 +7300,6 @@ static void test_h264_decoder(void) check_sample(output.pSample, nv12_frame_data, output_file); -skip_nv12_tests: ret = IMFSample_Release(output.pSample); ok(ret == 0, "Release returned %lu\n", ret); diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c index f98f3bdd14b..daf5b69cc5c 100644 --- a/dlls/winegstreamer/wg_transform.c +++ b/dlls/winegstreamer/wg_transform.c @@ -56,6 +56,7 @@ struct wg_transform GstSegment segment; GstBufferList *input; guint input_max_length; + guint output_plane_align; GstAtomicQueue *output_queue; GstSample *output_sample; bool output_caps_changed; @@ -73,6 +74,20 @@ static bool is_caps_video(GstCaps *caps) return g_str_has_prefix(media_type, "video/"); } +static bool align_video_info_planes(gsize plane_align, GstVideoInfo *info, GstVideoAlignment *align) +{ + gst_video_alignment_reset(align); + + align->padding_right = ((plane_align + 1) - (info->width & plane_align)) & plane_align; + align->padding_bottom = ((plane_align + 1) - (info->height & plane_align)) & plane_align; + align->stride_align[0] = plane_align; + align->stride_align[1] = plane_align; + align->stride_align[2] = plane_align; + align->stride_align[3] = plane_align; + + return gst_video_info_align(info, align); +} + static GstFlowReturn transform_sink_chain_cb(GstPad *pad, GstObject *parent, GstBuffer *buffer) { struct wg_transform *transform = gst_pad_get_element_private(pad); @@ -106,7 +121,9 @@ static gboolean transform_sink_query_cb(GstPad *pad, GstObject *parent, GstQuery { case GST_QUERY_ALLOCATION: { - GstStructure *config; + gsize plane_align = transform->output_plane_align; + GstStructure *config, *params; + GstVideoAlignment align; gboolean needs_pool; GstBufferPool *pool; GstVideoInfo info; @@ -117,19 +134,36 @@ static gboolean transform_sink_query_cb(GstPad *pad, GstObject *parent, GstQuery break; if (!gst_video_info_from_caps(&info, caps) + || !align_video_info_planes(plane_align, &info, &align) || !(pool = gst_video_buffer_pool_new())) break; + if ((params = gst_structure_new("video-meta", + "padding-top", G_TYPE_UINT, align.padding_top, + "padding-bottom", G_TYPE_UINT, align.padding_bottom, + "padding-left", G_TYPE_UINT, align.padding_left, + "padding-right", G_TYPE_UINT, align.padding_right, + NULL))) + gst_query_add_allocation_meta(query, GST_VIDEO_META_API_TYPE, params); + if (!(config = gst_buffer_pool_get_config(pool))) GST_ERROR("Failed to get pool %p config.", pool); else { + gst_buffer_pool_config_add_option(config, GST_BUFFER_POOL_OPTION_VIDEO_META); + gst_buffer_pool_config_add_option(config, GST_BUFFER_POOL_OPTION_VIDEO_ALIGNMENT); + gst_buffer_pool_config_set_video_alignment(config, &align); + gst_buffer_pool_config_set_params(config, caps, info.size, 0, 0); if (!gst_buffer_pool_set_config(pool, config)) GST_ERROR("Failed to set pool %p config.", pool); } + /* Prevent pool reconfiguration, we don't want another alignment. */ + if (!gst_buffer_pool_set_active(pool, true)) + GST_ERROR("Pool %p failed to activate.", pool); + if (gst_query_get_n_allocation_pools(query) > 0) gst_query_set_nth_allocation_pool(query, 0, pool, info.size, 0, 0); else @@ -299,6 +333,7 @@ NTSTATUS wg_transform_create(void *args) if (!(transform->output_queue = gst_atomic_queue_new(8))) goto out; transform->input_max_length = 1; + transform->output_plane_align = 0; if (!(src_caps = wg_format_to_caps(&input_format))) goto out; @@ -340,6 +375,7 @@ NTSTATUS wg_transform_create(void *args) * to match its expectations. */ transform->input_max_length = 16; + transform->output_plane_align = 15; if (!(element = create_element("h264parse", "base")) || !transform_append_element(transform, element, &first, &last)) goto out; @@ -501,16 +537,67 @@ NTSTATUS wg_transform_push_data(void *args) return STATUS_SUCCESS; } -static NTSTATUS read_transform_output_data(GstBuffer *buffer, struct wg_sample *sample) +static bool copy_video_buffer(GstBuffer *buffer, GstCaps *caps, gsize plane_align, + struct wg_sample *sample, gsize *total_size) { - GstMapInfo info; + GstVideoFrame src_frame, dst_frame; + GstVideoInfo src_info, *dst_info; + GstVideoAlignment align; + GstBuffer *dst_buffer; + bool ret = false; - if (!gst_buffer_map(buffer, &info, GST_MAP_READ)) + if (!gst_video_info_from_caps(&src_info, caps)) + return false; + if (!(dst_info = gst_video_info_copy(&src_info))) + return false; + if (!(dst_buffer = gst_buffer_new_wrapped_full(0, sample->data, sample->max_size, + 0, sample->max_size, 0, NULL))) + goto out; + + if (!align_video_info_planes(plane_align, dst_info, &align)) { - GST_ERROR("Failed to map buffer %p", buffer); - sample->size = 0; - return STATUS_UNSUCCESSFUL; + GST_ERROR("Failed to align video info."); + goto out; } + if (sample->max_size < dst_info->size) + { + GST_ERROR("Output buffer too small."); + goto out; + } + gst_buffer_set_size(dst_buffer, dst_info->size); + *total_size = sample->size = dst_info->size; + + if (!gst_video_frame_map(&src_frame, &src_info, buffer, GST_MAP_READ)) + GST_ERROR("Failed to map source frame."); + else + { + if (!gst_video_frame_map(&dst_frame, dst_info, dst_buffer, GST_MAP_WRITE)) + GST_ERROR("Failed to map destination frame."); + else + { + if (!(ret = gst_video_frame_copy(&dst_frame, &src_frame))) + GST_ERROR("Failed to copy video frame."); + gst_video_frame_unmap(&dst_frame); + } + gst_video_frame_unmap(&src_frame); + } + +out: + if (dst_buffer) + gst_buffer_unref(dst_buffer); + if (dst_info) + gst_video_info_free(dst_info); + + return ret; +} + +static bool copy_buffer(GstBuffer *buffer, GstCaps *caps, struct wg_sample *sample, + gsize *total_size) +{ + GstMapInfo info; + + if (!gst_buffer_map(buffer, &info, GST_MAP_READ)) + return false; if (sample->max_size >= info.size) sample->size = info.size; @@ -526,6 +613,23 @@ static NTSTATUS read_transform_output_data(GstBuffer *buffer, struct wg_sample * if (sample->flags & WG_SAMPLE_FLAG_INCOMPLETE) gst_buffer_resize(buffer, sample->size, -1); + *total_size = info.size; + return true; +} + +static NTSTATUS read_transform_output_data(GstBuffer *buffer, GstCaps *caps, gsize plane_align, + struct wg_sample *sample) +{ + gsize total_size; + + if (!(is_caps_video(caps) ? copy_video_buffer(buffer, caps, plane_align, sample, &total_size) + : copy_buffer(buffer, caps, sample, &total_size))) + { + GST_ERROR("Failed to copy buffer %p", buffer); + sample->size = 0; + return STATUS_UNSUCCESSFUL; + } + if (GST_BUFFER_PTS_IS_VALID(buffer)) { sample->flags |= WG_SAMPLE_FLAG_HAS_PTS; @@ -535,7 +639,7 @@ static NTSTATUS read_transform_output_data(GstBuffer *buffer, struct wg_sample * { GstClockTime duration = GST_BUFFER_DURATION(buffer) / 100; - duration = (duration * sample->size) / info.size; + duration = (duration * sample->size) / total_size; GST_BUFFER_DURATION(buffer) -= duration * 100; if (GST_BUFFER_PTS_IS_VALID(buffer)) GST_BUFFER_PTS(buffer) += duration * 100; @@ -599,7 +703,8 @@ NTSTATUS wg_transform_read_data(void *args) return STATUS_SUCCESS; } - if ((status = read_transform_output_data(output_buffer, sample))) + if ((status = read_transform_output_data(output_buffer, output_caps, + transform->output_plane_align, sample))) return status; if (!(sample->flags & WG_SAMPLE_FLAG_INCOMPLETE)) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/139
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=115759 Your paranoid android. === debian11 (build log) === 0104:err:winediag:wma_decoder_create GStreamer doesn't support WMA decoding, please install appropriate plugins 0104:err:winediag:h264_decoder_create GStreamer doesn't support H.264 decoding, please install appropriate plugins 0100:err:winediag:wma_decoder_create GStreamer doesn't support WMA decoding, please install appropriate plugins 0100:err:winediag:h264_decoder_create GStreamer doesn't support H.264 decoding, please install appropriate plugins 0100:err:winediag:wma_decoder_create GStreamer doesn't support WMA decoding, please install appropriate plugins 0100:err:winediag:h264_decoder_create GStreamer doesn't support H.264 decoding, please install appropriate plugins 0100:err:winediag:wma_decoder_create GStreamer doesn't support WMA decoding, please install appropriate plugins 0100:err:winediag:h264_decoder_create GStreamer doesn't support H.264 decoding, please install appropriate plugins 0100:err:winediag:wma_decoder_create GStreamer doesn't support WMA decoding, please install appropriate plugins 0100:err:winediag:h264_decoder_create GStreamer doesn't support H.264 decoding, please install appropriate plugins 0108:err:winediag:wma_decoder_create GStreamer doesn't support WMA decoding, please install appropriate plugins 0108:err:winediag:h264_decoder_create GStreamer doesn't support H.264 decoding, please install appropriate plugins 0100:err:winediag:wma_decoder_create GStreamer doesn't support WMA decoding, please install appropriate plugins 0100:err:winediag:h264_decoder_create GStreamer doesn't support H.264 decoding, please install appropriate plugins 0100:err:winediag:wma_decoder_create GStreamer doesn't support WMA decoding, please install appropriate plugins 0100:err:winediag:h264_decoder_create GStreamer doesn't support H.264 decoding, please install appropriate plugins === debian11 (build log) === 0100:err:winediag:wma_decoder_create GStreamer doesn't support WMA decoding, please install appropriate plugins 0100:err:winediag:h264_decoder_create GStreamer doesn't support H.264 decoding, please install appropriate plugins
On 5/30/22 03:20, Rémi Bernon wrote:
dlls/mf/tests/mf.c | 3 - dlls/winegstreamer/wg_transform.c | 123 +++++++++++++++++++++++++++--- 2 files changed, 114 insertions(+), 12 deletions(-)
Looks clear and correct, just a couple of nitpicks...
+ if (!(dst_info = gst_video_info_copy(&src_info))) + return false;
Any reason not to allocate dst_info on stack?
+ if (!(is_caps_video(caps) ? copy_video_buffer(buffer, caps, plane_align, sample, &total_size) + : copy_buffer(buffer, caps, sample, &total_size)))
This is a little hard to read. Would you mind either using a "ret" variable, or perhaps putting the copy_video_buffer() call inside of copy_buffer()?
From: Rémi Bernon <rbernon(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=45988 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=47084 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=49715 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52183 Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/mf/tests/mf.c | 40 +++++++++++++++---------------- dlls/winegstreamer/h264_decoder.c | 16 +++++++++++++ dlls/winegstreamer/mfplat.c | 25 +++++++++++++++++++ dlls/winegstreamer/unixlib.h | 1 + dlls/winegstreamer/wg_format.c | 3 ++- dlls/winegstreamer/wg_transform.c | 23 ++++++++++++++++++ 6 files changed, 87 insertions(+), 21 deletions(-) diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c index 81bfafbda04..4f55a67f30b 100644 --- a/dlls/mf/tests/mf.c +++ b/dlls/mf/tests/mf.c @@ -6842,70 +6842,70 @@ static void test_h264_decoder(void) ATTR_GUID(MF_MT_SUBTYPE, MFVideoFormat_NV12), ATTR_RATIO(MF_MT_PIXEL_ASPECT_RATIO, 1, 1), ATTR_RATIO(MF_MT_FRAME_RATE, 60000, 1000), - ATTR_RATIO(MF_MT_FRAME_SIZE, actual_width, actual_height, .todo_value = TRUE), - ATTR_UINT32(MF_MT_SAMPLE_SIZE, actual_width * actual_height * 3 / 2, .todo_value = TRUE), - ATTR_UINT32(MF_MT_DEFAULT_STRIDE, actual_width, .todo_value = TRUE), + ATTR_RATIO(MF_MT_FRAME_SIZE, actual_width, actual_height), + ATTR_UINT32(MF_MT_SAMPLE_SIZE, actual_width * actual_height * 3 / 2), + ATTR_UINT32(MF_MT_DEFAULT_STRIDE, actual_width), /* ATTR_UINT32(MF_MT_VIDEO_ROTATION, 0), missing on Win7 */ ATTR_UINT32(MF_MT_INTERLACE_MODE, 7), ATTR_UINT32(MF_MT_FIXED_SIZE_SAMPLES, 1), ATTR_UINT32(MF_MT_ALL_SAMPLES_INDEPENDENT, 1), - ATTR_BLOB(MF_MT_MINIMUM_DISPLAY_APERTURE, &actual_aperture, 16, .todo = TRUE), + ATTR_BLOB(MF_MT_MINIMUM_DISPLAY_APERTURE, &actual_aperture, 16), }, { ATTR_GUID(MF_MT_MAJOR_TYPE, MFMediaType_Video), ATTR_GUID(MF_MT_SUBTYPE, MFVideoFormat_YV12), ATTR_RATIO(MF_MT_PIXEL_ASPECT_RATIO, 1, 1), ATTR_RATIO(MF_MT_FRAME_RATE, 60000, 1000), - ATTR_RATIO(MF_MT_FRAME_SIZE, actual_width, actual_height, .todo_value = TRUE), - ATTR_UINT32(MF_MT_SAMPLE_SIZE, actual_width * actual_height * 3 / 2, .todo_value = TRUE), - ATTR_UINT32(MF_MT_DEFAULT_STRIDE, actual_width, .todo_value = TRUE), + ATTR_RATIO(MF_MT_FRAME_SIZE, actual_width, actual_height), + ATTR_UINT32(MF_MT_SAMPLE_SIZE, actual_width * actual_height * 3 / 2), + ATTR_UINT32(MF_MT_DEFAULT_STRIDE, actual_width), /* ATTR_UINT32(MF_MT_VIDEO_ROTATION, 0), missing on Win7 */ ATTR_UINT32(MF_MT_INTERLACE_MODE, 7), ATTR_UINT32(MF_MT_FIXED_SIZE_SAMPLES, 1), ATTR_UINT32(MF_MT_ALL_SAMPLES_INDEPENDENT, 1), - ATTR_BLOB(MF_MT_MINIMUM_DISPLAY_APERTURE, &actual_aperture, 16, .todo = TRUE), + ATTR_BLOB(MF_MT_MINIMUM_DISPLAY_APERTURE, &actual_aperture, 16), }, { ATTR_GUID(MF_MT_MAJOR_TYPE, MFMediaType_Video), ATTR_GUID(MF_MT_SUBTYPE, MFVideoFormat_IYUV), ATTR_RATIO(MF_MT_PIXEL_ASPECT_RATIO, 1, 1), ATTR_RATIO(MF_MT_FRAME_RATE, 60000, 1000), - ATTR_RATIO(MF_MT_FRAME_SIZE, actual_width, actual_height, .todo_value = TRUE), - ATTR_UINT32(MF_MT_SAMPLE_SIZE, actual_width * actual_height * 3 / 2, .todo_value = TRUE), - ATTR_UINT32(MF_MT_DEFAULT_STRIDE, actual_width, .todo_value = TRUE), + ATTR_RATIO(MF_MT_FRAME_SIZE, actual_width, actual_height), + ATTR_UINT32(MF_MT_SAMPLE_SIZE, actual_width * actual_height * 3 / 2), + ATTR_UINT32(MF_MT_DEFAULT_STRIDE, actual_width), /* ATTR_UINT32(MF_MT_VIDEO_ROTATION, 0), missing on Win7 */ ATTR_UINT32(MF_MT_INTERLACE_MODE, 7), ATTR_UINT32(MF_MT_FIXED_SIZE_SAMPLES, 1), ATTR_UINT32(MF_MT_ALL_SAMPLES_INDEPENDENT, 1), - ATTR_BLOB(MF_MT_MINIMUM_DISPLAY_APERTURE, &actual_aperture, 16, .todo = TRUE), + ATTR_BLOB(MF_MT_MINIMUM_DISPLAY_APERTURE, &actual_aperture, 16), }, { ATTR_GUID(MF_MT_MAJOR_TYPE, MFMediaType_Video), ATTR_GUID(MF_MT_SUBTYPE, MFVideoFormat_I420), ATTR_RATIO(MF_MT_PIXEL_ASPECT_RATIO, 1, 1), ATTR_RATIO(MF_MT_FRAME_RATE, 60000, 1000), - ATTR_RATIO(MF_MT_FRAME_SIZE, actual_width, actual_height, .todo_value = TRUE), - ATTR_UINT32(MF_MT_SAMPLE_SIZE, actual_width * actual_height * 3 / 2, .todo_value = TRUE), - ATTR_UINT32(MF_MT_DEFAULT_STRIDE, actual_width, .todo_value = TRUE), + ATTR_RATIO(MF_MT_FRAME_SIZE, actual_width, actual_height), + ATTR_UINT32(MF_MT_SAMPLE_SIZE, actual_width * actual_height * 3 / 2), + ATTR_UINT32(MF_MT_DEFAULT_STRIDE, actual_width), /* ATTR_UINT32(MF_MT_VIDEO_ROTATION, 0), missing on Win7 */ ATTR_UINT32(MF_MT_INTERLACE_MODE, 7), ATTR_UINT32(MF_MT_FIXED_SIZE_SAMPLES, 1), ATTR_UINT32(MF_MT_ALL_SAMPLES_INDEPENDENT, 1), - ATTR_BLOB(MF_MT_MINIMUM_DISPLAY_APERTURE, &actual_aperture, 16, .todo = TRUE), + ATTR_BLOB(MF_MT_MINIMUM_DISPLAY_APERTURE, &actual_aperture, 16), }, { ATTR_GUID(MF_MT_MAJOR_TYPE, MFMediaType_Video), ATTR_GUID(MF_MT_SUBTYPE, MFVideoFormat_YUY2), ATTR_RATIO(MF_MT_PIXEL_ASPECT_RATIO, 1, 1), ATTR_RATIO(MF_MT_FRAME_RATE, 60000, 1000), - ATTR_RATIO(MF_MT_FRAME_SIZE, actual_width, actual_height, .todo_value = TRUE), - ATTR_UINT32(MF_MT_SAMPLE_SIZE, actual_width * actual_height * 2, .todo_value = TRUE), - ATTR_UINT32(MF_MT_DEFAULT_STRIDE, actual_width * 2, .todo_value = TRUE), + ATTR_RATIO(MF_MT_FRAME_SIZE, actual_width, actual_height), + ATTR_UINT32(MF_MT_SAMPLE_SIZE, actual_width * actual_height * 2), + ATTR_UINT32(MF_MT_DEFAULT_STRIDE, actual_width * 2), /* ATTR_UINT32(MF_MT_VIDEO_ROTATION, 0), missing on Win7 */ ATTR_UINT32(MF_MT_INTERLACE_MODE, 7), ATTR_UINT32(MF_MT_FIXED_SIZE_SAMPLES, 1), ATTR_UINT32(MF_MT_ALL_SAMPLES_INDEPENDENT, 1), - ATTR_BLOB(MF_MT_MINIMUM_DISPLAY_APERTURE, &actual_aperture, 16, .todo = TRUE), + ATTR_BLOB(MF_MT_MINIMUM_DISPLAY_APERTURE, &actual_aperture, 16), }, }; diff --git a/dlls/winegstreamer/h264_decoder.c b/dlls/winegstreamer/h264_decoder.c index f97b6cf18c2..e39b9340be1 100644 --- a/dlls/winegstreamer/h264_decoder.c +++ b/dlls/winegstreamer/h264_decoder.c @@ -173,6 +173,22 @@ static HRESULT fill_output_media_type(struct h264_decoder *decoder, IMFMediaType return hr; } + if (FAILED(hr = IMFMediaType_GetItem(media_type, &MF_MT_MINIMUM_DISPLAY_APERTURE, NULL)) + && !IsRectEmpty(&wg_format->u.video.padding)) + { + MFVideoArea aperture = + { + .OffsetX = {.value = wg_format->u.video.padding.left}, + .OffsetY = {.value = wg_format->u.video.padding.top}, + .Area.cx = wg_format->u.video.width - wg_format->u.video.padding.right - wg_format->u.video.padding.left, + .Area.cy = wg_format->u.video.height - wg_format->u.video.padding.bottom - wg_format->u.video.padding.top, + }; + + if (FAILED(hr = IMFMediaType_SetBlob(media_type, &MF_MT_MINIMUM_DISPLAY_APERTURE, + (BYTE *)&aperture, sizeof(aperture)))) + return hr; + } + return S_OK; } diff --git a/dlls/winegstreamer/mfplat.c b/dlls/winegstreamer/mfplat.c index 2e53afb0542..0226e7a2e45 100644 --- a/dlls/winegstreamer/mfplat.c +++ b/dlls/winegstreamer/mfplat.c @@ -688,6 +688,20 @@ static IMFMediaType *mf_media_type_from_wg_format_video(const struct wg_format * IMFMediaType_SetUINT32(type, &MF_MT_ALL_SAMPLES_INDEPENDENT, TRUE); IMFMediaType_SetUINT32(type, &MF_MT_VIDEO_ROTATION, MFVideoRotationFormat_0); + if (!IsRectEmpty(&format->u.video.padding)) + { + MFVideoArea aperture = + { + .OffsetX = {.value = format->u.video.padding.left}, + .OffsetY = {.value = format->u.video.padding.top}, + .Area.cx = format->u.video.width - format->u.video.padding.right - format->u.video.padding.left, + .Area.cy = format->u.video.height - format->u.video.padding.bottom - format->u.video.padding.top, + }; + + IMFMediaType_SetBlob(type, &MF_MT_MINIMUM_DISPLAY_APERTURE, + (BYTE *)&aperture, sizeof(aperture)); + } + return type; } } @@ -770,7 +784,9 @@ static void mf_media_type_to_wg_format_audio(IMFMediaType *type, const GUID *sub static void mf_media_type_to_wg_format_video(IMFMediaType *type, const GUID *subtype, struct wg_format *format) { UINT64 frame_rate, frame_size; + MFVideoArea aperture; unsigned int i; + UINT32 size; if (FAILED(IMFMediaType_GetUINT64(type, &MF_MT_FRAME_SIZE, &frame_size))) { @@ -784,6 +800,15 @@ static void mf_media_type_to_wg_format_video(IMFMediaType *type, const GUID *sub format->u.video.fps_n = 1; format->u.video.fps_d = 1; + if (SUCCEEDED(IMFMediaType_GetBlob(type, &MF_MT_MINIMUM_DISPLAY_APERTURE, (BYTE *)&aperture, + sizeof(aperture), &size)) && size == sizeof(aperture)) + { + format->u.video.padding.left = aperture.OffsetX.value; + format->u.video.padding.top = aperture.OffsetY.value; + format->u.video.padding.right = format->u.video.width - aperture.Area.cx - aperture.OffsetX.value; + format->u.video.padding.bottom = format->u.video.height - aperture.Area.cy - aperture.OffsetY.value; + } + if (SUCCEEDED(IMFMediaType_GetUINT64(type, &MF_MT_FRAME_RATE, &frame_rate)) && (UINT32)frame_rate) { format->u.video.fps_n = (UINT32)(frame_rate >> 32); diff --git a/dlls/winegstreamer/unixlib.h b/dlls/winegstreamer/unixlib.h index e8cdfaf7217..f334a168bd1 100644 --- a/dlls/winegstreamer/unixlib.h +++ b/dlls/winegstreamer/unixlib.h @@ -68,6 +68,7 @@ struct wg_format } format; int32_t width, height; uint32_t fps_n, fps_d; + RECT padding; } video; struct { diff --git a/dlls/winegstreamer/wg_format.c b/dlls/winegstreamer/wg_format.c index 4cebeac9182..3b568ea2fec 100644 --- a/dlls/winegstreamer/wg_format.c +++ b/dlls/winegstreamer/wg_format.c @@ -552,7 +552,8 @@ bool wg_format_compare(const struct wg_format *a, const struct wg_format *b) /* Do not compare FPS. */ return a->u.video.format == b->u.video.format && a->u.video.width == b->u.video.width - && abs(a->u.video.height) == abs(b->u.video.height); + && abs(a->u.video.height) == abs(b->u.video.height) + && EqualRect( &a->u.video.padding, &b->u.video.padding ); } assert(0); diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c index daf5b69cc5c..7dd9407952d 100644 --- a/dlls/winegstreamer/wg_transform.c +++ b/dlls/winegstreamer/wg_transform.c @@ -696,8 +696,31 @@ NTSTATUS wg_transform_read_data(void *args) GST_MINI_OBJECT_FLAG_UNSET(transform->output_sample, GST_SAMPLE_FLAG_WG_CAPS_CHANGED); if (format) + { + gsize plane_align = transform->output_plane_align; + GstVideoAlignment align; + GstVideoInfo info; + wg_format_from_caps(format, output_caps); + if (format->major_type == WG_MAJOR_TYPE_VIDEO + && gst_video_info_from_caps(&info, output_caps) + && align_video_info_planes(plane_align, &info, &align)) + { + GST_INFO("Returning video alignment left %u, top %u, right %u, bottom %u.", align.padding_left, + align.padding_top, align.padding_right, align.padding_bottom); + + format->u.video.padding.left = align.padding_left; + format->u.video.width += format->u.video.padding.left; + format->u.video.padding.right = align.padding_right; + format->u.video.width += format->u.video.padding.right; + format->u.video.padding.top = align.padding_top; + format->u.video.height += format->u.video.padding.top; + format->u.video.padding.bottom = align.padding_bottom; + format->u.video.height += format->u.video.padding.bottom; + } + } + params->result = MF_E_TRANSFORM_STREAM_CHANGE; GST_INFO("Format changed detected, returning no output"); return STATUS_SUCCESS; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/139
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=115760 Your paranoid android. === debian11 (build log) === 0104:err:winediag:wma_decoder_create GStreamer doesn't support WMA decoding, please install appropriate plugins 0104:err:winediag:h264_decoder_create GStreamer doesn't support H.264 decoding, please install appropriate plugins 0100:err:winediag:wma_decoder_create GStreamer doesn't support WMA decoding, please install appropriate plugins 0100:err:winediag:h264_decoder_create GStreamer doesn't support H.264 decoding, please install appropriate plugins 0100:err:winediag:wma_decoder_create GStreamer doesn't support WMA decoding, please install appropriate plugins 0100:err:winediag:h264_decoder_create GStreamer doesn't support H.264 decoding, please install appropriate plugins 0104:err:winediag:wma_decoder_create GStreamer doesn't support WMA decoding, please install appropriate plugins 0104:err:winediag:h264_decoder_create GStreamer doesn't support H.264 decoding, please install appropriate plugins 0100:err:winediag:wma_decoder_create GStreamer doesn't support WMA decoding, please install appropriate plugins 0100:err:winediag:h264_decoder_create GStreamer doesn't support H.264 decoding, please install appropriate plugins 0104:err:winediag:wma_decoder_create GStreamer doesn't support WMA decoding, please install appropriate plugins 0104:err:winediag:h264_decoder_create GStreamer doesn't support H.264 decoding, please install appropriate plugins 0100:err:winediag:wma_decoder_create GStreamer doesn't support WMA decoding, please install appropriate plugins 0100:err:winediag:h264_decoder_create GStreamer doesn't support H.264 decoding, please install appropriate plugins 0100:err:winediag:wma_decoder_create GStreamer doesn't support WMA decoding, please install appropriate plugins 0100:err:winediag:h264_decoder_create GStreamer doesn't support H.264 decoding, please install appropriate plugins === debian11 (build log) === 0100:err:winediag:wma_decoder_create GStreamer doesn't support WMA decoding, please install appropriate plugins 0100:err:winediag:h264_decoder_create GStreamer doesn't support H.264 decoding, please install appropriate plugins
From: Rémi Bernon <rbernon(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=45988 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=47084 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=49715 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52183 Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/mf/tests/mf.c | 1 - dlls/winegstreamer/h264_decoder.c | 6 ++++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c index 4f55a67f30b..03035aa5e9f 100644 --- a/dlls/mf/tests/mf.c +++ b/dlls/mf/tests/mf.c @@ -7311,7 +7311,6 @@ static void test_h264_decoder(void) ok(hr == S_OK, "MFCreateMediaType returned %#lx\n", hr); init_media_type(media_type, is_win7 ? output_type_desc_win7 : output_type_desc, -1); hr = IMFTransform_SetOutputType(transform, 0, media_type, 0); - todo_wine ok(hr == MF_E_INVALIDMEDIATYPE, "SetOutputType returned %#lx.\n", hr); init_media_type(media_type, is_win7 ? new_output_type_desc_win7 : new_output_type_desc, -1); hr = IMFTransform_SetOutputType(transform, 0, media_type, 0); diff --git a/dlls/winegstreamer/h264_decoder.c b/dlls/winegstreamer/h264_decoder.c index e39b9340be1..19a36a9a77a 100644 --- a/dlls/winegstreamer/h264_decoder.c +++ b/dlls/winegstreamer/h264_decoder.c @@ -433,6 +433,7 @@ static HRESULT WINAPI transform_SetOutputType(IMFTransform *iface, DWORD id, IMF { struct h264_decoder *decoder = impl_from_IMFTransform(iface); GUID major, subtype; + UINT64 frame_size; HRESULT hr; ULONG i; @@ -454,6 +455,11 @@ static HRESULT WINAPI transform_SetOutputType(IMFTransform *iface, DWORD id, IMF if (i == ARRAY_SIZE(h264_decoder_output_types)) return MF_E_INVALIDMEDIATYPE; + if (FAILED(hr = IMFMediaType_GetUINT64(type, &MF_MT_FRAME_SIZE, &frame_size)) + || (frame_size >> 32) != decoder->wg_format.u.video.width + || (UINT32)frame_size != decoder->wg_format.u.video.height) + return MF_E_INVALIDMEDIATYPE; + if (decoder->output_type) IMFMediaType_Release(decoder->output_type); IMFMediaType_AddRef((decoder->output_type = type)); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/139
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=115761 Your paranoid android. === debian11 (build log) === 0104:err:winediag:wma_decoder_create GStreamer doesn't support WMA decoding, please install appropriate plugins 0104:err:winediag:h264_decoder_create GStreamer doesn't support H.264 decoding, please install appropriate plugins 0104:err:winediag:wma_decoder_create GStreamer doesn't support WMA decoding, please install appropriate plugins 0104:err:winediag:h264_decoder_create GStreamer doesn't support H.264 decoding, please install appropriate plugins 0104:err:winediag:wma_decoder_create GStreamer doesn't support WMA decoding, please install appropriate plugins 0104:err:winediag:h264_decoder_create GStreamer doesn't support H.264 decoding, please install appropriate plugins 0108:err:winediag:wma_decoder_create GStreamer doesn't support WMA decoding, please install appropriate plugins 0108:err:winediag:h264_decoder_create GStreamer doesn't support H.264 decoding, please install appropriate plugins 0108:err:winediag:wma_decoder_create GStreamer doesn't support WMA decoding, please install appropriate plugins 0108:err:winediag:h264_decoder_create GStreamer doesn't support H.264 decoding, please install appropriate plugins 0100:err:winediag:wma_decoder_create GStreamer doesn't support WMA decoding, please install appropriate plugins 0100:err:winediag:h264_decoder_create GStreamer doesn't support H.264 decoding, please install appropriate plugins 0100:err:winediag:wma_decoder_create GStreamer doesn't support WMA decoding, please install appropriate plugins 0100:err:winediag:h264_decoder_create GStreamer doesn't support H.264 decoding, please install appropriate plugins 0104:err:winediag:wma_decoder_create GStreamer doesn't support WMA decoding, please install appropriate plugins 0104:err:winediag:h264_decoder_create GStreamer doesn't support H.264 decoding, please install appropriate plugins === debian11 (build log) === 0100:err:winediag:wma_decoder_create GStreamer doesn't support WMA decoding, please install appropriate plugins 0100:err:winediag:h264_decoder_create GStreamer doesn't support H.264 decoding, please install appropriate plugins
v2: Added a commit first to remove the use of `gst_sample_set_info`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/139#note_1367
On Wed Jun 1 23:20:29 2022 +0000, **** wrote:
Zebediah Figura replied on the mailing list: ``` On 5/30/22 03:20, Rémi Bernon wrote:
+ if (gst_query_get_n_allocation_pools(query) > 0) + gst_query_set_nth_allocation_pool(query, 0, pool, info.size, 0, 0); + else + gst_query_add_allocation_pool(query, pool, info.size, 0, 0); GStreamer documentation isn't exactly clear about this, but my understanding is that, as the sink, we are supposed to be the first element to add things to this query, and as such there shouldn't be any pools already. Have you encountered cases where that wasn't true? Or were you modeling this after some GStreamer code I didn't find? FWIW, the above seems to be a common pattern for filters or source elements *after* receiving a query response from the sink, but that's not the case here.
Yes, it's not clear how it is supposed to be used, and I just wanted to make sure our pool would be prioritized in all cases.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/139#note_1497
On Wed Jun 1 23:20:21 2022 +0000, **** wrote:
Zebediah Figura replied on the mailing list: ``` On 5/30/22 03:20, Rémi Bernon wrote:
dlls/mf/tests/mf.c | 3 - dlls/winegstreamer/wg_transform.c | 123 +++++++++++++++++++++++++++--- 2 files changed, 114 insertions(+), 12 deletions(-) Looks clear and correct, just a couple of nitpicks... + if (!(dst_info = gst_video_info_copy(&src_info))) + return false; Any reason not to allocate dst_info on stack? + if (!(is_caps_video(caps) ? copy_video_buffer(buffer, caps, plane_align, sample, &total_size) + : copy_buffer(buffer, caps, sample, &total_size))) This is a little hard to read. Would you mind either using a "ret" variable, or perhaps putting the copy_video_buffer() call inside of copy_buffer()?
Any reason not to allocate dst_info on stack?
No real reason I guess, I was worried about the `finfo` pointer but looks like it can just be copied as is. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/139#note_1498
participants (3)
-
Marvin -
Rémi Bernon -
Zebediah Figura