[PATCH v3 0/7] MR3636: winegstreamer: Implement wg_muxer.
Since 1.20, gst_element_request_pad_simple is available and gst_element_get_request_pad is marked as deprecated. -- v3: winegstreamer: Add enum wg_container_type. https://gitlab.winehq.org/wine/wine/-/merge_requests/3636
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/winegstreamer/unixlib.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/dlls/winegstreamer/unixlib.c b/dlls/winegstreamer/unixlib.c index 6ed38260536..4fcf7d0d469 100644 --- a/dlls/winegstreamer/unixlib.c +++ b/dlls/winegstreamer/unixlib.c @@ -87,15 +87,21 @@ GstElement *find_element(GstElementFactoryListType type, GstCaps *src_caps, GstC if (!(transforms = gst_element_factory_list_get_elements(type, GST_RANK_MARGINAL))) goto done; - tmp = gst_element_factory_list_filter(transforms, src_caps, GST_PAD_SINK, FALSE); - gst_plugin_feature_list_free(transforms); - if (!(transforms = tmp)) - goto done; + if (src_caps) + { + tmp = gst_element_factory_list_filter(transforms, src_caps, GST_PAD_SINK, FALSE); + gst_plugin_feature_list_free(transforms); + if (!(transforms = tmp)) + goto done; + } - tmp = gst_element_factory_list_filter(transforms, sink_caps, GST_PAD_SRC, FALSE); - gst_plugin_feature_list_free(transforms); - if (!(transforms = tmp)) - goto done; + if (sink_caps) + { + tmp = gst_element_factory_list_filter(transforms, sink_caps, GST_PAD_SRC, FALSE); + gst_plugin_feature_list_free(transforms); + if (!(transforms = tmp)) + goto done; + } transforms = g_list_sort(transforms, gst_plugin_feature_rank_compare_func); for (tmp = transforms; tmp != NULL && element == NULL; tmp = tmp->next) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3636
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/winegstreamer/unixlib.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/dlls/winegstreamer/unixlib.c b/dlls/winegstreamer/unixlib.c index 4fcf7d0d469..f2e0b1ca2fe 100644 --- a/dlls/winegstreamer/unixlib.c +++ b/dlls/winegstreamer/unixlib.c @@ -145,16 +145,17 @@ bool append_element(GstElement *container, GstElement *element, GstElement **fir if (!gst_bin_add(GST_BIN(container), element) || !gst_element_sync_state_with_parent(element) || - (*last && !gst_element_link(*last, element))) + (last && *last && !gst_element_link(*last, element))) { GST_ERROR("Failed to link %s element.", name); } else { GST_DEBUG("Linked %s element %p.", name, element); - if (!*first) + if (first && !*first) *first = element; - *last = element; + if (last) + *last = element; success = true; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3636
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/winegstreamer/unix_private.h | 1 + dlls/winegstreamer/unixlib.c | 14 ++++++++++++++ dlls/winegstreamer/wg_parser.c | 12 ++++++------ dlls/winegstreamer/wg_transform.c | 16 ++++++++-------- 4 files changed, 29 insertions(+), 14 deletions(-) diff --git a/dlls/winegstreamer/unix_private.h b/dlls/winegstreamer/unix_private.h index db6bf3ebb2e..8bef7b2b2bd 100644 --- a/dlls/winegstreamer/unix_private.h +++ b/dlls/winegstreamer/unix_private.h @@ -39,6 +39,7 @@ extern GstElement *find_element(GstElementFactoryListType type, GstCaps *src_cap extern bool append_element(GstElement *container, GstElement *element, GstElement **first, GstElement **last) DECLSPEC_HIDDEN; extern bool link_src_to_element(GstPad *src_pad, GstElement *element) DECLSPEC_HIDDEN; extern bool link_element_to_sink(GstElement *element, GstPad *sink_pad) DECLSPEC_HIDDEN; +extern bool push_event(GstPad *pad, GstEvent *event) DECLSPEC_HIDDEN; /* wg_format.c */ diff --git a/dlls/winegstreamer/unixlib.c b/dlls/winegstreamer/unixlib.c index f2e0b1ca2fe..eca2bb8aacc 100644 --- a/dlls/winegstreamer/unixlib.c +++ b/dlls/winegstreamer/unixlib.c @@ -209,6 +209,20 @@ bool link_element_to_sink(GstElement *element, GstPad *sink_pad) return !ret; } +bool push_event(GstPad *pad, GstEvent *event) +{ + if (!gst_pad_push_event(pad, event)) + { + const gchar *type_name = gst_event_type_get_name(GST_EVENT_TYPE(event)); + gchar *pad_name = gst_pad_get_name(pad); + + GST_ERROR("Failed to push %s event %p to pad %s.", type_name, event, pad_name); + g_free(pad_name); + return false; + } + return true; +} + NTSTATUS wg_init_gstreamer(void *arg) { char arg0[] = "wine"; diff --git a/dlls/winegstreamer/wg_parser.c b/dlls/winegstreamer/wg_parser.c index 298c05c6b88..5556b52829c 100644 --- a/dlls/winegstreamer/wg_parser.c +++ b/dlls/winegstreamer/wg_parser.c @@ -258,7 +258,7 @@ static NTSTATUS wg_parser_stream_enable(void *args) gst_util_set_object_arg(G_OBJECT(stream->flip), "method", flip ? "vertical-flip" : "none"); } - gst_pad_push_event(stream->my_sink, gst_event_new_reconfigure()); + push_event(stream->my_sink, gst_event_new_reconfigure()); return S_OK; } @@ -450,7 +450,7 @@ static NTSTATUS wg_parser_stream_seek(void *args) if ((stop_flags & AM_SEEKING_PositioningBitsMask) == AM_SEEKING_NoPositioning) stop_type = GST_SEEK_TYPE_NONE; - if (!gst_pad_push_event(get_stream(params->stream)->my_sink, gst_event_new_seek(params->rate, GST_FORMAT_TIME, + if (!push_event(get_stream(params->stream)->my_sink, gst_event_new_seek(params->rate, GST_FORMAT_TIME, flags, start_type, params->start_pos * 100, stop_type, params->stop_pos * 100))) GST_ERROR("Failed to seek.\n"); @@ -480,7 +480,7 @@ static NTSTATUS wg_parser_stream_notify_qos(void *args) if (!(event = gst_event_new_qos(params->underflow ? GST_QOS_TYPE_UNDERFLOW : GST_QOS_TYPE_OVERFLOW, params->proportion, params->diff * 100, stream_time))) GST_ERROR("Failed to create QOS event.\n"); - gst_pad_push_event(stream->my_sink, event); + push_event(stream->my_sink, event); return S_OK; } @@ -1292,7 +1292,7 @@ static void *push_data(void *arg) gst_buffer_unref(buffer); - gst_pad_push_event(parser->my_src, gst_event_new_eos()); + push_event(parser->my_src, gst_event_new_eos()); GST_DEBUG("Stopping push thread."); @@ -1421,7 +1421,7 @@ static gboolean src_perform_seek(struct wg_parser *parser, GstEvent *event) { flush_event = gst_event_new_flush_start(); gst_event_set_seqnum(flush_event, seqnum); - gst_pad_push_event(parser->my_src, flush_event); + push_event(parser->my_src, flush_event); if (thread) gst_pad_set_active(parser->my_src, 1); } @@ -1433,7 +1433,7 @@ static gboolean src_perform_seek(struct wg_parser *parser, GstEvent *event) { flush_event = gst_event_new_flush_stop(TRUE); gst_event_set_seqnum(flush_event, seqnum); - gst_pad_push_event(parser->my_src, flush_event); + push_event(parser->my_src, flush_event); if (thread) gst_pad_set_active(parser->my_src, 1); } diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c index 775fed7a46c..38adf560892 100644 --- a/dlls/winegstreamer/wg_transform.c +++ b/dlls/winegstreamer/wg_transform.c @@ -443,10 +443,10 @@ NTSTATUS wg_transform_create(void *args) goto out; if (!(event = gst_event_new_stream_start("stream")) - || !gst_pad_push_event(transform->my_src, event)) + || !push_event(transform->my_src, event)) goto out; if (!(event = gst_event_new_caps(src_caps)) - || !gst_pad_push_event(transform->my_src, event)) + || !push_event(transform->my_src, event)) goto out; /* We need to use GST_FORMAT_TIME here because it's the only format @@ -455,7 +455,7 @@ NTSTATUS wg_transform_create(void *args) transform->segment.start = 0; transform->segment.stop = -1; if (!(event = gst_event_new_segment(&transform->segment)) - || !gst_pad_push_event(transform->my_src, event)) + || !push_event(transform->my_src, event)) goto out; gst_caps_unref(src_caps); @@ -531,7 +531,7 @@ NTSTATUS wg_transform_set_output_format(void *args) value = "none"; gst_util_set_object_arg(G_OBJECT(transform->video_flip), "method", value); } - if (!gst_pad_push_event(transform->my_sink, gst_event_new_reconfigure())) + if (!push_event(transform->my_sink, gst_event_new_reconfigure())) { GST_ERROR("Failed to reconfigure transform %p.", transform); return STATUS_UNSUCCESSFUL; @@ -892,16 +892,16 @@ NTSTATUS wg_transform_drain(void *args) } if (!(event = gst_event_new_segment_done(GST_FORMAT_TIME, -1)) - || !gst_pad_push_event(transform->my_src, event)) + || !push_event(transform->my_src, event)) goto error; if (!(event = gst_event_new_eos()) - || !gst_pad_push_event(transform->my_src, event)) + || !push_event(transform->my_src, event)) goto error; if (!(event = gst_event_new_stream_start("stream")) - || !gst_pad_push_event(transform->my_src, event)) + || !push_event(transform->my_src, event)) goto error; if (!(event = gst_event_new_segment(&transform->segment)) - || !gst_pad_push_event(transform->my_src, event)) + || !push_event(transform->my_src, event)) goto error; return STATUS_SUCCESS; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3636
From: Ziqing Hui <zhui(a)codeweavers.com> Since 1.20, gst_element_request_pad_simple is available and gst_element_get_request_pad is marked as deprecated. --- configure | 54 +++++++++++++++++++++++++++++++ configure.ac | 3 +- dlls/winegstreamer/unix_private.h | 1 + dlls/winegstreamer/unixlib.c | 35 ++++++++++++++++++++ include/config.h.in | 3 ++ 5 files changed, 95 insertions(+), 1 deletion(-) diff --git a/configure b/configure index db95cc3588d..3b5dee728ae 100755 --- a/configure +++ b/configure @@ -16992,6 +16992,60 @@ then : : fi + { printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking for -lgstreamer-1.0" >&5 +printf %s "checking for -lgstreamer-1.0... " >&6; } +if test ${ac_cv_lib_soname_gstreamer_1_0+y} +then : + printf %s "(cached) " >&6 +else $as_nop + ac_check_soname_save_LIBS=$LIBS +LIBS="-lgstreamer-1.0 $LIBS" + cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ + +/* Override any GCC internal prototype to avoid an error. + Use char because int might match the return type of a GCC + builtin and then its argument prototype would still apply. */ +char gst_init (); +int +main (void) +{ +return gst_init (); + ; + return 0; +} +_ACEOF +if ac_fn_c_try_link "$LINENO" +then : + case "$LIBEXT" in + dll) ac_cv_lib_soname_gstreamer_1_0=`$ac_cv_path_LDD conftest.exe | grep "gstreamer-1.0" | sed -e "s/dll.*/dll/"';2,$d'` ;; + dylib) ac_cv_lib_soname_gstreamer_1_0=`$OTOOL -L conftest$ac_exeext | grep "libgstreamer-1.0\\.[0-9A-Za-z.]*dylib" | sed -e "s/^.*\/\(libgstreamer-1.0\.[0-9A-Za-z.]*dylib\).*$/\1/"';2,$d'` ;; + *) ac_cv_lib_soname_gstreamer_1_0=`$READELF -d conftest$ac_exeext | grep "NEEDED.*libgstreamer-1.0\\.$LIBEXT" | sed -e "s/^.*\\[\\(libgstreamer-1.0\\.$LIBEXT[^ ]*\\)\\].*$/\1/"';2,$d'` + if ${ac_cv_lib_soname_gstreamer_1_0:+false} : +then : + ac_cv_lib_soname_gstreamer_1_0=`$LDD conftest$ac_exeext | grep "libgstreamer-1.0\\.$LIBEXT" | sed -e "s/^.*\(libgstreamer-1.0\.$LIBEXT[^ ]*\).*$/\1/"';2,$d'` +fi ;; + esac +else $as_nop + ac_cv_lib_soname_gstreamer_1_0= +fi +rm -f core conftest.err conftest.$ac_objext conftest.beam \ + conftest$ac_exeext conftest.$ac_ext + LIBS=$ac_check_soname_save_LIBS +fi +if ${ac_cv_lib_soname_gstreamer_1_0:+false} : +then : + { printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: not found" >&5 +printf "%s\n" "not found" >&6; } + +else $as_nop + { printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: $ac_cv_lib_soname_gstreamer_1_0" >&5 +printf "%s\n" "$ac_cv_lib_soname_gstreamer_1_0" >&6; } + +printf "%s\n" "#define SONAME_LIBGSTREAMER_1_0 \"$ac_cv_lib_soname_gstreamer_1_0\"" >>confdefs.h + + +fi else $as_nop { printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: no" >&5 printf "%s\n" "no" >&6; } diff --git a/configure.ac b/configure.ac index e17d28c8601..3b6e297528c 100644 --- a/configure.ac +++ b/configure.ac @@ -1588,7 +1588,8 @@ then AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <gst/gst.h>]], [[static int a[sizeof(gint64) > 4 ? 1 : -1]; if (a[0]) return 0;]])], [AC_MSG_RESULT([yes]) - AC_CHECK_LIB(gstreamer-1.0,gst_pad_new,[:],,[$GSTREAMER_LIBS])], + AC_CHECK_LIB(gstreamer-1.0,gst_pad_new,[:],,[$GSTREAMER_LIBS]) + WINE_CHECK_SONAME(gstreamer-1.0, gst_init)], [AC_MSG_RESULT([no]) ac_glib2_broken=yes enable_winegstreamer=${enable_winegstreamer:-no} diff --git a/dlls/winegstreamer/unix_private.h b/dlls/winegstreamer/unix_private.h index 8bef7b2b2bd..b39252d4d9a 100644 --- a/dlls/winegstreamer/unix_private.h +++ b/dlls/winegstreamer/unix_private.h @@ -40,6 +40,7 @@ extern bool append_element(GstElement *container, GstElement *element, GstElemen extern bool link_src_to_element(GstPad *src_pad, GstElement *element) DECLSPEC_HIDDEN; extern bool link_element_to_sink(GstElement *element, GstPad *sink_pad) DECLSPEC_HIDDEN; extern bool push_event(GstPad *pad, GstEvent *event) DECLSPEC_HIDDEN; +extern GstPad *request_pad(GstElement *element, const char *name) DECLSPEC_HIDDEN; /* wg_format.c */ diff --git a/dlls/winegstreamer/unixlib.c b/dlls/winegstreamer/unixlib.c index eca2bb8aacc..32311acaf7d 100644 --- a/dlls/winegstreamer/unixlib.c +++ b/dlls/winegstreamer/unixlib.c @@ -27,6 +27,7 @@ #include <assert.h> #include <stdarg.h> #include <stdio.h> +#include <dlfcn.h> #include <gst/gst.h> #include <gst/video/video.h> @@ -47,6 +48,9 @@ GST_DEBUG_CATEGORY(wine); +GstPad * (*pgst_element_request_pad_simple)(GstElement * element, const gchar * name); +GstPad * (*pgst_element_get_request_pad)(GstElement * element, const gchar * name); + GstStreamType stream_type_from_caps(GstCaps *caps) { const gchar *media_type; @@ -223,6 +227,25 @@ bool push_event(GstPad *pad, GstEvent *event) return true; } +GstPad *request_pad(GstElement *element, const char *name) +{ + GstPad *pad = NULL; + + if (pgst_element_request_pad_simple) + pad = pgst_element_request_pad_simple(element, name); + else if (pgst_element_get_request_pad) + pad = pgst_element_get_request_pad(element, name); + + if (!pad) + { + gchar *element_name = gst_element_get_name(element); + GST_ERROR("Failed to request pad %s from element %s.", name, element_name); + g_free(element_name); + } + + return pad; +} + NTSTATUS wg_init_gstreamer(void *arg) { char arg0[] = "wine"; @@ -230,8 +253,20 @@ NTSTATUS wg_init_gstreamer(void *arg) char *args[] = {arg0, arg1, NULL}; int argc = ARRAY_SIZE(args) - 1; char **argv = args; + void *gst_handle; GError *err; + if (!(gst_handle = dlopen(SONAME_LIBGSTREAMER_1_0, RTLD_NOW))) + { + fprintf(stderr, "Failed to find gstreamer library: %s.\n", SONAME_LIBGSTREAMER_1_0); + return STATUS_DLL_NOT_FOUND; + } + +#define LOAD_OPTIONAL_FUNCPTR(f) p##f = dlsym(gst_handle, #f) + LOAD_OPTIONAL_FUNCPTR(gst_element_request_pad_simple); + LOAD_OPTIONAL_FUNCPTR(gst_element_get_request_pad); +#undef LOAD_OPTIONAL_FUNCPTR + if (!gst_init_check(&argc, &argv, &err)) { fprintf(stderr, "winegstreamer: failed to initialize GStreamer: %s\n", err->message); diff --git a/include/config.h.in b/include/config.h.in index fa234a82f84..94684d09b49 100644 --- a/include/config.h.in +++ b/include/config.h.in @@ -744,6 +744,9 @@ /* Define to the soname of the libgssapi_krb5 library. */ #undef SONAME_LIBGSSAPI_KRB5 +/* Define to the soname of the libgstreamer-1.0 library. */ +#undef SONAME_LIBGSTREAMER_1_0 + /* Define to the soname of the libkrb5 library. */ #undef SONAME_LIBKRB5 -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3636
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/winegstreamer/mfplat.c | 11 ++++++++++- dlls/winegstreamer/unixlib.h | 2 ++ dlls/winegstreamer/wg_format.c | 14 ++++++++++++++ 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/dlls/winegstreamer/mfplat.c b/dlls/winegstreamer/mfplat.c index 7a37f25fcb2..d85568e5ecd 100644 --- a/dlls/winegstreamer/mfplat.c +++ b/dlls/winegstreamer/mfplat.c @@ -804,8 +804,9 @@ static void mf_media_type_to_wg_format_audio_wma(IMFMediaType *type, const GUID static void mf_media_type_to_wg_format_video_h264(IMFMediaType *type, struct wg_format *format) { + UINT32 profile, level, codec_data_len; UINT64 frame_rate, frame_size; - UINT32 profile, level; + BYTE *codec_data; memset(format, 0, sizeof(*format)); format->major_type = WG_MAJOR_TYPE_VIDEO_H264; @@ -832,6 +833,14 @@ static void mf_media_type_to_wg_format_video_h264(IMFMediaType *type, struct wg_ if (SUCCEEDED(IMFMediaType_GetUINT32(type, &MF_MT_MPEG2_LEVEL, &level))) format->u.video_h264.level = level; + + if (SUCCEEDED(IMFMediaType_GetAllocatedBlob(type, &MF_MT_MPEG_SEQUENCE_HEADER, &codec_data, &codec_data_len))) + { + assert(codec_data_len <= sizeof(format->u.video_h264.codec_data)); + format->u.video_h264.codec_data_len = codec_data_len; + memcpy(format->u.video_h264.codec_data, codec_data, codec_data_len); + CoTaskMemFree(codec_data); + } } static void mf_media_type_to_wg_format_video_indeo(IMFMediaType *type, uint32_t version, struct wg_format *format) diff --git a/dlls/winegstreamer/unixlib.h b/dlls/winegstreamer/unixlib.h index 702bd7aa69b..15e0605fdde 100644 --- a/dlls/winegstreamer/unixlib.h +++ b/dlls/winegstreamer/unixlib.h @@ -148,6 +148,8 @@ struct wg_format uint32_t fps_n, fps_d; uint32_t profile; uint32_t level; + uint32_t codec_data_len; + unsigned char codec_data[64]; } video_h264; struct { diff --git a/dlls/winegstreamer/wg_format.c b/dlls/winegstreamer/wg_format.c index bd0ebf892c1..2353839bbc4 100644 --- a/dlls/winegstreamer/wg_format.c +++ b/dlls/winegstreamer/wg_format.c @@ -553,6 +553,7 @@ static GstCaps *wg_format_to_caps_audio_wma(const struct wg_format *format) static GstCaps *wg_format_to_caps_video_h264(const struct wg_format *format) { const char *profile, *level; + GstBuffer *buffer; GstCaps *caps; if (!(caps = gst_caps_new_empty_simple("video/x-h264"))) @@ -609,6 +610,19 @@ static GstCaps *wg_format_to_caps_video_h264(const struct wg_format *format) if (level) gst_caps_set_simple(caps, "level", G_TYPE_STRING, level, NULL); + if (format->u.video_h264.codec_data_len) + { + if (!(buffer = gst_buffer_new_and_alloc(format->u.video_h264.codec_data_len))) + { + gst_caps_unref(caps); + return NULL; + } + + gst_buffer_fill(buffer, 0, format->u.video_h264.codec_data, format->u.video_h264.codec_data_len); + gst_caps_set_simple(caps, "codec_data", GST_TYPE_BUFFER, buffer, NULL); + gst_buffer_unref(buffer); + } + return caps; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3636
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/mf/tests/mf.c | 37 +++++++++++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c index 4f4a282a9d8..3f1e288a967 100644 --- a/dlls/mf/tests/mf.c +++ b/dlls/mf/tests/mf.c @@ -81,6 +81,20 @@ HRESULT (WINAPI *pMFGetTopoNodeCurrentType)(IMFTopologyNode *node, DWORD stream, HRESULT (WINAPI *pMFCreateDXGIDeviceManager)(UINT *token, IMFDXGIDeviceManager **manager); BOOL has_video_processor; +const static BYTE test_h264_sequence_header[] = +{ + 0x00, 0x00, 0x01, 0x67, 0x64, 0x00, 0x14, 0xac, 0xd9, 0x46, 0x36, 0xc0, + 0x5a, 0x83, 0x03, 0x03, 0x52, 0x80, 0x00, 0x00, 0x03, 0x00, 0x80, 0x00, + 0x00, 0x03, 0x01, 0x47, 0x8a, 0x14, 0xcb, 0x00, 0x00, 0x01, 0x68, 0xeb, + 0xec, 0xb2, 0x2c, +}; + +const static BYTE test_aac_codec_data[] = +{ + 0x00, 0x00, 0x29, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x12, 0x08, +}; + static BOOL is_vista(void) { return !pMFGetTopoNodeCurrentType; @@ -6756,7 +6770,7 @@ static void test_mpeg4_media_sink(void) { IMFMediaSink *sink = NULL, *sink2 = NULL, *sink_audio = NULL, *sink_video = NULL, *sink_empty = NULL; IMFByteStream *bytestream, *bytestream_audio, *bytestream_video, *bytestream_empty; - DWORD id, count, flags, width = 16, height = 16, fps = 10; + DWORD id, count, flags, width = 96, height = 96, fps = 1; IMFMediaType *audio_type, *video_type, *media_type; IMFMediaTypeHandler *type_handler = NULL; IMFPresentationClock *clock; @@ -6772,23 +6786,34 @@ static void test_mpeg4_media_sink(void) hr = IMFMediaType_SetGUID(audio_type, &MF_MT_MAJOR_TYPE, &MFMediaType_Audio); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - hr = IMFMediaType_SetGUID(audio_type, &MF_MT_SUBTYPE, &MFAudioFormat_PCM); + hr = IMFMediaType_SetGUID(audio_type, &MF_MT_SUBTYPE, &MFAudioFormat_AAC); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - hr = IMFMediaType_SetUINT32(audio_type, &MF_MT_AUDIO_NUM_CHANNELS, 2); + hr = IMFMediaType_SetUINT32(audio_type, &MF_MT_AUDIO_NUM_CHANNELS, 1); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - hr = IMFMediaType_SetUINT32(audio_type, &MF_MT_AUDIO_SAMPLES_PER_SECOND, 48000); + hr = IMFMediaType_SetUINT32(audio_type, &MF_MT_AUDIO_BITS_PER_SAMPLE, 16); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - hr = IMFMediaType_SetUINT32(audio_type, &MF_MT_AUDIO_BITS_PER_SAMPLE, 8); + hr = IMFMediaType_SetUINT32(audio_type, &MF_MT_AUDIO_SAMPLES_PER_SECOND, 44100); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = IMFMediaType_SetUINT32(audio_type, &MF_MT_AUDIO_AVG_BYTES_PER_SECOND, 12000); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = IMFMediaType_SetUINT32(audio_type, &MF_MT_AAC_AUDIO_PROFILE_LEVEL_INDICATION, 41); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = IMFMediaType_SetUINT32(audio_type, &MF_MT_AAC_PAYLOAD_TYPE, 0); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = IMFMediaType_SetBlob(audio_type, &MF_MT_USER_DATA, test_aac_codec_data, sizeof(test_aac_codec_data)); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); hr = IMFMediaType_SetGUID(video_type, &MF_MT_MAJOR_TYPE, &MFMediaType_Video); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - hr = IMFMediaType_SetGUID(video_type, &MF_MT_SUBTYPE, &MFVideoFormat_RGB24); + hr = IMFMediaType_SetGUID(video_type, &MF_MT_SUBTYPE, &MFVideoFormat_H264); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); hr = IMFMediaType_SetUINT64(video_type, &MF_MT_FRAME_SIZE, ((UINT64)width << 32) | height); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); hr = IMFMediaType_SetUINT64(video_type, &MF_MT_FRAME_RATE, ((UINT64)fps << 32) | 1); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = IMFMediaType_SetBlob(video_type, &MF_MT_MPEG_SEQUENCE_HEADER, + test_h264_sequence_header, sizeof(test_h264_sequence_header)); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); hr = MFCreateTempFile(MF_ACCESSMODE_WRITE, MF_OPENMODE_DELETE_IF_EXIST, 0, &bytestream_audio); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3636
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/mf/tests/mf.c | 43 ++++++++--------- dlls/mfreadwrite/tests/mfplat.c | 13 +----- dlls/winegstreamer/gst_private.h | 3 +- dlls/winegstreamer/main.c | 7 ++- dlls/winegstreamer/media_sink.c | 49 ++++++++++++++++---- dlls/winegstreamer/unix_private.h | 1 + dlls/winegstreamer/unixlib.h | 8 ++++ dlls/winegstreamer/wg_format.c | 22 +++++++++ dlls/winegstreamer/winegstreamer_classes.idl | 7 +++ 9 files changed, 107 insertions(+), 46 deletions(-) diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c index 3f1e288a967..3e6b031f273 100644 --- a/dlls/mf/tests/mf.c +++ b/dlls/mf/tests/mf.c @@ -6825,58 +6825,39 @@ static void test_mpeg4_media_sink(void) ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); hr = MFCreateMPEG4MediaSink(NULL, NULL, NULL, NULL); - todo_wine ok(hr == E_POINTER, "Unexpected hr %#lx.\n", hr); sink = (void *)0xdeadbeef; hr = MFCreateMPEG4MediaSink(NULL, NULL, NULL, &sink); - todo_wine ok(hr == E_POINTER, "Unexpected hr %#lx.\n", hr); ok(sink == (void *)0xdeadbeef, "Unexpected pointer %p.\n", sink); sink = NULL; hr = MFCreateMPEG4MediaSink(bytestream_empty, NULL, NULL, &sink_empty); - todo_wine ok(hr == S_OK || broken(hr == E_INVALIDARG), "Unexpected hr %#lx.\n", hr); hr = MFCreateMPEG4MediaSink(bytestream_audio, NULL, audio_type, &sink_audio); - todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); hr = MFCreateMPEG4MediaSink(bytestream_video, video_type, NULL, &sink_video); - todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); hr = MFCreateMPEG4MediaSink(bytestream, video_type, audio_type, &sink); - todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - if (!sink) - { - if (sink_video) - IMFMediaSink_Release(sink_video); - if (sink_audio) - IMFMediaSink_Release(sink_audio); - if (sink_empty) - IMFMediaSink_Release(sink_empty); - IMFByteStream_Release(bytestream); - IMFByteStream_Release(bytestream_empty); - IMFByteStream_Release(bytestream_video); - IMFByteStream_Release(bytestream_audio); - IMFMediaType_Release(video_type); - IMFMediaType_Release(audio_type); - return; - } - /* Test sink. */ + flags = 0xdeadbeef; hr = IMFMediaSink_GetCharacteristics(sink, &flags); + todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + todo_wine ok(flags == MEDIASINK_RATELESS || broken(flags == (MEDIASINK_RATELESS | MEDIASINK_FIXED_STREAMS)), "Unexpected flags %#lx.\n", flags); check_interface(sink, &IID_IMFMediaEventGenerator, TRUE); check_interface(sink, &IID_IMFFinalizableMediaSink, TRUE); check_interface(sink, &IID_IMFClockStateSink, TRUE); + todo_wine check_interface(sink, &IID_IMFGetService, TRUE); /* Test sink stream count. */ @@ -6991,8 +6972,11 @@ static void test_mpeg4_media_sink(void) hr = MFCreatePresentationClock(&clock); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); hr = IMFMediaSink_SetPresentationClock(sink, NULL); + todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + todo_wine hr = IMFMediaSink_SetPresentationClock(sink, clock); + todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); IMFPresentationClock_Release(clock); @@ -7011,13 +6995,18 @@ static void test_mpeg4_media_sink(void) ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); hr = IMFMediaTypeHandler_GetMajorType(type_handler, NULL); + todo_wine ok(hr == E_POINTER, "Unexpected hr %#lx.\n", hr); hr = IMFMediaTypeHandler_GetMajorType(type_handler, &guid); + todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + todo_wine ok(IsEqualGUID(&guid, &MFMediaType_Audio), "Unexpected major type.\n"); hr = IMFMediaTypeHandler_GetMediaTypeCount(type_handler, &count); + todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + todo_wine ok(count == 1, "Unexpected count %lu.\n", count); hr = IMFMediaTypeHandler_GetCurrentMediaType(type_handler, &media_type); @@ -7027,8 +7016,10 @@ static void test_mpeg4_media_sink(void) ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); hr = IMFMediaTypeHandler_SetCurrentMediaType(type_handler, NULL); + todo_wine ok(hr == E_POINTER, "Unexpected hr %#lx.\n", hr); hr = IMFMediaTypeHandler_SetCurrentMediaType(type_handler, media_type); + todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); IMFMediaType_Release(media_type); @@ -7047,19 +7038,25 @@ static void test_mpeg4_media_sink(void) ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); hr = IMFMediaTypeHandler_GetMajorType(type_handler, NULL); + todo_wine ok(hr == E_POINTER, "Unexpected hr %#lx.\n", hr); hr = IMFMediaTypeHandler_GetMajorType(type_handler, &guid); + todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); IMFStreamSink_Release(stream_sink); hr = IMFMediaSink_AddStreamSink(sink, 0, audio_type, &stream_sink); + todo_wine ok(hr == MF_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr); hr = IMFMediaSink_GetStreamSinkByIndex(sink, 0, &stream_sink); + todo_wine ok(hr == MF_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr); hr = IMFMediaSink_GetStreamSinkById(sink, 0, &stream_sink); + todo_wine ok(hr == MF_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr); hr = IMFMediaSink_GetCharacteristics(sink, &flags); + todo_wine ok(hr == MF_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr); IMFMediaTypeHandler_Release(type_handler); diff --git a/dlls/mfreadwrite/tests/mfplat.c b/dlls/mfreadwrite/tests/mfplat.c index 3b54aa4ff4c..17e64fcc0bf 100644 --- a/dlls/mfreadwrite/tests/mfplat.c +++ b/dlls/mfreadwrite/tests/mfplat.c @@ -1468,44 +1468,35 @@ static void test_sink_writer_mp4(void) ok(!writer, "Unexpected pointer %p.\n", writer); hr = MFCreateSinkWriterFromURL(NULL, stream, attr, &writer); - todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); if (hr == S_OK) IMFSinkWriter_Release(writer); hr = MFCreateSinkWriterFromURL(tmp_file, NULL, NULL, &writer); - todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); if (hr == S_OK) IMFSinkWriter_Release(writer); hr = MFCreateSinkWriterFromURL(tmp_file, NULL, attr, &writer); - todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); if (hr == S_OK) IMFSinkWriter_Release(writer); hr = MFCreateSinkWriterFromURL(tmp_file, stream, NULL, &writer); - todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); if (hr == S_OK) IMFSinkWriter_Release(writer); hr = MFCreateSinkWriterFromURL(tmp_file, stream, attr, &writer); - todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - if (hr != S_OK) - { - IMFByteStream_Release(stream); - IMFAttributes_Release(attr); - return; - } /* Test GetServiceForStream. */ sink = (void *)0xdeadbeef; hr = IMFSinkWriter_GetServiceForStream(writer, MF_SINK_WRITER_MEDIASINK, &GUID_NULL, &IID_IMFMediaSink, (void **)&sink); + todo_wine ok(hr == MF_E_UNSUPPORTED_SERVICE, "Unexpected hr %#lx.\n", hr); + todo_wine ok(!sink, "Unexpected pointer %p.\n", sink); DeleteFileW(tmp_file); diff --git a/dlls/winegstreamer/gst_private.h b/dlls/winegstreamer/gst_private.h index ed867f741d9..2edd2d19107 100644 --- a/dlls/winegstreamer/gst_private.h +++ b/dlls/winegstreamer/gst_private.h @@ -122,7 +122,8 @@ HRESULT wma_decoder_create(IUnknown *outer, IUnknown **out); HRESULT wmv_decoder_create(IUnknown *outer, IUnknown **out); HRESULT resampler_create(IUnknown *outer, IUnknown **out); HRESULT color_convert_create(IUnknown *outer, IUnknown **out); -HRESULT sink_class_factory_create(IUnknown *outer, IUnknown **out); +HRESULT mp3_sink_class_factory_create(IUnknown *outer, IUnknown **out); +HRESULT mpeg4_sink_class_factory_create(IUnknown *outer, IUnknown **out); bool amt_from_wg_format(AM_MEDIA_TYPE *mt, const struct wg_format *format, bool wm); bool amt_to_wg_format(const AM_MEDIA_TYPE *mt, struct wg_format *format); diff --git a/dlls/winegstreamer/main.c b/dlls/winegstreamer/main.c index 9624c469314..6a4825a227c 100644 --- a/dlls/winegstreamer/main.c +++ b/dlls/winegstreamer/main.c @@ -612,7 +612,8 @@ static struct class_factory wma_decoder_cf = {{&class_factory_vtbl}, wma_decoder static struct class_factory wmv_decoder_cf = {{&class_factory_vtbl}, wmv_decoder_create}; static struct class_factory resampler_cf = {{&class_factory_vtbl}, resampler_create}; static struct class_factory color_convert_cf = {{&class_factory_vtbl}, color_convert_create}; -static struct class_factory sink_class_factory_cf = {{&class_factory_vtbl}, sink_class_factory_create}; +static struct class_factory mp3_sink_class_factory_cf = {{&class_factory_vtbl}, mp3_sink_class_factory_create}; +static struct class_factory mpeg4_sink_class_factory_cf = {{&class_factory_vtbl}, mpeg4_sink_class_factory_create}; HRESULT WINAPI DllGetClassObject(REFCLSID clsid, REFIID iid, void **out) { @@ -648,7 +649,9 @@ HRESULT WINAPI DllGetClassObject(REFCLSID clsid, REFIID iid, void **out) else if (IsEqualGUID(clsid, &CLSID_CColorConvertDMO)) factory = &color_convert_cf; else if (IsEqualGUID(clsid, &CLSID_MFMP3SinkClassFactory)) - factory = &sink_class_factory_cf; + factory = &mp3_sink_class_factory_cf; + else if (IsEqualGUID(clsid, &CLSID_MFMPEG4SinkClassFactory)) + factory = &mpeg4_sink_class_factory_cf; else { FIXME("%s not implemented, returning CLASS_E_CLASSNOTAVAILABLE.\n", debugstr_guid(clsid)); diff --git a/dlls/winegstreamer/media_sink.c b/dlls/winegstreamer/media_sink.c index fcea67eb8f1..20d345974be 100644 --- a/dlls/winegstreamer/media_sink.c +++ b/dlls/winegstreamer/media_sink.c @@ -72,6 +72,7 @@ struct media_sink STATE_PAUSED, STATE_SHUTDOWN, } state; + wg_container_type type; IMFByteStream *bytestream; IMFMediaEventQueue *event_queue; @@ -1022,7 +1023,7 @@ static const IMFAsyncCallbackVtbl media_sink_callback_vtbl = media_sink_callback_Invoke, }; -static HRESULT media_sink_create(IMFByteStream *bytestream, struct media_sink **out) +static HRESULT media_sink_create(IMFByteStream *bytestream, wg_container_type type, struct media_sink **out) { struct media_sink *media_sink; HRESULT hr; @@ -1047,6 +1048,7 @@ static HRESULT media_sink_create(IMFByteStream *bytestream, struct media_sink ** media_sink->async_callback.lpVtbl = &media_sink_callback_vtbl; media_sink->refcount = 1; media_sink->state = STATE_OPENED; + media_sink->type = type; InitializeCriticalSection(&media_sink->cs); media_sink->cs.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": cs"); IMFByteStream_AddRef((media_sink->bytestream = bytestream)); @@ -1082,8 +1084,8 @@ static ULONG WINAPI sink_class_factory_Release(IMFSinkClassFactory *iface) return 1; } -static HRESULT WINAPI sink_class_factory_CreateMediaSink(IMFSinkClassFactory *iface, IMFByteStream *bytestream, - IMFMediaType *video_type, IMFMediaType *audio_type, IMFMediaSink **out) +static HRESULT WINAPI sink_class_factory_create_media_sink(IMFSinkClassFactory *iface, IMFByteStream *bytestream, + wg_container_type type, IMFMediaType *video_type, IMFMediaType *audio_type, IMFMediaSink **out) { IMFFinalizableMediaSink *media_sink_iface; struct media_sink *media_sink; @@ -1092,7 +1094,7 @@ static HRESULT WINAPI sink_class_factory_CreateMediaSink(IMFSinkClassFactory *if TRACE("iface %p, bytestream %p, video_type %p, audio_type %p, out %p.\n", iface, bytestream, video_type, audio_type, out); - if (FAILED(hr = media_sink_create(bytestream, &media_sink))) + if (FAILED(hr = media_sink_create(bytestream, type, &media_sink))) return hr; media_sink_iface = &media_sink->IMFFinalizableMediaSink_iface; @@ -1119,18 +1121,47 @@ static HRESULT WINAPI sink_class_factory_CreateMediaSink(IMFSinkClassFactory *if return S_OK; } -static const IMFSinkClassFactoryVtbl sink_class_factory_vtbl = +static HRESULT WINAPI mp3_sink_class_factory_CreateMediaSink(IMFSinkClassFactory *iface, IMFByteStream *bytestream, + IMFMediaType *video_type, IMFMediaType *audio_type, IMFMediaSink **out) +{ + return sink_class_factory_create_media_sink(iface, + bytestream, WG_CONTAINER_TYPE_ID3, video_type, audio_type, out); +} + +static HRESULT WINAPI mpeg4_sink_class_factory_CreateMediaSink(IMFSinkClassFactory *iface, IMFByteStream *bytestream, + IMFMediaType *video_type, IMFMediaType *audio_type, IMFMediaSink **out) +{ + return sink_class_factory_create_media_sink(iface, + bytestream, WG_CONTAINER_TYPE_QUICKTIME, video_type, audio_type, out); +} + +static const IMFSinkClassFactoryVtbl mp3_sink_class_factory_vtbl = { sink_class_factory_QueryInterface, sink_class_factory_AddRef, sink_class_factory_Release, - sink_class_factory_CreateMediaSink, + mp3_sink_class_factory_CreateMediaSink, }; -static IMFSinkClassFactory sink_class_factory = { &sink_class_factory_vtbl }; +static const IMFSinkClassFactoryVtbl mpeg4_sink_class_factory_vtbl = +{ + sink_class_factory_QueryInterface, + sink_class_factory_AddRef, + sink_class_factory_Release, + mpeg4_sink_class_factory_CreateMediaSink, +}; + +static IMFSinkClassFactory mp3_sink_class_factory = { &mp3_sink_class_factory_vtbl }; +static IMFSinkClassFactory mpeg4_sink_class_factory = { &mpeg4_sink_class_factory_vtbl }; + +HRESULT mp3_sink_class_factory_create(IUnknown *outer, IUnknown **out) +{ + *out = (IUnknown *)&mp3_sink_class_factory; + return S_OK; +} -HRESULT sink_class_factory_create(IUnknown *outer, IUnknown **out) +HRESULT mpeg4_sink_class_factory_create(IUnknown *outer, IUnknown **out) { - *out = (IUnknown *)&sink_class_factory; + *out = (IUnknown *)&mpeg4_sink_class_factory; return S_OK; } diff --git a/dlls/winegstreamer/unix_private.h b/dlls/winegstreamer/unix_private.h index b39252d4d9a..60819f0a865 100644 --- a/dlls/winegstreamer/unix_private.h +++ b/dlls/winegstreamer/unix_private.h @@ -47,6 +47,7 @@ extern GstPad *request_pad(GstElement *element, const char *name) DECLSPEC_HIDDE extern void wg_format_from_caps(struct wg_format *format, const GstCaps *caps) DECLSPEC_HIDDEN; extern bool wg_format_compare(const struct wg_format *a, const struct wg_format *b) DECLSPEC_HIDDEN; extern GstCaps *wg_format_to_caps(const struct wg_format *format) DECLSPEC_HIDDEN; +extern GstCaps *wg_container_type_to_caps(enum wg_container_type container_type) DECLSPEC_HIDDEN; /* wg_transform.c */ diff --git a/dlls/winegstreamer/unixlib.h b/dlls/winegstreamer/unixlib.h index 15e0605fdde..a5e1e6ffd0c 100644 --- a/dlls/winegstreamer/unixlib.h +++ b/dlls/winegstreamer/unixlib.h @@ -166,6 +166,14 @@ struct wg_format } u; }; +typedef UINT32 wg_container_type; +enum wg_container_type +{ + WG_CONTAINER_TYPE_UNKNOWN = 0, + WG_CONTAINER_TYPE_ID3, + WG_CONTAINER_TYPE_QUICKTIME, +}; + enum wg_sample_flag { WG_SAMPLE_FLAG_INCOMPLETE = 1, diff --git a/dlls/winegstreamer/wg_format.c b/dlls/winegstreamer/wg_format.c index 2353839bbc4..a1b22ad8abb 100644 --- a/dlls/winegstreamer/wg_format.c +++ b/dlls/winegstreamer/wg_format.c @@ -767,3 +767,25 @@ bool wg_format_compare(const struct wg_format *a, const struct wg_format *b) assert(0); return false; } + +GstCaps *wg_container_type_to_caps(enum wg_container_type container_type) +{ + GstCaps *caps = NULL; + + switch (container_type) + { + case WG_CONTAINER_TYPE_ID3: + caps = gst_caps_new_empty_simple("application/x-id3"); + break; + case WG_CONTAINER_TYPE_QUICKTIME: + caps = gst_caps_new_empty_simple("video/quicktime"); + gst_caps_set_simple(caps, "variant", G_TYPE_STRING, "iso", NULL); + break; + case WG_CONTAINER_TYPE_UNKNOWN: + default: + GST_WARNING("Container type %u not implemented.\n", container_type); + break; + } + + return caps; +} diff --git a/dlls/winegstreamer/winegstreamer_classes.idl b/dlls/winegstreamer/winegstreamer_classes.idl index 2bc85e70b3a..3e9b19c90e9 100644 --- a/dlls/winegstreamer/winegstreamer_classes.idl +++ b/dlls/winegstreamer/winegstreamer_classes.idl @@ -117,3 +117,10 @@ coclass CColorConvertDMO {} uuid(11275a82-5e5a-47fd-a01c-3683c12fb196) ] coclass MFMP3SinkClassFactory {} + +[ + helpstring("MF MPEG4 Sink Class Factory"), + threading(both), + uuid(a22c4fc7-6e91-4e1d-89e9-53b2667b72ba) +] +coclass MFMPEG4SinkClassFactory {} -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3636
1/7, 2/7, and 4/7 add dead code, and I cannot easily guess how 1/7 and 2/7 will be used.1/7, 2/7, and 4/7 add dead code, and I cannot easily guess how 1/7 and 2/7 will be used.
1/7 and 2/7 is used here: https://gitlab.winehq.org/wine/wine/-/merge_requests/3303/diffs?commit_id=cb... 4/7 is just to do dynamic linking like you said below.
How much utility does 3/7 add? It's just push_event() but it emits GST_ERROR. In what cases is GStreamer not already emitting that error? Then again, if it makes debugging easier I won't argue against it.
The main purpose of 3/7 is to display the event type name which I think makes debug easiser.
7/7 does not at all do what it says it does.
Any suggestion on 7/7? Change the commit message or split into 2 patches? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3636#note_43142
1/7, 2/7, and 4/7 add dead code, and I cannot easily guess how 1/7 and 2/7 will be used.1/7, 2/7, and 4/7 add dead code, and I cannot easily guess how 1/7 and 2/7 will be used.
1/7 and 2/7 is used here: https://gitlab.winehq.org/wine/wine/-/merge_requests/3303/diffs?commit_id=cb...
In general, if a patch introduces some new code, it should be justified by the patch itself. That means that, in principle, no patch should introduce dead code, period. Of course, it gets tricky when you have a large patch that you'd like to split up for reviewability. In that case it may be fine to split up the patch. But if that's going to happen, I'd assert that (a) it should be done in the same series as its user, (b) it should be justified in the patch subject. Reviewers in general are going to read one patch at a time; looking forward should be avoided, especially implicitly.
7/7 does not at all do what it says it does.
Any suggestion on 7/7? Change the commit message or split into 2 patches?
From skimming, it seems to do three things:
* Add CLSID_MFMPEG4SinkClassFactory, which is most of the patch; * actually introduce the wg_container_type enumeration, which is a scant few lines and should probably be deferred until it's actually going to be interpreted; * add wg_container_type_to_caps(), which is currently dead code and should definitely be deferred until it's actually going to be used. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3636#note_43214
participants (3)
-
Zebediah Figura (＠zfigura) -
Ziqing Hui
-
Ziqing Hui (＠zhui)