On 06/09/14 17:28, wm4 wrote: > On Sat, 06 Sep 2014 18:10:17 +0300 > Rémi Denis-Courmont <[email protected]> wrote: > >> Hello, >> >> Le vendredi 5 septembre 2014, 18:13:49 wm4 a écrit : >>>> +static VdpStatus vdp_enosys(/* Unspecified, not void - leave empty */) >>>> +{ >>>> + return VDP_STATUS_NO_IMPLEMENTATION; >>>> +} >>>> + >>>> +static void *ff_vdpau_get_proc_address(AVCodecContext *avctx, VdpFuncId >>>> id) +{ >>>> + AVVDPAUContext *hwctx = avctx->hwaccel_context; >>>> + void *ret; >>>> + >>>> + av_assert0(hwctx->device != VDP_INVALID_HANDLE); >>>> + >>>> + if (hwctx->get_proc_address(hwctx->device, id, &ret) != >>>> VDP_STATUS_OK) >>>> + ret = vdp_enosys; >>> >>> Isn't this very questionable? IMO initialization should just fail if a >>> required function is missing. But if a C language lawyer can confirm >>> that this use of vdp_enosys is indeed legal, why not (although I >>> have my doubt). >> >> I think vdp_enosys is legal thanks to the C calling convention (i.e. the >> called function does not need to know its parameters - this is necessary for >> variadic functions). I can change it to NULL but it will mostly result in >> more >> extra error handling boilerplate - only to handle completely broken VDPAU >> drivers (of which I know none). >> >> For such hypothetically broken driver, I think not failing safe is good >> enough. > > No, I'm pretty sure it's invalid C. Casting function types is > legal, but dereferencing (i.e. calling) a function pointer is legal > only if the actual types match. > > Maybe use some macros to define the function tables and the code to > load it. > > Well, I don't care that much - even if invalid, it's unlikely that > compilers suddenly start caring, and it miscompilation happens, we > still can fix it later. So no blocker. > >>> The idea seems generally ok (and making libavcodec handle more aspects >>> of hw decoding is welcome). But I have some doubts whether it will >>> still be possible for me to handle preemption as well as I could, and I >>> wouldn't appreciate if a small incremental improvement breaks all of my >>> code. >> >> Unless you have a _concrete_ enhancement request before next Tuesday, I will >> go ahead and try to address Diego and any further feedback. I am definitely >> not >> waiting an indefinite amount of time for the sake of an abstract request, >> and >> potentially holding up new codec support. Plus I would like to fix this >> before >> VLC 2.2.0. >> >> Specifically as regards preemption, I think it remains doable. In >> principles, >> this makes no real difference; you can still wrap the callback in a >> similarly >> inconvenient manner as before and with the same limitations. > > Have you actually implemented it? I don't feel like having to update my > code to an incrementally changing API that doesn't even consider this > use case. Maybe implement preemption recovery in VLC (AFAIK VLC > doesn't handle it yet?). Then we could come up with something > reasonable, instead of creating theories or declaring it as broken. > > Now I'm not even against your patches. I just hate it figuring out this > preemption crap again every time. I don't actually like preemption, but > handling it is kind of needed. > >> I think however that you have already demonstrated that preemption needs >> fixing >> at the back-end. But the venue for that discussion is the VDPAU FreeDesktop >> list, not libav-devel. Regardless, this patch does not increase the risk of >> needing yet another API change for preemption for those apps that do care; I >> don't see why yet another interface break would be needed either way. >> >>> Also, think of the API users: do they have to update their vdpau code >>> every month now? WTF? >> >> I fail to see how this forces anybody to update. This should be backward >> compatible with the previous three variants of the VDPAU hardware >> acceleration. Once a month? obvious troll is obvious. > > But it's declared as deprecated. The next major bump will disable it.
Making deprecated features stay for longer had been requested and is fine, I updated the to make it clear this point (the alert block was miswritten) https://wiki.libav.org/ReleaseProcess lu _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
