Comments are inline

> -----Original Message-----
> From: libav-devel [mailto:[email protected]] On Behalf Of Diego
> Biurrun
> Sent: Tuesday, February 27, 2018 2:11 PM
> To: libav development
> Subject: Re: [libav-devel] [PATCH] AMF SDK integration code cleanup:
> remove writer_id option & move AMF_COMMON_OPTIONS out from
> amfenc.h
>
> This patch accmoplishes multiple unrelated things. Ideally you would submit
> one patch per change. Also, there is no explanation of why you are doing
> these changes in the commit message.

- Hi, I am collaborating with AMD on AMF integration tasks.  Probably I should 
start in thread: "[libav-devel] Add HW H.264 and HEVC encoding for AMD GPUs 
based on AMF SDK"

- I have put changes to one patch, because all of them solves 'options' cleanup 
task. If it is required can split this to the following separated changes
1. Remove writer_id
2. move AMF_COMMON_OPTIONS out from amfenc.h

>
> On Tue, Feb 27, 2018 at 10:34:27AM +0000, Kravchenko, Alexander wrote:
> > From 4d0efe3f5fd03db188f41d52ee9549a046939d1d Mon Sep 17 00:00:00
> 2001
> > From: diamond88 <[email protected]>
>
> Looks like your Git is misconfigured, please set your name properly.

Ok

>
> > --- a/libavcodec/amfenc.h
> > +++ b/libavcodec/amfenc.h
> > @@ -151,8 +148,4 @@ 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 } \
> > -
> > --- a/libavcodec/amfenc_h264.c
> > +++ b/libavcodec/amfenc_h264.c
> >
> > -    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 },
> >
> > --- a/libavcodec/amfenc_hevc.c
> > +++ b/libavcodec/amfenc_hevc.c
> > @@ -92,7 +91,7 @@ static const AVOption options[] = {
> >
> > -    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 },
>
> Why is this an improvement?

- Current code cause warning described here  
https://lists.libav.org/pipermail/libav-devel/2018-February/085705.html

----------
/home/diego/src/libav.vanilla/libavcodec/amfenc.c: In function 
‘amf_init_context’:
/home/diego/src/libav.vanilla/libavcodec/amfenc.c:159:51: warning: passing 
argument 2 of ‘ctx->trace->pVtbl->RegisterWriter’ from incompatible pointer 
type [-Wincompatible-pointer-types]
     ctx->trace->pVtbl->RegisterWriter(ctx->trace, ctx->writer_id, 
(AMFTraceWriter *)&ctx->tracer, 1);
                                                   ^~~
/home/diego/src/libav.vanilla/libavcodec/amfenc.c:159:51: note: expected ‘const 
wchar_t * {aka const int *}’ but argument is of type ‘char *’
/home/diego/src/libav.vanilla/libavcodec/amfenc.c:160:51: warning: passing 
argument 2 of ‘ctx->trace->pVtbl->SetWriterLevel’ from incompatible pointer 
type [-Wincompatible-pointer-types]
     ctx->trace->pVtbl->SetWriterLevel(ctx->trace, ctx->writer_id, 
AMF_TRACE_TRACE);
                                                   ^~~
/home/diego/src/libav.vanilla/libavcodec/amfenc.c:160:51: note: expected ‘const 
wchar_t * {aka const int *}’ but argument is of type ‘char *’
/home/diego/src/libav.vanilla/libavcodec/amfenc.c: In function 
‘ff_amf_encode_close’:
/home/diego/src/libav.vanilla/libavcodec/amfenc.c:265:57: warning: passing 
argument 2 of ‘ctx->trace->pVtbl->UnregisterWriter’ from incompatible pointer 
type [-Wincompatible-pointer-types]
         ctx->trace->pVtbl->UnregisterWriter(ctx->trace, ctx->writer_id);
                                                         ^~~
/home/diego/src/libav.vanilla/libavcodec/amfenc.c:265:57: note: expected ‘const 
wchar_t * {aka const int *}’ but argument is of type ‘char *’
----------

- I don’t see common practice to configure component logging like this. It look 
like this should be done by another way (config file in working dir or some 
other way)
This will allow to have interface clean

>
> Diego
> _______________________________________________
> libav-devel mailing list
> [email protected]
> https://lists.libav.org/mailman/listinfo/libav-devel




________________________________

This e-mail and any attachment(s) are intended only for the recipient(s) named 
above and others who have been specifically authorized to receive them. They 
may contain confidential information. If you are not the intended recipient, 
please do not read this email or its attachment(s). Furthermore, you are hereby 
notified that any dissemination, distribution or copying of this e-mail and any 
attachment(s) is strictly prohibited. If you have received this e-mail in 
error, please immediately notify the sender by replying to this e-mail and then 
delete this e-mail and any attachment(s) or copies thereof from your system. 
Thank you.
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to