On Sat, Jul 7, 2012 at 10:39 AM, Diego Biurrun <[email protected]> wrote:
> On Sat, Jul 07, 2012 at 06:41:39PM +0200, Reinhard Tartler wrote:
>> On Sat, Jul 7, 2012 at 6:04 PM, Diego Biurrun <[email protected]> wrote:
>> > On Sat, Jul 07, 2012 at 08:54:12AM -0700, Ronald S. Bultje wrote:
>> >> On Sat, Jul 7, 2012 at 5:26 AM, Diego Biurrun <[email protected]> wrote:
>> >> > On Fri, Jul 06, 2012 at 09:59:50PM -0700, Ronald S. Bultje wrote:
>> >> >> On Fri, Jul 6, 2012 at 6:53 PM, Diego Biurrun <[email protected]> wrote:
>> >> >> > On Fri, Jul 06, 2012 at 10:33:19AM -0700, Ronald S. Bultje wrote:
>> >> >> >> On Fri, Jul 6, 2012 at 10:27 AM, Luca Barbato <[email protected]> 
>> >> >> >> wrote:
>> >> >> >> > On 07/06/2012 07:13 PM, Ronald S. Bultje wrote:
>> >> >> >> >> On Wed, Jul 4, 2012 at 2:09 PM, Måns Rullgård <[email protected]> 
>> >> >> >> >> wrote:
>> >> >> >> >>> "Ronald S. Bultje" <[email protected]> writes:
>> >> >> >> >>>> On Tue, Jul 3, 2012 at 8:14 PM, Ronald S. Bultje 
>> >> >> >> >>>> <[email protected]> wrote:
>> >> >> >> >>>>> From: "Ronald S. Bultje" <[email protected]>
>> >> >> >> >>>>>
>> >> >> >> >>>>> This allows compiling and running these tests on systems 
>> >> >> >> >>>>> lacking a built-
>> >> >> >> >>>>> in version of getopt(), such as MSVC.
>> >> >> >> >>>>> ---
>> >> >> >> >>>>>  configure             |    2 ++
>> >> >> >> >>>>>  libavcodec/dct-test.c |    7 +++++
>> >> >> >> >>>>>  libavcodec/fft-test.c |    6 ++++
>> >> >> >> >>>>>  libavcodec/getopt.c   |   84 
>> >> >> >> >>>>> +++++++++++++++++++++++++++++++++++++++++++++++++
>> >> >> >> >>>>>  4 files changed, 99 insertions(+)
>> >> >> >> >>>>>  create mode 100644 libavcodec/getopt.c
>> >> >> >> >>>>
>> >> >> >> >>>> Ping.
>> >> >> >> >>>
>> >> >> >> >>> No matter what, a replacement getopt.c does *not* belong in 
>> >> >> >> >>> libavcodec/
>> >> >> >> >>
>> >> >> >> >> So where does it go? Also, ping re: rest of the patch.
>> >> >> >> >
>> >> >> >> > Ops my email got lost...
>> >> >> >> >
>> >> >> >> > libavutil probably, is it the only place in which getopt is used?
>> >> >> >>
>> >> >> >> git says:
>> >> >> >> tools/graph2dot.c
>> >> >> >> libavcodec/motion-test.c
>> >> >> >> libavcodec/fft-test.c
>> >> >> >> libavcodec/dct-test.c
>> >> >> >
>> >> >> > IMO this is not worth the trouble.  Test for getopt in configure and
>> >> >> > compile those programs conditionally.
>> >> >>
>> >> >> They're part of fate.
>> >> >
>> >> > So?  Just run the fate tests conditionally as well.
>> >> >
>> >> >> I don't understand the trouble part. I already did all the effort.
>> >> >> What more trouble could there possibly be? Is deciding where to put
>> >> >> getopt.c too much trouble?
>> >> >
>> >> > The trouble is having ever more replacements for basic system functions
>> >> > in libav.  That creates a maintenance burden going into the future,
>> >> > which is in no way worth the gain of running two tests under MSVC.
>> >>
>> >> It is already written, so there is no burden.
>>
>> I concur with Luca, libavutil seems like the proper place for the
>> getopt replacement.
>>
>> >
>> > Let me be more explicit then: This patch is rejected.
>> >

Shouting rejected whenever you see something you don't like is hardly
constructive. How about you try saying something less harsh like "this
could use a little more work."

>> >> Libavutil/ sounds ok to me for placement, any other comments (Mans?)
>> >> or can I commit it as such? I can also add a new directory called
>> >> "libbrokenos/" and place it there if people prefer that.
>> >
>> > No.
>>
>> Why? What's the problem with the getopt replacement? Having FATE
>> running on a msvc compiler sounds like a worthwile goal for me.
>> And replacing the test programs that currently use getopt with
>> something that does not seems just ridiculous to me. So what do you
>> suggest to achieve the goal instead?
>
> There is no harm in not running fft-test and dct-test as part of fate
> on msvc targets IMO.  There are plenty of other tests that exercise
> that code.  Furthermore we already have native option parsing code
> that we can leverage.
>
> Those tests are simply not essential parts of fate, unlike the code
> they test (in isolation).  Just witness how late those tests were
> added to the fate framework.
>

I disagree, Windows has a strange ABI in regard to floating point XMM
registers. Beyond that fft-test was added to fate to stop people from
complaining about broken AAC and Vorbis on their weird ports when the
real culprit was the FFT/MDCT.
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to