[vlc-devel] [PATCH 1/3] vdpau/display: remove unused has_pictures_invalid

[Rémi Denis-Courmont]

I am not interested in beating dead horses. Lock/unlock callbacks are broken. Failing resize is broken. Those problems have been discussed to death several times already.

Le 18 janvier 2019 10:16:20 GMT+02:00, Steve Lhomme <robux4 at ycbcr.xyz> a écrit :
>On 17/01/2019 16:59, Rémi Denis-Courmont wrote:
>> Le torstaina 17. tammikuuta 2019, 16.34.08 EET Steve Lhomme a écrit :
>>> On 16/01/2019 16:59, Rémi Denis-Courmont wrote:
>>>> Le keskiviikkona 16. tammikuuta 2019, 14.33.52 EET Steve Lhomme a
>écrit :
>>>>> OK so the documentation of "has_pictures_invalid" needs to be
>changed.
>>>>> vout_display_SendEventPicturesInvalid() indirectly calls
>>>>> VOUT_DISPLAY_RESET_PICTURES which is also what happens when a
>>>>> resize/crop/etc error occurs but synchronously.
>>>>>
>>>>> So has_pictures_invalid means the display handles
>>>>> VOUT_DISPLAY_RESET_PICTURES.
>>>> It rather means that the display may need
>VOUT_DISPLAY_RESET_PICTURES.
>>>> Though in practice, it only really affects pool allocation, and
>should be
>>>> irrelevant if the display uses push-buffers.
>>> "if the display uses push buffers" yes. But there are some cases
>where
>>> it's not possible. In many display modules the display pool has its
>own
>>> lock/unlock callbacks.
>> I stopped reading here.
>
>That's not a correct way of handling comments on your code. I was 
>explaining the changes you made on VOUT_DISPLAY_CHANGE_xxx to reset the
>
>display introduced the error you complained about. My code just
>exhibits 
>the issue more often.
>
>Also in general ignoring an error is a bad coding practice. I am 
>proposing to handle the case where resizing a buffer fails (which can 
>happen, since it happened to you).
>
>> Lock/unlock has been flaky, not to say plainly broken,
>> for as long as I remember - which is at least as long as
>picture_pool_t
>> existed. I don't recall if/how the callbacks worked before.
>The lock/unlock of the memory is still going to be needed. It may be 
>possible to fake direct rendering in these (if you read the whole thing
>
>you would have seen it's only an issue in "indirect" rendering) and do 
>the lock/copy/unlock during the Prepare. Many of these modules 
>(direct3d9, direct3d11, android opengl/display, mmal) currently do it
>on 
>more than one picture though. There could be tearing issues if there's 
>no double buffering involved. It will have to be handled internally by 
>each module.
>
>So in the long run it may be better, I don't think it's mandatory
>change 
>needed for the push model. At least it works without for me.
>
>BTW, I mentioned that mmal handles lock and VOUT_DISPLAY_RESET_PICTURES
>
>but that's not the case.
>
>_______________________________________________
>vlc-devel mailing list
>To unsubscribe or modify your subscription options:
>https://mailman.videolan.org/listinfo/vlc-devel

-- 
Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20190118/de8d707b/attachment.html>