[FFmpeg-cvslog] r29354 - trunk/libswscale/swscale-example.c

Fri Jun 12 02:20:10 CEST 2009

On Thu, Jun 11, 2009 at 7:20 PM, Michael Niedermayer<michaelni at gmx.at> wrote:
> On Thu, Jun 11, 2009 at 05:51:19PM -0300, Ramiro Polla wrote:
>> On Thu, Jun 11, 2009 at 5:23 PM, Diego Biurrun<diego at biurrun.de> wrote:
>> > On Thu, Jun 11, 2009 at 09:42:27PM +0200, Michael Niedermayer wrote:
>> >> On Thu, Jun 11, 2009 at 09:32:05PM +0200, Michael Niedermayer wrote:
>> >> > On Thu, Jun 11, 2009 at 09:31:49PM +0200, Diego Biurrun wrote:
>> >> > > On Thu, Jun 11, 2009 at 08:42:57PM +0200, Michael Niedermayer wrote:
>> >> > > > On Thu, Jun 11, 2009 at 07:27:35PM +0200, Diego Biurrun wrote:
>> >> > > > > On Thu, Jun 11, 2009 at 06:41:41PM +0200, Michael Niedermayer wrote:
>> >> > > > > > On Thu, Jun 11, 2009 at 06:08:35PM +0200, Diego Biurrun wrote:
>> >> > > > > > > On Thu, Jun 11, 2009 at 05:46:11PM +0200, Michael Niedermayer wrote:
>> >> > > > > > > > On Thu, Jun 11, 2009 at 05:15:43PM +0200, diego wrote:
>> >> > > > > > > > >
>> >> > > > > > > > > Log:
>> >> > > > > > > > > Fix compilation: #undef standard library functions that are
>> >> > > > > > > > > forbidden within FFmpeg, but allowed in example code.
>> >> > > > > > > >
>> >> > > > > > > > this commit is not the correct solution
>> >> > > > > > >
>> >> > > > > > > Whoever knows the correct solution shall step forward...
>> >> > > > > >
>> >> > > > > > I do, revert your previous commit to swscale (r29353) as well and it works
>> >> > > > >
>> >> > > > > No, it does not.
>> >> > > >
>> >> > > > yes it does, i tested it before making that claim
>> >> > > >
>> >> > > > upon further investigation libavutil/mem.h here contained DECLARE_ALIGNED
>> >> > > > and was in C state, svn skiped it during updates
>> >> > > >
>> >> > > > now if you also revert your r16781 commit, swscale-example.c compiles
>> >> > > >
>> >> > > > to quote your commit message from r16781:
>> >> > > > ? ? Move DECLARE_ALIGNED and DECLARE_ASM_CONST to internal.h.
>> >> > > > ? ? Their definition depends on preprocessor directives from config.h,
>> >> > > > ? ? thus they cannot be declared in a public header since public headers
>> >> > > > ? ? cannot #include config.h.
>> >> > > >
>> >> > > > internal.h is no public header -> it cannot be used outside libavutil
>> >> > > > but DECLARE_ALIGNED is used all over the place outside libavutil
>> >> > >
>> >> > > So in summary, as I said, reverting my commits is not the correct
>> >> > > solution.
>> >> >
>> >> > iam not saying that blindly reverting them is the correct solution
>> >> > but i belive the correct solution will involve reverting them
>> >>
>> >> more correctly, some of them ...
>> >>
>> >>
>> >> also, the following are wrong beyond doubt:
>> >>
>> >> libavformat/adtsenc.c:#include "libavcodec/internal.h"
>> >
>> > libavformat/adtsenc.c:66: warning: implicit declaration of function `ff_log_missing_feature'
>> >
>> > ------------------------------------------------------------------------
>> > r17859 | alexc | 2009-03-06 22:19:16 +0100 (Fri, 06 Mar 2009) | 2 lines
>> > Changed paths:
>> > ? M /trunk/libavformat/adtsenc.c
>> >
>> > ADTS: Increased protection against writing illegal/nonsense files.
>> > ------------------------------------------------------------------------
>> >
>> >> libavcodec/aac.h:#include "libavutil/internal.h"
>> >> libavcodec/ac3dec.h:#include "libavutil/internal.h"
>> >
>> > These two appear to be unnecessary, removed.
>> >
>> >> libswscale/swscale_internal.h:#include "libavutil/internal.h"
>> >
>> > ------------------------------------------------------------------------
>> > r29350 | ramiro | 2009-06-05 00:50:38 +0200 (Fri, 05 Jun 2009) | 1 line
>> > Changed paths:
>> > ? M /trunk/libswscale/ppc/swscale_altivec_template.c
>> > ? M /trunk/libswscale/ppc/yuv2rgb_altivec.c
>> >
>> > Use DECLARE_ALIGNED macro instead of __attribute__((aligned)) for ppc
>> > code.
>> > ------------------------------------------------------------------------
>> > r29349 | ramiro | 2009-06-05 00:10:52 +0200 (Fri, 05 Jun 2009) | 1 line
>> > Changed paths:
>> > ? M /trunk/libswscale/swscale_template.c
>> >
>> > Replace more uses of __attribute__((aligned)) by DECLARE_ALIGNED.
>> > ------------------------------------------------------------------------
>> > r29348 | ramiro | 2009-06-04 23:55:52 +0200 (Thu, 04 Jun 2009) | 2 lines
>> > Changed paths:
>> > ? M /trunk/libswscale/swscale.c
>> > ? M /trunk/libswscale/swscale_internal.h
>> >
>> > Use DECLARE_ALIGNED macro instead of gcc __attribute__.
>> > Patch by Pavel Pavlov <pavel at summit-tech dot ca>
>> > ------------------------------------------------------------------------
>> >
>> >
>> > So I call upon Alex Converse and Ramiro Polla to fix the problems they
>> > introduced. ?I'm innocent :)
>>
>> How does libavcodec use DECLARE_ALIGNED all over the place? At some
>> point libavutil/internal.h must be included, probably indirectly, but
>> still...
>
> it #includes common.h which under #ifdef HAVE_AV_CONFIG_H
> #includes internal.h
> this of course is also not the way it should be, still it is not as
> wrong as litterally including an internal header
> it at least keeps the hack as one line of common.h and most important
> it only includes the header within libav* as user applications and
> examples do not define HAVE_AV_CONFIG_H
>
> * I suspect we all agree that the DECLARE_ALIGNED and co macros
> ?should ideally simply be part of libavutils public API, if they
> ?where, they could just be used trivially, and there no doubt is
> ?interrest to use such macros ...
> ?and after all they where in mem.h in the past ...
> * now DECLARE_ALIGNED depends on config.h and config.h is not part
> ?of the public API, but this dependancy is a joke, its just
> ?a HAVE_INLINE_ASM that does an #error
>
> -> simply moving DECLARE_ALIGNED back to mem.h and #including
> config.h into mem.h under #ifdef HAVE_AV_CONFIG_H seems like
> it could be a solution

revert_declare_aligned_to_internal.diff moves it back from internal.h
to mem.h, and splits
-#elif HAVE_INLINE_ASM
-    #error The asm code needs alignment, but we do not know how to do
it for this compiler.
-#else
into
+#elif defined(HAVE_AV_CONFIG_H)
+#if HAVE_INLINE_ASM
+    #error The asm code needs alignment, but we do not know how to do
it for this compiler.
+#endif
+#else

This split will be separate, but I'm just lazy to create one more patch.

According to Mans,
#elif defined(HAVE_AV_CONFIG_H) && HAVE_INLINE_ASM
should give warnings on some compilers, and
#if HAVE_INLINE_ASM
when HAVE_INLINE_ASM hasn't been defined is undefined behaviour, so
even though it works with gcc (treats it as #if 0), we shouldn't rely
on it.

and then revert_including_internal.diff

Ramiro Polla
-------------- next part --------------
A non-text attachment was scrubbed...
Name: revert_declare_aligned_to_internal.diff
Type: text/x-patch
Size: 2354 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/attachments/20090611/92ceecbc/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: revert_including_internal.diff
Type: text/x-patch
Size: 848 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/attachments/20090611/92ceecbc/attachment-0001.bin>