On Mon, Dec 03, 2012 at 10:34:33AM -0500, Don Moir wrote:
> av_set_log_level (AV_LOG_FATAL) still allows pretty much every log
> level. While I can check the level it bothers me that in some cases
> I can get like 1000 calls over a minutes time that I have no use
> for.
>
> I had mentioned in the past to allow NULL for av_log_callback and
> that was put in av_vlog but I wish the check for NULL was also at
> the top of av_log just to do nothing. The point being short-circuit
> av_log as soon as possible. Then would still need allowable
> av_assert0 replacement for the av_log call which default would be
> av_log.
>
> Am I nit-picking too much ? I don't think so in the sense why perform actions that consume time and are not needed.
Well, in the sense that we are (by your numbers) talking about
1000 per minute at maybe 20 instructions, or around
0.00001 % of CPU time I'd say yes.
However I notice two API concerns
1) Why do we apply log_level_offset_offset only to av_log but
not av_vlog? This is also not documented. Is this really intentional?
2) Why are we handling the log level only in av_log_default_callback
and not in common code like av_vlog?
> #define av_assert0(cond) do { \
> if (!(cond)) {
> - av_log(NULL, AV_LOG_FATAL, "Assertion %s failed at %s:%d\n", \
> + av_fatal_callback(NULL, AV_LOG_FATAL, "Assertion %s failed at %s:%d\n", \
> AV_STRINGIFY(cond), __FILE__, __LINE__); \
> abort(); \
> } \
> } while (0)
>
> All pretty frekin simple with no real code impact. Needs function to set av_fatal_callback.
I'd prefer to not increase the size of our already unmanageable
API as long as the only argument is a nearly non-measurable amount
of CPU usage :-)