[FFmpeg-devel] [PATCH] Add AVOptions for libopenh264 encoder message log level and log function.
Gregory J Wolfe
gregory.wolfe at kodakalaris.com
Wed Sep 2 22:42:25 CEST 2015
> -----Original Message-----
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf
> Of wm4
> Sent: Wednesday, September 02, 2015 2:28 PM
> To: ffmpeg-devel at 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.wolfe at kodakalaris.com> wrote:
>
> > > -----Original Message-----
> > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at 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.wolfe at 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.
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?
>
> > > >
> > > > Signed-off-by: Gregory J. Wolfe <gregory.wolfe at 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 at ffmpeg.org
> > > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > > _______________________________________________
> > > ffmpeg-devel mailing list
> > > ffmpeg-devel at ffmpeg.org
> > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
More information about the ffmpeg-devel
mailing list