[PATCH v5 0/10] MR22: winegstreamer: Implement more sample attributes and timestamps.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=45988 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=47084 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=49715 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52183 -- v5: winegstreamer: Implement Receive() for MPEG audio decoder sink. quartz/tests: Add tests for MPEG audio decoder sample processing. winegstreamer: Create wg_transform in MPEG audio decoder. winegstreamer: Check whether transform is supported when creating MPEG audio decoder. quartz/tests: Don't fail tests when MPEG audio decoder creation fails. winegstreamer: Support MPEG-1 audio in wg_transform. winegstreamer: Convert MPEG-1 audio to a major type. winegstreamer: Only resize buffer when read is incomplete. winegstreamer: Support the MFSampleExtension_CleanPoint sample attribute. winegstreamer: Add timestamp and duration to struct wg_sample. https://gitlab.winehq.org/wine/wine/-/merge_requests/22
From: Rémi Bernon <rbernon(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=45988 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=47084 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=49715 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52183 Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/mf/tests/mf.c | 6 ------ dlls/winegstreamer/mfplat.c | 17 +++++++++++++++++ dlls/winegstreamer/unixlib.h | 5 +++++ dlls/winegstreamer/wg_transform.c | 25 +++++++++++++++++++++++++ 4 files changed, 47 insertions(+), 6 deletions(-) diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c index a9706aa7647..5abf910ad71 100644 --- a/dlls/mf/tests/mf.c +++ b/dlls/mf/tests/mf.c @@ -6500,19 +6500,15 @@ static void test_wma_decoder(void) ok(flags == 0, "got flags %#lx\n", flags); time = 0xdeadbeef; hr = IMFSample_GetSampleTime(sample, &time); - todo_wine ok(hr == S_OK, "GetSampleTime returned %#lx\n", hr); - todo_wine ok(time == i * 928798, "got time %I64d\n", time); duration = 0xdeadbeef; hr = IMFSample_GetSampleDuration(sample, &duration); - todo_wine ok(hr == S_OK, "GetSampleDuration returned %#lx\n", hr); if (output.dwStatus == MFT_OUTPUT_DATA_BUFFER_INCOMPLETE || broken(output.dwStatus == (MFT_OUTPUT_DATA_BUFFER_INCOMPLETE|7))) { ok(length == wmadec_block_size, "got length %lu\n", length); - todo_wine ok(duration == 928798, "got duration %I64d\n", duration); check_sample_pcm16(sample, wmadec_data, output_file, TRUE); wmadec_data += wmadec_block_size; @@ -7242,9 +7238,7 @@ static void test_h264_decoder(void) /* doesn't matter what frame rate we've selected, duration is defined by the stream */ duration = 0xdeadbeef; hr = IMFSample_GetSampleDuration(output.pSample, &duration); - todo_wine ok(hr == S_OK, "GetSampleDuration returned %#lx\n", hr); - todo_wine ok(duration - 333666 <= 2, "got duration %I64d\n", duration); /* Win8 and before pad the data with garbage instead of original diff --git a/dlls/winegstreamer/mfplat.c b/dlls/winegstreamer/mfplat.c index 97e27bb7301..9dcfc558963 100644 --- a/dlls/winegstreamer/mfplat.c +++ b/dlls/winegstreamer/mfplat.c @@ -903,6 +903,7 @@ HRESULT mf_create_wg_sample(IMFSample *sample, struct wg_sample **out) { DWORD current_length, max_length; struct mf_sample *mf_sample; + LONGLONG time, duration; BYTE *buffer; HRESULT hr; @@ -913,6 +914,17 @@ HRESULT mf_create_wg_sample(IMFSample *sample, struct wg_sample **out) if (FAILED(hr = IMFMediaBuffer_Lock(mf_sample->media_buffer, &buffer, &max_length, ¤t_length))) goto out; + if (SUCCEEDED(IMFSample_GetSampleTime(sample, &time))) + { + mf_sample->wg_sample.flags |= WG_SAMPLE_FLAG_HAS_PTS; + mf_sample->wg_sample.pts = time; + } + if (SUCCEEDED(IMFSample_GetSampleDuration(sample, &duration))) + { + mf_sample->wg_sample.flags |= WG_SAMPLE_FLAG_HAS_DURATION; + mf_sample->wg_sample.duration = duration; + } + IMFSample_AddRef((mf_sample->sample = sample)); mf_sample->wg_sample.data = buffer; mf_sample->wg_sample.size = current_length; @@ -937,6 +949,11 @@ void mf_destroy_wg_sample(struct wg_sample *wg_sample) IMFMediaBuffer_SetCurrentLength(mf_sample->media_buffer, wg_sample->size); IMFMediaBuffer_Release(mf_sample->media_buffer); + if (wg_sample->flags & WG_SAMPLE_FLAG_HAS_PTS) + IMFSample_SetSampleTime(mf_sample->sample, wg_sample->pts); + if (wg_sample->flags & WG_SAMPLE_FLAG_HAS_DURATION) + IMFSample_SetSampleDuration(mf_sample->sample, wg_sample->duration); + IMFSample_Release(mf_sample->sample); free(mf_sample); } diff --git a/dlls/winegstreamer/unixlib.h b/dlls/winegstreamer/unixlib.h index f4e2ea4966b..32e5b068187 100644 --- a/dlls/winegstreamer/unixlib.h +++ b/dlls/winegstreamer/unixlib.h @@ -114,10 +114,15 @@ struct wg_format enum wg_sample_flag { WG_SAMPLE_FLAG_INCOMPLETE = 1, + WG_SAMPLE_FLAG_HAS_PTS = 2, + WG_SAMPLE_FLAG_HAS_DURATION = 4, }; struct wg_sample { + /* timestamp and duration are in 100-nanosecond units. */ + UINT64 pts; + UINT64 duration; UINT32 flags; UINT32 max_size; UINT32 size; diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c index 49c7bfaa927..afee8478b9b 100644 --- a/dlls/winegstreamer/wg_transform.c +++ b/dlls/winegstreamer/wg_transform.c @@ -223,6 +223,9 @@ NTSTATUS wg_transform_create(void *args) * to match its expectations. */ transform->input_max_length = 16; + if (!(element = create_element("h264parse", "base")) + || !transform_append_element(transform, element, &first, &last)) + goto out; /* fallthrough */ case WG_MAJOR_TYPE_WMA: if (!(element = transform_find_element(GST_ELEMENT_FACTORY_TYPE_DECODER, src_caps, raw_caps)) @@ -362,6 +365,10 @@ NTSTATUS wg_transform_push_data(void *args) return STATUS_NO_MEMORY; } gst_buffer_fill(buffer, 0, sample->data, sample->size); + 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; gst_buffer_list_insert(transform->input, -1, buffer); GST_INFO("Copied %u bytes from sample %p to input buffer list", sample->size, sample); @@ -391,6 +398,24 @@ static NTSTATUS read_transform_output_data(GstBuffer *buffer, struct wg_sample * gst_buffer_unmap(buffer, &info); gst_buffer_resize(buffer, sample->size, -1); + if (GST_BUFFER_PTS_IS_VALID(buffer)) + { + sample->flags |= WG_SAMPLE_FLAG_HAS_PTS; + sample->pts = GST_BUFFER_PTS(buffer) / 100; + } + if (GST_BUFFER_DURATION_IS_VALID(buffer)) + { + GstClockTime duration = GST_BUFFER_DURATION(buffer) / 100; + + duration = (duration * sample->size) / info.size; + GST_BUFFER_DURATION(buffer) -= duration * 100; + if (GST_BUFFER_PTS_IS_VALID(buffer)) + GST_BUFFER_PTS(buffer) += duration * 100; + + sample->flags |= WG_SAMPLE_FLAG_HAS_DURATION; + sample->duration = duration; + } + GST_INFO("Copied %u bytes, sample %p, flags %#x", sample->size, sample, sample->flags); return STATUS_SUCCESS; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/22
From: Rémi Bernon <rbernon(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=45988 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=47084 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=49715 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52183 Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/mf/tests/mf.c | 2 -- dlls/winegstreamer/mfplat.c | 5 +++++ dlls/winegstreamer/unixlib.h | 1 + dlls/winegstreamer/wg_transform.c | 4 ++++ 4 files changed, 10 insertions(+), 2 deletions(-) diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c index 5abf910ad71..a32005863d3 100644 --- a/dlls/mf/tests/mf.c +++ b/dlls/mf/tests/mf.c @@ -6488,9 +6488,7 @@ static void test_wma_decoder(void) ok(status == 0, "got status %#lx\n", status); value = 0xdeadbeef; hr = IMFSample_GetUINT32(sample, &MFSampleExtension_CleanPoint, &value); - todo_wine ok(hr == S_OK, "GetUINT32 MFSampleExtension_CleanPoint returned %#lx\n", hr); - todo_wine ok(value == 1, "got MFSampleExtension_CleanPoint %u\n", value); hr = IMFSample_GetTotalLength(sample, &length); ok(hr == S_OK, "GetTotalLength returned %#lx\n", hr); diff --git a/dlls/winegstreamer/mfplat.c b/dlls/winegstreamer/mfplat.c index 9dcfc558963..ee3a1f5e024 100644 --- a/dlls/winegstreamer/mfplat.c +++ b/dlls/winegstreamer/mfplat.c @@ -904,6 +904,7 @@ HRESULT mf_create_wg_sample(IMFSample *sample, struct wg_sample **out) DWORD current_length, max_length; struct mf_sample *mf_sample; LONGLONG time, duration; + UINT32 value; BYTE *buffer; HRESULT hr; @@ -924,6 +925,8 @@ HRESULT mf_create_wg_sample(IMFSample *sample, struct wg_sample **out) mf_sample->wg_sample.flags |= WG_SAMPLE_FLAG_HAS_DURATION; mf_sample->wg_sample.duration = duration; } + if (SUCCEEDED(IMFSample_GetUINT32(sample, &MFSampleExtension_CleanPoint, &value)) && value) + mf_sample->wg_sample.flags |= WG_SAMPLE_FLAG_SYNC_POINT; IMFSample_AddRef((mf_sample->sample = sample)); mf_sample->wg_sample.data = buffer; @@ -953,6 +956,8 @@ void mf_destroy_wg_sample(struct wg_sample *wg_sample) IMFSample_SetSampleTime(mf_sample->sample, wg_sample->pts); if (wg_sample->flags & WG_SAMPLE_FLAG_HAS_DURATION) IMFSample_SetSampleDuration(mf_sample->sample, wg_sample->duration); + if (wg_sample->flags & WG_SAMPLE_FLAG_SYNC_POINT) + IMFSample_SetUINT32(mf_sample->sample, &MFSampleExtension_CleanPoint, 1); IMFSample_Release(mf_sample->sample); free(mf_sample); diff --git a/dlls/winegstreamer/unixlib.h b/dlls/winegstreamer/unixlib.h index 32e5b068187..b8dd2194bbe 100644 --- a/dlls/winegstreamer/unixlib.h +++ b/dlls/winegstreamer/unixlib.h @@ -116,6 +116,7 @@ enum wg_sample_flag WG_SAMPLE_FLAG_INCOMPLETE = 1, WG_SAMPLE_FLAG_HAS_PTS = 2, WG_SAMPLE_FLAG_HAS_DURATION = 4, + WG_SAMPLE_FLAG_SYNC_POINT = 8, }; struct wg_sample diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c index afee8478b9b..02f93bd95c0 100644 --- a/dlls/winegstreamer/wg_transform.c +++ b/dlls/winegstreamer/wg_transform.c @@ -369,6 +369,8 @@ NTSTATUS wg_transform_push_data(void *args) 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); gst_buffer_list_insert(transform->input, -1, buffer); GST_INFO("Copied %u bytes from sample %p to input buffer list", sample->size, sample); @@ -415,6 +417,8 @@ static NTSTATUS read_transform_output_data(GstBuffer *buffer, struct wg_sample * sample->flags |= WG_SAMPLE_FLAG_HAS_DURATION; sample->duration = duration; } + if (!GST_BUFFER_FLAG_IS_SET(buffer, GST_BUFFER_FLAG_DELTA_UNIT)) + sample->flags |= WG_SAMPLE_FLAG_SYNC_POINT; GST_INFO("Copied %u bytes, sample %p, flags %#x", sample->size, sample, sample->flags); return STATUS_SUCCESS; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/22
From: Rémi Bernon <rbernon(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=45988 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=47084 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=49715 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52183 Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winegstreamer/wg_transform.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c index 02f93bd95c0..3e285e8c943 100644 --- a/dlls/winegstreamer/wg_transform.c +++ b/dlls/winegstreamer/wg_transform.c @@ -398,7 +398,9 @@ static NTSTATUS read_transform_output_data(GstBuffer *buffer, struct wg_sample * memcpy(sample->data, info.data, sample->size); gst_buffer_unmap(buffer, &info); - gst_buffer_resize(buffer, sample->size, -1); + + if (sample->flags & WG_SAMPLE_FLAG_INCOMPLETE) + gst_buffer_resize(buffer, sample->size, -1); if (GST_BUFFER_PTS_IS_VALID(buffer)) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/22
From: Anton Baskanov <baskanov(a)gmail.com> Signed-off-by: Anton Baskanov <baskanov(a)gmail.com> Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/winegstreamer/mfplat.c | 1 + dlls/winegstreamer/quartz_parser.c | 149 ++++++++++++++++------------- dlls/winegstreamer/unixlib.h | 11 ++- dlls/winegstreamer/wg_format.c | 22 ++--- dlls/winegstreamer/wg_transform.c | 2 + dlls/winegstreamer/wm_reader.c | 4 + 6 files changed, 104 insertions(+), 85 deletions(-) diff --git a/dlls/winegstreamer/mfplat.c b/dlls/winegstreamer/mfplat.c index ee3a1f5e024..4b177f79924 100644 --- a/dlls/winegstreamer/mfplat.c +++ b/dlls/winegstreamer/mfplat.c @@ -661,6 +661,7 @@ IMFMediaType *mf_media_type_from_wg_format(const struct wg_format *format) { case WG_MAJOR_TYPE_H264: case WG_MAJOR_TYPE_WMA: + case WG_MAJOR_TYPE_MPEG1_AUDIO: FIXME("Format %u not implemented!\n", format->major_type); /* fallthrough */ case WG_MAJOR_TYPE_UNKNOWN: diff --git a/dlls/winegstreamer/quartz_parser.c b/dlls/winegstreamer/quartz_parser.c index 7b82f3efbc9..34848c0b503 100644 --- a/dlls/winegstreamer/quartz_parser.c +++ b/dlls/winegstreamer/quartz_parser.c @@ -111,50 +111,6 @@ static bool amt_from_wg_format_audio(AM_MEDIA_TYPE *mt, const struct wg_format * case WG_AUDIO_FORMAT_UNKNOWN: return false; - case WG_AUDIO_FORMAT_MPEG1_LAYER1: - case WG_AUDIO_FORMAT_MPEG1_LAYER2: - { - MPEG1WAVEFORMAT *wave_format; - - if (!(wave_format = CoTaskMemAlloc(sizeof(*wave_format)))) - return false; - memset(wave_format, 0, sizeof(*wave_format)); - - mt->subtype = MEDIASUBTYPE_MPEG1AudioPayload; - mt->cbFormat = sizeof(*wave_format); - mt->pbFormat = (BYTE *)wave_format; - wave_format->wfx.wFormatTag = WAVE_FORMAT_MPEG; - wave_format->wfx.nChannels = format->u.audio.channels; - wave_format->wfx.nSamplesPerSec = format->u.audio.rate; - wave_format->wfx.cbSize = sizeof(*wave_format) - sizeof(WAVEFORMATEX); - wave_format->fwHeadLayer = (format->u.audio.format == WG_AUDIO_FORMAT_MPEG1_LAYER1 ? 1 : 2); - return true; - } - - case WG_AUDIO_FORMAT_MPEG1_LAYER3: - { - MPEGLAYER3WAVEFORMAT *wave_format; - - if (!(wave_format = CoTaskMemAlloc(sizeof(*wave_format)))) - return false; - memset(wave_format, 0, sizeof(*wave_format)); - - mt->subtype = MEDIASUBTYPE_MP3; - mt->cbFormat = sizeof(*wave_format); - mt->pbFormat = (BYTE *)wave_format; - wave_format->wfx.wFormatTag = WAVE_FORMAT_MPEGLAYER3; - wave_format->wfx.nChannels = format->u.audio.channels; - wave_format->wfx.nSamplesPerSec = format->u.audio.rate; - wave_format->wfx.cbSize = sizeof(*wave_format) - sizeof(WAVEFORMATEX); - /* FIXME: We can't get most of the MPEG data from the caps. We may have - * to manually parse the header. */ - wave_format->wID = MPEGLAYER3_ID_MPEG; - wave_format->fdwFlags = MPEGLAYER3_FLAG_PADDING_ON; - wave_format->nFramesPerBlock = 1; - wave_format->nCodecDelay = 1393; - return true; - } - case WG_AUDIO_FORMAT_U8: case WG_AUDIO_FORMAT_S16LE: case WG_AUDIO_FORMAT_S24LE: @@ -238,6 +194,62 @@ static bool amt_from_wg_format_audio(AM_MEDIA_TYPE *mt, const struct wg_format * return false; } +static bool amt_from_wg_format_mpeg1_audio(AM_MEDIA_TYPE *mt, const struct wg_format *format) +{ + mt->majortype = MEDIATYPE_Audio; + mt->formattype = FORMAT_WaveFormatEx; + + switch (format->u.mpeg1_audio.layer) + { + case 1: + case 2: + { + MPEG1WAVEFORMAT *wave_format; + + if (!(wave_format = CoTaskMemAlloc(sizeof(*wave_format)))) + return false; + memset(wave_format, 0, sizeof(*wave_format)); + + mt->subtype = MEDIASUBTYPE_MPEG1AudioPayload; + mt->cbFormat = sizeof(*wave_format); + mt->pbFormat = (BYTE *)wave_format; + wave_format->wfx.wFormatTag = WAVE_FORMAT_MPEG; + wave_format->wfx.nChannels = format->u.mpeg1_audio.channels; + wave_format->wfx.nSamplesPerSec = format->u.mpeg1_audio.rate; + wave_format->wfx.cbSize = sizeof(*wave_format) - sizeof(WAVEFORMATEX); + wave_format->fwHeadLayer = format->u.mpeg1_audio.layer; + return true; + } + + case 3: + { + MPEGLAYER3WAVEFORMAT *wave_format; + + if (!(wave_format = CoTaskMemAlloc(sizeof(*wave_format)))) + return false; + memset(wave_format, 0, sizeof(*wave_format)); + + mt->subtype = MEDIASUBTYPE_MP3; + mt->cbFormat = sizeof(*wave_format); + mt->pbFormat = (BYTE *)wave_format; + wave_format->wfx.wFormatTag = WAVE_FORMAT_MPEGLAYER3; + wave_format->wfx.nChannels = format->u.mpeg1_audio.channels; + wave_format->wfx.nSamplesPerSec = format->u.mpeg1_audio.rate; + wave_format->wfx.cbSize = sizeof(*wave_format) - sizeof(WAVEFORMATEX); + /* FIXME: We can't get most of the MPEG data from the caps. We may have + * to manually parse the header. */ + wave_format->wID = MPEGLAYER3_ID_MPEG; + wave_format->fdwFlags = MPEGLAYER3_FLAG_PADDING_ON; + wave_format->nFramesPerBlock = 1; + wave_format->nCodecDelay = 1393; + return true; + } + } + + assert(0); + return false; +} + #define ALIGN(n, alignment) (((n) + (alignment) - 1) & ~((alignment) - 1)) unsigned int wg_format_get_max_size(const struct wg_format *format) @@ -312,15 +324,6 @@ unsigned int wg_format_get_max_size(const struct wg_format *format) case WG_AUDIO_FORMAT_F64LE: return rate * channels * 8; - case WG_AUDIO_FORMAT_MPEG1_LAYER1: - return 56000; - - case WG_AUDIO_FORMAT_MPEG1_LAYER2: - return 48000; - - case WG_AUDIO_FORMAT_MPEG1_LAYER3: - return 40000; - case WG_AUDIO_FORMAT_UNKNOWN: FIXME("Cannot guess maximum sample size for unknown audio format.\n"); return 0; @@ -328,6 +331,20 @@ unsigned int wg_format_get_max_size(const struct wg_format *format) break; } + case WG_MAJOR_TYPE_MPEG1_AUDIO: + switch (format->u.mpeg1_audio.layer) + { + case 1: + return 56000; + + case 2: + return 48000; + + case 3: + return 40000; + } + break; + case WG_MAJOR_TYPE_H264: case WG_MAJOR_TYPE_WMA: FIXME("Format %u not implemented!\n", format->major_type); @@ -431,6 +448,9 @@ bool amt_from_wg_format(AM_MEDIA_TYPE *mt, const struct wg_format *format, bool case WG_MAJOR_TYPE_UNKNOWN: return false; + case WG_MAJOR_TYPE_MPEG1_AUDIO: + return amt_from_wg_format_mpeg1_audio(mt, format); + case WG_MAJOR_TYPE_AUDIO: return amt_from_wg_format_audio(mt, format); @@ -526,17 +546,10 @@ static bool amt_to_wg_format_audio_mpeg1(const AM_MEDIA_TYPE *mt, struct wg_form return false; } - format->major_type = WG_MAJOR_TYPE_AUDIO; - format->u.audio.channels = audio_format->wfx.nChannels; - format->u.audio.rate = audio_format->wfx.nSamplesPerSec; - if (audio_format->fwHeadLayer == 1) - format->u.audio.format = WG_AUDIO_FORMAT_MPEG1_LAYER1; - else if (audio_format->fwHeadLayer == 2) - format->u.audio.format = WG_AUDIO_FORMAT_MPEG1_LAYER2; - else if (audio_format->fwHeadLayer == 3) - format->u.audio.format = WG_AUDIO_FORMAT_MPEG1_LAYER3; - else - return false; + format->major_type = WG_MAJOR_TYPE_MPEG1_AUDIO; + format->u.mpeg1_audio.channels = audio_format->wfx.nChannels; + format->u.mpeg1_audio.rate = audio_format->wfx.nSamplesPerSec; + format->u.mpeg1_audio.layer = audio_format->fwHeadLayer; return true; } @@ -555,10 +568,10 @@ static bool amt_to_wg_format_audio_mpeg1_layer3(const AM_MEDIA_TYPE *mt, struct return false; } - format->major_type = WG_MAJOR_TYPE_AUDIO; - format->u.audio.channels = audio_format->wfx.nChannels; - format->u.audio.rate = audio_format->wfx.nSamplesPerSec; - format->u.audio.format = WG_AUDIO_FORMAT_MPEG1_LAYER3; + format->major_type = WG_MAJOR_TYPE_MPEG1_AUDIO; + format->u.mpeg1_audio.channels = audio_format->wfx.nChannels; + format->u.mpeg1_audio.rate = audio_format->wfx.nSamplesPerSec; + format->u.mpeg1_audio.layer = 3; return true; } diff --git a/dlls/winegstreamer/unixlib.h b/dlls/winegstreamer/unixlib.h index b8dd2194bbe..5911278530d 100644 --- a/dlls/winegstreamer/unixlib.h +++ b/dlls/winegstreamer/unixlib.h @@ -37,6 +37,7 @@ struct wg_format WG_MAJOR_TYPE_UNKNOWN, WG_MAJOR_TYPE_VIDEO, WG_MAJOR_TYPE_AUDIO, + WG_MAJOR_TYPE_MPEG1_AUDIO, WG_MAJOR_TYPE_WMA, WG_MAJOR_TYPE_H264, } major_type; @@ -80,10 +81,6 @@ struct wg_format WG_AUDIO_FORMAT_S32LE, WG_AUDIO_FORMAT_F32LE, WG_AUDIO_FORMAT_F64LE, - - WG_AUDIO_FORMAT_MPEG1_LAYER1, - WG_AUDIO_FORMAT_MPEG1_LAYER2, - WG_AUDIO_FORMAT_MPEG1_LAYER3, } format; uint32_t channels; @@ -91,6 +88,12 @@ struct wg_format uint32_t rate; } audio; struct + { + uint32_t layer; + uint32_t rate; + uint32_t channels; + } mpeg1_audio; + struct { uint32_t version; uint32_t bitrate; diff --git a/dlls/winegstreamer/wg_format.c b/dlls/winegstreamer/wg_format.c index ff9238a6a69..608070b78e8 100644 --- a/dlls/winegstreamer/wg_format.c +++ b/dlls/winegstreamer/wg_format.c @@ -177,7 +177,7 @@ static void wg_format_from_video_info(struct wg_format *format, const GstVideoIn format->u.video.fps_d = GST_VIDEO_INFO_FPS_D(info); } -static void wg_format_from_caps_audio_mpeg(struct wg_format *format, const GstCaps *caps) +static void wg_format_from_caps_audio_mpeg1(struct wg_format *format, const GstCaps *caps) { const GstStructure *structure = gst_caps_get_structure(caps, 0); gint layer, channels, rate; @@ -198,17 +198,10 @@ static void wg_format_from_caps_audio_mpeg(struct wg_format *format, const GstCa return; } - format->major_type = WG_MAJOR_TYPE_AUDIO; - - if (layer == 1) - format->u.audio.format = WG_AUDIO_FORMAT_MPEG1_LAYER1; - else if (layer == 2) - format->u.audio.format = WG_AUDIO_FORMAT_MPEG1_LAYER2; - else if (layer == 3) - format->u.audio.format = WG_AUDIO_FORMAT_MPEG1_LAYER3; - - format->u.audio.channels = channels; - format->u.audio.rate = rate; + format->major_type = WG_MAJOR_TYPE_MPEG1_AUDIO; + format->u.mpeg1_audio.layer = layer; + format->u.mpeg1_audio.channels = channels; + format->u.mpeg1_audio.rate = rate; } static void wg_format_from_caps_video_cinepak(struct wg_format *format, const GstCaps *caps) @@ -263,7 +256,7 @@ void wg_format_from_caps(struct wg_format *format, const GstCaps *caps) } else if (!strcmp(name, "audio/mpeg")) { - wg_format_from_caps_audio_mpeg(format, caps); + wg_format_from_caps_audio_mpeg1(format, caps); } else if (!strcmp(name, "video/x-cinepak")) { @@ -501,6 +494,8 @@ GstCaps *wg_format_to_caps(const struct wg_format *format) { case WG_MAJOR_TYPE_UNKNOWN: return gst_caps_new_any(); + case WG_MAJOR_TYPE_MPEG1_AUDIO: + return NULL; case WG_MAJOR_TYPE_WMA: return wg_format_to_caps_wma(format); case WG_MAJOR_TYPE_H264: @@ -521,6 +516,7 @@ bool wg_format_compare(const struct wg_format *a, const struct wg_format *b) switch (a->major_type) { + case WG_MAJOR_TYPE_MPEG1_AUDIO: case WG_MAJOR_TYPE_WMA: case WG_MAJOR_TYPE_H264: GST_FIXME("Format %u not implemented!", a->major_type); diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c index 3e285e8c943..08e0b4aca32 100644 --- a/dlls/winegstreamer/wg_transform.c +++ b/dlls/winegstreamer/wg_transform.c @@ -236,6 +236,7 @@ NTSTATUS wg_transform_create(void *args) } break; + case WG_MAJOR_TYPE_MPEG1_AUDIO: case WG_MAJOR_TYPE_AUDIO: case WG_MAJOR_TYPE_VIDEO: case WG_MAJOR_TYPE_UNKNOWN: @@ -269,6 +270,7 @@ NTSTATUS wg_transform_create(void *args) case WG_MAJOR_TYPE_VIDEO: break; + case WG_MAJOR_TYPE_MPEG1_AUDIO: case WG_MAJOR_TYPE_H264: case WG_MAJOR_TYPE_WMA: case WG_MAJOR_TYPE_UNKNOWN: diff --git a/dlls/winegstreamer/wm_reader.c b/dlls/winegstreamer/wm_reader.c index 57ba8633a84..03adea8a318 100644 --- a/dlls/winegstreamer/wm_reader.c +++ b/dlls/winegstreamer/wm_reader.c @@ -1686,6 +1686,7 @@ HRESULT wm_reader_get_output_format_count(struct wm_reader *reader, DWORD output *count = ARRAY_SIZE(video_formats); break; + case WG_MAJOR_TYPE_MPEG1_AUDIO: case WG_MAJOR_TYPE_WMA: case WG_MAJOR_TYPE_H264: FIXME("Format %u not implemented!\n", format.major_type); @@ -1736,6 +1737,7 @@ HRESULT wm_reader_get_output_format(struct wm_reader *reader, DWORD output, format.u.audio.format = WG_AUDIO_FORMAT_S16LE; break; + case WG_MAJOR_TYPE_MPEG1_AUDIO: case WG_MAJOR_TYPE_WMA: case WG_MAJOR_TYPE_H264: FIXME("Format %u not implemented!\n", format.major_type); @@ -1815,6 +1817,8 @@ static const char *get_major_type_string(enum wg_major_type type) return "video"; case WG_MAJOR_TYPE_UNKNOWN: return "unknown"; + case WG_MAJOR_TYPE_MPEG1_AUDIO: + return "mpeg1-audio"; case WG_MAJOR_TYPE_WMA: return "wma"; case WG_MAJOR_TYPE_H264: -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/22
From: Anton Baskanov <baskanov(a)gmail.com> Signed-off-by: Anton Baskanov <baskanov(a)gmail.com> Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/winegstreamer/wg_format.c | 18 +++++++++++++++++- dlls/winegstreamer/wg_transform.c | 2 +- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/dlls/winegstreamer/wg_format.c b/dlls/winegstreamer/wg_format.c index 608070b78e8..5b3a5617ff1 100644 --- a/dlls/winegstreamer/wg_format.c +++ b/dlls/winegstreamer/wg_format.c @@ -331,6 +331,22 @@ static void wg_channel_mask_to_gst(GstAudioChannelPosition *positions, uint32_t } } +static GstCaps *wg_format_to_caps_mpeg1_audio(const struct wg_format *format) +{ + GstCaps *caps; + + if (!(caps = gst_caps_new_empty_simple("audio/mpeg"))) + return NULL; + + gst_caps_set_simple(caps, "mpegversion", G_TYPE_INT, 1, NULL); + gst_caps_set_simple(caps, "layer", G_TYPE_INT, format->u.mpeg1_audio.layer, NULL); + gst_caps_set_simple(caps, "rate", G_TYPE_INT, format->u.mpeg1_audio.rate, NULL); + gst_caps_set_simple(caps, "channels", G_TYPE_INT, format->u.mpeg1_audio.channels, NULL); + gst_caps_set_simple(caps, "parsed", G_TYPE_BOOLEAN, TRUE, NULL); + + return caps; +} + static GstCaps *wg_format_to_caps_audio(const struct wg_format *format) { GstAudioChannelPosition positions[32]; @@ -495,7 +511,7 @@ GstCaps *wg_format_to_caps(const struct wg_format *format) case WG_MAJOR_TYPE_UNKNOWN: return gst_caps_new_any(); case WG_MAJOR_TYPE_MPEG1_AUDIO: - return NULL; + return wg_format_to_caps_mpeg1_audio(format); case WG_MAJOR_TYPE_WMA: return wg_format_to_caps_wma(format); case WG_MAJOR_TYPE_H264: diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c index 08e0b4aca32..21392a82509 100644 --- a/dlls/winegstreamer/wg_transform.c +++ b/dlls/winegstreamer/wg_transform.c @@ -227,6 +227,7 @@ NTSTATUS wg_transform_create(void *args) || !transform_append_element(transform, element, &first, &last)) goto out; /* fallthrough */ + case WG_MAJOR_TYPE_MPEG1_AUDIO: case WG_MAJOR_TYPE_WMA: if (!(element = transform_find_element(GST_ELEMENT_FACTORY_TYPE_DECODER, src_caps, raw_caps)) || !transform_append_element(transform, element, &first, &last)) @@ -236,7 +237,6 @@ NTSTATUS wg_transform_create(void *args) } break; - case WG_MAJOR_TYPE_MPEG1_AUDIO: case WG_MAJOR_TYPE_AUDIO: case WG_MAJOR_TYPE_VIDEO: case WG_MAJOR_TYPE_UNKNOWN: -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/22
From: Anton Baskanov <baskanov(a)gmail.com> Signed-off-by: Anton Baskanov <baskanov(a)gmail.com> Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/quartz/tests/mpegaudio.c | 60 +++++++++-------------------------- 1 file changed, 15 insertions(+), 45 deletions(-) diff --git a/dlls/quartz/tests/mpegaudio.c b/dlls/quartz/tests/mpegaudio.c index 296f5f479a0..a43adfbbd6d 100644 --- a/dlls/quartz/tests/mpegaudio.c +++ b/dlls/quartz/tests/mpegaudio.c @@ -216,16 +216,9 @@ static void check_interface_(unsigned int line, void *iface_ptr, REFIID iid, BOO static void test_interfaces(void) { - IBaseFilter *filter; + IBaseFilter *filter = create_mpeg_audio_codec(); IPin *pin; - filter = create_mpeg_audio_codec(); - if (!filter) - { - skip("Failed to create MPEG audio decoder instance, skipping tests.\n"); - return; - } - check_interface(filter, &IID_IBaseFilter, TRUE); check_interface(filter, &IID_IMediaFilter, TRUE); check_interface(filter, &IID_IPersist, TRUE); @@ -322,11 +315,6 @@ static void test_aggregation(void) hr = CoCreateInstance(&CLSID_CMpegAudioCodec, &test_outer, CLSCTX_INPROC_SERVER, &IID_IUnknown, (void **)&unk); ok(hr == S_OK, "Got hr %#lx.\n", hr); - if (FAILED(hr)) - { - skip("Failed to create MPEG audio decoder instance, skipping tests.\n"); - return; - } ok(outer_ref == 1, "Got unexpected refcount %ld.\n", outer_ref); ok(unk != &test_outer, "Returned IUnknown should not be outer IUnknown.\n"); ref = get_refcount(unk); @@ -372,18 +360,11 @@ static void test_aggregation(void) static void test_unconnected_filter_state(void) { - IBaseFilter *filter; + IBaseFilter *filter = create_mpeg_audio_codec(); FILTER_STATE state; HRESULT hr; ULONG ref; - filter = create_mpeg_audio_codec(); - if (!filter) - { - skip("Failed to create MPEG audio decoder instance, skipping tests.\n"); - return; - } - hr = IBaseFilter_GetState(filter, 0, &state); ok(hr == S_OK, "Got hr %#lx.\n", hr); ok(state == State_Stopped, "Got state %u.\n", state); @@ -436,19 +417,12 @@ static void test_unconnected_filter_state(void) static void test_enum_pins(void) { - IBaseFilter *filter; + IBaseFilter *filter = create_mpeg_audio_codec(); IEnumPins *enum1, *enum2; ULONG count, ref; IPin *pins[3]; HRESULT hr; - filter = create_mpeg_audio_codec(); - if (!filter) - { - skip("Failed to create MPEG audio decoder instance, skipping tests.\n"); - return; - } - ref = get_refcount(filter); ok(ref == 1, "Got unexpected refcount %ld.\n", ref); @@ -564,19 +538,12 @@ static void test_enum_pins(void) static void test_find_pin(void) { - IBaseFilter *filter; + IBaseFilter *filter = create_mpeg_audio_codec(); IEnumPins *enum_pins; IPin *pin, *pin2; HRESULT hr; ULONG ref; - filter = create_mpeg_audio_codec(); - if (!filter) - { - skip("Failed to create MPEG audio decoder instance, skipping tests.\n"); - return; - } - hr = IBaseFilter_EnumPins(filter, &enum_pins); ok(hr == S_OK, "Got hr %#lx.\n", hr); @@ -612,7 +579,7 @@ static void test_find_pin(void) static void test_pin_info(void) { - IBaseFilter *filter; + IBaseFilter *filter = create_mpeg_audio_codec(); PIN_DIRECTION dir; PIN_INFO info; HRESULT hr; @@ -620,13 +587,6 @@ static void test_pin_info(void) ULONG ref; IPin *pin; - filter = create_mpeg_audio_codec(); - if (!filter) - { - skip("Failed to create MPEG audio decoder instance, skipping tests.\n"); - return; - } - hr = IBaseFilter_FindPin(filter, L"In", &pin); ok(hr == S_OK, "Got hr %#lx.\n", hr); ref = get_refcount(filter); @@ -1393,8 +1353,18 @@ static void test_connect_pin(void) START_TEST(mpegaudio) { + IBaseFilter *filter; + CoInitialize(NULL); + if (FAILED(CoCreateInstance(&CLSID_CMpegAudioCodec, NULL, CLSCTX_INPROC_SERVER, + &IID_IBaseFilter, (void **)&filter))) + { + skip("Failed to create MPEG audio decoder instance.\n"); + return; + } + IBaseFilter_Release(filter); + test_interfaces(); test_aggregation(); test_unconnected_filter_state(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/22
From: Anton Baskanov <baskanov(a)gmail.com> Signed-off-by: Anton Baskanov <baskanov(a)gmail.com> Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/winegstreamer/quartz_transform.c | 31 +++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/dlls/winegstreamer/quartz_transform.c b/dlls/winegstreamer/quartz_transform.c index 375c549aad5..c383e6774d2 100644 --- a/dlls/winegstreamer/quartz_transform.c +++ b/dlls/winegstreamer/quartz_transform.c @@ -21,6 +21,7 @@ #include "gst_private.h" WINE_DEFAULT_DEBUG_CHANNEL(quartz); +WINE_DECLARE_DEBUG_CHANNEL(winediag); struct transform { @@ -296,9 +297,39 @@ static const struct transform_ops mpeg_audio_codec_transform_ops = HRESULT mpeg_audio_codec_create(IUnknown *outer, IUnknown **out) { + static const struct wg_format output_format = + { + .major_type = WG_MAJOR_TYPE_AUDIO, + .u.audio = + { + .format = WG_AUDIO_FORMAT_S16LE, + .channel_mask = 1, + .channels = 1, + .rate = 44100, + }, + }; + static const struct wg_format input_format = + { + .major_type = WG_MAJOR_TYPE_MPEG1_AUDIO, + .u.mpeg1_audio = + { + .layer = 2, + .channels = 1, + .rate = 44100, + }, + }; + struct wg_transform *transform; struct transform *object; HRESULT hr; + transform = wg_transform_create(&input_format, &output_format); + if (!transform) + { + ERR_(winediag)("GStreamer doesn't support MPEG-1 audio decoding, please install appropriate plugins.\n"); + return E_FAIL; + } + wg_transform_destroy(transform); + hr = transform_create(outer, &CLSID_CMpegAudioCodec, &mpeg_audio_codec_transform_ops, &object); if (FAILED(hr)) return hr; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/22
From: Anton Baskanov <baskanov(a)gmail.com> Signed-off-by: Anton Baskanov <baskanov(a)gmail.com> Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/winegstreamer/quartz_transform.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/dlls/winegstreamer/quartz_transform.c b/dlls/winegstreamer/quartz_transform.c index c383e6774d2..ab61e5eab94 100644 --- a/dlls/winegstreamer/quartz_transform.c +++ b/dlls/winegstreamer/quartz_transform.c @@ -30,6 +30,8 @@ struct transform struct strmbase_sink sink; struct strmbase_source source; + struct wg_transform *transform; + const struct transform_ops *ops; }; @@ -70,10 +72,21 @@ static void transform_destroy(struct strmbase_filter *iface) static HRESULT transform_init_stream(struct strmbase_filter *iface) { struct transform *filter = impl_from_strmbase_filter(iface); + struct wg_format input_format, output_format; HRESULT hr; if (filter->source.pin.peer) { + if (!amt_to_wg_format(&filter->sink.pin.mt, &input_format)) + return E_FAIL; + + if (!amt_to_wg_format(&filter->source.pin.mt, &output_format)) + return E_FAIL; + + filter->transform = wg_transform_create(&input_format, &output_format); + if (!filter->transform) + return E_FAIL; + hr = IMemAllocator_Commit(filter->source.pAllocator); if (FAILED(hr)) ERR("Failed to commit allocator, hr %#lx.\n", hr); @@ -87,8 +100,12 @@ static HRESULT transform_cleanup_stream(struct strmbase_filter *iface) struct transform *filter = impl_from_strmbase_filter(iface); if (filter->source.pin.peer) + { IMemAllocator_Decommit(filter->source.pAllocator); + wg_transform_destroy(filter->transform); + } + return S_OK; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/22
From: Anton Baskanov <baskanov(a)gmail.com> Signed-off-by: Anton Baskanov <baskanov(a)gmail.com> Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/quartz/tests/mpegaudio.c | 75 +++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/dlls/quartz/tests/mpegaudio.c b/dlls/quartz/tests/mpegaudio.c index a43adfbbd6d..b34f09e137f 100644 --- a/dlls/quartz/tests/mpegaudio.c +++ b/dlls/quartz/tests/mpegaudio.c @@ -794,6 +794,7 @@ struct testfilter struct strmbase_source source; struct strmbase_sink sink; const AM_MEDIA_TYPE *mt; + unsigned int got_sample; }; static inline struct testfilter *impl_from_strmbase_filter(struct strmbase_filter *iface) @@ -867,11 +868,27 @@ static HRESULT testsink_connect(struct strmbase_sink *iface, IPin *peer, const A return S_OK; } +static HRESULT WINAPI testsink_Receive(struct strmbase_sink *iface, IMediaSample *sample) +{ + struct testfilter *filter = impl_from_strmbase_filter(iface->pin.filter); + LONG size; + + ++filter->got_sample; + + size = IMediaSample_GetSize(sample); + ok(size == 3072, "Got size %lu.\n", size); + size = IMediaSample_GetActualDataLength(sample); + ok(size == 768, "Got valid size %lu.\n", size); + + return S_OK; +} + static const struct strmbase_sink_ops testsink_ops = { .base.pin_query_interface = testsink_query_interface, .base.pin_get_media_type = testsink_get_media_type, .sink_connect = testsink_connect, + .pfnReceive = testsink_Receive, }; static void testfilter_init(struct testfilter *filter) @@ -1040,6 +1057,62 @@ static void test_source_allocator(IFilterGraph2 *graph, IMediaControl *control, IFilterGraph2_Disconnect(graph, &testsource->source.pin.IPin_iface); } +static void test_sample_processing(IMediaControl *control, IMemInputPin *input, struct testfilter *sink) +{ + IMemAllocator *allocator; + IMediaSample *sample; + HRESULT hr; + BYTE *data; + LONG size; + + hr = IMemInputPin_ReceiveCanBlock(input); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + hr = IMemInputPin_GetAllocator(input, &allocator); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + hr = IMediaControl_Pause(control); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + hr = IMemAllocator_GetBuffer(allocator, &sample, NULL, NULL, 0); + ok(hr == VFW_E_NOT_COMMITTED, "Got hr %#lx.\n", hr); + + hr = IMemAllocator_Commit(allocator); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IMemAllocator_GetBuffer(allocator, &sample, NULL, NULL, 0); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + hr = IMediaSample_GetPointer(sample, &data); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + size = IMediaSample_GetSize(sample); + ok(size == 256, "Got size %ld.\n", size); + memset(data, 0, 48); + data[0] = 0xff; + data[1] = 0xff; + data[2] = 0x18; + data[3] = 0xc4; + hr = IMediaSample_SetActualDataLength(sample, 48); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + hr = IMemInputPin_Receive(input, sample); + todo_wine ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IMemInputPin_Receive(input, sample); + todo_wine ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IMemInputPin_Receive(input, sample); + todo_wine ok(hr == S_OK, "Got hr %#lx.\n", hr); + todo_wine ok(sink->got_sample >= 1, "Got %u calls to Receive().\n", sink->got_sample); + sink->got_sample = 0; + + hr = IMediaControl_Stop(control); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + hr = IMemInputPin_Receive(input, sample); + todo_wine ok(hr == VFW_E_WRONG_STATE, "Got hr %#lx.\n", hr); + + IMediaSample_Release(sample); + IMemAllocator_Release(allocator); +} + static void test_connect_pin(void) { IBaseFilter *filter = create_mpeg_audio_codec(); @@ -1233,6 +1306,8 @@ static void test_connect_pin(void) hr = IMediaControl_Stop(control); ok(hr == S_OK, "Got hr %#lx.\n", hr); + test_sample_processing(control, meminput, &testsink); + hr = IFilterGraph2_Disconnect(graph, source); ok(hr == S_OK, "Got hr %#lx.\n", hr); hr = IFilterGraph2_Disconnect(graph, source); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/22
From: Anton Baskanov <baskanov(a)gmail.com> Signed-off-by: Anton Baskanov <baskanov(a)gmail.com> Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/quartz/tests/mpegaudio.c | 10 ++-- dlls/winegstreamer/quartz_transform.c | 86 +++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 5 deletions(-) diff --git a/dlls/quartz/tests/mpegaudio.c b/dlls/quartz/tests/mpegaudio.c index b34f09e137f..105469edaf1 100644 --- a/dlls/quartz/tests/mpegaudio.c +++ b/dlls/quartz/tests/mpegaudio.c @@ -1095,19 +1095,19 @@ static void test_sample_processing(IMediaControl *control, IMemInputPin *input, ok(hr == S_OK, "Got hr %#lx.\n", hr); hr = IMemInputPin_Receive(input, sample); - todo_wine ok(hr == S_OK, "Got hr %#lx.\n", hr); + ok(hr == S_OK, "Got hr %#lx.\n", hr); hr = IMemInputPin_Receive(input, sample); - todo_wine ok(hr == S_OK, "Got hr %#lx.\n", hr); + ok(hr == S_OK, "Got hr %#lx.\n", hr); hr = IMemInputPin_Receive(input, sample); - todo_wine ok(hr == S_OK, "Got hr %#lx.\n", hr); - todo_wine ok(sink->got_sample >= 1, "Got %u calls to Receive().\n", sink->got_sample); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + ok(sink->got_sample >= 1, "Got %u calls to Receive().\n", sink->got_sample); sink->got_sample = 0; hr = IMediaControl_Stop(control); ok(hr == S_OK, "Got hr %#lx.\n", hr); hr = IMemInputPin_Receive(input, sample); - todo_wine ok(hr == VFW_E_WRONG_STATE, "Got hr %#lx.\n", hr); + ok(hr == VFW_E_WRONG_STATE, "Got hr %#lx.\n", hr); IMediaSample_Release(sample); IMemAllocator_Release(allocator); diff --git a/dlls/winegstreamer/quartz_transform.c b/dlls/winegstreamer/quartz_transform.c index ab61e5eab94..5019cc6032b 100644 --- a/dlls/winegstreamer/quartz_transform.c +++ b/dlls/winegstreamer/quartz_transform.c @@ -20,6 +20,8 @@ #include "gst_private.h" +#include "mferror.h" + WINE_DEFAULT_DEBUG_CHANNEL(quartz); WINE_DECLARE_DEBUG_CHANNEL(winediag); @@ -137,10 +139,94 @@ static HRESULT transform_sink_query_interface(struct strmbase_pin *pin, REFIID i return S_OK; } +static HRESULT WINAPI transform_sink_receive(struct strmbase_sink *pin, IMediaSample *sample) +{ + struct transform *filter = impl_from_strmbase_filter(pin->pin.filter); + struct wg_sample input_wg_sample = {0}; + HRESULT hr; + + /* We do not expect pin connection state to change while the filter is + * running. This guarantee is necessary, since otherwise we would have to + * take the filter lock, and we can't take the filter lock from a streaming + * thread. */ + if (!filter->source.pMemInputPin) + { + WARN("Source is not connected, returning VFW_E_NOT_CONNECTED.\n"); + return VFW_E_NOT_CONNECTED; + } + + if (filter->filter.state == State_Stopped) + return VFW_E_WRONG_STATE; + + if (filter->sink.flushing) + return S_FALSE; + + input_wg_sample.max_size = IMediaSample_GetSize(sample); + input_wg_sample.size = IMediaSample_GetActualDataLength(sample); + + hr = IMediaSample_GetPointer(sample, &input_wg_sample.data); + if (FAILED(hr)) + return hr; + + hr = wg_transform_push_data(filter->transform, &input_wg_sample); + if (FAILED(hr)) + return hr; + + for (;;) + { + struct wg_sample output_wg_sample = {0}; + IMediaSample *output_sample; + + hr = IMemAllocator_GetBuffer(filter->source.pAllocator, &output_sample, NULL, NULL, 0); + if (FAILED(hr)) + return hr; + + output_wg_sample.max_size = IMediaSample_GetSize(output_sample); + + hr = IMediaSample_GetPointer(output_sample, &output_wg_sample.data); + if (FAILED(hr)) + { + IMediaSample_Release(output_sample); + return hr; + } + + hr = wg_transform_read_data(filter->transform, &output_wg_sample); + if (hr == MF_E_TRANSFORM_NEED_MORE_INPUT) + { + IMediaSample_Release(output_sample); + break; + } + if (FAILED(hr)) + { + IMediaSample_Release(output_sample); + return hr; + } + + hr = IMediaSample_SetActualDataLength(output_sample, output_wg_sample.size); + if (FAILED(hr)) + { + IMediaSample_Release(output_sample); + return hr; + } + + hr = IMemInputPin_Receive(filter->source.pMemInputPin, output_sample); + if (FAILED(hr)) + { + IMediaSample_Release(output_sample); + return hr; + } + + IMediaSample_Release(output_sample); + } + + return S_OK; +} + static const struct strmbase_sink_ops sink_ops = { .base.pin_query_accept = transform_sink_query_accept, .base.pin_query_interface = transform_sink_query_interface, + .pfnReceive = transform_sink_receive, }; static HRESULT transform_source_query_accept(struct strmbase_pin *pin, const AM_MEDIA_TYPE *mt) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/22
I updated the MR to resolve conflicts and include the other winegstreamer series that has been sent on the ML and save another unnecessary iteration. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/22#note_718
On Fri May 6 16:32:08 2022 +0000, **** wrote:
Zebediah Figura (she/her) replied on the mailing list: ``` On 5/6/22 02:01, Rémi Bernon (@rbernon) wrote:
Actually, for that matter, why do we need to store the wg_format on the PE side? Can't we just store it on the unix side?
(Maybe even check it in the chain function instead of in read_data, and store it in a flag on the object with gst_mini_object_set_qdata(), and then that'd remove the need to allocate GstSample objects. I don't remember if there was another impetus for using those?)
I don't see what benefit it would have, we need to return the information about the new format to the client anyway so that it can update the properties it exposes to the MF caller.
Er, right, I forgot we still need to store the format anyway, so ignore that part :-)
Still, the first part seems reasonable, unless I'm missing something?
Or, frankly, we could make it output-only, and set the "format changed" flag entirely on the client side. The way things currently are, the logic is kind of split in two, and it feels awkward.
There are cases, like when the application calls ProcessMessage with BEGIN_STREAMING, where we want to reset the format and receive format change notification, again. Having the stream format stored on the MF transform side lets us do that by reseting it to its default, and trigger the format change again. Sure, then the question becomes, can we move the entire logic to the client side? That would require a separate call to retrieve the type of the current sample, given the below (or alternatively some extra queuing on the client side, but that seems more complex than necessary), but structurally it feels a lot less awkward.
Call of Duty: Black Ops 3 depends on these notifications.
I'll try something, but that game, and the others using the H264 have been really sensitive to tiny behavior changes.
@@ -427,7 +462,18 @@ NTSTATUS wg_transform_read_data(void *args) return STATUS_SUCCESS; }
- if ((status = read_transform_output_data(transform->output_buffer, sample))) + if (sample->format && (caps = gst_sample_get_caps(transform->output_sample))) + { + wg_format_from_caps(&format, caps); + if (!wg_format_compare(&format, sample->format)) + { + *sample->format = format; + params->result = MF_E_TRANSFORM_STREAM_CHANGE; + return STATUS_SUCCESS; + } + }
This looks wrong; aren't we dropping the sample data on the floor?
No? We're not releasing output_sample there.
Sorry, not dropping it on the floor exactly, but we're also not filling the buffer, whereas the mfplat code (and the tests) looks like it expects the buffer to be filled.
The tests check that the returned sample, on format change, is empty, which should be the case, and mf_destroy_wg_sample will convert the wg sample properties back to the MF side.
After a stream change you need, and can, call ProcessOutput once more, immediately, to get the sample data.
Okay, I missed that. That's a bizarre API design; I don't know why they'd send an empty sample instead of no sample at all... ``` Sure, then the question becomes, can we move the entire logic to the client side? That would require a separate call to retrieve the type of the current sample, given the below (or alternatively some extra queuing on the client side, but that seems more complex than necessary), but structurally it feels a lot less awkward.
It would only makes things more complicated, with an additional entry point which you would have to call on every sample to get their format, back to the client to compare them, and then only eventually read it. I don't see how this is awkward at all, the client optionally provides the format it believes the stream has and that it expects to be for the output samples, the transform returns either with a read success and unmodified format, or a format change result and the update format. It's overall a dozen lines of code. Having an additional entry point will be much larger change, with wrappers, etc. If at any point we need all this to be thread safe, because right now it's definitely not and I hope we won't need it, the separate entry point would make everything much more difficult whereas with this design you could just lock the read_data and either return a format change or read success atomically.
Okay, I missed that. That's a bizarre API design; I don't know why they'd send an empty sample instead of no sample at all...
The sample is not sent by the transform, it is provided by the caller, and returned to it. The transform only fills it when there's data. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/22#note_729
On 5/6/22 12:20, Rémi Bernon (@rbernon) wrote:
Sure, then the question becomes, can we move the entire logic to the client side? That would require a separate call to retrieve the type of the current sample, given the below (or alternatively some extra queuing on the client side, but that seems more complex than necessary), but structurally it feels a lot less awkward.
It would only makes things more complicated, with an additional entry point which you would have to call on every sample to get their format, back to the client to compare them, and then only eventually read it.
I don't see how this is awkward at all, the client optionally provides the format it believes the stream has and that it expects to be for the output samples, the transform returns either with a read success and unmodified format, or a format change result and the update format.
It's overall a dozen lines of code. Having an additional entry point will be much larger change, with wrappers, etc. I've already tried to explain why this feels awkward to me. I'd rather have two function calls that make sense structurally, than try to shove everything into one call, regardless of how many lines it takes.
(For that matter, we don't even need two function calls. With the current state of things, all we need to do is pass a maximum size of zero to the read_data function. It's not clear how that'll work with a zero-copy infrastructure, but we could easily just add a "peek" flag.)
If at any point we need all this to be thread safe, because right now it's definitely not and I hope we won't need it, the separate entry point would make everything much more difficult whereas with this design you could just lock the read_data and either return a format change or read success atomically.
I don't see how thread safety makes this any more complex?
On Fri May 6 17:46:13 2022 +0000, **** wrote:
Zebediah Figura (she/her) replied on the mailing list: ``` On 5/6/22 12:20, Rémi Bernon (@rbernon) wrote:
Sure, then the question becomes, can we move the entire logic to the client side? That would require a separate call to retrieve the type of the current sample, given the below (or alternatively some extra queuing on the client side, but that seems more complex than necessary), but structurally it feels a lot less awkward.
It would only makes things more complicated, with an additional entry point which you would have to call on every sample to get their format, back to the client to compare them, and then only eventually read it.
I don't see how this is awkward at all, the client optionally provides the format it believes the stream has and that it expects to be for the output samples, the transform returns either with a read success and unmodified format, or a format change result and the update format.
It's overall a dozen lines of code. Having an additional entry point will be much larger change, with wrappers, etc. I've already tried to explain why this feels awkward to me. I'd rather have two function calls that make sense structurally, than try to shove everything into one call, regardless of how many lines it takes. (For that matter, we don't even need two function calls. With the current state of things, all we need to do is pass a maximum size of zero to the read_data function. It's not clear how that'll work with a zero-copy infrastructure, but we could easily just add a "peek" flag.) If at any point we need all this to be thread safe, because right now it's definitely not and I hope we won't need it, the separate entry point would make everything much more difficult whereas with this design you could just lock the read_data and either return a format change or read success atomically. I don't see how thread safety makes this any more complex?
I've already tried to explain why this feels awkward to me. I'd rather have two function calls that make sense structurally, than try to shove everything into one call, regardless of how many lines it takes.
I really don't see how it make more sense "structurally" one way or the other, it's imho only a matter of taste.
(For that matter, we don't even need two function calls. With the current state of things, all we need to do is pass a maximum size of zero to the read_data function. It's not clear how that'll work with a zero-copy infrastructure, but we could easily just add a "peek" flag.)
Yes, and as I already have done some investigation and implementation of zero copy on the output side, I can say already that it will make things much more complicated. We cannot hold on the samples that were provided by the client, so having a single unix entry point that is supposed to atomically take a buffer, checks the expected format, eventually uses it to decode, and return it to the client either filled or empty with the new format, is going to be much easier to work with. Output data is generally going to be produced, synchronously, when we push the input buffers to the pipeline, from the gst_pad_push_list call. To achieve zero copy, we need to prepare the output buffer beforehand, knowing its expected format, put it in a pool that we propose to the decoder as a reply to its allocation query. A caps change can also happen during that call, and in which case we will have to release the buffer instead, as it would not be compatible anymore, but also because we don't want it to be used and return the format change instead, allocate a unix one for the decoder and return from the call. Only after that call, we can tell whether format has changed, and if not if our buffer has been used or if we need to copy from the output sample buffer instead. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/22#note_732
participants (3)
-
Anton Baskanov -
Rémi Bernon
-
Zebediah Figura (she/her)