[PATCH 2/3] amstream: Implement AMAudioStream::SetFormat.
Zebediah Figura
zfigura at codeweavers.com
Mon Feb 24 14:27:04 CST 2020
On 2/24/20 12:04 PM, Anton Baskanov wrote:
> Signed-off-by: Anton Baskanov <baskanov at 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
>
More information about the wine-devel
mailing list