Re: [PATCH v2] winegstreamer: Ensure that the source reports both PCM and floating point audio formats.
On 5/13/21 10:47 AM, Giovanni Mascellani wrote:
Hi,
Il 13/05/21 09:18, Nikolay Sivov ha scritto:
On 5/12/21 6:37 PM, Giovanni Mascellani wrote:
diff --git a/dlls/winegstreamer/media_source.c b/dlls/winegstreamer/media_source.c index eb5b9e366ec..5249bc9c41d 100644 --- a/dlls/winegstreamer/media_source.c +++ b/dlls/winegstreamer/media_source.c @@ -806,6 +806,31 @@ static HRESULT media_stream_init_desc(struct media_stream *stream) goto done; } } + else if (format.major_type == WG_MAJOR_TYPE_AUDIO) + { + /* Expose at least one PCM and one floating point type for the + consumer to pick from. */ + static const enum wg_audio_format audio_types[] = + { + WG_AUDIO_FORMAT_S16LE, + WG_AUDIO_FORMAT_F64LE, + }; Is F64LE correct? I thought before you used an equivalent of F32LE, if that's what (Float, MF_MT_AUDIO_BITS_PER_SAMPLE=32) means.
Ugh, I meant F32LE. My mistake.
+ + stream_types = malloc( sizeof(IMFMediaType *) * (ARRAY_SIZE(audio_types) + 1) ); + + stream_types[0] = mf_media_type_from_wg_format(&format); + type_count = 1; + + for (i = 0; i < ARRAY_SIZE(audio_types); i++) + { + struct wg_format new_format; + if (format.u.audio.format == audio_types[i]) + continue; + new_format = format; + new_format.u.audio.format = audio_types[i]; + stream_types[type_count++] = mf_media_type_from_wg_format(&new_format); + } + } else { stream_type = mf_media_type_from_wg_format(&format); Thanks, that looks fine to me. It could be reduced a bit more still, by removing 'new_format' for example, but that's minor.
That would be wrong, wouldn't it? If I change format.u.audio.format, then the "if () continue" at the beginning of the loop doesn't make sense any more. Unless I remember the original format in another variable, which would then have comparable complexity.
Now we need someone who knows gstreamer to comment, specifically if resampling PCM -> PCM/float -> float would work. Or maybe you can test that yourself with some sin wave data with rate not matching output device rate.
I can't see how this is related to my patch: I don't think Gstreamer does any rate change at this level. It just advertises the native rate the audio stream supports. Whatever rate matching is required, it will be done downstream from the media source (both before and after my patch, which is only concerned with the sample format, not its rate). Original patch added PCM->F32LE and float->S16LE conversions, this one also adds conversions between different PCM and different float formats. So the question is does it work? It was wrong to call it rate, I meant bps of course, or format as whole.
Thanks, Giovanni.
On 5/13/21 2:53 AM, Nikolay Sivov wrote:
On 5/13/21 10:47 AM, Giovanni Mascellani wrote:
Hi,
Il 13/05/21 09:18, Nikolay Sivov ha scritto:
On 5/12/21 6:37 PM, Giovanni Mascellani wrote:
diff --git a/dlls/winegstreamer/media_source.c b/dlls/winegstreamer/media_source.c index eb5b9e366ec..5249bc9c41d 100644 --- a/dlls/winegstreamer/media_source.c +++ b/dlls/winegstreamer/media_source.c @@ -806,6 +806,31 @@ static HRESULT media_stream_init_desc(struct media_stream *stream) goto done; } } + else if (format.major_type == WG_MAJOR_TYPE_AUDIO) + { + /* Expose at least one PCM and one floating point type for the + consumer to pick from. */ + static const enum wg_audio_format audio_types[] = + { + WG_AUDIO_FORMAT_S16LE, + WG_AUDIO_FORMAT_F64LE, + }; Is F64LE correct? I thought before you used an equivalent of F32LE, if that's what (Float, MF_MT_AUDIO_BITS_PER_SAMPLE=32) means.
Ugh, I meant F32LE. My mistake.
+ + stream_types = malloc( sizeof(IMFMediaType *) * (ARRAY_SIZE(audio_types) + 1) ); + + stream_types[0] = mf_media_type_from_wg_format(&format); + type_count = 1; + + for (i = 0; i < ARRAY_SIZE(audio_types); i++) + { + struct wg_format new_format; + if (format.u.audio.format == audio_types[i]) + continue; + new_format = format; + new_format.u.audio.format = audio_types[i]; + stream_types[type_count++] = mf_media_type_from_wg_format(&new_format); + } + } else { stream_type = mf_media_type_from_wg_format(&format); Thanks, that looks fine to me. It could be reduced a bit more still, by removing 'new_format' for example, but that's minor.
That would be wrong, wouldn't it? If I change format.u.audio.format, then the "if () continue" at the beginning of the loop doesn't make sense any more. Unless I remember the original format in another variable, which would then have comparable complexity.
Now we need someone who knows gstreamer to comment, specifically if resampling PCM -> PCM/float -> float would work. Or maybe you can test that yourself with some sin wave data with rate not matching output device rate.
I can't see how this is related to my patch: I don't think Gstreamer does any rate change at this level. It just advertises the native rate the audio stream supports. Whatever rate matching is required, it will be done downstream from the media source (both before and after my patch, which is only concerned with the sample format, not its rate). Original patch added PCM->F32LE and float->S16LE conversions, this one also adds conversions between different PCM and different float formats. So the question is does it work? It was wrong to call it rate, I meant bps of course, or format as whole.
audioconvert should be perfectly capable of this, yes.
participants (2)
-
Nikolay Sivov -
Zebediah Figura (she/her)