Re: [FFmpeg-devel] [PATCH] lavu/tx: stop using av_log(NULL, )

[Anton Khirnov <anton@khirnov.net>]

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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".