[PATCH v3 0/5] MR1384: winegstreamer: Fix some quartz transform issues.
- v2: - Allocate 3 buffers instead of 100. - Add support for MFSampleExtension_Discontinuity. - v3: Destroy the queue instead of flusing it. -- v3: winegstreamer: Set the discontinuity flag in wg_transform. include: Add MFSampleExtension_Discontinuity to mfapi.h. winegstreamer: Allocate at least 3 buffers for MPEG audio in quartz parser. winegstreamer: Destroy the sample queue when stopping the quartz transform. winegstreamer: Hold the streaming lock while destroying wg_transform. https://gitlab.winehq.org/wine/wine/-/merge_requests/1384
From: Anton Baskanov <baskanov(a)gmail.com> Otherwise, the streaming thread might try to access it while it's being destroyed. --- dlls/winegstreamer/quartz_transform.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dlls/winegstreamer/quartz_transform.c b/dlls/winegstreamer/quartz_transform.c index 6520f697ce4..ccdec9454a6 100644 --- a/dlls/winegstreamer/quartz_transform.c +++ b/dlls/winegstreamer/quartz_transform.c @@ -129,7 +129,9 @@ static HRESULT transform_cleanup_stream(struct strmbase_filter *iface) { IMemAllocator_Decommit(filter->source.pAllocator); + EnterCriticalSection(&filter->filter.stream_cs); wg_transform_destroy(filter->transform); + LeaveCriticalSection(&filter->filter.stream_cs); } return S_OK; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1384
From: Anton Baskanov <baskanov(a)gmail.com> This releases the samples wg_transform might have referenced before destruction. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53342 --- dlls/winegstreamer/quartz_transform.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/dlls/winegstreamer/quartz_transform.c b/dlls/winegstreamer/quartz_transform.c index ccdec9454a6..09ad4862410 100644 --- a/dlls/winegstreamer/quartz_transform.c +++ b/dlls/winegstreamer/quartz_transform.c @@ -78,7 +78,6 @@ static void transform_destroy(struct strmbase_filter *iface) strmbase_sink_cleanup(&filter->sink); strmbase_filter_cleanup(&filter->filter); - wg_sample_queue_destroy(filter->sample_queue); free(filter); } @@ -109,9 +108,15 @@ static HRESULT transform_init_stream(struct strmbase_filter *iface) if (!amt_to_wg_format(&filter->source.pin.mt, &output_format)) return E_FAIL; + if (FAILED(hr = wg_sample_queue_create(&filter->sample_queue))) + return hr; + filter->transform = wg_transform_create(&input_format, &output_format); if (!filter->transform) + { + wg_sample_queue_destroy(filter->sample_queue); return E_FAIL; + } hr = IMemAllocator_Commit(filter->source.pAllocator); if (FAILED(hr)) @@ -131,6 +136,7 @@ static HRESULT transform_cleanup_stream(struct strmbase_filter *iface) EnterCriticalSection(&filter->filter.stream_cs); wg_transform_destroy(filter->transform); + wg_sample_queue_destroy(filter->sample_queue); LeaveCriticalSection(&filter->filter.stream_cs); } @@ -538,18 +544,11 @@ static const IQualityControlVtbl source_quality_control_vtbl = static HRESULT transform_create(IUnknown *outer, const CLSID *clsid, const struct transform_ops *ops, struct transform **out) { struct transform *object; - HRESULT hr; object = calloc(1, sizeof(*object)); if (!object) return E_OUTOFMEMORY; - if (FAILED(hr = wg_sample_queue_create(&object->sample_queue))) - { - free(object); - return hr; - } - strmbase_filter_init(&object->filter, outer, clsid, &filter_ops); strmbase_sink_init(&object->sink, &object->filter, L"In", &sink_ops, NULL); strmbase_source_init(&object->source, &object->filter, L"Out", &source_ops); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1384
From: Anton Baskanov <baskanov(a)gmail.com> --- dlls/winegstreamer/quartz_parser.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/dlls/winegstreamer/quartz_parser.c b/dlls/winegstreamer/quartz_parser.c index 87185312585..2a7845be737 100644 --- a/dlls/winegstreamer/quartz_parser.c +++ b/dlls/winegstreamer/quartz_parser.c @@ -1649,6 +1649,7 @@ static HRESULT WINAPI GSTOutPin_DecideBufferSize(struct strmbase_source *iface, IMemAllocator *allocator, ALLOCATOR_PROPERTIES *props) { struct parser_source *pin = impl_source_from_IPin(&iface->pin.IPin_iface); + unsigned int buffer_count = 1; unsigned int buffer_size = 16384; ALLOCATOR_PROPERTIES ret_props; @@ -1664,11 +1665,18 @@ static HRESULT WINAPI GSTOutPin_DecideBufferSize(struct strmbase_source *iface, WAVEFORMATEX *format = (WAVEFORMATEX *)pin->pin.pin.mt.pbFormat; buffer_size = format->nAvgBytesPerSec; } + else if (IsEqualGUID(&pin->pin.pin.mt.subtype, &MEDIASUBTYPE_MPEG1AudioPayload) + || IsEqualGUID(&pin->pin.pin.mt.subtype, &MEDIASUBTYPE_MP3)) + { + /* mpg123audiodec requires at least 3 buffers as it will keep + * references to the last 2 samples. */ + buffer_count = 3; + } /* We do need to drop any buffers that might have been sent with the old * caps, but this will be handled in parser_init_stream(). */ - props->cBuffers = max(props->cBuffers, 1); + props->cBuffers = max(props->cBuffers, buffer_count); props->cbBuffer = max(props->cbBuffer, buffer_size); props->cbAlign = max(props->cbAlign, 1); return IMemAllocator_SetProperties(allocator, props, &ret_props); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1384
From: Anton Baskanov <baskanov(a)gmail.com> --- include/mfapi.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/mfapi.h b/include/mfapi.h index ab336a3f4e8..24f2d6dc560 100644 --- a/include/mfapi.h +++ b/include/mfapi.h @@ -393,6 +393,7 @@ DEFINE_GUID(MFMediaType_Video, 0x73646976, 0x0000, 0x0010, 0x80, 0x0 DEFINE_GUID(MFSampleExtension_DecodeTimestamp, 0x73a954d4, 0x09e2, 0x4861, 0xbe, 0xfc, 0x94, 0xbd, 0x97, 0xc0, 0x8e, 0x6e); DEFINE_GUID(MFSampleExtension_CleanPoint, 0x9cdf01d8, 0xa0f0, 0x43ba, 0xb0, 0x77, 0xea, 0xa0, 0x6c, 0xbd, 0x72, 0x8a); +DEFINE_GUID(MFSampleExtension_Discontinuity, 0x9cdf01d9, 0xa0f0, 0x43ba, 0xb0, 0x77, 0xea, 0xa0, 0x6c, 0xbd, 0x72, 0x8a); DEFINE_GUID(MFSampleExtension_Timestamp, 0x1e436999, 0x69be, 0x4c7a, 0x93, 0x69, 0x70, 0x06, 0x8c, 0x02, 0x60, 0xcb); DEFINE_GUID(MFSampleExtension_Token, 0x8294da66, 0xf328, 0x4805, 0xb5, 0x51, 0x00, 0xde, 0xb4, 0xc5, 0x7a, 0x61); DEFINE_GUID(MFSampleExtension_3DVideo, 0xf86f97a4, 0xdd54, 0x4e2e, 0x9a, 0x5e, 0x55, 0xfc, 0x2d, 0x74, 0xa0, 0x05); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1384
From: Anton Baskanov <baskanov(a)gmail.com> This is required to avoid glitches when seeking, as some formats (e.g. MP3) may use data from previous frames. --- dlls/winegstreamer/unixlib.h | 1 + dlls/winegstreamer/wg_sample.c | 8 ++++++++ dlls/winegstreamer/wg_transform.c | 4 ++++ 3 files changed, 13 insertions(+) diff --git a/dlls/winegstreamer/unixlib.h b/dlls/winegstreamer/unixlib.h index be76d366cae..09aa68eb4cc 100644 --- a/dlls/winegstreamer/unixlib.h +++ b/dlls/winegstreamer/unixlib.h @@ -135,6 +135,7 @@ enum wg_sample_flag WG_SAMPLE_FLAG_HAS_PTS = 2, WG_SAMPLE_FLAG_HAS_DURATION = 4, WG_SAMPLE_FLAG_SYNC_POINT = 8, + WG_SAMPLE_FLAG_DISCONTINUITY = 0x10, }; struct wg_sample diff --git a/dlls/winegstreamer/wg_sample.c b/dlls/winegstreamer/wg_sample.c index f48639a6822..eb4af86c381 100644 --- a/dlls/winegstreamer/wg_sample.c +++ b/dlls/winegstreamer/wg_sample.c @@ -277,6 +277,8 @@ HRESULT wg_transform_push_mf(struct wg_transform *transform, IMFSample *sample, } 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; wg_sample_queue_begin_append(queue, wg_sample); hr = wg_transform_push_data(transform, wg_sample); @@ -320,6 +322,8 @@ HRESULT wg_transform_read_mf(struct wg_transform *transform, IMFSample *sample, IMFSample_SetSampleDuration(sample, wg_sample->duration); if (wg_sample->flags & WG_SAMPLE_FLAG_SYNC_POINT) IMFSample_SetUINT32(sample, &MFSampleExtension_CleanPoint, 1); + if (wg_sample->flags & WG_SAMPLE_FLAG_DISCONTINUITY) + IMFSample_SetUINT32(sample, &MFSampleExtension_Discontinuity, 1); if (SUCCEEDED(hr = IMFSample_ConvertToContiguousBuffer(sample, &buffer))) { @@ -354,6 +358,8 @@ HRESULT wg_transform_push_quartz(struct wg_transform *transform, struct wg_sampl if (IMediaSample_IsSyncPoint(sample->u.quartz.sample) == S_OK) wg_sample->flags |= WG_SAMPLE_FLAG_SYNC_POINT; + if (IMediaSample_IsDiscontinuity(sample->u.quartz.sample) == S_OK) + wg_sample->flags |= WG_SAMPLE_FLAG_DISCONTINUITY; wg_sample_queue_begin_append(queue, wg_sample); hr = wg_transform_push_data(transform, wg_sample); @@ -397,6 +403,8 @@ HRESULT wg_transform_read_quartz(struct wg_transform *transform, struct wg_sampl value = !!(wg_sample->flags & WG_SAMPLE_FLAG_SYNC_POINT); IMediaSample_SetSyncPoint(sample->u.quartz.sample, value); + value = !!(wg_sample->flags & WG_SAMPLE_FLAG_DISCONTINUITY); + IMediaSample_SetDiscontinuity(sample->u.quartz.sample, value); return S_OK; } diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c index 34b8d25c2e5..a52d7e1f722 100644 --- a/dlls/winegstreamer/wg_transform.c +++ b/dlls/winegstreamer/wg_transform.c @@ -648,6 +648,8 @@ NTSTATUS wg_transform_push_data(void *args) 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); gst_atomic_queue_push(transform->input_queue, buffer); params->result = S_OK; @@ -781,6 +783,8 @@ static NTSTATUS read_transform_output_data(GstBuffer *buffer, GstCaps *caps, gsi } if (!GST_BUFFER_FLAG_IS_SET(buffer, GST_BUFFER_FLAG_DELTA_UNIT)) sample->flags |= WG_SAMPLE_FLAG_SYNC_POINT; + if (GST_BUFFER_FLAG_IS_SET(buffer, GST_BUFFER_FLAG_DISCONT)) + sample->flags |= WG_SAMPLE_FLAG_DISCONTINUITY; if (needs_copy) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1384
On Fri Nov 18 17:24:26 2022 +0000, Anton Baskanov wrote:
changed this line in [version 3 of the diff](/wine/wine/-/merge_requests/1384/diffs?diff_id=19536&start_sha=f415bf550a1df2071c29f8b064607a68e57f6fa7#fcef7dc08ba245f492c5595be05a2d0fb7d03284_134_139) Done in v3.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1384#note_16392
This merge request was approved by Rémi Bernon. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1384
This merge request was approved by Zebediah Figura. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1384
participants (4)
-
Anton Baskanov -
Anton Baskanov (＠baskanov)
-
Rémi Bernon
-
Zebediah Figura (＠zfigura)