On Wed, Feb 07, 2018 at 07:35:05PM +0000, Mironov, Mikhail wrote:
> Regarding PRId64 macro:
> Started to look into this and found that at least in Microsoft's version of 
> inttypes.h there no definitions for Unicode strings. All AMF tracing /logging 
> is Unicode. 
> So the thought was to have a full set of macros in one place: LPRId64 and 
> AMFPRId64 rather to just add missing macros.

Been there, done that. It works better if you just define missing macros
for the (fewer and fewer) legacy platforms. You avoid the clutter of
locally-defined macros in your code and you only have to maintain a few
fallback definitions.

> > -----Original Message-----
> > From: libav-devel [mailto:libav-devel-boun...@libav.org] On Behalf Of
> > Mironov, Mikhail
> > Sent: February 7, 2018 1:40 PM
> > To: libav development <libav-devel@libav.org>
> > Subject: Re: [libav-devel] Add HW H.264 and HEVC encoding for AMD GPUs
> > based on AMF SDK
> > 
> > Not yet, I mean we will fix them, in libav or in AMF.
> > I quickly searched but did not find any formal standard on include guards. I
> > did see some recommendations in forums though.
> > To get some statistics: I checked few header files in Windows SDK, OpenCL
> > SDK, OpenGL, Intel Media SDK - I see all kind of styles including  
> > __FILE_H__.
> > For example: https://github.com/Intel-Media-
> > SDK/MediaSDK/blob/master/api/include/mfxvideo.h
> > 
> > Mikhail
> > 
> > > -----Original Message-----
> > > From: libav-devel [mailto:libav-devel-boun...@libav.org] On Behalf Of
> > > Diego Biurrun
> > > Sent: February 7, 2018 12:37 PM
> > > To: libav development <libav-devel@libav.org>
> > > Subject: Re: [libav-devel] Add HW H.264 and HEVC encoding for AMD GPUs
> > > based on AMF SDK
> > >
> > > On Wed, Feb 07, 2018 at 04:27:30PM +0000, Mironov, Mikhail wrote:
> > > > Warnings should be fixed. More comments inline.
> > >
> > > Fixed as in fixed if I pull in fresh headers from Git?
> > >
> > > > > On Sun, Feb 04, 2018 at 07:04:39PM +0000, Mironov, Mikhail wrote:
> > > > > > I have a developer who will cover AMF integration on regular
> > > > > > basis
> > > > > (together with me). He starts tomorrow and will be up to date soon.
> > > > > > I will ask him to cleanup the things you mentioned if you will
> > > > > > not submit the
> > > > > changes first.
> > > > > > He is on BCC for now.
> > > > >
> > > > > It seems a bit more cleanup is needed on your side as well:
> > > > >
> > > > > /home/libav/libs/include/AMF/core/Platform.h:431:16: error:
> > > > > implicit declaration of function ‘memcmp’
> > > > > [-Werror=implicit-function-declaration]
> > > > >
> > > > > Indeed that header is missing a string.h #include and things break
> > > > > on one of my systems.
> > > > >
> > > > > Also, there are a bunch of warnings:
> > > > >
> > > > > ~/src/build/amf $ make libavcodec/amfenc.o
> > > > > CC      libavcodec/amfenc.o
> > > > > /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
> > > *’
> > > > >
> > > > > One could just cast them away, but probably it's better to ajust the
> > types.
> > > > >
> > > > >
> > > > > Also, I noticed two issues in your headers (at a glance):
> > > > >
> > > > > 1) You recreate parts of inttypes.h, i.e.
> > > > >
> > > > >    #define AMFPRId64    "lld"
> > > > >
> > > > >    and similar - why, oh, why? What ass-backwards systems are you
> > > > > trying
> > > to
> > > > >    support there?
> > > >
> > > > I don’t remember right now which system required the custom macro
> > > > but it
> > > was real issue five years ago.
> > > > This is AMF SDK issue, not libav integration, we should clean this
> > > > up
> > > though.
> > >
> > > 5 years ago on Linux or with some of those horrible, now-obsolete MSVC
> > > versions? The latter I believe right away, but finding Linux systems
> > > that were not C99 compliant was difficult even in 2012...
> > >
> > > > > 2) You are invading system namespace:
> > > > >
> > > > >    #ifndef __AMFPlatform_h__
> > > > >
> > > > >    Identifiers starting with __ (or _ and an uppercase letter) are 
> > > > > reserved
> > > > >    for the system. Should be easy enough to replace these with
> > > >
> > > > This is a decision made for AMF SDK which is external for libav
> > integration.
> > > Which standard or recommendation are you refer to?
> > >
> > > The C standard reserves those identifiers for the system, i.e. the
> > > kernel or the C library.
> > >
> > > Diego
> > > _______________________________________________
> > > libav-devel mailing list
> > > libav-devel@libav.org
> > > https://lists.libav.org/mailman/listinfo/libav-devel
> > _______________________________________________
> > libav-devel mailing list
> > libav-devel@libav.org
> > https://lists.libav.org/mailman/listinfo/libav-devel
> _______________________________________________
> libav-devel mailing list
> libav-devel@libav.org
> https://lists.libav.org/mailman/listinfo/libav-devel
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to