[vlc-devel] [vlc-commits] video_output: win32: fix recursive call to vout_display_SendEventDisplaySize

[Steve Lhomme]

On 2018-05-28 6:25 PM, Rémi Denis-Courmont wrote:
> Le lundi 28 mai 2018, 10:38:23 EEST Steve Lhomme a écrit :
>> vlc | branch: master | Steve Lhomme <robux4 at ycbcr.xyz> | Mon May 28 09:35:58
>> 2018 +0200| [39ee73994e860bfbfc45d2a3ac51e50d8a799e5d] | committer: Steve
>> Lhomme
>>
>> video_output: win32: fix recursive call to vout_display_SendEventDisplaySize
> Recursive??? I don't see any recursive call here, before or after a4942. Do
> you mean live-looping?

Not sure what live-looping but it's an indirect recursion. A new event 
is posted and treated afterwards with different (past) values.

>
>> A changing value may be receive while another one is being treated,
>> resulting in an infinite loop (via event calls).
> That wouldn't be a regression from a4942. That would be much much more likely
> to occur with a4942, but that would be pre-existinging... That means 3.0 still
> needs fixing too.
>
> And then... I can readily believe (although I have obviously not tested) that
> this fixes video playback with the Qt or skin engine window providers.
> Moreover I am obviously in favor of removing display-size events from display
> plug-ins as far as possible. There are multiple reasons:
> - it feels like the job of the window provider,
> - it avoids duplicating code in display plug-ins,
> - it avoids conflicts and races like this patch tries to address.
>
> However I doubt that this patch will work with LibVLC embedded video, or with
> stand-alone (non-embedded) video at this point.

I didn't test libvlc but --no-embedded works. I'll have to see on 
Winstore since it detects size changes internally.

>
> Specifically as far as I know LibVLC on Windows still uses the "my" old much
> too simplistic "drawable" plug-in. It is incapable of sending the window size
> events, or really any events. If it is not used by LibVLC, it should be
> deleted.
>
> Likewise, I cannot find any window size events in the case of non-embedded
> video, because there are no proper Win32 window provider plug-in at all so far
> that I would be aware of. There should most probably be one such plug-in for
> the reasons noted above, but there is not so far.
>
> tl;dr:
> The bug is probably older than you think and it is probably still present.
>