[PATCH v2 0/5] MR1384: winegstreamer: Fix some quartz transform issues.
- v2: - Allocate 3 buffers instead of 100. - Add support for MFSampleExtension_Discontinuity. -- v2: 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: Release the samples 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> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53342 --- dlls/winegstreamer/quartz_transform.c | 1 + 1 file changed, 1 insertion(+) diff --git a/dlls/winegstreamer/quartz_transform.c b/dlls/winegstreamer/quartz_transform.c index ccdec9454a6..61044ac78dc 100644 --- a/dlls/winegstreamer/quartz_transform.c +++ b/dlls/winegstreamer/quartz_transform.c @@ -131,6 +131,7 @@ static HRESULT transform_cleanup_stream(struct strmbase_filter *iface) EnterCriticalSection(&filter->filter.stream_cs); wg_transform_destroy(filter->transform); + wg_sample_queue_flush(filter->sample_queue, true); LeaveCriticalSection(&filter->filter.stream_cs); } -- 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
Rémi Bernon (@rbernon) commented about dlls/winegstreamer/quartz_transform.c:
EnterCriticalSection(&filter->filter.stream_cs); wg_transform_destroy(filter->transform); + wg_sample_queue_flush(filter->sample_queue, true);
Any reason to keep the queue alive after this? You could move the `wg_sample_queue_destroy` here it would flush it too. Maybe then move `wg_sample_queue_create` to `transform_init_stream` to keep the symmetry. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1384#note_16363
participants (3)
-
Anton Baskanov -
Anton Baskanov (＠baskanov)
-
Rémi Bernon