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

[Rémi Bernon]

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.

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.
-- 
Rémi Bernon <rbernon at codeweavers.com>