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

Ramiro Polla ramiro.polla
Thu Jul 9 00:41:08 CEST 2009


Hi,

Sorry for the very late reply, I was incredibly busy.

On Thu, Jun 11, 2009 at 7:02 PM, Michael Niedermayer<michaelni at gmx.at> wrote:
> On Thu, Jun 11, 2009 at 10:34:46PM +0200, Diego Biurrun wrote:
>> On Thu, Jun 11, 2009 at 10:02:12PM +0200, Michael Niedermayer wrote:
>> > On Thu, Jun 11, 2009 at 09:54:50PM +0200, Diego Biurrun 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
>> > > >
>> > > >
>> > > > > Go flame whoever introduced DECLARE_ALIGNED into example
>> > > > > code.
>> > > >
>> > > > you did in r20037
>> > >
>> > > No:
>> > >
>> > > silver:/var/tmp/mplayer_vanilla $ svn diff -c 20037
>> > > --- libswscale/swscale-example.c ? ? ? ?(revision 20036)
>> > > +++ libswscale/swscale-example.c ? ? ? ?(revision 20037)
>> > > @@ -25,6 +25,8 @@
>> > > ?#undef HAVE_AV_CONFIG_H
>> > > ?#include "avutil.h"
>> > > ?#include "swscale.h"
>> > > +#include "swscale_internal.h"
>> > > +#include "rgb2rgb.h"
>> >
>> > grep -c DECLARE_AL libswscale/swscale-example.c
>> > 0
>> > grep -c DECLARE_AL libswscale/swscale_internal.h
>> > 28
>> >
>> > so yes, your #include swscale_internal.h introduced them
>>
>> That was necessary because swscale-example uses sws_format_name and
>> isALPHA, which are declared/defined in swscale_internal.h...
>
> sws_format_name() could be moved to swscale.h

IMHO pixel format information should all be moved to one single place.
Currently we have 3 lists with pixel format names. Moving it to
swscale.h (making it public) only to remove it later when a cleaner
system is ready doesn't sound like a good idea to me.

> isALPHA could until someone has a better idea just be copied into
> swscale-example.c, this is ugly but if it avoids the #includes it seems
> an acceptable compromise

Hack for both isALPHA and sws_format_name() done in swscale-example.c.
Note that I think a better way to find out isALPHA is also part of a
cleaner pixel format system, so both should be fixed at the same time.

Taking mans' and diego's comments into consideration, I propose these patches:
1_drop_error.diff drops the error case for DECLARE_ALIGNED
2_move_back_to_mem_h.diff moves the macros back to mem.h
3_swscale.diff cleans the swscale stuff by using the hacks I mentioned.

Ramiro Polla
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 1_drop_error.diff
Type: text/x-patch
Size: 541 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/attachments/20090708/63f28556/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 2_move_back_to_mem_h.diff
Type: text/x-patch
Size: 1888 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/attachments/20090708/63f28556/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 3_swscale.diff
Type: text/x-patch
Size: 1296 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/attachments/20090708/63f28556/attachment-0002.bin>



More information about the ffmpeg-cvslog mailing list