[PATCH 0/3] MR3737: winegstreamer: Implement wg_muxer.
This patch set is part of !3303 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3737
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/winegstreamer/Makefile.in | 1 + dlls/winegstreamer/gst_private.h | 3 + dlls/winegstreamer/main.c | 26 ++++++ dlls/winegstreamer/unix_private.h | 6 ++ dlls/winegstreamer/unixlib.c | 29 +++--- dlls/winegstreamer/unixlib.h | 33 +++++++ dlls/winegstreamer/wg_format.c | 39 +++++++++ dlls/winegstreamer/wg_muxer.c | 141 ++++++++++++++++++++++++++++++ dlls/winegstreamer/wg_parser.c | 25 +++++- 9 files changed, 291 insertions(+), 12 deletions(-) create mode 100644 dlls/winegstreamer/wg_muxer.c diff --git a/dlls/winegstreamer/Makefile.in b/dlls/winegstreamer/Makefile.in index 78bdd6c0ef1..f963dcea7f0 100644 --- a/dlls/winegstreamer/Makefile.in +++ b/dlls/winegstreamer/Makefile.in @@ -22,6 +22,7 @@ C_SRCS = \ video_processor.c \ wg_allocator.c \ wg_format.c \ + wg_muxer.c \ wg_parser.c \ wg_sample.c \ wg_transform.c \ diff --git a/dlls/winegstreamer/gst_private.h b/dlls/winegstreamer/gst_private.h index ed867f741d9..0c814148447 100644 --- a/dlls/winegstreamer/gst_private.h +++ b/dlls/winegstreamer/gst_private.h @@ -110,6 +110,9 @@ bool wg_transform_get_status(wg_transform_t transform, bool *accepts_input); HRESULT wg_transform_drain(wg_transform_t transform); HRESULT wg_transform_flush(wg_transform_t transform); +HRESULT wg_muxer_create(struct wg_container_format *format, wg_muxer_t *muxer); +void wg_muxer_destroy(wg_muxer_t muxer); + unsigned int wg_format_get_max_size(const struct wg_format *format); HRESULT avi_splitter_create(IUnknown *outer, IUnknown **out); diff --git a/dlls/winegstreamer/main.c b/dlls/winegstreamer/main.c index c78d73537f7..e1ddf1e52f7 100644 --- a/dlls/winegstreamer/main.c +++ b/dlls/winegstreamer/main.c @@ -456,6 +456,32 @@ HRESULT wg_transform_flush(wg_transform_t transform) return S_OK; } +HRESULT wg_muxer_create(struct wg_container_format *format, wg_muxer_t *muxer) +{ + struct wg_muxer_create_params params = + { + .format = format, + }; + NTSTATUS status; + + TRACE("format %p, muxer %p.\n", format, muxer); + + if (SUCCEEDED(status = WINE_UNIX_CALL(unix_wg_muxer_create, ¶ms))) + { + *muxer = params.muxer; + TRACE("Created wg_muxer %#I64x.\n", params.muxer); + } + + return status; +} + +void wg_muxer_destroy(wg_muxer_t muxer) +{ + TRACE("muxer %#I64x.\n", muxer); + + WINE_UNIX_CALL(unix_wg_muxer_destroy, &muxer); +} + #define ALIGN(n, alignment) (((n) + (alignment) - 1) & ~((alignment) - 1)) unsigned int wg_format_get_stride(const struct wg_format *format) diff --git a/dlls/winegstreamer/unix_private.h b/dlls/winegstreamer/unix_private.h index 8bef7b2b2bd..90a31f53f38 100644 --- a/dlls/winegstreamer/unix_private.h +++ b/dlls/winegstreamer/unix_private.h @@ -46,6 +46,7 @@ extern bool push_event(GstPad *pad, GstEvent *event) DECLSPEC_HIDDEN; 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_format_to_caps(const struct wg_container_format *format) DECLSPEC_HIDDEN; /* wg_transform.c */ @@ -58,6 +59,11 @@ extern NTSTATUS wg_transform_get_status(void *args) DECLSPEC_HIDDEN; extern NTSTATUS wg_transform_drain(void *args) DECLSPEC_HIDDEN; extern NTSTATUS wg_transform_flush(void *args) DECLSPEC_HIDDEN; +/* wg_muxer.c */ + +extern NTSTATUS wg_muxer_create(void *args) DECLSPEC_HIDDEN; +extern NTSTATUS wg_muxer_destroy(void *args) DECLSPEC_HIDDEN; + /* wg_allocator.c */ static inline BYTE *wg_sample_data(struct wg_sample *sample) diff --git a/dlls/winegstreamer/unixlib.c b/dlls/winegstreamer/unixlib.c index 7e5ef34c4d7..eca2bb8aacc 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) @@ -139,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; } diff --git a/dlls/winegstreamer/unixlib.h b/dlls/winegstreamer/unixlib.h index 15e0605fdde..6fa5fb99039 100644 --- a/dlls/winegstreamer/unixlib.h +++ b/dlls/winegstreamer/unixlib.h @@ -166,6 +166,29 @@ 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, +}; + +typedef UINT32 wg_quicktime_variant; +enum wg_quicktime_variant +{ + WG_QUICKTIME_VARIANT_UNKNOWN = 0, + WG_QUICKTIME_VARIANT_APPLE, + WG_QUICKTIME_VARIANT_ISO, + WG_QUICKTIME_VARIANT_3GPP, +}; + +struct wg_container_format +{ + wg_container_type type; + wg_quicktime_variant quicktime_variant; +}; + enum wg_sample_flag { WG_SAMPLE_FLAG_INCOMPLETE = 1, @@ -209,6 +232,7 @@ enum wg_parser_type typedef UINT64 wg_parser_t; typedef UINT64 wg_parser_stream_t; typedef UINT64 wg_transform_t; +typedef UINT64 wg_muxer_t; struct wg_parser_create_params { @@ -365,6 +389,12 @@ struct wg_transform_get_status_params UINT32 accepts_input; }; +struct wg_muxer_create_params +{ + wg_muxer_t muxer; + const struct wg_container_format *format; +}; + enum unix_funcs { unix_wg_init_gstreamer, @@ -404,6 +434,9 @@ enum unix_funcs unix_wg_transform_get_status, unix_wg_transform_drain, unix_wg_transform_flush, + + unix_wg_muxer_create, + unix_wg_muxer_destroy, }; #endif /* __WINE_WINEGSTREAMER_UNIXLIB_H */ diff --git a/dlls/winegstreamer/wg_format.c b/dlls/winegstreamer/wg_format.c index 2353839bbc4..029c6cf8b22 100644 --- a/dlls/winegstreamer/wg_format.c +++ b/dlls/winegstreamer/wg_format.c @@ -767,3 +767,42 @@ bool wg_format_compare(const struct wg_format *a, const struct wg_format *b) assert(0); return false; } + +GstCaps *wg_container_format_to_caps(const struct wg_container_format *format) +{ + GstCaps *caps = NULL; + + switch (format->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"); + switch(format->quicktime_variant) + { + case WG_QUICKTIME_VARIANT_APPLE: + gst_caps_set_simple(caps, "variant", G_TYPE_STRING, "apple", NULL); + break; + case WG_QUICKTIME_VARIANT_ISO: + gst_caps_set_simple(caps, "variant", G_TYPE_STRING, "iso", NULL); + break; + case WG_QUICKTIME_VARIANT_3GPP: + gst_caps_set_simple(caps, "variant", G_TYPE_STRING, "3gpp", NULL); + break; + case WG_QUICKTIME_VARIANT_UNKNOWN: + default: + GST_WARNING("Quicktime variant %u not implemented.\n", format->type); + break; + } + break; + + case WG_CONTAINER_TYPE_UNKNOWN: + default: + GST_WARNING("Container type %u not implemented.\n", format->type); + break; + } + + return caps; +} diff --git a/dlls/winegstreamer/wg_muxer.c b/dlls/winegstreamer/wg_muxer.c new file mode 100644 index 00000000000..06045a3e6a3 --- /dev/null +++ b/dlls/winegstreamer/wg_muxer.c @@ -0,0 +1,141 @@ +/* + * GStreamer muxer backend + * + * Copyright 2023 Ziqing Hui for CodeWeavers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#if 0 +#pragma makedep unix +#endif + +#include "ntstatus.h" +#define WIN32_NO_STATUS +#include "winternl.h" + +#include "unix_private.h" + +struct wg_muxer +{ + GstElement *container, *muxer; + + GstPad *my_sink; + GstAtomicQueue *output_queue; + GstMapInfo map_info; + GstBuffer *buffer; + + pthread_mutex_t mutex; + pthread_cond_t cond; +}; + +static struct wg_muxer *get_muxer(wg_muxer_t muxer) +{ + return (struct wg_muxer *)(ULONG_PTR)muxer; +} + +static void muxer_free_buffer(struct wg_muxer *muxer) +{ + gst_buffer_unmap(muxer->buffer, &muxer->map_info); + gst_buffer_unref(muxer->buffer); + muxer->buffer = NULL; +} + +static void muxer_destroy(struct wg_muxer *muxer) +{ + pthread_cond_destroy(&muxer->cond); + pthread_mutex_destroy(&muxer->mutex); + if (muxer->buffer) + muxer_free_buffer(muxer); + if (muxer->output_queue) + gst_atomic_queue_unref(muxer->output_queue); + if (muxer->my_sink) + gst_object_unref(muxer->my_sink); + if (muxer->container) + { + gst_element_set_state(muxer->container, GST_STATE_NULL); + gst_object_unref(muxer->container); + } + free(muxer); +} + +NTSTATUS wg_muxer_create(void *args) +{ + struct wg_muxer_create_params *params = args; + GstPadTemplate *template = NULL; + GstCaps *sink_caps = NULL; + NTSTATUS status = E_FAIL; + struct wg_muxer *muxer; + + /* Create wg_muxer object. */ + if (!(muxer = calloc(1, sizeof(*muxer)))) + return E_OUTOFMEMORY; + pthread_mutex_init(&muxer->mutex, NULL); + pthread_cond_init(&muxer->cond, NULL); + if (!(muxer->container = gst_bin_new("wg_muxer"))) + goto out; + if (!(muxer->output_queue = gst_atomic_queue_new(8))) + goto out; + + /* Create sink pad. */ + if (!(sink_caps = wg_container_format_to_caps(params->format))) + goto out; + if (!(template = gst_pad_template_new("sink", GST_PAD_SINK, GST_PAD_ALWAYS, sink_caps))) + goto out; + muxer->my_sink = gst_pad_new_from_template(template, "wg_muxer_sink"); + if (!muxer->my_sink) + goto out; + gst_pad_set_element_private(muxer->my_sink, muxer); + + /* Create gstreamer muxer element. */ + if (!(muxer->muxer = find_element(GST_ELEMENT_FACTORY_TYPE_MUXER | GST_ELEMENT_FACTORY_TYPE_FORMATTER, + NULL, sink_caps))) + goto out; + if (!append_element(muxer->container, muxer->muxer, NULL, NULL)) + goto out; + + /* Link muxer to sink pad. */ + if (!link_element_to_sink(muxer->muxer, muxer->my_sink)) + goto out; + if (!gst_pad_set_active(muxer->my_sink, 1)) + goto out; + + /* Set to pause state. */ + gst_element_set_state(muxer->container, GST_STATE_PAUSED); + if (!gst_element_get_state(muxer->container, NULL, NULL, -1)) + goto out; + + gst_caps_unref(sink_caps); + + GST_INFO("Created winegstreamer muxer %p.", muxer); + params->muxer = (wg_transform_t)(ULONG_PTR)muxer; + + return S_OK; + +out: + if (sink_caps) + gst_caps_unref(sink_caps); + if (template) + gst_object_unref(template); + muxer_destroy(muxer); + return status; +} + +NTSTATUS wg_muxer_destroy(void *args) +{ + struct wg_muxer *muxer = get_muxer(*(wg_muxer_t *)args); + muxer_destroy(muxer); + return S_OK; +} diff --git a/dlls/winegstreamer/wg_parser.c b/dlls/winegstreamer/wg_parser.c index 5556b52829c..016691d448d 100644 --- a/dlls/winegstreamer/wg_parser.c +++ b/dlls/winegstreamer/wg_parser.c @@ -1945,6 +1945,9 @@ const unixlib_entry_t __wine_unix_call_funcs[] = X(wg_transform_get_status), X(wg_transform_drain), X(wg_transform_flush), + + X(wg_muxer_create), + X(wg_muxer_destroy), }; #ifdef _WIN64 @@ -2052,7 +2055,6 @@ static NTSTATUS wow64_wg_parser_stream_copy_buffer(void *args) return wg_parser_stream_copy_buffer(¶ms); } - static NTSTATUS wow64_wg_parser_stream_get_tag(void *args) { struct @@ -2152,6 +2154,24 @@ NTSTATUS wow64_wg_transform_read_data(void *args) return ret; } +NTSTATUS wow64_wg_muxer_create(void *args) +{ + struct + { + wg_muxer_t muxer; + PTR32 format; + } *params32 = args; + struct wg_muxer_create_params params = + { + .format = ULongToPtr(params32->format), + }; + NTSTATUS ret; + + ret = wg_muxer_create(¶ms); + params32->muxer = params.muxer; + return ret; +} + const unixlib_entry_t __wine_unix_call_wow64_funcs[] = { #define X64(name) [unix_ ## name] = wow64_ ## name @@ -2192,6 +2212,9 @@ const unixlib_entry_t __wine_unix_call_wow64_funcs[] = X(wg_transform_get_status), X(wg_transform_drain), X(wg_transform_flush), + + X64(wg_muxer_create), + X(wg_muxer_destroy), }; #endif /* _WIN64 */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3737
From: Ziqing Hui <zhui(a)codeweavers.com> This makes muxer element know that it can perform seeking. --- dlls/winegstreamer/wg_muxer.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/dlls/winegstreamer/wg_muxer.c b/dlls/winegstreamer/wg_muxer.c index 06045a3e6a3..67fa849cb10 100644 --- a/dlls/winegstreamer/wg_muxer.c +++ b/dlls/winegstreamer/wg_muxer.c @@ -71,6 +71,24 @@ static void muxer_destroy(struct wg_muxer *muxer) free(muxer); } +static gboolean muxer_sink_query_cb(GstPad *pad, GstObject *parent, GstQuery *query) +{ + struct wg_muxer *muxer = gst_pad_get_element_private(pad); + + GST_DEBUG("pad %p, parent %p, query %p, muxer %p, type \"%s\".", + pad, parent, query, muxer, gst_query_type_get_name(query->type)); + + switch (query->type) + { + case GST_QUERY_SEEKING: + gst_query_set_seeking(query, GST_FORMAT_BYTES, true, 0, -1); + return true; + default: + GST_WARNING("Ignoring \"%s\" query.", gst_query_type_get_name(query->type)); + return gst_pad_query_default(pad, parent, query); + } +} + NTSTATUS wg_muxer_create(void *args) { struct wg_muxer_create_params *params = args; @@ -98,6 +116,7 @@ NTSTATUS wg_muxer_create(void *args) if (!muxer->my_sink) goto out; gst_pad_set_element_private(muxer->my_sink, muxer); + gst_pad_set_query_function(muxer->my_sink, muxer_sink_query_cb); /* Create gstreamer muxer element. */ if (!(muxer->muxer = find_element(GST_ELEMENT_FACTORY_TYPE_MUXER | GST_ELEMENT_FACTORY_TYPE_FORMATTER, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3737
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/winegstreamer/gst_private.h | 3 +- dlls/winegstreamer/main.c | 7 ++-- dlls/winegstreamer/media_sink.c | 72 ++++++++++++++++++++++++++------ 3 files changed, 65 insertions(+), 17 deletions(-) diff --git a/dlls/winegstreamer/gst_private.h b/dlls/winegstreamer/gst_private.h index 0c814148447..243dab16c72 100644 --- a/dlls/winegstreamer/gst_private.h +++ b/dlls/winegstreamer/gst_private.h @@ -125,7 +125,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 e1ddf1e52f7..e4c93076eea 100644 --- a/dlls/winegstreamer/main.c +++ b/dlls/winegstreamer/main.c @@ -638,7 +638,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) { @@ -674,9 +675,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 = &sink_class_factory_cf; + 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..392ffb17bf9 100644 --- a/dlls/winegstreamer/media_sink.c +++ b/dlls/winegstreamer/media_sink.c @@ -72,11 +72,14 @@ struct media_sink STATE_PAUSED, STATE_SHUTDOWN, } state; + struct wg_container_format format; IMFByteStream *bytestream; IMFMediaEventQueue *event_queue; struct list stream_sinks; + + wg_muxer_t muxer; }; static struct stream_sink *impl_from_IMFStreamSink(IMFStreamSink *iface) @@ -567,6 +570,7 @@ static ULONG WINAPI media_sink_Release(IMFFinalizableMediaSink *iface) IMFByteStream_Release(media_sink->bytestream); media_sink->cs.DebugInfo->Spare[0] = 0; DeleteCriticalSection(&media_sink->cs); + wg_muxer_destroy(media_sink->muxer); free(media_sink); } @@ -1022,7 +1026,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, struct wg_container_format *format, struct media_sink **out) { struct media_sink *media_sink; HRESULT hr; @@ -1035,11 +1039,10 @@ static HRESULT media_sink_create(IMFByteStream *bytestream, struct media_sink ** if (!(media_sink = calloc(1, sizeof(*media_sink)))) return E_OUTOFMEMORY; + if (FAILED(hr = wg_muxer_create(format, &media_sink->muxer))) + goto fail; if (FAILED(hr = MFCreateEventQueue(&media_sink->event_queue))) - { - free(media_sink); - return hr; - } + goto fail; media_sink->IMFFinalizableMediaSink_iface.lpVtbl = &media_sink_vtbl; media_sink->IMFMediaEventGenerator_iface.lpVtbl = &media_sink_event_vtbl; @@ -1047,6 +1050,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->format = *format; InitializeCriticalSection(&media_sink->cs); media_sink->cs.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": cs"); IMFByteStream_AddRef((media_sink->bytestream = bytestream)); @@ -1056,6 +1060,14 @@ static HRESULT media_sink_create(IMFByteStream *bytestream, struct media_sink ** TRACE("Created media sink %p.\n", media_sink); return S_OK; + +fail: + if (media_sink->event_queue) + IMFMediaEventQueue_Release(media_sink->event_queue); + if (media_sink->muxer) + wg_muxer_destroy(media_sink->muxer); + free(media_sink); + return hr; } static HRESULT WINAPI sink_class_factory_QueryInterface(IMFSinkClassFactory *iface, REFIID riid, void **out) @@ -1082,8 +1094,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, + struct wg_container_format *format, IMFMediaType *video_type, IMFMediaType *audio_type, IMFMediaSink **out) { IMFFinalizableMediaSink *media_sink_iface; struct media_sink *media_sink; @@ -1092,7 +1104,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, format, &media_sink))) return hr; media_sink_iface = &media_sink->IMFFinalizableMediaSink_iface; @@ -1119,18 +1131,52 @@ 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) +{ + struct wg_container_format format; + + format.type = WG_CONTAINER_TYPE_ID3; + return sink_class_factory_create_media_sink(iface, bytestream, &format, 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) +{ + struct wg_container_format format; + + format.type = WG_CONTAINER_TYPE_QUICKTIME; + format.quicktime_variant = WG_QUICKTIME_VARIANT_ISO; + return sink_class_factory_create_media_sink(iface, bytestream, &format, 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, + mp3_sink_class_factory_CreateMediaSink, +}; + +static const IMFSinkClassFactoryVtbl mpeg4_sink_class_factory_vtbl = { sink_class_factory_QueryInterface, sink_class_factory_AddRef, sink_class_factory_Release, - sink_class_factory_CreateMediaSink, + mpeg4_sink_class_factory_CreateMediaSink, }; -static IMFSinkClassFactory sink_class_factory = { &sink_class_factory_vtbl }; +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; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3737
Rémi Bernon (@rbernon) commented about dlls/winegstreamer/unixlib.h:
+}; + +typedef UINT32 wg_quicktime_variant; +enum wg_quicktime_variant +{ + WG_QUICKTIME_VARIANT_UNKNOWN = 0, + WG_QUICKTIME_VARIANT_APPLE, + WG_QUICKTIME_VARIANT_ISO, + WG_QUICKTIME_VARIANT_3GPP, +}; + +struct wg_container_format +{ + wg_container_type type; + wg_quicktime_variant quicktime_variant; +}; What about passing the GStreamer caps string directly instead of requiring some intermediate enum and conversion?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3737#note_44527
Rémi Bernon (@rbernon) commented about dlls/winegstreamer/wg_muxer.c:
+static void muxer_destroy(struct wg_muxer *muxer) +{ + pthread_cond_destroy(&muxer->cond); + pthread_mutex_destroy(&muxer->mutex); + if (muxer->buffer) + muxer_free_buffer(muxer); + if (muxer->output_queue) + gst_atomic_queue_unref(muxer->output_queue); + if (muxer->my_sink) + gst_object_unref(muxer->my_sink); + if (muxer->container) + { + gst_element_set_state(muxer->container, GST_STATE_NULL); + gst_object_unref(muxer->container); + } + free(muxer); I think we prefer avoiding ifs in the destructors and keep that logic only in the constructor error path. I know this may feel like duplication but it also allow you to assume that once successfully constructed, none of these fields can be NULL.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3737#note_44531
Rémi Bernon (@rbernon) commented about dlls/winegstreamer/wg_muxer.c:
+#endif + +#include "ntstatus.h" +#define WIN32_NO_STATUS +#include "winternl.h" + +#include "unix_private.h" + +struct wg_muxer +{ + GstElement *container, *muxer; + + GstPad *my_sink; + GstAtomicQueue *output_queue; + GstMapInfo map_info; + GstBuffer *buffer; The buffer and map_info fields are currently not really used anywhere, better to delay adding them to when they will be needed.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3737#note_44529
Rémi Bernon (@rbernon) commented about dlls/winegstreamer/wg_muxer.c:
+ + pthread_mutex_t mutex; + pthread_cond_t cond; +}; + +static struct wg_muxer *get_muxer(wg_muxer_t muxer) +{ + return (struct wg_muxer *)(ULONG_PTR)muxer; +} + +static void muxer_free_buffer(struct wg_muxer *muxer) +{ + gst_buffer_unmap(muxer->buffer, &muxer->map_info); + gst_buffer_unref(muxer->buffer); + muxer->buffer = NULL; +} Same for this helper. Also I don't know how the buffer is used, but I believe it's a bad GStreamer practice to keep the buffers mapped. Ideally you'd only map it / unmap shortly when needed.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3737#note_44530
Rémi Bernon (@rbernon) commented about dlls/winegstreamer/unixlib.c:
!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; + if (last) *last = element; success = true; }
I'd prefer not allowing NULL, this will avoid mistakes breaking the list, in case of mismatched NULL / non-NULL calls. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3737#note_44528
Rémi Bernon (@rbernon) commented about dlls/winegstreamer/media_sink.c:
TRACE("Created media sink %p.\n", media_sink);
return S_OK; + +fail: + if (media_sink->event_queue) + IMFMediaEventQueue_Release(media_sink->event_queue);
This condition can never be true. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3737#note_44533
Rémi Bernon (@rbernon) commented about dlls/winegstreamer/wg_muxer.c:
+ + return S_OK; + +out: + if (sink_caps) + gst_caps_unref(sink_caps); + if (template) + gst_object_unref(template); + muxer_destroy(muxer); + return status; +} + +NTSTATUS wg_muxer_destroy(void *args) +{ + struct wg_muxer *muxer = get_muxer(*(wg_muxer_t *)args); + muxer_destroy(muxer); You could probably then merge the helper inside this one.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3737#note_44532
How are we using the output queue? It doesn't conceptually match how I'd naïvely expect a muxer to work. Can you give an overview of the output interface for wg_muxer? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3737#note_44589
On Fri Sep 8 01:32:39 2023 +0000, Zebediah Figura wrote:
How are we using the output queue? It doesn't conceptually match how I'd naïvely expect a muxer to work. Can you give an overview of the output interface for wg_muxer? See the 2 commits：
https://gitlab.winehq.org/wine/wine/-/merge_requests/3303/diffs?commit_id=d1... https://gitlab.winehq.org/wine/wine/-/merge_requests/3303/diffs?commit_id=54... The output interface looks like this: ``` gst_private.h: HRESULT wg_muxer_get_buffer(wg_muxer_t muxer, uint32_t *size, uint64_t *offset); HRESULT wg_muxer_copy_buffer(wg_muxer_t muxer, void *buffer, UINT32 size); void wg_muxer_free_buffer(wg_muxer_t muxer); ``` We will use them like this: (https://gitlab.winehq.org/wine/wine/-/merge_requests/3303/diffs?commit_id=54...) ``` media_sink.c static HRESULT media_sink_write_stream(struct media_sink *media_sink) { BYTE *data = NULL; ULONG written; QWORD offset; UINT32 size; HRESULT hr; while (SUCCEEDED(hr = wg_muxer_get_buffer(media_sink->muxer, &size, &offset))) { if (!(data = calloc(1, size))) return E_OUTOFMEMORY; if (FAILED(hr = wg_muxer_copy_buffer(media_sink->muxer, data, size))) { free(data); WARN("Failed to copy buffer, hr %#lx.", hr); return hr; } if ((offset != UINT64_MAX && FAILED(hr = IMFByteStream_SetCurrentPosition(media_sink->bytestream, offset))) || FAILED(hr = IMFByteStream_Write(media_sink->bytestream, data, size, &written))) { free(data); return hr; } free(data); wg_muxer_free_buffer(media_sink->muxer); } return S_OK; } ``` And we implement them like this: (https://gitlab.winehq.org/wine/wine/-/merge_requests/3303/diffs?commit_id=d1...) ``` wg_muxer.c: NTSTATUS wg_muxer_get_buffer(void *args) { struct wg_muxer_get_buffer_params *params = args; struct wg_muxer *muxer = get_muxer(params->muxer); if (!muxer->buffer) { if (!(muxer->buffer = gst_atomic_queue_pop(muxer->output_queue))) return MF_E_SINK_NO_SAMPLES_PROCESSED; if (!gst_buffer_map(muxer->buffer, &muxer->map_info, GST_MAP_READ)) { GST_ERROR("Failed to map buffer %p, dropped.", muxer->buffer); gst_buffer_unref(muxer->buffer); muxer->buffer = NULL; return E_FAIL; } } params->size = muxer->map_info.size; if (GST_BUFFER_OFFSET_IS_VALID(muxer->buffer)) params->offset = GST_BUFFER_OFFSET(muxer->buffer); return S_OK; } NTSTATUS wg_muxer_copy_buffer(void *args) { struct wg_muxer_copy_buffer_params *params = args; struct wg_muxer *muxer = get_muxer(params->muxer); if (!muxer->buffer) return MF_E_SINK_NO_SAMPLES_PROCESSED; if (params->size < muxer->map_info.size) return MF_E_BUFFERTOOSMALL; memcpy(params->buffer, muxer->map_info.data, muxer->map_info.size); return S_OK; } NTSTATUS wg_muxer_free_buffer(void *args) { struct wg_muxer *muxer = get_muxer(*(wg_muxer_t *)args); if (muxer->buffer) muxer_free_buffer(muxer); return S_OK; } ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3737#note_44649
On Thu Sep 7 08:53:11 2023 +0000, Rémi Bernon wrote:
What about passing the GStreamer caps string directly instead of requiring some intermediate enum and conversion? If we hold a `char *` in `wg_container_format`, it requires a pointer conversion from PE pointer to unix pointer when we are calling `wg_container_format_to_caps` in wg_muxer.c.
Or we hold a `char[]` array in the struct to avoid the pointer conversion. But I preffer the intermediate enum approach. What do you think? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3737#note_44654
On Fri Sep 8 02:58:50 2023 +0000, Ziqing Hui wrote:
If we hold a `char *` in `wg_container_format`, it requires a pointer conversion from PE pointer to unix pointer when we are calling `wg_container_format_to_caps` in wg_muxer.c. Or we hold a `char[]` array in the struct to avoid the pointer conversion. But I preffer the intermediate enum approach. What do you think? I don't know what you mean with pointer conversion but you have a wow64 thunk already anyway, so just do the conversion here.
I think the enum is just adding unnecessary abstraction and intermediate steps. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3737#note_44658
On Fri Sep 8 01:32:38 2023 +0000, Ziqing Hui wrote:
See the 2 commits： https://gitlab.winehq.org/wine/wine/-/merge_requests/3303/diffs?commit_id=d1... https://gitlab.winehq.org/wine/wine/-/merge_requests/3303/diffs?commit_id=54... The output interface looks like this: ``` gst_private.h: HRESULT wg_muxer_get_buffer(wg_muxer_t muxer, uint32_t *size, uint64_t *offset); HRESULT wg_muxer_copy_buffer(wg_muxer_t muxer, void *buffer, UINT32 size); void wg_muxer_free_buffer(wg_muxer_t muxer); ``` We will use them like this: (https://gitlab.winehq.org/wine/wine/-/merge_requests/3303/diffs?commit_id=54...) ``` media_sink.c static HRESULT media_sink_write_stream(struct media_sink *media_sink) { BYTE *data = NULL; ULONG written; QWORD offset; UINT32 size; HRESULT hr; while (SUCCEEDED(hr = wg_muxer_get_buffer(media_sink->muxer, &size, &offset))) { if (!(data = calloc(1, size))) return E_OUTOFMEMORY; if (FAILED(hr = wg_muxer_copy_buffer(media_sink->muxer, data, size))) { free(data); WARN("Failed to copy buffer, hr %#lx.", hr); return hr; } if ((offset != UINT64_MAX && FAILED(hr = IMFByteStream_SetCurrentPosition(media_sink->bytestream, offset))) || FAILED(hr = IMFByteStream_Write(media_sink->bytestream, data, size, &written))) { free(data); return hr; } free(data); wg_muxer_free_buffer(media_sink->muxer); } return S_OK; } ``` And we implement them like this: (https://gitlab.winehq.org/wine/wine/-/merge_requests/3303/diffs?commit_id=d1...) ``` wg_muxer.c: NTSTATUS wg_muxer_get_buffer(void *args) { struct wg_muxer_get_buffer_params *params = args; struct wg_muxer *muxer = get_muxer(params->muxer); if (!muxer->buffer) { if (!(muxer->buffer = gst_atomic_queue_pop(muxer->output_queue))) return MF_E_SINK_NO_SAMPLES_PROCESSED; if (!gst_buffer_map(muxer->buffer, &muxer->map_info, GST_MAP_READ)) { GST_ERROR("Failed to map buffer %p, dropped.", muxer->buffer); gst_buffer_unref(muxer->buffer); muxer->buffer = NULL; return E_FAIL; } } params->size = muxer->map_info.size; if (GST_BUFFER_OFFSET_IS_VALID(muxer->buffer)) params->offset = GST_BUFFER_OFFSET(muxer->buffer); return S_OK; } NTSTATUS wg_muxer_copy_buffer(void *args) { struct wg_muxer_copy_buffer_params *params = args; struct wg_muxer *muxer = get_muxer(params->muxer); if (!muxer->buffer) return MF_E_SINK_NO_SAMPLES_PROCESSED; if (params->size < muxer->map_info.size) return MF_E_BUFFERTOOSMALL; memcpy(params->buffer, muxer->map_info.data, muxer->map_info.size); return S_OK; } NTSTATUS wg_muxer_free_buffer(void *args) { struct wg_muxer *muxer = get_muxer(*(wg_muxer_t *)args); if (muxer->buffer) muxer_free_buffer(muxer); return S_OK; } ``` What about simply one wg_muxer_get_data where you would just read some requested number of bytes, or less, from the output buffer, popping additional buffers from the output queue if needed and if available?
I don't think there's any reason to express individual buffers to the PE side, as it's all written to a stream anyway. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3737#note_44659
participants (4)
-
Rémi Bernon -
Zebediah Figura (＠zfigura)
-
Ziqing Hui
-
Ziqing Hui (＠zhui)