[vlc-devel] [vlc-commits] opengl: forward the error code from the resize callback

[Steve Lhomme]

On 17/01/2019 17:06, Rémi Denis-Courmont wrote:
> Le torstaina 17. tammikuuta 2019, 17.46.15 EET Steve Lhomme a écrit :
>> On 16/01/2019 12:41, Rémi Denis-Courmont wrote:
>>> This is obviously wrong since it ends with an assertion failure at
>>> opengl/display.c:228. And thus the commits before it make no sense.
>> It asserts there because it's not meant to handle
>> VOUT_DISPLAY_RESET_PICTURES. And it receives that because during a
>> VOUT_DISPLAY_CHANGE_DISPLAY_SIZE/VOUT_DISPLAY_CHANGE_DISPLAY_FILLED/VOUT_DIS
>> PLAY_CHANGE_ZOOM it returned an error from a call to vlc_gl_Resize(). If the
>> code is not ready to handle such an error (why it occurs for you is another
>> thing to look at) then it will also assert on the call right after:
>>
>> if(vlc_gl_MakeCurrent(sys->gl)!=VLC_SUCCESS)
>>
>>       returnVLC_EGENERIC;
>>
>> So I don't see how the commit is wrong. Either the DISPLAY_CHANGE_*
>> events can never return an error from there, or the code needs to be
>> ready to handle it.
> Returning an error from those controls just means you need to reset the
> display input format,

This wasn't clear to me.
> and the pool if the display was not ported yet.

The display pool ? What if direct rendering is on ? That means the 
decoder pool as well. Right now the comment you added says "internal 
buffers" that's rather vague.

>
> Returning an error does *not* mean that you reject the change. You *cannot*
> reject the change. It simply does not work and cannot that way. Since the
> control is dispatched asynchronously, it is too late to reject the change.
>
> I am planning to address that for other reasons than error handling, but even
> then, most (all except maybe Wayland) windowing systems lack provisions for
> rejecting a window size change.
>
> So if you have a failure at that stage, then you have two options:
> - render wrong going forward, or
> - stop rendering and discard pictures until further notice.
>
> Either way, you don't need to return an error to the core.
That would be true if OpenGL wasn't handled in the core. But this 
function clearly returns whether the surface dimensions were succesfully 
changed:
http://git.videolan.org/?p=vlc.git;a=blob;f=src/video_output/opengl.c;h=c288fd83b429dfeaabdef4432dfacc96650e5c80;hb=HEAD#l173

Telling it that it worked when it didn't will not do any good.