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
