[PATCH 1/3] mf/tests: Add some WMA encoder ProcessOutput tests.

[Rémi Bernon]

On 2/4/22 20:55, Nikolay Sivov wrote:
> 
> 
> On 2/4/22 15:49, Rémi Bernon wrote:
>> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51931
>> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52391
>> Signed-off-by: Rémi Bernon <rbernon at codeweavers.com>
>> ---
>>   dlls/mf/tests/mf.c | 132 +++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 132 insertions(+)
>>
>> diff --git a/dlls/mf/tests/mf.c b/dlls/mf/tests/mf.c
>> index ebf42bcad9c..03d0250d51a 100644
>> --- a/dlls/mf/tests/mf.c
>> +++ b/dlls/mf/tests/mf.c
>> @@ -5492,9 +5492,103 @@ static IMFSample *create_sample(const BYTE 
>> *data, ULONG size)
>>       return sample;
>>   }
>> +#define check_sample(a, b, c) check_sample_(__LINE__, a, b, c)
>> +static void check_sample_(int line, IMFSample *sample, const void 
>> *expect_buf, ULONG expect_len)
>> +{
>> +    IMFMediaBuffer *media_buffer;
>> +    DWORD length;
>> +    BYTE *buffer;
>> +    HRESULT hr;
>> +    ULONG ret;
>> +
>> +    hr = IMFSample_ConvertToContiguousBuffer(sample, &media_buffer);
>> +    ok_(__FILE__, line)(hr == S_OK, "ConvertToContiguousBuffer 
>> returned %#x\n", hr);
>> +    hr = IMFMediaBuffer_Lock(media_buffer, &buffer, NULL, &length);
>> +    ok_(__FILE__, line)(hr == S_OK, "Lock returned %#x\n", hr);
>> +    ok_(__FILE__, line)(expect_len == length, "got length %u\n", 
>> length);
>> +    if (length && length == expect_len)
>> +    {
>> +        ok_(__FILE__, line)(!memcmp(expect_buf, buffer, expect_len),
>> +                "unexpected buffer data\n");
>> +    }
>> +    hr = IMFMediaBuffer_Unlock(media_buffer);
>> +    ok_(__FILE__, line)(hr == S_OK, "Unlock returned %#x\n", hr);
>> +    ret = IMFMediaBuffer_Release(media_buffer);
>> +    ok_(__FILE__, line)(ret == 1, "Release returned %u\n", ret);
>> +}
>> +
>>   static const BYTE wma_codec_data[10] = {0, 0x44, 0, 0, 0x17, 0, 0, 
>> 0, 0, 0};
>>   static const BYTE wma_decoded_data[0x4000] = {0};
>>   static const ULONG wma_block_size = 1487;
>> +static const BYTE wma_encoded_data[][1487] =
>> +{
> 
> Sorry, I forgot to reply to this one. Would it make sense to have this 
> blob in the resources like we occasionally do?
> Is it possible to reduce the size of it, and provide a way to regenerate 
> it, or a note on how it was generated at least?
> 


Well that's the sole purpose of the encoder tests. Sure, it doesn't 
really print it, but it's just a matter of adding a few hexdump lines to 
print the packets. Maybe a comment about that would be appropriate.

Regarding its size, it wasn't obviously easy to have it smaller, I tried 
a few things and this was the simplest case for which the encoder was 
satisfied with the amount of input, and accepted to output encoded data, 
and for which the encoder had also enough input packets to decode something.

It didn't feel so large so I didn't try much harder. I think it may also 
be because we encode buffers full of 0s, so the encoded data is very 
small, and somehow it causes the encoder to eat a lot of data before 
some packets.


> I realize that it's not pretty to add a whole wma stream, because you'll 
> need to parse through first,
> using source methods, which means even more code.
> 


Yes, I wanted to avoid having to parse a container file if possible.

First, because I used Windows WMA encoder to generate the WMA packets (I 
couldn't make ffmpeg encoder work with Windows decoder), it'd be less 
convenient if it needed to be put in a container. Currently with just a 
few added hexdump lines the test output can be used to generate new data.

I think the question is still actually going to stand later, as I intend 
to add tests for the H264 encoder. There too we would need to get our 
hands on raw H264 packets, but as the open-source encoders are working 
fine it's possible to generate the stream in a container.

In any case, it is indeed a bit of a burden to have to parse a container 
to get the raw buffers, and I hoped that keeping the buffers small 
enough it could be fine to embed them like this (even H264 can stay 
relatively small with a small picture size).


> If there is going to be more tests for WMx, and since decoders/encoders 
> don't strictly belong to mf.dll, maybe moving this to additional test 
> file is an option.
> 

Yeah, well I could move them to wmadmod now, for the WMA transform, as 
it's supposed to live there. I'm not sure where the H264 transform is 
supposed to be though?

-- 
Rémi Bernon <rbernon at codeweavers.com>