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? > > > > > > Signed-off-by: Gregory J. Wolfe <gregory.wo...@kodakalaris.com> > > > --- > > > libavcodec/libopenh264enc.c | 22 ++++++++++++++++++++++ > > > 1 file changed, 22 insertions(+) > > > > > > diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c > > > index b315e8b..146bbaf 100644 > > > --- a/libavcodec/libopenh264enc.c > > > +++ b/libavcodec/libopenh264enc.c > > > @@ -37,6 +37,9 @@ typedef struct SVCContext { > > > int slice_mode; > > > int loopfilter; > > > char *profile; > > > + int OPTION_TRACE_LEVEL; > > > + int64_t OPTION_TRACE_CALLBACK; // message logging function, type is > > > + // actually WelsTraceCallback > > > } SVCContext; > > > > > > #define OPENH264_VER_AT_LEAST(maj, min) \ @@ -52,6 +55,17 @@ > > static > > > const AVOption options[] = { > > > { "auto", "Automatic number of slices according to number of > > threads", 0, AV_OPT_TYPE_CONST, { .i64 = SM_AUTO_SLICE }, 0, 0, VE, > > "slice_mode" }, > > > { "loopfilter", "Enable loop filter", OFFSET(loopfilter), > > AV_OPT_TYPE_INT, { .i64 = 1 }, 0, 1, VE }, > > > { "profile", "Set profile restrictions", OFFSET(profile), > > AV_OPT_TYPE_STRING, { 0 }, 0, 0, VE }, > > > + { "OPTION_TRACE_LEVEL", "Message logging level", > > OFFSET(OPTION_TRACE_LEVEL), AV_OPT_TYPE_INT, { .i64 = > > WELS_LOG_DEFAULT }, WELS_LOG_QUIET, WELS_LOG_RESV, VE, > > "OPTION_TRACE_LEVEL" }, > > > + { "default", "default libopenh264 message logging", 0, > > AV_OPT_TYPE_CONST, { .i64 = WELS_LOG_DEFAULT }, 0, 0, VE, > > "OPTION_TRACE_LEVEL" }, > > > + { "quiet", "quiet mode", 0, AV_OPT_TYPE_CONST, { .i64 = > > WELS_LOG_QUIET }, 0, 0, VE, "OPTION_TRACE_LEVEL" }, > > > + { "error", "log error messages", 0, AV_OPT_TYPE_CONST, { .i64 = > > WELS_LOG_ERROR }, 0, 0, VE, "OPTION_TRACE_LEVEL" }, > > > + { "warning", "log warning+err messages", 0, AV_OPT_TYPE_CONST, { > > .i64 = WELS_LOG_WARNING }, 0, 0, VE, "OPTION_TRACE_LEVEL" }, > > > + { "info", "log informational+wrn+err messages", 0, > > AV_OPT_TYPE_CONST, { .i64 = WELS_LOG_INFO }, 0, 0, VE, > > "OPTION_TRACE_LEVEL" > > }, > > > + { "debug", "log debug+inf+wrn+err messages", 0, > > > + AV_OPT_TYPE_CONST, { > > .i64 = WELS_LOG_DEBUG }, 0, 0, VE, "OPTION_TRACE_LEVEL" }, > > > + { "detail", "log detail+dbg+inf+wrn+err messages", 0, > > AV_OPT_TYPE_CONST, { .i64 = WELS_LOG_DETAIL }, 0, 0, VE, > > "OPTION_TRACE_LEVEL" }, > > > + // for OPTION_TRACE_CALLBACK, default value of 0 means do not > > change; otherwise, it is the > > > + // address of the WelsTraceCallback message logging function, > > > + passed > > in as an int64_t > > > + { "OPTION_TRACE_CALLBACK", "Message logging function, 0 means do > > > + not > > change", OFFSET(OPTION_TRACE_CALLBACK), AV_OPT_TYPE_INT64, { .i64 = > > 0 }, INT64_MIN, INT64_MAX, VE, "OPTION_TRACE_CALLBACK" }, > > > { NULL } > > > }; > > > > > > @@ -89,6 +103,14 @@ static av_cold int svc_encode_init(AVCodecContext > > *avctx) > > > av_log(avctx, AV_LOG_ERROR, "Unable to create encoder\n"); > > > return AVERROR_UNKNOWN; > > > } > > > + // set message logging options > > > + // message logging level > > > + > > (*s->encoder)->SetOption(s- > > >encoder,ENCODER_OPTION_TRACE_LEVEL,&s->OPTION_TRACE_LEVEL); > > > + if ( s->OPTION_TRACE_CALLBACK ) > > > + {//set message logging function > > > + WelsTraceCallback OPTION_TRACE_CALLBACK = (WelsTraceCallback) > > ((uintptr_t)s->OPTION_TRACE_CALLBACK); > > > + > > (*s->encoder)->SetOption(s- > > >encoder,ENCODER_OPTION_TRACE_CALLBACK,(void > > *)&OPTION_TRACE_CALLBACK); > > > + }//set message logging function > > > > > > (*s->encoder)->GetDefaultParams(s->encoder, ¶m); > > > > > > -- > > > 1.8.5.2.msysgit.0 > > > > > > _______________________________________________ > > > ffmpeg-devel mailing list > > > ffmpeg-devel@ffmpeg.org > > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
