[FFmpeg-devel] [PATCH] lavu/tx: stop using av_log(NULL, )
Anton Khirnov
anton at khirnov.net
Fri Jul 26 11:43:39 EEST 2024
Quoting Lynne via ffmpeg-devel (2024-07-26 10:33:24)
> On 26/07/2024 09:47, Anton Khirnov wrote:
> > Quoting Lynne via ffmpeg-devel (2024-07-26 08:42:11)
> >> Its not feasible to add an AVClass in the main context, as
> >> it would waste space, as the main context is recursive, and
> >> every bit of assembly would need to be changed.
> >>
> >> While its true that on paper av_log has access to the main
> >> context, that functionality is not used as no options are
> >> available for setting. No options will be exposed either,
> >> and it makes no sense.
> >>
> >> mpv has recently started warning if a NULL AVClass is used
> >> as an FFmpeg bug. While I don't fully agree nor disagree with
> >> this, this is a simple patch which fixes the issue.
> >
> > No, it just hides the issue for the time being.
>
> If this means "it may get broken eventually, its not forbidden
> anywhere", then IMO we should just codify the current behavior such that
> it won't, unless there's some use-case you can think of.
It means "you're offloading fixing this issue properly on someone else",
which is a bad thing in my book. In my understanding of the API, it _is_
forbidden and UB.
> > I am against this patch, just add a proper AVClass. AVTXContext is
> > entirely opaque, so it should definitely be feasible.
> I'd like to avoid adding a pointer and allocating it if it can't be
> helped. And properly integrating each context into the AVClass system as
> a child of the parent context.
Consider the possibility that if your code depends on fixed layout of a
big and complex struct, then your code is...suboptimal. If it's about
offsets for SIMD, you could either
* generate them at build time
* move the things used by SIMD into a smaller struct embedded in the
main context
> If you think a NULL av_log is valid (you implied that a year ago), then
> I'm more than happy to drop this patch.
I didn't and I don't. To the contrary, I consider av_log(NULL) a
mispattern that we should be rid of.
--
Anton Khirnov
More information about the ffmpeg-devel
mailing list