[PATCH 3/3] winegstreamer: Use videobox to align and padd NV12 video buffers.

[Zebediah Figura (she/her)]

On 10/27/21 16:05, Rémi Bernon wrote:
> On 10/27/21 10:59 PM, Zebediah Figura (she/her) wrote:
>> On 10/27/21 15:36, Rémi Bernon wrote:
>>> On 10/27/21 10:22 PM, Zebediah Figura (she/her) wrote:
>>>> On 10/27/21 10:04, Rémi Bernon wrote:
>>>>> Native MF aligns the NV12 planes on 16 pixels, and Greedfall 
>>>>> expects it.
>>>>>
>>>>> Signed-off-by: Rémi Bernon <rbernon at codeweavers.com>
>>>>> ---
>>>>>   dlls/winegstreamer/wg_parser.c | 8 +++++++-
>>>>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>>>>
>>>>
>>>> This leaves me with a lot of questions, not least of which is "can 
>>>> we have tests for this"? Others are:
>>>>
>>>
>>> Well, I'm not sure how to add tests? Should I add a sample video 
>>> file? I think I saw one already somewhere but it doesn't look we have 
>>> that many and it can grow big if we add more.
>>
>> Yes, we have some test video files already, including several in 
>> dlls/quartz/tests. Ideally it should be possible to reproduce with 
>> small frame sizes and short videos (just a couple of frames).
>>
> 
> Okay, I'll add some.
> 
>>>
>>>> * This will change the actual height of the image, not just its 
>>>> layout. Is that really correct?
>>>>
>>>
>>> Yes, as far as I could check it is the case.
>>>
>>>> * Is it really correct to change the height and not the width?
>>>>
>>>
>>> Well, the width isn't an issue with this specific game, the videos 
>>> are 1920x1080 (and decoded as 1920x1088). If some other need it maybe 
>>> it should.
>>
>> I'll admit I don't really like the idea of not even testing unaligned 
>> widths...
>>
> 
> Like below, it's not broken as far as we know, so it doesn't seems like 
> it needs any fixing for the moment? (I don't really mind, just saying)
> 
>>>
>>>> * Is it really correct to change the reported dimensions in all 
>>>> circumstances? E.g. for all formats? I'm already pretty sure that 
>>>> it's wrong for DirectShow...
>>>>
>>>
>>> I guess, not sure how we can tell which one is used.
>>
>> I'm not sure I understand what you mean, but in theory it should be a 
>> matter of testing what output formats are enumerated for an unaligned 
>> video (say 33x33). I'm assuming that NV12 isn't the only available 
>> format; are all of the formats aligned to 16 pixels? Does this hold 
>> for different video codecs, including uncompressed RGB or YUV video?
>>
>> I'm not immediately sure it makes sense to test *all* these things 
>> programmatically, although it would be nice if it ends up being 
>> possible to do so easily.
>>
>> Playing fast and loose with what video formats we report is one 
>> thing—applications are going to find a way to blit it to the display 
>> one way or another. Changing the video *size* is another thing, and I 
>> really want to make sure we're only doing it when we're supposed to.
>>
> 
> Yeah maybe they should be aligned too, but that could be added if we 
> find it's needed? And if things work fine with these other formats maybe 
> not changing them is safer.

Reporting multiple different formats with inconsistent sizes is 
something else that makes me nervous...

> 
> I tried changing the reported formats or ordering but that game seems to 
> explicitly request NV12 and isn't happy if it's not reported.

By "explicitly request" I assume it's not doing anything like 
constructing its own NV12 format, right? I mean, I assume it's not 
explicitly asking a different size or alignment or anything either...?

I was suggesting to test without the game; just create a custom file, 
load it into the source reader, and see what media types are offered.