Signed-off-by: Anton Baskanov baskanov@gmail.com --- dlls/amstream/audiostream.c | 41 +++++++++- dlls/amstream/tests/amstream.c | 142 +++++++++++++++++++++++++++++++++ 2 files changed, 181 insertions(+), 2 deletions(-)
diff --git a/dlls/amstream/audiostream.c b/dlls/amstream/audiostream.c index 8a16512dcd..abc82f3a0e 100644 --- a/dlls/amstream/audiostream.c +++ b/dlls/amstream/audiostream.c @@ -183,6 +183,7 @@ struct audio_stream IPin *peer; IMemAllocator *allocator; AM_MEDIA_TYPE mt; + WAVEFORMATEX format; };
static inline struct audio_stream *impl_from_IAMMediaStream(IAMMediaStream *iface) @@ -512,13 +513,49 @@ static HRESULT WINAPI audio_IAudioMediaStream_GetFormat(IAudioMediaStream *iface return S_OK; }
+static BOOL is_format_compatible(const WAVEFORMATEX *current_format, const WAVEFORMATEX *format) +{ + if (format->wFormatTag != WAVE_FORMAT_PCM) + return FALSE; + + if (current_format->wFormatTag == WAVE_FORMAT_PCM) + { + if (current_format->nChannels != format->nChannels) + return FALSE; + + if (current_format->nSamplesPerSec != format->nSamplesPerSec) + return FALSE; + + if (current_format->nAvgBytesPerSec != format->nAvgBytesPerSec) + return FALSE; + + if (current_format->nBlockAlign != format->nBlockAlign) + return FALSE; + + if (current_format->wBitsPerSample != format->wBitsPerSample) + return FALSE; + } + return TRUE; +} + static HRESULT WINAPI audio_IAudioMediaStream_SetFormat(IAudioMediaStream *iface, const WAVEFORMATEX *wave_format) { struct audio_stream *This = impl_from_IAudioMediaStream(iface); + const WAVEFORMATEX *ref_format;
- FIXME("(%p/%p)->(%p) stub!\n", iface, This, wave_format); + TRACE("(%p/%p)->(%p)\n", iface, This, wave_format);
- return E_NOTIMPL; + if (!wave_format) + return E_POINTER; + + ref_format = This->peer ? (WAVEFORMATEX *)This->mt.pbFormat : &This->format; + + if (!is_format_compatible(ref_format, wave_format)) + return E_INVALIDARG; + + This->format = *wave_format; + + return S_OK; }
static HRESULT WINAPI audio_IAudioMediaStream_CreateSample(IAudioMediaStream *iface, IAudioData *audio_data, diff --git a/dlls/amstream/tests/amstream.c b/dlls/amstream/tests/amstream.c index 1fb3cc239b..853949a87b 100644 --- a/dlls/amstream/tests/amstream.c +++ b/dlls/amstream/tests/amstream.c @@ -2627,6 +2627,147 @@ static void test_audiostream_get_format(void) IAMMultiMediaStream_Release(mmstream); }
+static void check_audiostream_format(const WAVEFORMATEX *format, HRESULT expected_hr) +{ + IAMMultiMediaStream *mmstream = create_ammultimediastream(); + IMediaStream *stream = NULL; + IAudioMediaStream *audio_stream = NULL; + HRESULT hr; + + hr = IAMMultiMediaStream_AddMediaStream(mmstream, NULL, &MSPID_PrimaryAudio, 0, &stream); + ok(hr == S_OK, "Got hr %#x.\n", hr); + hr = IMediaStream_QueryInterface(stream, &IID_IAudioMediaStream, (void **)&audio_stream); + ok(hr == S_OK, "Got hr %#x.\n", hr); + + hr = IAudioMediaStream_SetFormat(audio_stream, format); + ok(hr == expected_hr, "Got hr %#x.\n", hr); + + IAudioMediaStream_Release(audio_stream); + IMediaStream_Release(stream); + IAMMultiMediaStream_Release(mmstream); +} + +static void test_audiostream_set_format(void) +{ + IAMMultiMediaStream *mmstream; + IMediaStream *stream = NULL; + IAudioMediaStream *audio_stream = NULL; + IPin *pin = NULL; + WAVEFORMATPCMEX valid_format = {{0}}; + WAVEFORMATPCMEX format; + AM_MEDIA_TYPE valid_media_type = {0}; + HRESULT hr; + + valid_format.Format.wFormatTag = WAVE_FORMAT_PCM; + valid_format.Format.nChannels = 2; + valid_format.Format.nSamplesPerSec = 44100; + valid_format.Format.nAvgBytesPerSec = 176400; + valid_format.Format.nBlockAlign = 4; + valid_format.Format.wBitsPerSample = 16; + valid_format.Format.cbSize = 0; + + valid_media_type.majortype = MEDIATYPE_Audio; + valid_media_type.subtype = MEDIASUBTYPE_PCM; + valid_media_type.bFixedSizeSamples = TRUE; + valid_media_type.bTemporalCompression = FALSE; + valid_media_type.lSampleSize = 2; + valid_media_type.formattype = FORMAT_WaveFormatEx; + valid_media_type.pUnk = NULL; + valid_media_type.cbFormat = sizeof(WAVEFORMATEX); + valid_media_type.pbFormat = (BYTE *)&valid_format; + + check_audiostream_format(&valid_format.Format, S_OK); + check_audiostream_format(NULL, E_POINTER); + + format = valid_format; + format.Format.wFormatTag = WAVE_FORMAT_EXTENSIBLE; + format.Format.cbSize = 22; + format.Samples.wValidBitsPerSample = 16; + format.dwChannelMask = KSAUDIO_SPEAKER_STEREO; + format.SubFormat = KSDATAFORMAT_SUBTYPE_PCM; + check_audiostream_format(&format.Format, E_INVALIDARG); + + format = valid_format; + format.Format.nBlockAlign = 1; + check_audiostream_format(&format.Format, S_OK); + + format = valid_format; + format.Format.nAvgBytesPerSec = 1234; + check_audiostream_format(&format.Format, S_OK); + + mmstream = create_ammultimediastream(); + + hr = IAMMultiMediaStream_AddMediaStream(mmstream, NULL, &MSPID_PrimaryAudio, 0, &stream); + ok(hr == S_OK, "Got hr %#x.\n", hr); + hr = IMediaStream_QueryInterface(stream, &IID_IAudioMediaStream, (void **)&audio_stream); + ok(hr == S_OK, "Got hr %#x.\n", hr); + + hr = IAudioMediaStream_SetFormat(audio_stream, &valid_format.Format); + ok(hr == S_OK, "Got hr %#x.\n", hr); + + format = valid_format; + format.Format.nChannels = 1; + hr = IAudioMediaStream_SetFormat(audio_stream, &format.Format); + ok(hr == E_INVALIDARG, "Got hr %#x.\n", hr); + + format = valid_format; + format.Format.nSamplesPerSec = 11025; + hr = IAudioMediaStream_SetFormat(audio_stream, &format.Format); + ok(hr == E_INVALIDARG, "Got hr %#x.\n", hr); + + format = valid_format; + format.Format.nAvgBytesPerSec = 1234; + hr = IAudioMediaStream_SetFormat(audio_stream, &format.Format); + ok(hr == E_INVALIDARG, "Got hr %#x.\n", hr); + + format = valid_format; + format.Format.nBlockAlign = 1; + hr = IAudioMediaStream_SetFormat(audio_stream, &format.Format); + ok(hr == E_INVALIDARG, "Got hr %#x.\n", hr); + + format = valid_format; + format.Format.wBitsPerSample = 8; + hr = IAudioMediaStream_SetFormat(audio_stream, &format.Format); + ok(hr == E_INVALIDARG, "Got hr %#x.\n", hr); + + hr = IAudioMediaStream_SetFormat(audio_stream, &valid_format.Format); + ok(hr == S_OK, "Got hr %#x.\n", hr); + + IAudioMediaStream_Release(audio_stream); + IMediaStream_Release(stream); + IAMMultiMediaStream_Release(mmstream); + + mmstream = create_ammultimediastream(); + + hr = IAMMultiMediaStream_AddMediaStream(mmstream, NULL, &MSPID_PrimaryAudio, 0, &stream); + ok(hr == S_OK, "Got hr %#x.\n", hr); + hr = IMediaStream_QueryInterface(stream, &IID_IAudioMediaStream, (void **)&audio_stream); + ok(hr == S_OK, "Got hr %#x.\n", hr); + hr = IMediaStream_QueryInterface(stream, &IID_IPin, (void **)&pin); + ok(hr == S_OK, "Got hr %#x.\n", hr); + + hr = IPin_ReceiveConnection(pin, &output_pin, &valid_media_type); + ok(hr == S_OK, "Got hr %#x.\n", hr); + + format = valid_format; + format.Format.nChannels = 1; + hr = IAudioMediaStream_SetFormat(audio_stream, &format.Format); + ok(hr == E_INVALIDARG, "Got hr %#x.\n", hr); + + hr = IPin_Disconnect(pin); + ok(hr == S_OK, "Got hr %#x.\n", hr); + + format = valid_format; + format.Format.nChannels = 1; + hr = IAudioMediaStream_SetFormat(audio_stream, &format.Format); + ok(hr == S_OK, "Got hr %#x.\n", hr); + + IPin_Release(pin); + IAudioMediaStream_Release(audio_stream); + IMediaStream_Release(stream); + IAMMultiMediaStream_Release(mmstream); +} + START_TEST(amstream) { HANDLE file; @@ -2661,6 +2802,7 @@ START_TEST(amstream) test_audiodata_set_format();
test_audiostream_get_format(); + test_audiostream_set_format();
CoUninitialize(); } -- 2.17.1
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=65714
Your paranoid android.
=== build (build log) ===
error: corrupt patch at line 12 Task: Patch failed to apply
=== debian10 (build log) ===
error: corrupt patch at line 12 Task: Patch failed to apply
=== debian10 (build log) ===
error: corrupt patch at line 12 Task: Patch failed to apply
On 2/24/20 12:04 PM, Anton Baskanov wrote:
Signed-off-by: Anton Baskanov baskanov@gmail.com
dlls/amstream/audiostream.c | 41 +++++++++- dlls/amstream/tests/amstream.c | 142 +++++++++++++++++++++++++++++++++ 2 files changed, 181 insertions(+), 2 deletions(-)
diff --git a/dlls/amstream/audiostream.c b/dlls/amstream/audiostream.c index 8a16512dcd..abc82f3a0e 100644 --- a/dlls/amstream/audiostream.c +++ b/dlls/amstream/audiostream.c @@ -183,6 +183,7 @@ struct audio_stream IPin *peer; IMemAllocator *allocator; AM_MEDIA_TYPE mt;
- WAVEFORMATEX format;
};
static inline struct audio_stream *impl_from_IAMMediaStream(IAMMediaStream *iface) @@ -512,13 +513,49 @@ static HRESULT WINAPI audio_IAudioMediaStream_GetFormat(IAudioMediaStream *iface return S_OK; }
+static BOOL is_format_compatible(const WAVEFORMATEX *current_format, const WAVEFORMATEX *format) +{
- if (format->wFormatTag != WAVE_FORMAT_PCM)
return FALSE;- if (current_format->wFormatTag == WAVE_FORMAT_PCM)
- {
if (current_format->nChannels != format->nChannels)return FALSE;if (current_format->nSamplesPerSec != format->nSamplesPerSec)return FALSE;if (current_format->nAvgBytesPerSec != format->nAvgBytesPerSec)return FALSE;if (current_format->nBlockAlign != format->nBlockAlign)return FALSE;if (current_format->wBitsPerSample != format->wBitsPerSample)return FALSE;- }
- return TRUE;
+}
This seems very confusing. I suspect it would be much easier just to track separately whether the format has been set, and then compare the entire format if it has.
Using the term "compatible" is a bit odd, too, since you're doing an exact match.
Also, is cbSize really not checked? If it is, I would recommend just doing a single memcmp(). Actually, even if it isn't, I wouldn't be opposed to memcmp(..., sizeof(PCMWAVEFORMAT)).
static HRESULT WINAPI audio_IAudioMediaStream_SetFormat(IAudioMediaStream *iface, const WAVEFORMATEX *wave_format) { struct audio_stream *This = impl_from_IAudioMediaStream(iface);
- const WAVEFORMATEX *ref_format;
- FIXME("(%p/%p)->(%p) stub!\n", iface, This, wave_format);
- TRACE("(%p/%p)->(%p)\n", iface, This, wave_format);
- return E_NOTIMPL;
- if (!wave_format)
return E_POINTER;- ref_format = This->peer ? (WAVEFORMATEX *)This->mt.pbFormat :
&This->format;
- if (!is_format_compatible(ref_format, wave_format))
return E_INVALIDARG;- This->format = *wave_format;
- return S_OK;
}
Similarly to GetFormat(), I suspect this should be protected by the stream's critical section.
static HRESULT WINAPI audio_IAudioMediaStream_CreateSample(IAudioMediaStream *iface, IAudioData *audio_data, diff --git a/dlls/amstream/tests/amstream.c b/dlls/amstream/tests/amstream.c index 1fb3cc239b..853949a87b 100644 --- a/dlls/amstream/tests/amstream.c +++ b/dlls/amstream/tests/amstream.c @@ -2627,6 +2627,147 @@ static void test_audiostream_get_format(void) IAMMultiMediaStream_Release(mmstream); }
+static void check_audiostream_format(const WAVEFORMATEX *format, HRESULT expected_hr) +{
- IAMMultiMediaStream *mmstream = create_ammultimediastream();
- IMediaStream *stream = NULL;
- IAudioMediaStream *audio_stream = NULL;
- HRESULT hr;
- hr = IAMMultiMediaStream_AddMediaStream(mmstream, NULL,
&MSPID_PrimaryAudio, 0, &stream);
- ok(hr == S_OK, "Got hr %#x.\n", hr);
- hr = IMediaStream_QueryInterface(stream, &IID_IAudioMediaStream,
(void **)&audio_stream);
- ok(hr == S_OK, "Got hr %#x.\n", hr);
- hr = IAudioMediaStream_SetFormat(audio_stream, format);
- ok(hr == expected_hr, "Got hr %#x.\n", hr);
- IAudioMediaStream_Release(audio_stream);
- IMediaStream_Release(stream);
- IAMMultiMediaStream_Release(mmstream);
+}
+static void test_audiostream_set_format(void) +{
- IAMMultiMediaStream *mmstream;
- IMediaStream *stream = NULL;
- IAudioMediaStream *audio_stream = NULL;
- IPin *pin = NULL;
- WAVEFORMATPCMEX valid_format = {{0}};
- WAVEFORMATPCMEX format;
- AM_MEDIA_TYPE valid_media_type = {0};
- HRESULT hr;
- valid_format.Format.wFormatTag = WAVE_FORMAT_PCM;
- valid_format.Format.nChannels = 2;
- valid_format.Format.nSamplesPerSec = 44100;
- valid_format.Format.nAvgBytesPerSec = 176400;
- valid_format.Format.nBlockAlign = 4;
- valid_format.Format.wBitsPerSample = 16;
- valid_format.Format.cbSize = 0;
- valid_media_type.majortype = MEDIATYPE_Audio;
- valid_media_type.subtype = MEDIASUBTYPE_PCM;
- valid_media_type.bFixedSizeSamples = TRUE;
- valid_media_type.bTemporalCompression = FALSE;
- valid_media_type.lSampleSize = 2;
- valid_media_type.formattype = FORMAT_WaveFormatEx;
- valid_media_type.pUnk = NULL;
- valid_media_type.cbFormat = sizeof(WAVEFORMATEX);
- valid_media_type.pbFormat = (BYTE *)&valid_format;
- check_audiostream_format(&valid_format.Format, S_OK);
- check_audiostream_format(NULL, E_POINTER);
- format = valid_format;
- format.Format.wFormatTag = WAVE_FORMAT_EXTENSIBLE;
- format.Format.cbSize = 22;
- format.Samples.wValidBitsPerSample = 16;
- format.dwChannelMask = KSAUDIO_SPEAKER_STEREO;
- format.SubFormat = KSDATAFORMAT_SUBTYPE_PCM;
- check_audiostream_format(&format.Format, E_INVALIDARG);
- format = valid_format;
- format.Format.nBlockAlign = 1;
- check_audiostream_format(&format.Format, S_OK);
- format = valid_format;
- format.Format.nAvgBytesPerSec = 1234;
- check_audiostream_format(&format.Format, S_OK);
- mmstream = create_ammultimediastream();
- hr = IAMMultiMediaStream_AddMediaStream(mmstream, NULL,
&MSPID_PrimaryAudio, 0, &stream);
- ok(hr == S_OK, "Got hr %#x.\n", hr);
- hr = IMediaStream_QueryInterface(stream, &IID_IAudioMediaStream,
(void **)&audio_stream);
- ok(hr == S_OK, "Got hr %#x.\n", hr);
- hr = IAudioMediaStream_SetFormat(audio_stream, &valid_format.Format);
- ok(hr == S_OK, "Got hr %#x.\n", hr);
- format = valid_format;
- format.Format.nChannels = 1;
- hr = IAudioMediaStream_SetFormat(audio_stream, &format.Format);
- ok(hr == E_INVALIDARG, "Got hr %#x.\n", hr);
- format = valid_format;
- format.Format.nSamplesPerSec = 11025;
- hr = IAudioMediaStream_SetFormat(audio_stream, &format.Format);
- ok(hr == E_INVALIDARG, "Got hr %#x.\n", hr);
- format = valid_format;
- format.Format.nAvgBytesPerSec = 1234;
- hr = IAudioMediaStream_SetFormat(audio_stream, &format.Format);
- ok(hr == E_INVALIDARG, "Got hr %#x.\n", hr);
- format = valid_format;
- format.Format.nBlockAlign = 1;
- hr = IAudioMediaStream_SetFormat(audio_stream, &format.Format);
- ok(hr == E_INVALIDARG, "Got hr %#x.\n", hr);
- format = valid_format;
- format.Format.wBitsPerSample = 8;
- hr = IAudioMediaStream_SetFormat(audio_stream, &format.Format);
- ok(hr == E_INVALIDARG, "Got hr %#x.\n", hr);
- hr = IAudioMediaStream_SetFormat(audio_stream, &valid_format.Format);
- ok(hr == S_OK, "Got hr %#x.\n", hr);
- IAudioMediaStream_Release(audio_stream);
- IMediaStream_Release(stream);
- IAMMultiMediaStream_Release(mmstream);
- mmstream = create_ammultimediastream();
- hr = IAMMultiMediaStream_AddMediaStream(mmstream, NULL,
&MSPID_PrimaryAudio, 0, &stream);
- ok(hr == S_OK, "Got hr %#x.\n", hr);
- hr = IMediaStream_QueryInterface(stream, &IID_IAudioMediaStream,
(void **)&audio_stream);
- ok(hr == S_OK, "Got hr %#x.\n", hr);
- hr = IMediaStream_QueryInterface(stream, &IID_IPin, (void **)&pin);
- ok(hr == S_OK, "Got hr %#x.\n", hr);
- hr = IPin_ReceiveConnection(pin, &output_pin, &valid_media_type);
- ok(hr == S_OK, "Got hr %#x.\n", hr);
- format = valid_format;
- format.Format.nChannels = 1;
- hr = IAudioMediaStream_SetFormat(audio_stream, &format.Format);
- ok(hr == E_INVALIDARG, "Got hr %#x.\n", hr);
- hr = IPin_Disconnect(pin);
- ok(hr == S_OK, "Got hr %#x.\n", hr);
- format = valid_format;
- format.Format.nChannels = 1;
- hr = IAudioMediaStream_SetFormat(audio_stream, &format.Format);
- ok(hr == S_OK, "Got hr %#x.\n", hr);
- IPin_Release(pin);
- IAudioMediaStream_Release(audio_stream);
- IMediaStream_Release(stream);
- IAMMultiMediaStream_Release(mmstream);
+}
I think it would be worth explicitly testing whether IAudioMediaStream::GetFormat() returns success after SetFormat() is called.
START_TEST(amstream) { HANDLE file; @@ -2661,6 +2802,7 @@ START_TEST(amstream) test_audiodata_set_format();
test_audiostream_get_format();
test_audiostream_set_format();
CoUninitialize();
}
2.17.1
-
Anton Baskanov -
Marvin -
Zebediah Figura