[PATCH 2/2] mfreadwrite: Accept new media type only when format data are equal.

[Nikolay Sivov]

On 5/7/21 1:56 PM, Giovanni Mascellani wrote:
> Hi,
>
> Il 07/05/21 10:30, Nikolay Sivov ha scritto:
>> I doesn't seem like a right thing to do. This helper is meant to check
>> if decoder is necessary, for that you only need a subtype test.
>> Admittedly existing naming does not make it obvious.
>>
>> What should happen in general case:
>>
>> - check major/subtype to see if decoder is necessary at all;
>> - if it's necessary, look for the decoder;
>> - unconditionally compare for format data to see if converter is
>> necessary, and append one.
>>
>> According to docs it should support local transforms, so registering
>> local dummy converter (that is only able to negotiate types) for 16 ->
>> 32bit resampling, and trying again could potentially succeed instead
>> where it returns MF_E_TOPO_CODEC_NOT_FOUND now. It's possible that on
>> Windows reader is either confused by raw types source returns and skips
>> that part, or fails to find a converter in first place (or conversion is
>> disabled by default despite what docs say).
>
> I admit my patch is not perfect, but still it seems better than what
> happens now.
>
> At some point the data in the requested media type must be compared
> with the native media types provided by the source. Currently this
> does not happen and if the caller requests a 16 bit format while the
> source provides a 32 bit format, the source reader will acknowledge
> the request with 16 bit but return 32 bit audio, which will result in
> garbled audio.
>
> Also, suppose that a media source exposes both a 32 bit and a 16 bit
> media type. I expect (and my tests on Windows seem to confirm it) that
> if you request 32 bit you get the first media type and if you request
> 16 bit you get the second, even if they both match at the
> major/subtype level. The current implementation can only match the
> first one. Since Windows matches the correct one, it seems that
> Windows does at least one round of matching taking also media type
> data into account.

Ok, I checked with video type too, type with different frame size is
also rejected, so patches are probably safe. Please resend both of them.

>
> According to [1], the search for a decoder only happens if the native
> type is compressed, otherwise only native media types are allowed. I
> understand that MSDN can be incorrect, but this seems to align with my
> (admittedly limited) testing. Do you have evidence of the contrary?
>
> [1]
> https://docs.microsoft.com/en-us/windows/win32/api/mfreadwrite/nf-mfreadwrite-imfsourcereader-setcurrentmediatype
>
> Thanks, Giovanni.
>
>