On Wed, 2 Sep 2015 20:42:25 +0000 Gregory J Wolfe <gregory.wo...@kodakalaris.com> wrote:
> > -----Original Message----- > > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf > > Of wm4 > > Sent: Wednesday, September 02, 2015 2:28 PM > > To: ffmpeg-devel@ffmpeg.org > > Subject: Re: [FFmpeg-devel] [PATCH] Add AVOptions for libopenh264 > > encoder message log level and log function. > > > > On Wed, 2 Sep 2015 17:36:30 +0000 > > Gregory J Wolfe <gregory.wo...@kodakalaris.com> wrote: > > > > > > -----Original Message----- > > > > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On > > > > Behalf Of Paul B Mahol > > > > Sent: Wednesday, September 02, 2015 1:15 PM > > > > To: FFmpeg development discussions and patches > > > > Subject: Re: [FFmpeg-devel] [PATCH] Add AVOptions for libopenh264 > > > > encoder message log level and log function. > > > > > > > > Dana 2. 9. 2015. 19:10 osoba "Gregory J. Wolfe" < > > > > gregory.wo...@kodakalaris.com> napisala je: > > > > > > > > > > By default, the libopenh264 encoder logs error and warning > > > > > messages to stderr. However, libopenh264 also provides a > > > > > SetOption() function that allows the client to set the message > > > > > logging level (e.g., error, warning, > > > > > debug) and a client-supplied message logging function (whose > > > > > signature must conform to libopenh264 type WelsTraceCallback). > > > > > > > > > > To enable this from the FFmpeg level, file libopenh264enc.c has > > > > > been modified to add AVOption entries OPTION_TRACEL_LEVEL (log > > > > > level) and OPTION_TRACE_CALLBACK (log function). The names are > > > > > derived from the enumerator values ENCODER_OPTION_TRACE_LEVEL > > and > > > > > ENCODER_OPTION_TRACE_CALLBACK which are passed to SetOption(). > > > > While > > > > > they are somewhat verbose, they are very likely unique across > > encoders. > > > > > > > > > > Some FFmpeg command line examples. To disable all libopenh264 > > > > messages: > > > > > > > > > > ffmpeg ... -OPTION_TRACE_LEVEL quiet > > > > > > > > > > To enable only error messages: > > > > > > > > > > ffmpeg ... -OPTION_TRACE_LEVEL error > > > > > > > > > > The OPTION_TRACE_CALLBACK option can obviously not be used from > > > > > the command line since it requires the address of a function. > > > > > When used programmatically, the function address must be converted > > > > > to a string representation of the function address as a signed 64-bit > > integer. > > > > > > > > > > The function signature defined by libopenh264 type > > > > > WelsTraceCallback can be found in source file > > > > > codec/api/svc/codec_api.h. The default implementation used by > > > > > libopenh264 is function welsStderrTrace(), and can be found in source > > file codec/common/source/welsCodecTrace.cpp. > > > > > > > > Why this doesnt make use of av_log functions? > > > > > > The libopenh264 library requires its own specific signature for the > > > log function that you give to it. If the client so desires, he/she > > > can create a message logging function with the required libopenh264 > > > signature that then invokes av_log. Note that libopenh264 already > > > formats its messages using its own prefix strings, similar to what FFmpeg > > does. > > > > > > > > > > > CAPS LOCK OPTIONS ARE UNACCEPTABLE > > > > > > > > +1 > > > > > I'm guessing that you're objecting to the command line switch values > > > like -OPTION_TRACE_LEVEL. I did not want to use something like > > > loglevel since it is already used by FFmpeg. libopenh264 uses its own > > > enumerator values for its message logging level, so I simply tried to > > > come up with something that would not conflict with any existing > > > AVOptions. Do the AVOption keywords have to be unique across all > > > formats/encoders/decoders? Please supply some guidance as to > > > requirements/restrictions and I will be happy to update the patch. > > > > I don't see the slightest point in this. You could setup a callback which > > in turn > > calls av_log(). This would be quite sane. It'd just work, and the user > > doesn't > > have to set an obscure encoder-specific option to get expected behavior. I'm > > not sure what you're objecting here. Is it maybe that the libopenh264 > > callback doesn't have a log level parameter? > > > > In addition to being extremely roundabout to the user (as opposed to using > > av_log), OPTION_TRACE_CALLBACK is also missing documentation. > > What signature does it have? > > I think I see what you're getting at. But first, some quick comments in > response: > > - Admittedly, my first attempt addressed, at only the very lowest level, > the minimum amount needed to be able to control messages from > libopenh264, and was not well integrated with FFmpeg w.r.t. av_log. > > - Regarding documentation, I did state in the git commit message: > > ... a client-supplied message logging function (whose signature must conform > to libopenh264 type WelsTraceCallback) .... The function signature defined by > libopenh264 type WelsTraceCallback can be found in source file > codec/api/svc/codec_api.h. The default implementation used by > libopenh264 is function welsStderrTrace(), and can be found in source > file codec/common/source/welsCodecTrace.cpp. Yeah, but no user potentially interested in getting libopenh264 log output is going to find the commit just to learn how to use it. > OK, time to move forward. Based on the discussion so far, here's what I > propose as a revision: > > (1) Eliminate OPTION_TRACE_LEVEL and OPTION_TRACE_CALLBACK. > > (2) Intelligently map the FFmpeg log levels to their equivalents > (more or less) in libopenh264, so that a change to the FFmpeg log level > has the corresponding effect on libopenh264 logging. > > (3) Add code to libopenh264enc.c that implements and sets up the needed > libopenh264 logging function, which itself uses av_log. > > These changes would then fully integrate the libopenh264 message logging > Into FFmpeg without the client having to do anything. > > Feedback? Sounds perfect. Note that API users may or may not do their own filtering using a custom av_log callback, rather than relying on av_log's filtering. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
