[PATCH 1/3] winegstreamer: Introduce new wg_transform struct.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51931 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52391 Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- This would serve a simpler purpose than the full-fledged wg_parser, and be used by synchronous transforms only (at least there's no thread on the Wine side), which simply work on the push then pull loop model. The wg_transform name is chosen to be generic and possibly cover both decoder and encoders (and possibly converters), but maybe it's better to keep it specific for now and only design it for decoders. This would simplify the format question in the next patch. dlls/winegstreamer/Makefile.in | 1 + dlls/winegstreamer/gst_private.h | 3 ++ dlls/winegstreamer/main.c | 14 +++++++ dlls/winegstreamer/unix_private.h | 31 ++++++++++++++ dlls/winegstreamer/unixlib.h | 8 ++++ dlls/winegstreamer/wg_parser.c | 20 +++++++-- dlls/winegstreamer/wg_transform.c | 69 +++++++++++++++++++++++++++++++ dlls/winegstreamer/wma_decoder.c | 20 +++++++++ 8 files changed, 162 insertions(+), 4 deletions(-) create mode 100644 dlls/winegstreamer/unix_private.h create mode 100644 dlls/winegstreamer/wg_transform.c diff --git a/dlls/winegstreamer/Makefile.in b/dlls/winegstreamer/Makefile.in index c53e914e246..52295418f0f 100644 --- a/dlls/winegstreamer/Makefile.in +++ b/dlls/winegstreamer/Makefile.in @@ -13,6 +13,7 @@ C_SRCS = \ mfplat.c \ quartz_parser.c \ wg_parser.c \ + wg_transform.c \ wm_asyncreader.c \ wm_reader.c \ wm_syncreader.c \ diff --git a/dlls/winegstreamer/gst_private.h b/dlls/winegstreamer/gst_private.h index 222bce3b2c7..df82b229143 100644 --- a/dlls/winegstreamer/gst_private.h +++ b/dlls/winegstreamer/gst_private.h @@ -96,6 +96,9 @@ uint64_t wg_parser_stream_get_duration(struct wg_parser_stream *stream) DECLSPEC void wg_parser_stream_seek(struct wg_parser_stream *stream, double rate, uint64_t start_pos, uint64_t stop_pos, DWORD start_flags, DWORD stop_flags) DECLSPEC_HIDDEN; +struct wg_transform *wg_transform_create(void) DECLSPEC_HIDDEN; +void wg_transform_destroy(struct wg_transform *transform) DECLSPEC_HIDDEN; + unsigned int wg_format_get_max_size(const struct wg_format *format); HRESULT avi_splitter_create(IUnknown *outer, IUnknown **out) DECLSPEC_HIDDEN; diff --git a/dlls/winegstreamer/main.c b/dlls/winegstreamer/main.c index 260dd208e2f..f23fa3abcdf 100644 --- a/dlls/winegstreamer/main.c +++ b/dlls/winegstreamer/main.c @@ -254,6 +254,20 @@ void wg_parser_stream_seek(struct wg_parser_stream *stream, double rate, __wine_unix_call(unix_handle, unix_wg_parser_stream_seek, ¶ms); } +struct wg_transform *wg_transform_create(void) +{ + struct wg_transform_create_params params = {0}; + + if (__wine_unix_call(unix_handle, unix_wg_transform_create, ¶ms)) + return NULL; + return params.transform; +} + +void wg_transform_destroy(struct wg_transform *transform) +{ + __wine_unix_call(unix_handle, unix_wg_transform_destroy, transform); +} + BOOL WINAPI DllMain(HINSTANCE instance, DWORD reason, void *reserved) { if (reason == DLL_PROCESS_ATTACH) diff --git a/dlls/winegstreamer/unix_private.h b/dlls/winegstreamer/unix_private.h new file mode 100644 index 00000000000..375d33e7728 --- /dev/null +++ b/dlls/winegstreamer/unix_private.h @@ -0,0 +1,31 @@ +/* + * winegstreamer Unix library interface + * + * Copyright 2020-2021 Zebediah Figura 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 + */ + +#ifndef __WINE_WINEGSTREAMER_UNIX_PRIVATE_H +#define __WINE_WINEGSTREAMER_UNIX_PRIVATE_H + +#include "unixlib.h" + +extern bool init_gstreamer(void) DECLSPEC_HIDDEN; + +extern NTSTATUS wg_transform_create(void *args) DECLSPEC_HIDDEN; +extern NTSTATUS wg_transform_destroy(void *args) DECLSPEC_HIDDEN; + +#endif /* __WINE_WINEGSTREAMER_UNIX_PRIVATE_H */ diff --git a/dlls/winegstreamer/unixlib.h b/dlls/winegstreamer/unixlib.h index 82bb534b938..c8b98da3a64 100644 --- a/dlls/winegstreamer/unixlib.h +++ b/dlls/winegstreamer/unixlib.h @@ -217,6 +217,11 @@ struct wg_parser_stream_seek_params DWORD start_flags, stop_flags; }; +struct wg_transform_create_params +{ + struct wg_transform *transform; +}; + enum unix_funcs { unix_wg_parser_create, @@ -245,6 +250,9 @@ enum unix_funcs unix_wg_parser_stream_get_duration, unix_wg_parser_stream_seek, + + unix_wg_transform_create, + unix_wg_transform_destroy, }; #endif /* __WINE_WINEGSTREAMER_UNIXLIB_H */ diff --git a/dlls/winegstreamer/wg_parser.c b/dlls/winegstreamer/wg_parser.c index 013566b25e9..ac8c5a2b95c 100644 --- a/dlls/winegstreamer/wg_parser.c +++ b/dlls/winegstreamer/wg_parser.c @@ -37,7 +37,7 @@ #include "winternl.h" #include "dshow.h" -#include "unixlib.h" +#include "unix_private.h" typedef enum { @@ -51,7 +51,7 @@ typedef enum * debug logging instead of Wine debug logging. In order to be safe we forbid * any use of Wine debug logging in this entire file. */ -GST_DEBUG_CATEGORY_STATIC(wine); +GST_DEBUG_CATEGORY(wine); #define GST_CAT_DEFAULT wine typedef BOOL (*init_gst_cb)(struct wg_parser *parser); @@ -1963,6 +1963,16 @@ static void init_gstreamer_once(void) gst_version_string(), GST_VERSION_MAJOR, GST_VERSION_MINOR, GST_VERSION_MICRO); } +bool init_gstreamer(void) +{ + static pthread_once_t init_once = PTHREAD_ONCE_INIT; + + if (pthread_once(&init_once, init_gstreamer_once)) + return false; + + return true; +} + static NTSTATUS wg_parser_create(void *args) { static const init_gst_cb init_funcs[] = @@ -1973,11 +1983,10 @@ static NTSTATUS wg_parser_create(void *args) [WG_PARSER_WAVPARSE] = wave_parser_init_gst, }; - static pthread_once_t once = PTHREAD_ONCE_INIT; struct wg_parser_create_params *params = args; struct wg_parser *parser; - if (pthread_once(&once, init_gstreamer_once)) + if (!init_gstreamer()) return E_FAIL; if (!(parser = calloc(1, sizeof(*parser)))) @@ -2044,4 +2053,7 @@ const unixlib_entry_t __wine_unix_call_funcs[] = X(wg_parser_stream_get_duration), X(wg_parser_stream_seek), + + X(wg_transform_create), + X(wg_transform_destroy), }; diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c new file mode 100644 index 00000000000..822740da0d7 --- /dev/null +++ b/dlls/winegstreamer/wg_transform.c @@ -0,0 +1,69 @@ +/* + * GStreamer transform backend + * + * Copyright 2022 Rémi Bernon 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 "config.h" + +#include <assert.h> +#include <stdarg.h> +#include <stdio.h> + +#include <gst/gst.h> +#include <gst/video/video.h> +#include <gst/audio/audio.h> + +#include "winternl.h" +#include "dshow.h" + +#include "unix_private.h" + +GST_DEBUG_CATEGORY_EXTERN(wine); +#define GST_CAT_DEFAULT wine + +struct wg_transform +{ +}; + +NTSTATUS wg_transform_destroy(void *args) +{ + struct wg_transform *transform = args; + + free(transform); + return S_OK; +} + +NTSTATUS wg_transform_create(void *args) +{ + struct wg_transform_create_params *params = args; + struct wg_transform *transform; + + if (!init_gstreamer()) + return E_FAIL; + + if (!(transform = calloc(1, sizeof(*transform)))) + return E_OUTOFMEMORY; + + GST_INFO("Created winegstreamer transform %p.", transform); + params->transform = transform; + return S_OK; +} diff --git a/dlls/winegstreamer/wma_decoder.c b/dlls/winegstreamer/wma_decoder.c index 78316059052..3b051230a9e 100644 --- a/dlls/winegstreamer/wma_decoder.c +++ b/dlls/winegstreamer/wma_decoder.c @@ -53,6 +53,8 @@ struct wma_decoder LONG refcount; IMFMediaType *input_type; IMFMediaType *output_type; + + struct wg_transform *wg_transform; }; static inline struct wma_decoder *impl_from_IUnknown(IUnknown *iface) @@ -60,6 +62,19 @@ static inline struct wma_decoder *impl_from_IUnknown(IUnknown *iface) return CONTAINING_RECORD(iface, struct wma_decoder, IUnknown_inner); } +static HRESULT try_create_wg_transform(struct wma_decoder *decoder) +{ + if (decoder->wg_transform) + wg_transform_destroy(decoder->wg_transform); + + decoder->wg_transform = wg_transform_create(); + if (decoder->wg_transform) + return S_OK; + + WARN("Failed to create wg_transform.\n"); + return E_FAIL; +} + static HRESULT WINAPI unknown_QueryInterface(IUnknown *iface, REFIID iid, void **out) { struct wma_decoder *decoder = impl_from_IUnknown(iface); @@ -104,6 +119,8 @@ static ULONG WINAPI unknown_Release(IUnknown *iface) if (!refcount) { + if (decoder->wg_transform) + wg_transform_destroy(decoder->wg_transform); if (decoder->input_type) IMFMediaType_Release(decoder->input_type); if (decoder->output_type) @@ -438,6 +455,9 @@ static HRESULT WINAPI transform_SetOutputType(IMFTransform *iface, DWORD id, IMF if (FAILED(hr = IMFMediaType_CopyAllItems(type, (IMFAttributes *)decoder->output_type))) goto failed; + if (FAILED(hr = try_create_wg_transform(decoder))) + goto failed; + return S_OK; failed: -- 2.34.1
And use it for decoder transform input types. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51931 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52391 Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- This uses a separate struct for encoded formats, as I believe it was suggested at some point (or I may be mistaken), but I'm not sure it's best. If we intend to support encoders or converters in the wg_transform the formats would need to be swapped, and in which case using the same struct may be cleaner. In that case, maybe using a separate major type for encoded formats, or a separate major type for each encoded format would be better. dlls/winegstreamer/gst_private.h | 1 + dlls/winegstreamer/mfplat.c | 84 ++++++++++++++++++++++++++++++++ dlls/winegstreamer/unixlib.h | 25 ++++++++++ dlls/winegstreamer/wma_decoder.c | 12 +++++ 4 files changed, 122 insertions(+) diff --git a/dlls/winegstreamer/gst_private.h b/dlls/winegstreamer/gst_private.h index df82b229143..cec52e976ec 100644 --- a/dlls/winegstreamer/gst_private.h +++ b/dlls/winegstreamer/gst_private.h @@ -117,6 +117,7 @@ extern HRESULT mfplat_DllRegisterServer(void) DECLSPEC_HIDDEN; IMFMediaType *mf_media_type_from_wg_format(const struct wg_format *format) DECLSPEC_HIDDEN; void mf_media_type_to_wg_format(IMFMediaType *type, struct wg_format *format) DECLSPEC_HIDDEN; +void mf_media_type_to_wg_encoded_format(IMFMediaType *type, struct wg_encoded_format *format) DECLSPEC_HIDDEN; HRESULT winegstreamer_stream_handler_create(REFIID riid, void **obj) DECLSPEC_HIDDEN; diff --git a/dlls/winegstreamer/mfplat.c b/dlls/winegstreamer/mfplat.c index a111bbe196d..61c7fe28a63 100644 --- a/dlls/winegstreamer/mfplat.c +++ b/dlls/winegstreamer/mfplat.c @@ -760,3 +760,87 @@ void mf_media_type_to_wg_format(IMFMediaType *type, struct wg_format *format) else FIXME("Unrecognized major type %s.\n", debugstr_guid(&major_type)); } + +static void mf_media_type_to_wg_encoded_format_wma(IMFMediaType *type, struct wg_encoded_format *format, + UINT32 version) +{ + UINT32 rate, depth, channels, block_align, bytes_per_second, codec_data_len; + BYTE codec_data[64]; + + if (FAILED(IMFMediaType_GetUINT32(type, &MF_MT_AUDIO_SAMPLES_PER_SECOND, &rate))) + { + FIXME("Sample rate is not set.\n"); + return; + } + if (FAILED(IMFMediaType_GetUINT32(type, &MF_MT_AUDIO_NUM_CHANNELS, &channels))) + { + FIXME("Channel count is not set.\n"); + return; + } + if (FAILED(IMFMediaType_GetUINT32(type, &MF_MT_AUDIO_BLOCK_ALIGNMENT, &block_align))) + { + FIXME("Block alignment is not set.\n"); + return; + } + if (FAILED(IMFMediaType_GetUINT32(type, &MF_MT_AUDIO_BITS_PER_SAMPLE, &depth))) + { + FIXME("Depth is not set.\n"); + return; + } + if (FAILED(IMFMediaType_GetBlob(type, &MF_MT_USER_DATA, codec_data, sizeof(codec_data), &codec_data_len))) + { + FIXME("Codec data is not set.\n"); + return; + } + if (FAILED(IMFMediaType_GetUINT32(type, &MF_MT_AUDIO_AVG_BYTES_PER_SECOND, &bytes_per_second))) + { + FIXME("Bitrate is not set.\n"); + bytes_per_second = 0; + } + + format->encoded_type = WG_ENCODED_TYPE_WMA; + format->u.xwma.version = version; + format->u.xwma.bitrate = bytes_per_second * 8; + format->u.xwma.rate = rate; + format->u.xwma.depth = depth; + format->u.xwma.channels = channels; + format->u.xwma.block_align = block_align; + format->u.xwma.codec_data_len = codec_data_len; + memcpy(format->u.xwma.codec_data, codec_data, codec_data_len); +} + +void mf_media_type_to_wg_encoded_format(IMFMediaType *type, struct wg_encoded_format *format) +{ + GUID major_type, subtype; + + memset(format, 0, sizeof(*format)); + + if (FAILED(IMFMediaType_GetMajorType(type, &major_type))) + { + FIXME("Major type is not set.\n"); + return; + } + if (FAILED(IMFMediaType_GetGUID(type, &MF_MT_SUBTYPE, &subtype))) + { + FIXME("Subtype is not set.\n"); + return; + } + + if (IsEqualGUID(&major_type, &MFMediaType_Audio)) + { + if (IsEqualGUID(&subtype, &MEDIASUBTYPE_MSAUDIO1)) + mf_media_type_to_wg_encoded_format_wma(type, format, 1); + else if (IsEqualGUID(&subtype, &MFAudioFormat_WMAudioV8)) + mf_media_type_to_wg_encoded_format_wma(type, format, 2); + else if (IsEqualGUID(&subtype, &MFAudioFormat_WMAudioV9)) + mf_media_type_to_wg_encoded_format_wma(type, format, 3); + else if (IsEqualGUID(&subtype, &MFAudioFormat_WMAudio_Lossless)) + mf_media_type_to_wg_encoded_format_wma(type, format, 4); + else + FIXME("Unimplemented audio subtype %s.\n", debugstr_guid(&subtype)); + } + else + { + FIXME("Unimplemented major type %s.\n", debugstr_guid(&major_type)); + } +} diff --git a/dlls/winegstreamer/unixlib.h b/dlls/winegstreamer/unixlib.h index c8b98da3a64..ea46de4cce1 100644 --- a/dlls/winegstreamer/unixlib.h +++ b/dlls/winegstreamer/unixlib.h @@ -91,6 +91,31 @@ struct wg_format } u; }; +struct wg_encoded_format +{ + enum wg_encoded_type + { + WG_ENCODED_TYPE_UNKNOWN, + WG_ENCODED_TYPE_WMA, + WG_ENCODED_TYPE_XMA, + } encoded_type; + + union + { + struct + { + uint32_t version; + uint32_t bitrate; + uint32_t rate; + uint32_t depth; + uint32_t channels; + uint32_t block_align; + uint32_t codec_data_len; + unsigned char codec_data[64]; + } xwma; + } u; +}; + enum wg_parser_event_type { WG_PARSER_EVENT_NONE = 0, diff --git a/dlls/winegstreamer/wma_decoder.c b/dlls/winegstreamer/wma_decoder.c index 3b051230a9e..b037795fc78 100644 --- a/dlls/winegstreamer/wma_decoder.c +++ b/dlls/winegstreamer/wma_decoder.c @@ -64,8 +64,20 @@ static inline struct wma_decoder *impl_from_IUnknown(IUnknown *iface) static HRESULT try_create_wg_transform(struct wma_decoder *decoder) { + struct wg_encoded_format input_format; + struct wg_format output_format; + if (decoder->wg_transform) wg_transform_destroy(decoder->wg_transform); + decoder->wg_transform = NULL; + + mf_media_type_to_wg_encoded_format(decoder->input_type, &input_format); + if (input_format.encoded_type == WG_ENCODED_TYPE_UNKNOWN) + return MF_E_INVALIDMEDIATYPE; + + mf_media_type_to_wg_format(decoder->output_type, &output_format); + if (output_format.major_type == WG_MAJOR_TYPE_UNKNOWN) + return MF_E_INVALIDMEDIATYPE; decoder->wg_transform = wg_transform_create(); if (decoder->wg_transform) -- 2.34.1
On 2/11/22 03:36, Rémi Bernon wrote:
And use it for decoder transform input types.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51931 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52391 Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> ---
This uses a separate struct for encoded formats, as I believe it was suggested at some point (or I may be mistaken), but I'm not sure it's best. If we intend to support encoders or converters in the wg_transform the formats would need to be swapped, and in which case using the same struct may be cleaner.
In that case, maybe using a separate major type for encoded formats, or a separate major type for each encoded format would be better.
I'm not sure if I suggested such a thing, but in any case I don't think it makes sense to use a separate struct. Note also that we already express some compressed formats using the wg_format structure, namely Cinepak and MPEG-1 audio. We don't use a separate major type for those, and that's essentially because they share almost all of the same other attributes (frame size, frame rate, channel count/mask, although not audio sample rate). On the other hand some compressed formats, like WMA, need extra data, so maybe it makes more sense to introduce a new major type for those. I don't have strong opinions either way. I also think it's fine to add a separate major type for some formats but not others, based on how different the data needs to be.
With caps created from the input / output formats. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51931 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52391 Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winegstreamer/gst_private.h | 3 +- dlls/winegstreamer/main.c | 9 ++- dlls/winegstreamer/unix_private.h | 1 + dlls/winegstreamer/unixlib.h | 2 + dlls/winegstreamer/wg_parser.c | 2 +- dlls/winegstreamer/wg_transform.c | 93 +++++++++++++++++++++++++++++++ dlls/winegstreamer/wma_decoder.c | 2 +- 7 files changed, 107 insertions(+), 5 deletions(-) diff --git a/dlls/winegstreamer/gst_private.h b/dlls/winegstreamer/gst_private.h index cec52e976ec..5d198f57dc7 100644 --- a/dlls/winegstreamer/gst_private.h +++ b/dlls/winegstreamer/gst_private.h @@ -96,7 +96,8 @@ uint64_t wg_parser_stream_get_duration(struct wg_parser_stream *stream) DECLSPEC void wg_parser_stream_seek(struct wg_parser_stream *stream, double rate, uint64_t start_pos, uint64_t stop_pos, DWORD start_flags, DWORD stop_flags) DECLSPEC_HIDDEN; -struct wg_transform *wg_transform_create(void) DECLSPEC_HIDDEN; +struct wg_transform *wg_transform_create(const struct wg_encoded_format *input_format, + const struct wg_format *output_format) DECLSPEC_HIDDEN; void wg_transform_destroy(struct wg_transform *transform) DECLSPEC_HIDDEN; 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 f23fa3abcdf..6dfa9eb5c82 100644 --- a/dlls/winegstreamer/main.c +++ b/dlls/winegstreamer/main.c @@ -254,9 +254,14 @@ void wg_parser_stream_seek(struct wg_parser_stream *stream, double rate, __wine_unix_call(unix_handle, unix_wg_parser_stream_seek, ¶ms); } -struct wg_transform *wg_transform_create(void) +struct wg_transform *wg_transform_create(const struct wg_encoded_format *input_format, + const struct wg_format *output_format) { - struct wg_transform_create_params params = {0}; + struct wg_transform_create_params params = + { + .input_format = input_format, + .output_format = output_format, + }; if (__wine_unix_call(unix_handle, unix_wg_transform_create, ¶ms)) return NULL; diff --git a/dlls/winegstreamer/unix_private.h b/dlls/winegstreamer/unix_private.h index 375d33e7728..38349eb5e8d 100644 --- a/dlls/winegstreamer/unix_private.h +++ b/dlls/winegstreamer/unix_private.h @@ -24,6 +24,7 @@ #include "unixlib.h" extern bool init_gstreamer(void) DECLSPEC_HIDDEN; +extern GstCaps *wg_format_to_caps(const struct wg_format *format) DECLSPEC_HIDDEN; extern NTSTATUS wg_transform_create(void *args) DECLSPEC_HIDDEN; extern NTSTATUS wg_transform_destroy(void *args) DECLSPEC_HIDDEN; diff --git a/dlls/winegstreamer/unixlib.h b/dlls/winegstreamer/unixlib.h index ea46de4cce1..96cda2e25aa 100644 --- a/dlls/winegstreamer/unixlib.h +++ b/dlls/winegstreamer/unixlib.h @@ -245,6 +245,8 @@ struct wg_parser_stream_seek_params struct wg_transform_create_params { struct wg_transform *transform; + const struct wg_encoded_format *input_format; + const struct wg_format *output_format; }; enum unix_funcs diff --git a/dlls/winegstreamer/wg_parser.c b/dlls/winegstreamer/wg_parser.c index ac8c5a2b95c..e7e80ecfddf 100644 --- a/dlls/winegstreamer/wg_parser.c +++ b/dlls/winegstreamer/wg_parser.c @@ -463,7 +463,7 @@ static GstCaps *wg_format_to_caps_video(const struct wg_format *format) return caps; } -static GstCaps *wg_format_to_caps(const struct wg_format *format) +GstCaps *wg_format_to_caps(const struct wg_format *format) { switch (format->major_type) { diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c index 822740da0d7..146cdd87ae7 100644 --- a/dlls/winegstreamer/wg_transform.c +++ b/dlls/winegstreamer/wg_transform.c @@ -42,12 +42,77 @@ GST_DEBUG_CATEGORY_EXTERN(wine); struct wg_transform { + GstPad *my_src, *my_sink; }; +static GstCaps *wg_format_to_caps_xwma(const struct wg_encoded_format *format) +{ + GstBuffer *buffer; + GstCaps *caps; + + if (format->encoded_type == WG_ENCODED_TYPE_WMA) + caps = gst_caps_new_empty_simple("audio/x-wma"); + else + caps = gst_caps_new_empty_simple("audio/x-xma"); + + if (format->u.xwma.version) + gst_caps_set_simple(caps, "wmaversion", G_TYPE_INT, format->u.xwma.version, NULL); + if (format->u.xwma.bitrate) + gst_caps_set_simple(caps, "bitrate", G_TYPE_INT, format->u.xwma.bitrate, NULL); + if (format->u.xwma.rate) + gst_caps_set_simple(caps, "rate", G_TYPE_INT, format->u.xwma.rate, NULL); + if (format->u.xwma.depth) + gst_caps_set_simple(caps, "depth", G_TYPE_INT, format->u.xwma.depth, NULL); + if (format->u.xwma.channels) + gst_caps_set_simple(caps, "channels", G_TYPE_INT, format->u.xwma.channels, NULL); + if (format->u.xwma.block_align) + gst_caps_set_simple(caps, "block_align", G_TYPE_INT, format->u.xwma.block_align, NULL); + + if (format->u.xwma.codec_data_len) + { + buffer = gst_buffer_new_and_alloc(format->u.xwma.codec_data_len); + gst_buffer_fill(buffer, 0, format->u.xwma.codec_data, format->u.xwma.codec_data_len); + gst_caps_set_simple(caps, "codec_data", GST_TYPE_BUFFER, buffer, NULL); + gst_buffer_unref(buffer); + } + + return caps; +} + +static GstCaps *wg_encoded_format_to_caps(const struct wg_encoded_format *format) +{ + switch (format->encoded_type) + { + case WG_ENCODED_TYPE_UNKNOWN: + return NULL; + case WG_ENCODED_TYPE_WMA: + case WG_ENCODED_TYPE_XMA: + return wg_format_to_caps_xwma(format); + } + assert(0); + return NULL; +} + +static GstFlowReturn transform_sink_chain_cb(GstPad *pad, GstObject *parent, GstBuffer *buffer) +{ + struct wg_transform *transform = gst_pad_get_element_private(pad); + + GST_INFO("transform %p, buffer %p.", transform, buffer); + + gst_buffer_unref(buffer); + + return GST_FLOW_OK; +} + NTSTATUS wg_transform_destroy(void *args) { struct wg_transform *transform = args; + if (transform->my_sink) + g_object_unref(transform->my_sink); + if (transform->my_src) + g_object_unref(transform->my_src); + free(transform); return S_OK; } @@ -55,7 +120,11 @@ NTSTATUS wg_transform_destroy(void *args) NTSTATUS wg_transform_create(void *args) { struct wg_transform_create_params *params = args; + struct wg_encoded_format input_format = *params->input_format; + struct wg_format output_format = *params->output_format; + GstCaps *src_caps, *sink_caps; struct wg_transform *transform; + GstPadTemplate *template; if (!init_gstreamer()) return E_FAIL; @@ -63,7 +132,31 @@ NTSTATUS wg_transform_create(void *args) if (!(transform = calloc(1, sizeof(*transform)))) return E_OUTOFMEMORY; + src_caps = wg_encoded_format_to_caps(&input_format); + assert(src_caps); + sink_caps = wg_format_to_caps(&output_format); + assert(sink_caps); + + template = gst_pad_template_new("src", GST_PAD_SRC, GST_PAD_ALWAYS, src_caps); + assert(template); + transform->my_src = gst_pad_new_from_template(template, "src"); + g_object_unref(template); + assert(transform->my_src); + + template = gst_pad_template_new("sink", GST_PAD_SINK, GST_PAD_ALWAYS, sink_caps); + assert(template); + transform->my_sink = gst_pad_new_from_template(template, "sink"); + g_object_unref(template); + assert(transform->my_sink); + + gst_pad_set_element_private(transform->my_sink, transform); + gst_pad_set_chain_function(transform->my_sink, transform_sink_chain_cb); + GST_INFO("Created winegstreamer transform %p.", transform); params->transform = transform; + + gst_caps_unref(src_caps); + gst_caps_unref(sink_caps); + return S_OK; } diff --git a/dlls/winegstreamer/wma_decoder.c b/dlls/winegstreamer/wma_decoder.c index b037795fc78..4183df43fed 100644 --- a/dlls/winegstreamer/wma_decoder.c +++ b/dlls/winegstreamer/wma_decoder.c @@ -79,7 +79,7 @@ static HRESULT try_create_wg_transform(struct wma_decoder *decoder) if (output_format.major_type == WG_MAJOR_TYPE_UNKNOWN) return MF_E_INVALIDMEDIATYPE; - decoder->wg_transform = wg_transform_create(); + decoder->wg_transform = wg_transform_create(&input_format, &output_format); if (decoder->wg_transform) return S_OK; -- 2.34.1
On 2/11/22 03:36, Rémi Bernon wrote:
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51931 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52391 Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> ---
This would serve a simpler purpose than the full-fledged wg_parser, and be used by synchronous transforms only (at least there's no thread on the Wine side), which simply work on the push then pull loop model.
Do you by chance have a complete branch somewhere that I can look at or mess with? I'd in particular like to see just how much code is identical, or explicitly different, between the parser and transform objects.
The wg_transform name is chosen to be generic and possibly cover both decoder and encoders (and possibly converters), but maybe it's better to keep it specific for now and only design it for decoders. This would simplify the format question in the next patch.
I suspect it makes more sense for it to be a generic transform object. From what I've seen GStreamer doesn't really discriminate between encoders, decoders, and other transforms, and I don't think that winegstreamer should either.
On 2/12/22 00:19, Zebediah Figura wrote:
On 2/11/22 03:36, Rémi Bernon wrote:
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51931 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52391 Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> ---
This would serve a simpler purpose than the full-fledged wg_parser, and be used by synchronous transforms only (at least there's no thread on the Wine side), which simply work on the push then pull loop model.
Do you by chance have a complete branch somewhere that I can look at or mess with? I'd in particular like to see just how much code is identical, or explicitly different, between the parser and transform objects.
The wg_transform name is chosen to be generic and possibly cover both decoder and encoders (and possibly converters), but maybe it's better to keep it specific for now and only design it for decoders. This would simplify the format question in the next patch.
I suspect it makes more sense for it to be a generic transform object. From what I've seen GStreamer doesn't really discriminate between encoders, decoders, and other transforms, and I don't think that winegstreamer should either.
I have three branches on top of each other, wip/wmadev/v1, wip/h264dec/v1, and wip/aacdec/v1 in https://github.com/rbernon/wine, although the last one is not polished at all and full of wip patches for tests. Probably the most interesting things are in the wmadec and h264dec patches. As far as I could see (although I didn't test it much) the aacdec didn't really require anything except the format mappings. Or, if you prefer a patch series: https://github.com/rbernon/wine/compare/6c7b746812...cb863ca5c2.patch Cheers, -- Rémi Bernon <rbernon(a)codeweavers.com>
On 2/11/22 17:46, Rémi Bernon wrote:
On 2/12/22 00:19, Zebediah Figura wrote:
On 2/11/22 03:36, Rémi Bernon wrote:
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51931 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52391 Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> ---
This would serve a simpler purpose than the full-fledged wg_parser, and be used by synchronous transforms only (at least there's no thread on the Wine side), which simply work on the push then pull loop model.
Do you by chance have a complete branch somewhere that I can look at or mess with? I'd in particular like to see just how much code is identical, or explicitly different, between the parser and transform objects.
The wg_transform name is chosen to be generic and possibly cover both decoder and encoders (and possibly converters), but maybe it's better to keep it specific for now and only design it for decoders. This would simplify the format question in the next patch.
I suspect it makes more sense for it to be a generic transform object. From what I've seen GStreamer doesn't really discriminate between encoders, decoders, and other transforms, and I don't think that winegstreamer should either.
I have three branches on top of each other, wip/wmadev/v1, wip/h264dec/v1, and wip/aacdec/v1 in https://github.com/rbernon/wine, although the last one is not polished at all and full of wip patches for tests.
Probably the most interesting things are in the wmadec and h264dec patches. As far as I could see (although I didn't test it much) the aacdec didn't really require anything except the format mappings.
Thanks, that's about what I was looking for. I only ended up looking at the WMA branch, but here are my thoughts: * There's surprisingly even less code in there than I thought, so I'm inclined to think that a separate wg_transform is a good idea. That said: * wg_transform is built without an equivalent of wg_parser_get_preferred_format() and all of the relevant support code. That gives me some pause. It doesn't give me a *lot* of pause, because the main point of wg_transform is to handle those applications that try to create *specific* decoders, and we probably want those specific decoders to report the same exact formats as they do on Windows. There are a few examples I can think of of applications that would be able to make use of a generic decoder, but if we have enough specific decoders already there may be no point anyway. * wg_transform is not designed to handle one-to-many elements and cuts out all of the relevant code. I think this is fine, ultimately; the only real example of that I can think of is the case of a demuxer that supports push mode, and if we ever need to support that (which we probably won't) it *probably* makes more sense to use wg_parser anyway. * The post-processing logic is kind of duplicated. Would it make sense to separate that into a helper function and then use it for both wg_parser and wg_transform? * sink_chain_cb is kind of redone. To be fair, we don't care about EOS or segment events. On the other hand, I'm not thrilled about the way we pull outputs in wg_parser, and we probably want to make it look more like what you have for wg_transform. [In specific I suspect we want to generate our own segment events for DirectShow rather than using GStreamer's, and once that's done we should make EOS a return value, and preëmptively grab sample buffers...] I can probably live with having two different implementations for a while, but ultimately I think we want to avoid that, at least in the unixlib API. * I think we want to handle GST_MESSAGE_ERROR and GST_MESSAGE_WARNING. We can't use bus_handler_cb() directly (since we don't want to handle GST_MESSAGE_DURATION_CHANGED) but we could probably add a helper for those two at least. * We might want QoS logic. That's probably not that impactful in terms of "is wg_transform worth it", though, and it doesn't need to be done immediately anyway... * More of a case of "if you don't want to do it then I will sooner or later", but if we're going to have two different objects I'd prefer to move the common code (which ends up mostly being all of the wg_format conversion) to a third file, just for maintainability's sake.
On 2/12/22 02:33, Zebediah Figura wrote:
I only ended up looking at the WMA branch, but here are my thoughts:
* There's surprisingly even less code in there than I thought, so I'm inclined to think that a separate wg_transform is a good idea. That said:
* wg_transform is built without an equivalent of wg_parser_get_preferred_format() and all of the relevant support code. That gives me some pause. It doesn't give me a *lot* of pause, because the main point of wg_transform is to handle those applications that try to create *specific* decoders, and we probably want those specific decoders to report the same exact formats as they do on Windows. There are a few examples I can think of of applications that would be able to make use of a generic decoder, but if we have enough specific decoders already there may be no point anyway.
I don't think this is required here, the way the transforms work they just provide a set of default supported formats, from which the application is supposed to chose, and, most of the time applications just select the first format, expecting it to be the same as on Windows. Then, after the transform has started processing some buffers, the output format may change, for instance if the stream has some metadata, and there's a notification that the application is supposed to handle. They just then usually query the new format and use it, I'm not even sure they can change it again at this point. For the H264 transform, this really depends on the same format as on Windows, the default output format is NV12 (*), and although some are looking for it and explicitly selecting it, some other games assume it is the first format. As they don't even check that it is, reporting some other format instead just makes the displayed frame incorrect. H264 buffer also usually have metadata to describe the actual stream format, so games don't even bother setting the output format properties, they use the first available one, and they wait and expect the stream output format change event to arrive - after which some query the output format properties, and some don't but still expect the change event. (*) And not standard NV12 but NV12 in the exact same way Microsoft H264 decoder outputs, which means with aligned planes, which GStreamer doesn't seem to provide and which requires some tweaking. We need several H264 transform patches to implement that.
* wg_transform is not designed to handle one-to-many elements and cuts out all of the relevant code. I think this is fine, ultimately; the only real example of that I can think of is the case of a demuxer that supports push mode, and if we ever need to support that (which we probably won't) it *probably* makes more sense to use wg_parser anyway.
Yes, from the IMFTransform interface I think they are supposed to support multiple streams, and probably there's some demuxer transforms, but I hope we won't need them for a while. And if we do, maybe it could be a matter of adding a stream index to the push / pull data functions.
* The post-processing logic is kind of duplicated. Would it make sense to separate that into a helper function and then use it for both wg_parser and wg_transform?
What post-processing do you mean?
* sink_chain_cb is kind of redone. To be fair, we don't care about EOS or segment events. On the other hand, I'm not thrilled about the way we pull outputs in wg_parser, and we probably want to make it look more like what you have for wg_transform. [In specific I suspect we want to generate our own segment events for DirectShow rather than using GStreamer's, and once that's done we should make EOS a return value, and preëmptively grab sample buffers...] I can probably live with having two different implementations for a while, but ultimately I think we want to avoid that, at least in the unixlib API.
* I think we want to handle GST_MESSAGE_ERROR and GST_MESSAGE_WARNING. We can't use bus_handler_cb() directly (since we don't want to handle GST_MESSAGE_DURATION_CHANGED) but we could probably add a helper for those two at least.
Currently there's no bus, I'm not sure we need one?
* More of a case of "if you don't want to do it then I will sooner or later", but if we're going to have two different objects I'd prefer to move the common code (which ends up mostly being all of the wg_format conversion) to a third file, just for maintainability's sake.
Sure, I'll do that. Cheers, -- Rémi Bernon <rbernon(a)codeweavers.com>
Ech, sorry, I managed to drop this thread on the floor... On 2/14/22 04:07, Rémi Bernon wrote:
* wg_transform is built without an equivalent of wg_parser_get_preferred_format() and all of the relevant support code. That gives me some pause. It doesn't give me a *lot* of pause, because the main point of wg_transform is to handle those applications that try to create *specific* decoders, and we probably want those specific decoders to report the same exact formats as they do on Windows. There are a few examples I can think of of applications that would be able to make use of a generic decoder, but if we have enough specific decoders already there may be no point anyway.
I don't think this is required here, the way the transforms work they just provide a set of default supported formats, from which the application is supposed to chose, and, most of the time applications just select the first format, expecting it to be the same as on Windows.
Then, after the transform has started processing some buffers, the output format may change, for instance if the stream has some metadata, and there's a notification that the application is supposed to handle. They just then usually query the new format and use it, I'm not even sure they can change it again at this point.
For the H264 transform, this really depends on the same format as on Windows, the default output format is NV12 (*), and although some are looking for it and explicitly selecting it, some other games assume it is the first format. As they don't even check that it is, reporting some other format instead just makes the displayed frame incorrect.
H264 buffer also usually have metadata to describe the actual stream format, so games don't even bother setting the output format properties, they use the first available one, and they wait and expect the stream output format change event to arrive - after which some query the output format properties, and some don't but still expect the change event.
(*) And not standard NV12 but NV12 in the exact same way Microsoft H264 decoder outputs, which means with aligned planes, which GStreamer doesn't seem to provide and which requires some tweaking. We need several H264 transform patches to implement that.
Right, I know that some objects really do need to output a hardcoded list of formats, that's what I was trying to say in my message. But I'm wondering if we at any point want a generic decoder. Note that mfplat is not the only concern here (we may need it as a DMO, as well as DirectShow via DMOs.) I'm not sure enough that we need a generic decoder that I think this is a priority, but I'm also not sure that we don't need one. Or, frankly, if games ask for a specific decoder but don't make assumptions about the output format, we should take advantage of that if at all possible. Format conversion is very much not cheap.
* wg_transform is not designed to handle one-to-many elements and cuts out all of the relevant code. I think this is fine, ultimately; the only real example of that I can think of is the case of a demuxer that supports push mode, and if we ever need to support that (which we probably won't) it *probably* makes more sense to use wg_parser anyway.
Yes, from the IMFTransform interface I think they are supposed to support multiple streams, and probably there's some demuxer transforms, but I hope we won't need them for a while. And if we do, maybe it could be a matter of adding a stream index to the push / pull data functions.
Probably. Note that part of the concern is elements without a fixed number of pads. Still, it's probably not worth worrying about.
* The post-processing logic is kind of duplicated. Would it make sense to separate that into a helper function and then use it for both wg_parser and wg_transform?
What post-processing do you mean?
I mean the format conversion, resampling, videoflip, etc. elements. It's something that can be deduplicated later, though, and will be at this rate if at all...
* sink_chain_cb is kind of redone. To be fair, we don't care about EOS or segment events. On the other hand, I'm not thrilled about the way we pull outputs in wg_parser, and we probably want to make it look more like what you have for wg_transform. [In specific I suspect we want to generate our own segment events for DirectShow rather than using GStreamer's, and once that's done we should make EOS a return value, and preëmptively grab sample buffers...] I can probably live with having two different implementations for a while, but ultimately I think we want to avoid that, at least in the unixlib API.
* I think we want to handle GST_MESSAGE_ERROR and GST_MESSAGE_WARNING. We can't use bus_handler_cb() directly (since we don't want to handle GST_MESSAGE_DURATION_CHANGED) but we could probably add a helper for those two at least.
Currently there's no bus, I'm not sure we need one?
Well, like I said, I think we should make a point of printing error and warning messages to console; they won't appear otherwise with default log settings.
* More of a case of "if you don't want to do it then I will sooner or later", but if we're going to have two different objects I'd prefer to move the common code (which ends up mostly being all of the wg_format conversion) to a third file, just for maintainability's sake.
Sure, I'll do that.
Thanks for taking care of that!
participants (3)
-
Rémi Bernon -
Zebediah Figura -
Zebediah Figura (she/her)