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. Mikhail
> -----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
