AMF SDK integration code cleanup: replace writer_id option to LIBAV_AMF_WRITER_ID
diff mbox series

Message ID 1006FD0F0FE06E479CEE8E19981AE070368044DC@ORO-MBOX-05.luxoft.com
State New
Headers show
Series
  • AMF SDK integration code cleanup: replace writer_id option to LIBAV_AMF_WRITER_ID
Related show

Commit Message

Kravchenko, Alexander Feb. 28, 2018, 6:33 a.m. 
AMFTraceWriter is abstraction to configure how AMF outputs its logs for current process, not for component.

Example instances of AMFTraceWriter can be
* FileWriter
* SocketWriter
* DebugOutputWriter
* LibavWriter (output using av_log function).

AMFTraceWriter can be Registered/Unregistered/Enabled/Disabled and configured to output particular level of trace output.

If we use multiple LibavWriter objects in one process, we will have duplication of output in avlib log.
To prevent this scenario we should use one constant writer_id .


---
 libavcodec/amfenc.c | 8 +++++---
 libavcodec/amfenc.h | 4 +---
 2 files changed, 6 insertions(+), 6 deletions(-)

--
2.16.2.windows.1

Comments

Luca Barbato Feb. 28, 2018, 3:42 p.m. | #1 
On 28/02/2018 07:33, Kravchenko, Alexander wrote:
> AMFTraceWriter is abstraction to configure how AMF outputs its logs for current process, not for component.
> 
> Example instances of AMFTraceWriter can be
> * FileWriter
> * SocketWriter
> * DebugOutputWriter
> * LibavWriter (output using av_log function).
> 
> AMFTraceWriter can be Registered/Unregistered/Enabled/Disabled and configured to output particular level of trace output.
> 
> If we use multiple LibavWriter objects in one process, we will have duplication of output in avlib log.
> To prevent this scenario we should use one constant writer_id .

Patch ok to me. Thank you

lu
Diego Biurrun March 1, 2018, 8:27 a.m. | #2 
On Wed, Feb 28, 2018 at 06:33:05AM +0000, Kravchenko, Alexander wrote:
> AMFTraceWriter is abstraction to configure how AMF outputs its logs for current process, not for component.
> 
> Example instances of AMFTraceWriter can be
> * FileWriter
> * SocketWriter
> * DebugOutputWriter
> * LibavWriter (output using av_log function).
> 
> AMFTraceWriter can be Registered/Unregistered/Enabled/Disabled and configured to output particular level of trace output.
> 
> If we use multiple LibavWriter objects in one process, we will have duplication of output in avlib log.
> To prevent this scenario we should use one constant writer_id .
> ---
>  libavcodec/amfenc.c | 8 +++++---
>  libavcodec/amfenc.h | 4 +---
>  2 files changed, 6 insertions(+), 6 deletions(-)

Queueing.

Do you mind if I switch your name to the "Alexander Kravchenko" standard
"first name then last name" order?

Diego
Kravchenko, Alexander March 1, 2018, 9:56 a.m. | #3 
> Do you mind if I switch your name to the "Alexander Kravchenko" standard
> "first name then last name" order?

of course, no problem
Diego Biurrun March 1, 2018, 12:09 p.m. | #4 
On Wed, Feb 28, 2018 at 06:33:05AM +0000, Kravchenko, Alexander wrote:
> If we use multiple LibavWriter objects in one process, we will have duplication of output in avlib log.

What is avlib log?

Diego
Kravchenko, Alexander March 1, 2018, 2:48 p.m. | #5 
> -----Original Message-----
> From: libav-devel [mailto:libav-devel-bounces@libav.org] On Behalf Of Diego
> Biurrun
> Sent: Thursday, March 1, 2018 3:10 PM
> To: libav development <libav-devel@libav.org>
> Subject: Re: [libav-devel] [PATCH] AMF SDK integration code cleanup:
> replace writer_id option to LIBAV_AMF_WRITER_ID
>
> On Wed, Feb 28, 2018 at 06:33:05AM +0000, Kravchenko, Alexander wrote:
> > If we use multiple LibavWriter objects in one process, we will have
> duplication of output in avlib log.
>
> What is avlib log?
>

I meant libav logger (av_log function)
Diego Biurrun March 7, 2018, 12:12 p.m. | #6 
On Thu, Mar 01, 2018 at 09:27:33AM +0100, Diego Biurrun wrote:
> On Wed, Feb 28, 2018 at 06:33:05AM +0000, Kravchenko, Alexander wrote:
> > AMFTraceWriter is abstraction to configure how AMF outputs its logs for current process, not for component.
> > 
> > Example instances of AMFTraceWriter can be
> > * FileWriter
> > * SocketWriter
> > * DebugOutputWriter
> > * LibavWriter (output using av_log function).
> > 
> > AMFTraceWriter can be Registered/Unregistered/Enabled/Disabled and configured to output particular level of trace output.
> > 
> > If we use multiple LibavWriter objects in one process, we will have duplication of output in avlib log.
> > To prevent this scenario we should use one constant writer_id .
> > ---
> >  libavcodec/amfenc.c | 8 +++++---
> >  libavcodec/amfenc.h | 4 +---
> >  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> Queueing.
> 
> Do you mind if I switch your name to the "Alexander Kravchenko" standard
> "first name then last name" order?

Also, I'd replace the log message with

    amf: Replace writer_id option with LIBAV_AMF_WRITER_ID constant

    AMFTraceWriter is an abstraction to configure how AMF outputs its logs
    for the current process and can be configured to output different levels
    of trace output. If multiple LibavWriter objects are used in one process,
    there will be duplication of output in av_log. Use a constant writer_id
    to prevent this scenario.

which is clearer to me. Correct me if I botched your intended meaning.

Diego
Kravchenko, Alexander March 7, 2018, 1:11 p.m. | #7 
> Also, I'd replace the log message with
>
>     amf: Replace writer_id option with LIBAV_AMF_WRITER_ID constant
>
>     AMFTraceWriter is an abstraction to configure how AMF outputs its logs
>     for the current process and can be configured to output different levels
>     of trace output. If multiple LibavWriter objects are used in one process,
>     there will be duplication of output in av_log. Use a constant writer_id
>     to prevent this scenario.
>
> which is clearer to me. Correct me if I botched your intended meaning.
>

Sounds good

Patch
diff mbox series 

diff --git a/libavcodec/amfenc.c b/libavcodec/amfenc.c
index f305a48bf..ef4350d1c 100644
--- a/libavcodec/amfenc.c
+++ b/libavcodec/amfenc.c
@@ -46,6 +46,8 @@ 
 #include <dlfcn.h>
 #endif

+#define LIBAV_AMF_WRITER_ID L"libav_log"
+
 #define PTS_PROP L"PtsProp"

 const enum AVPixelFormat ff_amf_pix_fmts[] = {
@@ -171,8 +173,8 @@  static int amf_init_context(AVCodecContext *avctx)
     // connect AMF logger to av_log
     ctx->tracer.vtbl = &tracer_vtbl;
     ctx->tracer.avctx = avctx;
-    ctx->trace->pVtbl->RegisterWriter(ctx->trace, ctx->writer_id, (AMFTraceWriter*)&ctx->tracer, 1);
-    ctx->trace->pVtbl->SetWriterLevel(ctx->trace, ctx->writer_id, AMF_TRACE_TRACE);
+    ctx->trace->pVtbl->RegisterWriter(ctx->trace, LIBAV_AMF_WRITER_ID,(AMFTraceWriter*)&ctx->tracer, 1);
+    ctx->trace->pVtbl->SetWriterLevel(ctx->trace, LIBAV_AMF_WRITER_ID, AMF_TRACE_TRACE);

     res = ctx->factory->pVtbl->CreateContext(ctx->factory, &ctx->context);
     AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "CreateContext() failed with error %d\n", res);
@@ -283,7 +285,7 @@  int av_cold ff_amf_encode_close(AVCodecContext *avctx)
     av_buffer_unref(&ctx->hw_frames_ctx);

     if (ctx->trace) {
-        ctx->trace->pVtbl->UnregisterWriter(ctx->trace, ctx->writer_id);
+        ctx->trace->pVtbl->UnregisterWriter(ctx->trace, LIBAV_AMF_WRITER_ID);
     }
     if (ctx->library) {
         dlclose(ctx->library);
diff --git a/libavcodec/amfenc.h b/libavcodec/amfenc.h
index f3b82be77..a8153ef12 100644
--- a/libavcodec/amfenc.h
+++ b/libavcodec/amfenc.h
@@ -80,7 +80,6 @@  typedef struct AmfContext {

     // common encoder options
     int                 log_to_dbg;
-    char                *writer_id;

     // Static options, have to be set before Init() call
     int                 usage;
@@ -152,7 +151,6 @@  extern const enum AVPixelFormat ff_amf_pix_fmts[];
     }

 #define AMF_COMMON_OPTIONS \
-    { "log_to_dbg",     "Enable AMF logging to debug output",   OFFSET(log_to_dbg), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 1, VE }, \
-    { "writer_id",      "Enable AMF logging to writer id",      OFFSET(writer_id),  AV_OPT_TYPE_STRING, { .str = "libavcodec" }, 0, 1, VE } \
+    { "log_to_dbg",     "Enable AMF logging to debug output",   OFFSET(log_to_dbg), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 1, VE } \

 #endif //AVCODEC_AMFENC_H