On Fri, Oct 06, 2017 at 01:48:14AM +0200, wm4 wrote: > On Fri, 6 Oct 2017 00:01:30 +0200 > Michael Niedermayer <mich...@niedermayer.cc> wrote: > > > The opaque_ref wraping is a really bad design. Iam not sure why > > people defend it. > > FFmpeg is full of this design. There are plenty of structs with > opaque/priv fields that change meaning depending on the context > (basically how the struct is used or what uses it). It affects all > decoders, encoders, filters, demuxers, muxers, the av_log() call, > functions that work with AVClass, AVOption, and probably more.
what you write is not true each decoder, demuxer, ... CLASS has its own type of private context nothing outside code specific to that class messes with it. A snow decoder has a snow context. If the outside structure is moved around its still a snow decoder with a snow private context. No amount of moving the structure around makes it invalid. OTOH, opaque_ref is defined by the user application. There is a single user application in the address space. Before the patch all AVFrame opaque_ref have the same type, no amount of moving/passing AVFrames around results in an invalid AVFrame. after the patch this is very different. Each AVFrame becomes tied to the code that created it, opaque_ref has a type that outside decoders, is defined by the user application inside decoders, its a internal structure. AVFrames are no longer a universal structure that can simply be passed around. This is bad design and it is fragile private and opaque are also intended to be very different things. In FFmpeg private is a internal thing like the internal context of an encoder. Opaque is a exteral thing, from the user application or caller You may have misunderstood me but I am against AVFrames depending on the code that created them. This is not just about this implementation (which is in fact buggy too), Pointing to the implementation issues and bugs was just intended to show how fragile this is. I do not understand how one can on one hand see all the problems this causes yet not see that the API that causes this is what is at fault. And it is MUCH easier to fix the API than to fix all the issues that context specific AVFrames would cause > > Are you saying that FFmpeg is really bad design? That's funny. > > There is nothing unclean about this use of opaque_ref, just that it > somewhat collides with awful legacy hacks like draw_horiz_band (but > which could be fixed anyway). > > Also what you said about nested decoders is simply incorrect. The It is not incorrect in case current implementations of decoders dont trigger it. [...] > > Now I'm very curious to hear what your "cleaner" solution to this > problem is. add a new field to AVFrame if you want a field with semantics that are different. you said you are against this IIRC. But thats the simple, robust and easy maintainable solution more so if that field is not void* it could provide type checking which most people consider a good thing. A system simiar to side data for opaque data could be used too, i would say thats overkill but some people like side data One entry for the users opaque structure One entry for the libavcodec private structure One entry for a future libavfilter private structure And this way you need no wraping or unwraping, a AVFrame either has some extra opaque fields or it doesnt. nothing could cast them to the wrong type. This is much more robust than putting all these things in opaque_ref and spending time and more time and more time writing code wraping and unwraping at every interface point and then cursing half the interface and likely in the future fighting over every bit of added interface as it massivly increases complexity with the wraping [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Breaking DRM is a little like attempting to break through a door even though the window is wide open and the only thing in the house is a bunch of things you dont want and which you would get tomorrow for free anyway
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
