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. 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 nested decoder is used only by the decoder implementation itself, and always uses the default get_buffer callback for the nested codec, so the returned AVFrame.opaque_ref is returned as NULL. Now it's possible that the new code will assert if the parent codec has AV_CODEC_CAP_DR1 set. This is because if AV_CODEC_CAP_DR1 is set, the AVFrame is expected to be allocated by the codec context's ff_get_buffer() (which would guarantee opaque_ref is set correctly). It looks like avrndec.c does not do this correctly if is_mjpeg is set. But in fact, this codec violates the API by declaring DR1 support, but not calling the user get_buffer2 callback. This could be fixed. An actually valid point is brought up by Mark Thompson (and not by you), but this can be fixed easily by wrapping the AVFrame properly before returning. (Assuming the desired semantics are that the opaque_ref is returned to the user as-is.) So, still no problem found. What is the problem? Now I'm very curious to hear what your "cleaner" solution to this problem is. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
