[PATCH 0/3] MR3810: winegstreamer: Implement add_stream and push_sample for muxer.
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/winegstreamer/Makefile.in | 2 +- dlls/winegstreamer/gst_private.h | 1 + dlls/winegstreamer/main.c | 14 +++ dlls/winegstreamer/media_sink.c | 9 ++ dlls/winegstreamer/unix_private.h | 1 + dlls/winegstreamer/unixlib.h | 8 ++ dlls/winegstreamer/wg_muxer.c | 143 ++++++++++++++++++++++++++++++ dlls/winegstreamer/wg_parser.c | 19 ++++ 8 files changed, 196 insertions(+), 1 deletion(-) diff --git a/dlls/winegstreamer/Makefile.in b/dlls/winegstreamer/Makefile.in index f963dcea7f0..5b8e9fcab2c 100644 --- a/dlls/winegstreamer/Makefile.in +++ b/dlls/winegstreamer/Makefile.in @@ -3,7 +3,7 @@ UNIXLIB = winegstreamer.so IMPORTLIB = winegstreamer IMPORTS = strmbase ole32 oleaut32 msdmo DELAYIMPORTS = mfplat mf -UNIX_CFLAGS = $(GSTREAMER_CFLAGS) +UNIX_CFLAGS = $(GSTREAMER_CFLAGS) -Wno-deprecated-declarations UNIX_LIBS = $(GSTREAMER_LIBS) $(PTHREAD_LIBS) C_SRCS = \ diff --git a/dlls/winegstreamer/gst_private.h b/dlls/winegstreamer/gst_private.h index 8cfadd10bfc..23b59766d0c 100644 --- a/dlls/winegstreamer/gst_private.h +++ b/dlls/winegstreamer/gst_private.h @@ -112,6 +112,7 @@ HRESULT wg_transform_flush(wg_transform_t transform); HRESULT wg_muxer_create(const char *format, wg_muxer_t *muxer); void wg_muxer_destroy(wg_muxer_t muxer); +HRESULT wg_muxer_add_stream(wg_muxer_t muxer, UINT32 stream_id, const struct wg_format *format); unsigned int wg_format_get_max_size(const struct wg_format *format); diff --git a/dlls/winegstreamer/main.c b/dlls/winegstreamer/main.c index 853907e1825..9dca66100c9 100644 --- a/dlls/winegstreamer/main.c +++ b/dlls/winegstreamer/main.c @@ -482,6 +482,20 @@ void wg_muxer_destroy(wg_muxer_t muxer) WINE_UNIX_CALL(unix_wg_muxer_destroy, &muxer); } +HRESULT wg_muxer_add_stream(wg_muxer_t muxer, UINT32 stream_id, const struct wg_format *format) +{ + struct wg_muxer_add_stream_params params = + { + .muxer = muxer, + .stream_id = stream_id, + .format = format, + }; + + TRACE("muxer %#I64x, stream_id %u, format %p.\n", muxer, stream_id, format); + + return WINE_UNIX_CALL(unix_wg_muxer_add_stream, ¶ms); +} + #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/media_sink.c b/dlls/winegstreamer/media_sink.c index 6bd9fdcfb7d..344134d1633 100644 --- a/dlls/winegstreamer/media_sink.c +++ b/dlls/winegstreamer/media_sink.c @@ -588,6 +588,7 @@ static HRESULT WINAPI media_sink_AddStreamSink(IMFFinalizableMediaSink *iface, D { struct media_sink *media_sink = impl_from_IMFFinalizableMediaSink(iface); struct stream_sink *object; + struct wg_format format; HRESULT hr; TRACE("iface %p, stream_sink_id %#lx, media_type %p, stream_sink %p.\n", @@ -608,6 +609,14 @@ static HRESULT WINAPI media_sink_AddStreamSink(IMFFinalizableMediaSink *iface, D return hr; } + mf_media_type_to_wg_format(media_type, &format); + if (FAILED(hr = wg_muxer_add_stream(media_sink->muxer, stream_sink_id, &format))) + { + LeaveCriticalSection(&media_sink->cs); + IMFStreamSink_Release(&object->IMFStreamSink_iface); + return hr; + } + list_add_tail(&media_sink->stream_sinks, &object->entry); LeaveCriticalSection(&media_sink->cs); diff --git a/dlls/winegstreamer/unix_private.h b/dlls/winegstreamer/unix_private.h index 305d69c12a8..9627845b402 100644 --- a/dlls/winegstreamer/unix_private.h +++ b/dlls/winegstreamer/unix_private.h @@ -62,6 +62,7 @@ extern NTSTATUS wg_transform_flush(void *args) DECLSPEC_HIDDEN; extern NTSTATUS wg_muxer_create(void *args) DECLSPEC_HIDDEN; extern NTSTATUS wg_muxer_destroy(void *args) DECLSPEC_HIDDEN; +extern NTSTATUS wg_muxer_add_stream(void *args) DECLSPEC_HIDDEN; /* wg_allocator.c */ diff --git a/dlls/winegstreamer/unixlib.h b/dlls/winegstreamer/unixlib.h index a5015ef9b22..3b320448510 100644 --- a/dlls/winegstreamer/unixlib.h +++ b/dlls/winegstreamer/unixlib.h @@ -372,6 +372,13 @@ struct wg_muxer_create_params const char *format; }; +struct wg_muxer_add_stream_params +{ + wg_muxer_t muxer; + UINT32 stream_id; + const struct wg_format *format; +}; + enum unix_funcs { unix_wg_init_gstreamer, @@ -414,6 +421,7 @@ enum unix_funcs unix_wg_muxer_create, unix_wg_muxer_destroy, + unix_wg_muxer_add_stream, }; #endif /* __WINE_WINEGSTREAMER_UNIXLIB_H */ diff --git a/dlls/winegstreamer/wg_muxer.c b/dlls/winegstreamer/wg_muxer.c index 6887bc015ba..d33c978fe97 100644 --- a/dlls/winegstreamer/wg_muxer.c +++ b/dlls/winegstreamer/wg_muxer.c @@ -22,16 +22,33 @@ #pragma makedep unix #endif +#include <stdio.h> + #include "ntstatus.h" #define WIN32_NO_STATUS #include "winternl.h" #include "unix_private.h" +#include "wine/list.h" + struct wg_muxer { GstElement *container, *muxer; GstPad *my_sink; + struct list streams; +}; + +struct wg_muxer_stream +{ + struct wg_muxer *muxer; + struct wg_format format; + uint32_t id; + + GstPad *my_src; + GstSegment segment; + + struct list entry; }; static struct wg_muxer *get_muxer(wg_muxer_t muxer) @@ -39,6 +56,23 @@ static struct wg_muxer *get_muxer(wg_muxer_t muxer) return (struct wg_muxer *)(ULONG_PTR)muxer; } +static gboolean muxer_src_query_cb(GstPad *pad, GstObject *parent, GstQuery *query) +{ + struct wg_muxer_stream *stream = gst_pad_get_element_private(pad); + + GST_DEBUG("pad %p, parent %p, query %p, stream %u, type \"%s\".", + pad, parent, query, stream->id, gst_query_type_get_name(query->type)); + + switch (query->type) + { + case GST_QUERY_LATENCY: + gst_query_set_latency(query, true, 0, 0); + return true; + default: + return gst_pad_query_default(pad, parent, query); + } +} + static gboolean muxer_sink_query_cb(GstPad *pad, GstObject *parent, GstQuery *query) { struct wg_muxer *muxer = gst_pad_get_element_private(pad); @@ -69,6 +103,7 @@ NTSTATUS wg_muxer_create(void *args) /* Create wg_muxer object. */ if (!(muxer = calloc(1, sizeof(*muxer)))) return E_OUTOFMEMORY; + list_init(&muxer->streams); if (!(muxer->container = gst_bin_new("wg_muxer"))) goto out; @@ -132,7 +167,15 @@ out: NTSTATUS wg_muxer_destroy(void *args) { struct wg_muxer *muxer = get_muxer(*(wg_muxer_t *)args); + struct wg_muxer_stream *stream, *next; + LIST_FOR_EACH_ENTRY_SAFE(stream, next, &muxer->streams, struct wg_muxer_stream, entry) + { + list_remove(&stream->entry); + if (stream->my_src) + gst_object_unref(stream->my_src); + free(stream); + } gst_object_unref(muxer->my_sink); gst_element_set_state(muxer->container, GST_STATE_NULL); gst_object_unref(muxer->container); @@ -140,3 +183,103 @@ NTSTATUS wg_muxer_destroy(void *args) return S_OK; } + +NTSTATUS wg_muxer_add_stream(void *args) +{ + struct wg_muxer_add_stream_params *params = args; + struct wg_muxer *muxer = get_muxer(params->muxer); + GstPadTemplate *template = NULL; + const char *muxer_sink_pad_name; + struct wg_muxer_stream *stream; + GstPad *muxer_sink = NULL; + NTSTATUS status = E_FAIL; + GstCaps *src_caps = NULL; + char src_pad_name[32]; + int ret; + + /* Create stream object. */ + if (!(stream = calloc(1, sizeof(*stream)))) + return E_OUTOFMEMORY; + stream->muxer = muxer; + stream->format = *params->format; + stream->id = params->stream_id; + + /* Create src pad. */ + if (!(src_caps = wg_format_to_caps(params->format))) + goto out; + if (params->format->major_type == WG_MAJOR_TYPE_VIDEO_H264) + gst_caps_set_simple(src_caps, "stream-format", G_TYPE_STRING, "avc", NULL); + if (!(template = gst_pad_template_new("src", GST_PAD_SRC, GST_PAD_ALWAYS, src_caps))) + goto out; + sprintf(src_pad_name, "wg_muxer_src_%u", stream->id); + stream->my_src = gst_pad_new_from_template(template, src_pad_name); + if (!stream->my_src) + goto out; + gst_pad_set_element_private(stream->my_src, stream); + gst_pad_set_query_function(stream->my_src, muxer_src_query_cb); + + /* Request muxer sink pad. */ + switch (stream->format.major_type) + { + case WG_MAJOR_TYPE_VIDEO: + case WG_MAJOR_TYPE_VIDEO_CINEPAK: + case WG_MAJOR_TYPE_VIDEO_H264: + case WG_MAJOR_TYPE_VIDEO_WMV: + case WG_MAJOR_TYPE_VIDEO_INDEO: + muxer_sink_pad_name = "video_%u"; + break; + case WG_MAJOR_TYPE_AUDIO: + case WG_MAJOR_TYPE_AUDIO_MPEG1: + case WG_MAJOR_TYPE_AUDIO_MPEG4: + case WG_MAJOR_TYPE_AUDIO_WMA: + muxer_sink_pad_name = "audio_%u"; + break; + default: + goto out; + } + if (!(muxer_sink = gst_element_get_request_pad(muxer->muxer, muxer_sink_pad_name))) + goto out; + + /* Link src pad to muxer sink pad. */ + if ((ret = gst_pad_link(stream->my_src, muxer_sink)) < 0) + { + GST_ERROR("Failed to link %s to muxer, ret %d.\n", src_pad_name, ret); + goto out; + } + if (!gst_pad_set_active(stream->my_src, 1)) + goto out; + + /* Push events to prepare for streaming. */ + if (!push_event(stream->my_src, gst_event_new_stream_start(src_pad_name))) + goto out; + if (!push_event(stream->my_src, gst_event_new_caps(src_caps))) + goto out; + gst_segment_init(&stream->segment, GST_FORMAT_TIME); + if (!push_event(stream->my_src, gst_event_new_segment(&stream->segment))) + goto out; + + list_add_tail(&muxer->streams, &stream->entry); + + gst_object_unref(muxer_sink); + gst_caps_unref(src_caps); + gst_object_unref(template); + GST_INFO("Created winegstreamer muxer stream %p.", stream); + + return S_OK; + +out: + if (muxer_sink) + { + gst_element_release_request_pad(muxer->muxer, muxer_sink); + gst_object_unref(muxer_sink); + } + if (stream->my_src) + gst_object_unref(stream->my_src); + if (src_caps) + gst_caps_unref(src_caps); + if (template) + gst_object_unref(template); + free(stream); + + return status; +} diff --git a/dlls/winegstreamer/wg_parser.c b/dlls/winegstreamer/wg_parser.c index 016691d448d..1880b052680 100644 --- a/dlls/winegstreamer/wg_parser.c +++ b/dlls/winegstreamer/wg_parser.c @@ -1948,6 +1948,7 @@ const unixlib_entry_t __wine_unix_call_funcs[] = X(wg_muxer_create), X(wg_muxer_destroy), + X(wg_muxer_add_stream), }; #ifdef _WIN64 @@ -2172,6 +2173,23 @@ NTSTATUS wow64_wg_muxer_create(void *args) return ret; } +NTSTATUS wow64_wg_muxer_add_stream(void *args) +{ + struct + { + wg_muxer_t muxer; + DWORD stream_id; + PTR32 format; + } *params32 = args; + struct wg_muxer_add_stream_params params = + { + .muxer = params32->muxer, + .stream_id = params32->stream_id, + .format = ULongToPtr(params32->format), + }; + return wg_muxer_add_stream(¶ms); +} + const unixlib_entry_t __wine_unix_call_wow64_funcs[] = { #define X64(name) [unix_ ## name] = wow64_ ## name @@ -2215,6 +2233,7 @@ const unixlib_entry_t __wine_unix_call_wow64_funcs[] = X64(wg_muxer_create), X(wg_muxer_destroy), + X64(wg_muxer_add_stream), }; #endif /* _WIN64 */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3810
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/winegstreamer/gst_private.h | 1 + dlls/winegstreamer/main.c | 13 +++ dlls/winegstreamer/unix_private.h | 1 + dlls/winegstreamer/unixlib.h | 8 ++ dlls/winegstreamer/wg_muxer.c | 148 ++++++++++++++++++++++++++++++ dlls/winegstreamer/wg_parser.c | 17 ++++ 6 files changed, 188 insertions(+) diff --git a/dlls/winegstreamer/gst_private.h b/dlls/winegstreamer/gst_private.h index 23b59766d0c..ac573515444 100644 --- a/dlls/winegstreamer/gst_private.h +++ b/dlls/winegstreamer/gst_private.h @@ -113,6 +113,7 @@ HRESULT wg_transform_flush(wg_transform_t transform); HRESULT wg_muxer_create(const char *format, wg_muxer_t *muxer); void wg_muxer_destroy(wg_muxer_t muxer); HRESULT wg_muxer_add_stream(wg_muxer_t muxer, UINT32 stream_id, const struct wg_format *format); +HRESULT wg_muxer_push_sample(wg_muxer_t muxer, struct wg_sample *sample); unsigned int wg_format_get_max_size(const struct wg_format *format); diff --git a/dlls/winegstreamer/main.c b/dlls/winegstreamer/main.c index 9dca66100c9..d984ed05bb0 100644 --- a/dlls/winegstreamer/main.c +++ b/dlls/winegstreamer/main.c @@ -496,6 +496,19 @@ HRESULT wg_muxer_add_stream(wg_muxer_t muxer, UINT32 stream_id, const struct wg_ return WINE_UNIX_CALL(unix_wg_muxer_add_stream, ¶ms); } +HRESULT wg_muxer_push_sample(wg_muxer_t muxer, struct wg_sample *sample) +{ + struct wg_muxer_push_sample_params params = + { + .muxer = muxer, + .sample = sample, + }; + + TRACE("muxer %#I64x, sample %p.\n", muxer, sample); + + return WINE_UNIX_CALL(unix_wg_muxer_push_sample, ¶ms); +} + #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 9627845b402..0e9a67471c9 100644 --- a/dlls/winegstreamer/unix_private.h +++ b/dlls/winegstreamer/unix_private.h @@ -63,6 +63,7 @@ extern NTSTATUS wg_transform_flush(void *args) DECLSPEC_HIDDEN; extern NTSTATUS wg_muxer_create(void *args) DECLSPEC_HIDDEN; extern NTSTATUS wg_muxer_destroy(void *args) DECLSPEC_HIDDEN; extern NTSTATUS wg_muxer_add_stream(void *args) DECLSPEC_HIDDEN; +extern NTSTATUS wg_muxer_push_sample(void *args) DECLSPEC_HIDDEN; /* wg_allocator.c */ diff --git a/dlls/winegstreamer/unixlib.h b/dlls/winegstreamer/unixlib.h index 3b320448510..6cbcab81c40 100644 --- a/dlls/winegstreamer/unixlib.h +++ b/dlls/winegstreamer/unixlib.h @@ -185,6 +185,7 @@ struct wg_sample UINT32 max_size; UINT32 size; UINT64 data; /* pointer to user memory */ + UINT32 stream_id; }; struct wg_parser_buffer @@ -379,6 +380,12 @@ struct wg_muxer_add_stream_params const struct wg_format *format; }; +struct wg_muxer_push_sample_params +{ + wg_muxer_t muxer; + struct wg_sample *sample; +}; + enum unix_funcs { unix_wg_init_gstreamer, @@ -422,6 +429,7 @@ enum unix_funcs unix_wg_muxer_create, unix_wg_muxer_destroy, unix_wg_muxer_add_stream, + unix_wg_muxer_push_sample, }; #endif /* __WINE_WINEGSTREAMER_UNIXLIB_H */ diff --git a/dlls/winegstreamer/wg_muxer.c b/dlls/winegstreamer/wg_muxer.c index d33c978fe97..b1d8978767b 100644 --- a/dlls/winegstreamer/wg_muxer.c +++ b/dlls/winegstreamer/wg_muxer.c @@ -27,6 +27,7 @@ #include "ntstatus.h" #define WIN32_NO_STATUS #include "winternl.h" +#include "mferror.h" #include "unix_private.h" @@ -35,7 +36,13 @@ struct wg_muxer { GstElement *container, *muxer; + GstPad *my_sink; + GstAtomicQueue *output_queue; + + pthread_mutex_t mutex; + guint64 offset; + struct list streams; }; @@ -56,6 +63,26 @@ static struct wg_muxer *get_muxer(wg_muxer_t muxer) return (struct wg_muxer *)(ULONG_PTR)muxer; } +static void sample_free_notify(void *arg) +{ + struct wg_sample *sample = arg; + GST_DEBUG("Releasing wg_sample %p", sample); + InterlockedDecrement(&sample->refcount); +} + +static struct wg_muxer_stream *muxer_get_stream_by_id(struct wg_muxer *muxer, DWORD id) +{ + struct wg_muxer_stream *stream; + + LIST_FOR_EACH_ENTRY(stream, &muxer->streams, struct wg_muxer_stream, entry) + { + if (stream->id == id) + return stream; + } + + return NULL; +} + static gboolean muxer_src_query_cb(GstPad *pad, GstObject *parent, GstQuery *query) { struct wg_muxer_stream *stream = gst_pad_get_element_private(pad); @@ -91,6 +118,66 @@ static gboolean muxer_sink_query_cb(GstPad *pad, GstObject *parent, GstQuery *qu } } +static gboolean muxer_sink_event_cb(GstPad *pad, GstObject *parent, GstEvent *event) +{ + struct wg_muxer *muxer = gst_pad_get_element_private(pad); + const GstSegment *segment; + + GST_DEBUG("pad %p, parent %p, event %p, muxer %p, type \"%s\".", + pad, parent, event, muxer, GST_EVENT_TYPE_NAME(event)); + + switch (event->type) + { + case GST_EVENT_SEGMENT: + pthread_mutex_lock(&muxer->mutex); + + gst_event_parse_segment(event, &segment); + if (segment->format != GST_FORMAT_BYTES) + { + pthread_mutex_unlock(&muxer->mutex); + GST_FIXME("Unhandled segment format \"%s\".", gst_format_get_name(segment->format)); + break; + } + muxer->offset = segment->start; + + pthread_mutex_unlock(&muxer->mutex); + break; + + default: + GST_WARNING("Ignoring \"%s\" event.", GST_EVENT_TYPE_NAME(event)); + break; + } + + gst_event_unref(event); + return TRUE; +} + +static GstFlowReturn muxer_sink_chain_cb(GstPad *pad, GstObject *parent, GstBuffer *buffer) +{ + struct wg_muxer *muxer = gst_pad_get_element_private(pad); + + GST_DEBUG("pad %p, parent %p, buffer %p, muxer %p.", pad, parent, buffer, muxer); + + pthread_mutex_lock(&muxer->mutex); + + GST_BUFFER_OFFSET(buffer) = GST_BUFFER_OFFSET_NONE; + if (muxer->offset != GST_BUFFER_OFFSET_NONE) + { + GST_BUFFER_OFFSET(buffer) = muxer->offset; + muxer->offset = GST_BUFFER_OFFSET_NONE; + } + gst_atomic_queue_push(muxer->output_queue, buffer); + + GST_LOG("Pushed buffer %p to output queue %p, buffer size %" G_GSIZE_FORMAT ", " + "offset %" G_GUINT64_FORMAT ", %u buffers in queue now.", + buffer, muxer->output_queue, gst_buffer_get_size(buffer), GST_BUFFER_OFFSET(buffer), + gst_atomic_queue_length(muxer->output_queue)); + + pthread_mutex_unlock(&muxer->mutex); + + return GST_FLOW_OK; +} + NTSTATUS wg_muxer_create(void *args) { struct wg_muxer_create_params *params = args; @@ -104,8 +191,12 @@ NTSTATUS wg_muxer_create(void *args) if (!(muxer = calloc(1, sizeof(*muxer)))) return E_OUTOFMEMORY; list_init(&muxer->streams); + muxer->offset = GST_BUFFER_OFFSET_NONE; + pthread_mutex_init(&muxer->mutex, 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 = gst_caps_from_string(params->format))) @@ -120,6 +211,8 @@ NTSTATUS wg_muxer_create(void *args) goto out; gst_pad_set_element_private(muxer->my_sink, muxer); gst_pad_set_query_function(muxer->my_sink, muxer_sink_query_cb); + gst_pad_set_event_function(muxer->my_sink, muxer_sink_event_cb); + gst_pad_set_chain_function(muxer->my_sink, muxer_sink_chain_cb); /* Create gstreamer muxer element. */ if (!(muxer->muxer = find_element(GST_ELEMENT_FACTORY_TYPE_MUXER | GST_ELEMENT_FACTORY_TYPE_FORMATTER, @@ -154,11 +247,13 @@ out: gst_object_unref(template); if (sink_caps) gst_caps_unref(sink_caps); + gst_atomic_queue_unref(muxer->output_queue); if (muxer->container) { gst_element_set_state(muxer->container, GST_STATE_NULL); gst_object_unref(muxer->container); } + pthread_mutex_destroy(&muxer->mutex); free(muxer); return status; @@ -168,6 +263,7 @@ NTSTATUS wg_muxer_destroy(void *args) { struct wg_muxer *muxer = get_muxer(*(wg_muxer_t *)args); struct wg_muxer_stream *stream, *next; + GstBuffer *buffer; LIST_FOR_EACH_ENTRY_SAFE(stream, next, &muxer->streams, struct wg_muxer_stream, entry) { @@ -176,9 +272,17 @@ NTSTATUS wg_muxer_destroy(void *args) gst_object_unref(stream->my_src); free(stream); } + + while ((buffer = gst_atomic_queue_pop(muxer->output_queue))) + gst_buffer_unref(buffer); + gst_atomic_queue_unref(muxer->output_queue); + gst_object_unref(muxer->my_sink); gst_element_set_state(muxer->container, GST_STATE_NULL); gst_object_unref(muxer->container); + + pthread_mutex_destroy(&muxer->mutex); + free(muxer); return S_OK; @@ -283,3 +387,47 @@ out: return status; } + +NTSTATUS wg_muxer_push_sample(void *args) +{ + struct wg_muxer_push_sample_params *params = args; + struct wg_muxer *muxer = get_muxer(params->muxer); + struct wg_sample *sample = params->sample; + struct wg_muxer_stream *stream; + GstFlowReturn ret; + GstBuffer *buffer; + + if (!(stream = muxer_get_stream_by_id(muxer, sample->stream_id))) + return MF_E_INVALIDSTREAMNUMBER; + + /* Create sample data buffer by wrapping wg_sample. */ + if (!(buffer = gst_buffer_new_wrapped_full(GST_MEMORY_FLAG_READONLY, wg_sample_data(sample), sample->max_size, + 0, sample->size, sample, sample_free_notify))) + { + GST_ERROR("Failed to allocate input buffer."); + return E_OUTOFMEMORY; + } + + InterlockedIncrement(&sample->refcount); + GST_INFO("Wrapped %u/%u bytes from sample %p to buffer %p.", sample->size, sample->max_size, sample, buffer); + + /* Set sample properties. */ + if (sample->flags & WG_SAMPLE_FLAG_HAS_PTS) + GST_BUFFER_PTS(buffer) = sample->pts * 100; + if (sample->flags & WG_SAMPLE_FLAG_HAS_DURATION) + GST_BUFFER_DURATION(buffer) = sample->duration * 100; + if (!(sample->flags & WG_SAMPLE_FLAG_SYNC_POINT)) + GST_BUFFER_FLAG_SET(buffer, GST_BUFFER_FLAG_DELTA_UNIT); + if (sample->flags & WG_SAMPLE_FLAG_DISCONTINUITY) + GST_BUFFER_FLAG_SET(buffer, GST_BUFFER_FLAG_DISCONT); + + /* Push sample data buffer to stream src pad. */ + if ((ret = gst_pad_push(stream->my_src, buffer)) < 0) + { + GST_ERROR("Failed to push buffer %p to pad %s, reason %s.", + buffer, gst_pad_get_name(stream->my_src), gst_flow_get_name(ret)); + return E_FAIL; + } + + return S_OK; +} diff --git a/dlls/winegstreamer/wg_parser.c b/dlls/winegstreamer/wg_parser.c index 1880b052680..30b30b405b2 100644 --- a/dlls/winegstreamer/wg_parser.c +++ b/dlls/winegstreamer/wg_parser.c @@ -1949,6 +1949,7 @@ const unixlib_entry_t __wine_unix_call_funcs[] = X(wg_muxer_create), X(wg_muxer_destroy), X(wg_muxer_add_stream), + X(wg_muxer_push_sample), }; #ifdef _WIN64 @@ -2190,6 +2191,21 @@ NTSTATUS wow64_wg_muxer_add_stream(void *args) return wg_muxer_add_stream(¶ms); } +NTSTATUS wow64_wg_muxer_push_sample(void *args) +{ + struct + { + wg_muxer_t muxer; + PTR32 sample; + } *params32 = args; + struct wg_muxer_push_sample_params params = + { + .muxer = params32->muxer, + .sample = ULongToPtr(params32->sample), + }; + return wg_muxer_push_sample(¶ms); +} + const unixlib_entry_t __wine_unix_call_wow64_funcs[] = { #define X64(name) [unix_ ## name] = wow64_ ## name @@ -2234,6 +2250,7 @@ const unixlib_entry_t __wine_unix_call_wow64_funcs[] = X64(wg_muxer_create), X(wg_muxer_destroy), X64(wg_muxer_add_stream), + X64(wg_muxer_push_sample), }; #endif /* _WIN64 */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3810
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/winegstreamer/media_sink.c | 85 ++++++++++++++++++++++++++++++++- 1 file changed, 83 insertions(+), 2 deletions(-) diff --git a/dlls/winegstreamer/media_sink.c b/dlls/winegstreamer/media_sink.c index 344134d1633..c153fb1f640 100644 --- a/dlls/winegstreamer/media_sink.c +++ b/dlls/winegstreamer/media_sink.c @@ -31,6 +31,7 @@ enum async_op ASYNC_START, ASYNC_STOP, ASYNC_PAUSE, + ASYNC_PROCESS, }; struct async_command @@ -39,6 +40,15 @@ struct async_command LONG refcount; enum async_op op; + + union + { + struct + { + IMFSample *sample; + UINT32 stream_id; + } process; + } u; }; struct stream_sink @@ -142,7 +152,11 @@ static ULONG WINAPI async_command_Release(IUnknown *iface) ULONG refcount = InterlockedDecrement(&command->refcount); if (!refcount) + { + if (command->op == ASYNC_PROCESS && command->u.process.sample) + IMFSample_Release(command->u.process.sample); free(command); + } return refcount; } @@ -301,9 +315,40 @@ static HRESULT WINAPI stream_sink_GetMediaTypeHandler(IMFStreamSink *iface, IMFM static HRESULT WINAPI stream_sink_ProcessSample(IMFStreamSink *iface, IMFSample *sample) { - FIXME("iface %p, sample %p stub!\n", iface, sample); + struct stream_sink *stream_sink = impl_from_IMFStreamSink(iface); + struct media_sink *media_sink = impl_from_IMFFinalizableMediaSink(stream_sink->media_sink); + struct async_command *command; + HRESULT hr; - return E_NOTIMPL; + TRACE("iface %p, sample %p.\n", iface, sample); + + EnterCriticalSection(&media_sink->cs); + + if (media_sink->state == STATE_SHUTDOWN) + { + LeaveCriticalSection(&media_sink->cs); + return MF_E_SHUTDOWN; + } + + if (media_sink->state != STATE_STARTED && media_sink->state != STATE_PAUSED) + { + LeaveCriticalSection(&media_sink->cs); + return MF_E_INVALIDREQUEST; + } + + if (FAILED(hr = (async_command_create(ASYNC_PROCESS, &command)))) + { + LeaveCriticalSection(&media_sink->cs); + return hr; + } + IMFSample_AddRef((command->u.process.sample = sample)); + command->u.process.stream_id = stream_sink->id; + + hr = MFPutWorkItem(MFASYNC_CALLBACK_QUEUE_STANDARD, &media_sink->async_callback, &command->IUnknown_iface); + + LeaveCriticalSection(&media_sink->cs); + + return hr; } static HRESULT WINAPI stream_sink_PlaceMarker(IMFStreamSink *iface, MFSTREAMSINK_MARKER_TYPE marker_type, @@ -515,6 +560,38 @@ static HRESULT media_sink_pause(struct media_sink *media_sink) return media_sink_queue_stream_event(media_sink, MEStreamSinkPaused); } +static HRESULT media_sink_process(struct media_sink *media_sink, IMFSample *sample, UINT32 stream_id) +{ + wg_muxer_t muxer = media_sink->muxer; + struct wg_sample *wg_sample; + LONGLONG time, duration; + UINT32 value; + HRESULT hr; + + TRACE("media_sink %p, sample %p, stream_id %u.\n", media_sink, sample, stream_id); + + if (FAILED(hr = wg_sample_create_mf(sample, &wg_sample))) + return hr; + wg_sample->stream_id = stream_id; + + if (SUCCEEDED(IMFSample_GetSampleTime(sample, &time))) + { + wg_sample->flags |= WG_SAMPLE_FLAG_HAS_PTS; + wg_sample->pts = time; + } + if (SUCCEEDED(IMFSample_GetSampleDuration(sample, &duration))) + { + wg_sample->flags |= WG_SAMPLE_FLAG_HAS_DURATION; + wg_sample->duration = duration; + } + if (SUCCEEDED(IMFSample_GetUINT32(sample, &MFSampleExtension_CleanPoint, &value)) && value) + wg_sample->flags |= WG_SAMPLE_FLAG_SYNC_POINT; + if (SUCCEEDED(IMFSample_GetUINT32(sample, &MFSampleExtension_Discontinuity, &value)) && value) + wg_sample->flags |= WG_SAMPLE_FLAG_DISCONTINUITY; + + return wg_muxer_push_sample(muxer, wg_sample); +} + static HRESULT WINAPI media_sink_QueryInterface(IMFFinalizableMediaSink *iface, REFIID riid, void **obj) { struct media_sink *media_sink = impl_from_IMFFinalizableMediaSink(iface); @@ -1015,6 +1092,10 @@ static HRESULT WINAPI media_sink_callback_Invoke(IMFAsyncCallback *iface, IMFAsy case ASYNC_PAUSE: hr = media_sink_pause(media_sink); break; + case ASYNC_PROCESS: + if (FAILED(hr = media_sink_process(media_sink, command->u.process.sample, command->u.process.stream_id))) + WARN("Failed to process sample, hr %#lx.\n", hr); + break; default: WARN("Unsupported op %u.\n", command->op); break; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3810
Rémi Bernon (@rbernon) commented about dlls/winegstreamer/Makefile.in:
IMPORTLIB = winegstreamer IMPORTS = strmbase ole32 oleaut32 msdmo DELAYIMPORTS = mfplat mf -UNIX_CFLAGS = $(GSTREAMER_CFLAGS) +UNIX_CFLAGS = $(GSTREAMER_CFLAGS) -Wno-deprecated-declarations Maybe we can avoid this, with something like that somewhere on the top of the file?
``` #if !GST_CHECK_VERSION(1,20,0) #define gst_element_request_pad_simple gst_element_get_request_pad #endif ``` And use `gst_element_request_pad_simple` instead of `gst_element_get_request_pad`? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45155
Rémi Bernon (@rbernon) commented about dlls/winegstreamer/main.c:
WINE_UNIX_CALL(unix_wg_muxer_destroy, &muxer); }
+HRESULT wg_muxer_add_stream(wg_muxer_t muxer, UINT32 stream_id, const struct wg_format *format) +{ + struct wg_muxer_add_stream_params params = + { + .muxer = muxer, + .stream_id = stream_id, + .format = format, + }; + + TRACE("muxer %#I64x, stream_id %u, format %p.\n", muxer, stream_id, format); + + return WINE_UNIX_CALL(unix_wg_muxer_add_stream, ¶ms);
I missed it in `wg_muxer_created` but I don't think we should cast NSTATUS as HRESULT implicitly. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45156
Rémi Bernon (@rbernon) commented about dlls/winegstreamer/wg_muxer.c:
+ GstCaps *src_caps = NULL; + char src_pad_name[32]; + int ret; + + /* Create stream object. */ + if (!(stream = calloc(1, sizeof(*stream)))) + return E_OUTOFMEMORY; + stream->muxer = muxer; + stream->format = *params->format; + stream->id = params->stream_id; + + /* Create src pad. */ + if (!(src_caps = wg_format_to_caps(params->format))) + goto out; + if (params->format->major_type == WG_MAJOR_TYPE_VIDEO_H264) + gst_caps_set_simple(src_caps, "stream-format", G_TYPE_STRING, "avc", NULL); Does this belong here? Should it be in our `wg_format_to_caps` instead? I don't really know what these H264 "stream-format" / "alignment" meant, but I believe @bshanks said something about it on macOS (vtdec only accepting avc stream-format).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45157
Rémi Bernon (@rbernon) commented about dlls/winegstreamer/wg_muxer.c:
+ case WG_MAJOR_TYPE_VIDEO: + case WG_MAJOR_TYPE_VIDEO_CINEPAK: + case WG_MAJOR_TYPE_VIDEO_H264: + case WG_MAJOR_TYPE_VIDEO_WMV: + case WG_MAJOR_TYPE_VIDEO_INDEO: + muxer_sink_pad_name = "video_%u"; + break; + case WG_MAJOR_TYPE_AUDIO: + case WG_MAJOR_TYPE_AUDIO_MPEG1: + case WG_MAJOR_TYPE_AUDIO_MPEG4: + case WG_MAJOR_TYPE_AUDIO_WMA: + muxer_sink_pad_name = "audio_%u"; + break; + default: + goto out; + } I think you can use `stream_type_from_caps` instead of another switch.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45158
Rémi Bernon (@rbernon) commented about dlls/winegstreamer/main.c:
return WINE_UNIX_CALL(unix_wg_muxer_add_stream, ¶ms); }
+HRESULT wg_muxer_push_sample(wg_muxer_t muxer, struct wg_sample *sample) +{ + struct wg_muxer_push_sample_params params = + { + .muxer = muxer, + .sample = sample, + }; + + TRACE("muxer %#I64x, sample %p.\n", muxer, sample); + + return WINE_UNIX_CALL(unix_wg_muxer_push_sample, ¶ms);
Same thing here regarding NTSTATUS. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45159
Rémi Bernon (@rbernon) commented about dlls/winegstreamer/gst_private.h:
HRESULT wg_muxer_create(const char *format, wg_muxer_t *muxer); void wg_muxer_destroy(wg_muxer_t muxer); HRESULT wg_muxer_add_stream(wg_muxer_t muxer, UINT32 stream_id, const struct wg_format *format); +HRESULT wg_muxer_push_sample(wg_muxer_t muxer, struct wg_sample *sample); The function is not used anywhere, and it is then basically dead code in this commit. Please make some code use it first, and then implement it.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45160
Rémi Bernon (@rbernon) commented about dlls/winegstreamer/unixlib.h:
UINT32 max_size; UINT32 size; UINT64 data; /* pointer to user memory */ + UINT32 stream_id;
I'm not sure it's generally useful in the sample, maybe it could be a `wg_muxer_push_sample` parameter instead? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45161
Rémi Bernon (@rbernon) commented about dlls/winegstreamer/wg_muxer.c:
+ + gst_event_parse_segment(event, &segment); + if (segment->format != GST_FORMAT_BYTES) + { + pthread_mutex_unlock(&muxer->mutex); + GST_FIXME("Unhandled segment format \"%s\".", gst_format_get_name(segment->format)); + break; + } + muxer->offset = segment->start; + + pthread_mutex_unlock(&muxer->mutex); + break; + + default: + GST_WARNING("Ignoring \"%s\" event.", GST_EVENT_TYPE_NAME(event)); + break; This seems unrelated to the buffers being pushed, and instead something that will be triggered by pushing the segment? Maybe it should be split?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45162
Rémi Bernon (@rbernon) commented about dlls/winegstreamer/wg_muxer.c:
+ GstFlowReturn ret; + GstBuffer *buffer; + + if (!(stream = muxer_get_stream_by_id(muxer, sample->stream_id))) + return MF_E_INVALIDSTREAMNUMBER; + + /* Create sample data buffer by wrapping wg_sample. */ + if (!(buffer = gst_buffer_new_wrapped_full(GST_MEMORY_FLAG_READONLY, wg_sample_data(sample), sample->max_size, + 0, sample->size, sample, sample_free_notify))) + { + GST_ERROR("Failed to allocate input buffer."); + return E_OUTOFMEMORY; + } + + InterlockedIncrement(&sample->refcount); + GST_INFO("Wrapped %u/%u bytes from sample %p to buffer %p.", sample->size, sample->max_size, sample, buffer); I don't think you should use zero-copy here, especially as in the next change you don't have a sample queue to garbage collect the released samples.
This is also likely going to use compressed samples only, so probably not very large, and I think copying data should be fine. You can do it with `gst_buffer_new_and_alloc` then `gst_buffer_fill`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45163
Rémi Bernon (@rbernon) commented about dlls/winegstreamer/media_sink.c:
+ + if (media_sink->state != STATE_STARTED && media_sink->state != STATE_PAUSED) + { + LeaveCriticalSection(&media_sink->cs); + return MF_E_INVALIDREQUEST; + } + + if (FAILED(hr = (async_command_create(ASYNC_PROCESS, &command)))) + { + LeaveCriticalSection(&media_sink->cs); + return hr; + } + IMFSample_AddRef((command->u.process.sample = sample)); + command->u.process.stream_id = stream_sink->id; + + hr = MFPutWorkItem(MFASYNC_CALLBACK_QUEUE_STANDARD, &media_sink->async_callback, &command->IUnknown_iface); You're leaking the command if `MFPutWorkItem` fails.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45164
Rémi Bernon (@rbernon) commented about dlls/winegstreamer/media_sink.c:
+ if (SUCCEEDED(IMFSample_GetSampleTime(sample, &time))) + { + wg_sample->flags |= WG_SAMPLE_FLAG_HAS_PTS; + wg_sample->pts = time; + } + if (SUCCEEDED(IMFSample_GetSampleDuration(sample, &duration))) + { + wg_sample->flags |= WG_SAMPLE_FLAG_HAS_DURATION; + wg_sample->duration = duration; + } + if (SUCCEEDED(IMFSample_GetUINT32(sample, &MFSampleExtension_CleanPoint, &value)) && value) + wg_sample->flags |= WG_SAMPLE_FLAG_SYNC_POINT; + if (SUCCEEDED(IMFSample_GetUINT32(sample, &MFSampleExtension_Discontinuity, &value)) && value) + wg_sample->flags |= WG_SAMPLE_FLAG_DISCONTINUITY; + + return wg_muxer_push_sample(muxer, wg_sample); Without a wg_sample_queue you need to release the wg_sample here, though you also need to remove zero-copy from the unix side as will complain otherwise.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45165
On Wed Sep 13 07:46:26 2023 +0000, Rémi Bernon wrote:
The function is not used anywhere, and it is then basically dead code in this commit. Please make some code use it first, and then implement it. Maybe exchange the order of PATCH 2 and PATCH 3?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45192
On Wed Sep 13 07:46:24 2023 +0000, Rémi Bernon wrote:
Does this belong here? Should it be in our `wg_format_to_caps` instead? I don't really know what these H264 "stream-format" / "alignment" meant, but I believe @bshanks said something about it on macOS (vtdec only accepting avc stream-format). This line is added because mp4mux element only accpet h264 stream with "stream-format" be set to "avc" or "avc3".
``` gst-inspect-1.0 mp4mux: [...] SINK template: 'video_%u' Availability: On request Capabilities: video/mpeg mpegversion: 4 systemstream: false width: [ 16, 2147483647 ] height: [ 16, 2147483647 ] video/x-divx divxversion: 5 width: [ 16, 2147483647 ] height: [ 16, 2147483647 ] video/x-h264 stream-format: { (string)avc, (string)avc3 } alignment: au width: [ 16, 2147483647 ] height: [ 16, 2147483647 ] video/x-h265 stream-format: { (string)hvc1, (string)hev1 } alignment: au width: [ 16, 2147483647 ] height: [ 16, 2147483647 ] video/x-mp4-part width: [ 16, 2147483647 ] height: [ 16, 2147483647 ] video/x-av1 width: [ 16, 2147483647 ] height: [ 16, 2147483647 ] [...] ``` Should this be in wg_format_to_caps_video_h264()? Currently wg_format_to_caps_video_h264 set "stream-format" to "byte-stream". Is it OK to change the "byte-stream" to "avc" in wg_format_to_caps_video_h264()? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45196
On Wed Sep 13 07:46:23 2023 +0000, Rémi Bernon wrote:
I missed it in `wg_muxer_created` but I don't think we should cast NSTATUS as HRESULT implicitly. OK, should I use HRESULT_FROM_NT in PE api side?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45199
On Wed Sep 13 10:36:37 2023 +0000, Ziqing Hui wrote:
OK, should I use HRESULT_FROM_NT in PE api side? I think it'd be better, but then you shouldn't return HRESULT values from the unix side. I'm not sure if these specific values are supposed to be meaningful but if it's just something like E_FAIL / E_OUTOFMEMORY you can replace them with their STATUS_* equivalent.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45205
On Wed Sep 13 10:34:46 2023 +0000, Ziqing Hui wrote:
This line is added because mp4mux element only accpet h264 stream with "stream-format" be set to "avc" or "avc3". ``` gst-inspect-1.0 mp4mux: [...] SINK template: 'video_%u' Availability: On request Capabilities: video/mpeg mpegversion: 4 systemstream: false width: [ 16, 2147483647 ] height: [ 16, 2147483647 ] video/x-divx divxversion: 5 width: [ 16, 2147483647 ] height: [ 16, 2147483647 ] video/x-h264 stream-format: { (string)avc, (string)avc3 } alignment: au width: [ 16, 2147483647 ] height: [ 16, 2147483647 ] video/x-h265 stream-format: { (string)hvc1, (string)hev1 } alignment: au width: [ 16, 2147483647 ] height: [ 16, 2147483647 ] video/x-mp4-part width: [ 16, 2147483647 ] height: [ 16, 2147483647 ] video/x-av1 width: [ 16, 2147483647 ] height: [ 16, 2147483647 ] [...] ``` Should this be in wg_format_to_caps_video_h264()? Currently wg_format_to_caps_video_h264 set "stream-format" to "byte-stream". Is it OK to change the "byte-stream" to "avc" in wg_format_to_caps_video_h264()? I have no idea whether that's right. Maybe it'd be better to figure the exact meaning, but as long as it doesn't break the H264 tests I'm probably fine with it.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45206
On Wed Sep 13 10:28:22 2023 +0000, Ziqing Hui wrote:
Maybe exchange the order of PATCH 2 and PATCH 3? Something like that, yeah.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45207
On Wed Sep 13 10:39:44 2023 +0000, Rémi Bernon wrote:
I think it'd be better, but then you shouldn't return HRESULT values from the unix side. I'm not sure if these specific values are supposed to be meaningful but if it's just something like E_FAIL / E_OUTOFMEMORY you can replace them with their STATUS_* equivalent. What's the problem with casting NTSTATUS to HRESULT?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45253
On Wed Sep 13 19:44:14 2023 +0000, Zebediah Figura wrote:
What's the problem with casting NTSTATUS to HRESULT? It's not the same type and the constants are different.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45254
On Wed Sep 13 07:46:22 2023 +0000, Rémi Bernon wrote:
Maybe we can avoid this, with something like that somewhere on the top of the file? ``` #if !GST_CHECK_VERSION(1,20,0) #define gst_element_request_pad_simple gst_element_get_request_pad #endif ``` And use `gst_element_request_pad_simple` instead of `gst_element_get_request_pad`? I don't really have an opinion on this, but if we do use -Wno-deprecated-declarations, it should probably go in the top-level configure, and we should check for compiler support like other -W options instead of assuming it's present.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45284
Should this be in wg_format_to_caps_video_h264()? Currently wg_format_to_caps_video_h264 set "stream-format" to "byte-stream". Is it OK to change the "byte-stream" to "avc" in wg_format_to_caps_video_h264()?
Not per se, no. According to my understanding, there are two variants of H.264 around (well, actually three, but let's set aside AVC3 for the moment). One is Annex B, which GStreamer calls "byte-stream", and which uses start codes (a fixed sequence of three bytes that identify the start of a packet). The other seems to be usually referred to as "AVC"; it does not use start codes, but instead prefixes packets with their length. Obviously the two are not compatible, but fortunately there is a very simple way to convert between them: prepend an h264parse element. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45285
This seems unrelated to the buffers being pushed, and instead something that will be triggered by pushing the segment?
No, this is correct; SEGMENT events are how elements perform random access on a sink. I'd argue that PTS would have been a more intuitive way to do this, but it is what it is. I think we should be using the "position" field of the segment rather than "start", although I don't know. The documentation is not clear, and the only real example (filesink/fdsink) uses "start" but with a FIXME saying it should use "position" instead. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45286
On Wed Sep 13 22:54:42 2023 +0000, Zebediah Figura wrote:
This seems unrelated to the buffers being pushed, and instead something that will be triggered by pushing the segment? No, this is correct; SEGMENT events are how elements perform random access on a sink. I'd argue that PTS would have been a more intuitive way to do this, but it is what it is. I think we should be using the "position" field of the segment rather than "start", although I don't know. The documentation is not clear, and the only real example (filesink/fdsink) uses "start" but with a FIXME saying it should use "position" instead. Like zf said, we have to record the random access positon of buffer beging pushed, and SEGMENT event is the one who sets the position.
I think we should be using the "position" field of the segment rather than "start", although I don't know. The documentation is not clear, and the only real example (filesink/fdsink) uses "start" but with a FIXME saying it should use "position" instead.
I made some test, the result that only "start" filed is holding the valid postion value, "positon" filed is 0, so have to use "start" here. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45305
On Wed Sep 13 22:54:42 2023 +0000, Zebediah Figura wrote:
I don't really have an opinion on this, but if we do use -Wno-deprecated-declarations, it should probably go in the top-level configure, and we should check for compiler support like other -W options instead of assuming it's present. I prefer the `GST_CHECK_VERSION` way which is simpler I think.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45306
On Wed Sep 13 22:54:42 2023 +0000, Zebediah Figura wrote:
Should this be in wg_format_to_caps_video_h264()? Currently wg_format_to_caps_video_h264 set "stream-format" to "byte-stream". Is it OK to change the "byte-stream" to "avc" in wg_format_to_caps_video_h264()? Not per se, no. According to my understanding, there are two variants of H.264 around (well, actually three, but let's set aside AVC3 for the moment). One is Annex B, which GStreamer calls "byte-stream", and which uses start codes (a fixed sequence of three bytes that identify the start of a packet). The other seems to be usually referred to as "AVC"; it does not use start codes, but instead prefixes packets with their length. Obviously the two are not compatible, but fortunately there is a very simple way to convert between them: prepend an h264parse element. So we should add a h264parse here?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45454
On Fri Sep 15 02:01:01 2023 +0000, Ziqing Hui wrote:
So we should add a h264parse here? I don't think so, probably rather change the stream-type to avc. The MFT does not expect start codes, and instead takes individual packets.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45463
On Fri Sep 15 06:38:00 2023 +0000, Rémi Bernon wrote:
I don't think so, probably rather change the stream-type to avc. The MFT does not expect start codes, and instead takes individual packets. The H264 samples we pass to MFTransform in mf/test/transform.c[1] are containing start code. Does it matters?
[1] https://gitlab.winehq.org/wine/wine/-/blob/master/dlls/mf/tests/transform.c?... -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45464
On Fri Sep 15 06:50:15 2023 +0000, Ziqing Hui wrote:
The H264 samples we pass to MFTransform in mf/test/transform.c[1] are containing start code. Does it matters? [1] https://gitlab.winehq.org/wine/wine/-/blob/master/dlls/mf/tests/transform.c?... Hmm... you're right maybe the MFT can handle either format somehow then, would be better to check what the games are actually doing in this case. Call of Duty Black Ops 3, Shadow Warrior 2 for instance are using the MFT decoder directly.
There's also the MF_NALU_LENGTH_SET and MF_NALU_LENGTH_SET attributes (https://learn.microsoft.com/en-us/windows/win32/medfound/mf-nalu-length-set) which maybe indicate that the stream is in a format or another. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45469
On Fri Sep 15 07:05:08 2023 +0000, Rémi Bernon wrote:
Hmm... you're right maybe the MFT can handle either format somehow then, would be better to check what the games are actually doing in this case. Call of Duty Black Ops 3, Shadow Warrior 2 for instance are using the MFT decoder directly. There's also the MF_NALU_LENGTH_SET and MF_NALU_LENGTH_INFORMATION attributes (https://learn.microsoft.com/en-us/windows/win32/medfound/mf-nalu-length-set) which maybe indicate that the stream is in a format or another. OK, so the conclusion is that for this patch, we cannot simply change "byte-stream" to "avc" in wg_format_to_caps_video_h264(). We'd better add a h264parse while we are adding a h264 stream to mp4muxer, correct?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45477
On Fri Sep 15 07:14:43 2023 +0000, Ziqing Hui wrote:
OK, so the conclusion is that for this patch, we cannot simply change "byte-stream" to "avc" in wg_format_to_caps_video_h264(). We'd better add a h264parse while we are adding a h264 stream to mp4muxer, correct? I'm not sure that's the best solution, although perhaps the easiest. The h264parse plugin is sometimes not available. We don't have it in Proton for instance.
I think the right solution would be to decide whether we should use "byte-stream" or "avc" depending on the media type. Maybe convert from "byte-stream" to "avc" ourselves, although I'm not entirely confident that this is fine wrt. patents, or rely on having another muxer selected instead. Btw, I think the muxer selection is not optimal for this situation, as it will not take into account the stream caps, but only the container. Maybe it should be done differently so that another muxer, which would support "byte-stream" format, could be used. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45481
On Fri Sep 15 07:41:52 2023 +0000, Rémi Bernon wrote:
I'm not sure that's the best solution, although perhaps the easiest. The h264parse plugin is sometimes not available. We don't have it in Proton for instance. I think the right solution would be to decide whether we should use "byte-stream" or "avc" depending on the media type. Maybe convert from "byte-stream" to "avc" ourselves, although I'm not entirely confident that this is fine wrt. patents, or rely on having another muxer selected instead. Btw, I think the muxer selection is not optimal for this situation, as it will not take into account the stream caps, but only the container. Maybe it should be done differently so that another muxer, which would support "byte-stream" format, could be used. Fwiw avmux_mp4 can handle byte-stream H264.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45483
On Fri Sep 15 07:55:47 2023 +0000, Rémi Bernon wrote:
Fwiw avmux_mp4 can handle byte-stream H264. avmux_mp4 is marked as "(not recommended, use mp4mux instead)", it has a rank of none(0).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45484
On Fri Sep 15 07:57:24 2023 +0000, Ziqing Hui wrote:
avmux_mp4 is marked as "(not recommended, use mp4mux instead)", it has a rank of none(0). It looks like it comes from https://bugzilla.gnome.org/show_bug.cgi?id=622773, which is not clear about the reasons, but it's also not obvious that it is completely broken.
In any case I'm not suggesting to explicitly use it, but at least allow it to be selected if mp4mux cannot be used. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45485
I'm not sure that's the best solution, although perhaps the easiest. The h264parse plugin is sometimes not available. We don't have it in Proton for instance.
True, but we need *something* that can do its job. I think when it comes to encoders, Proton has bigger problems than h264parse anyway. Ideally we shouldn't be creating h264parse manually, but rather autoplugging. It may be worth looking into encodebin. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45544
Hmm... you're right maybe the MFT can handle either format somehow then, would be better to check what the games are actually doing in this case. Call of Duty Black Ops 3, Shadow Warrior 2 for instance are using the MFT decoder directly.
It's possible, but I doubt it. The H.264 decoder documentation [1] states: Input data must conform to Annex B of ISO/IEC 14496-10. The data must include the start codes. The decoder skips bytes until it finds a valid sequence parameter set (SPS) and picture parameter set (PPS) in the byte stream. and the MPEG-4 file sink documentation [2] states: The bitstream must conform to the H.264 Annex B format specification. In particular, NALUs must be delimited with either 3-byte or 4-byte start codes. Of course, documentation is rarely to be trusted, and it's possible that the AVC formats are accepted anyway...
There's also the MF_NALU_LENGTH_SET and MF_NALU_LENGTH_INFORMATION attributes (https://learn.microsoft.com/en-us/windows/win32/medfound/mf-nalu-length-set) which maybe indicate that the stream is in a format or another.
I don't think so. MF_NALU_LENGTH_INFORMATION is specifically meant to provide NALU lengths out of line (cf. [3]). That would be a convenience if the data is in Annex B format, since the decoder no longer needs to scan for start codes, but it would be entirely unnecessary for AVC, since the lengths are already in line. [1] https://learn.microsoft.com/en-us/windows/win32/medfound/h-264-video-decoder [2] https://learn.microsoft.com/en-us/windows/win32/medfound/mpeg-4-file-sink [3] https://learn.microsoft.com/en-us/windows/win32/medfound/mf-nalu-length-info... -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45549
Ideally we shouldn't be creating h264parse manually, but rather autoplugging. It may be worth looking into encodebin.
Yeah, that sounds reasonable. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3810#note_45692
participants (4)
-
Rémi Bernon -
Zebediah Figura (＠zfigura)
-
Ziqing Hui
-
Ziqing Hui (＠zhui)