On 11/9/2017 10:37 AM, Aurelien Jacobs wrote:
> On Thu, Nov 09, 2017 at 12:52:34AM +0000, Rostislav Pehlivanov wrote:
>> On 8 November 2017 at 22:41, Aurelien Jacobs <au...@gnuage.org> wrote:
>>
>>> On Wed, Nov 08, 2017 at 06:26:03PM +0100, Michael Niedermayer wrote:
>>>> On Wed, Nov 08, 2017 at 02:06:09PM +0100, Aurelien Jacobs wrote:
>>>> [...]
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + * Compute the convolution of the signal with the coefficients, and
>>> reduce
>>>>> + * to 24 bits by applying the specified right shifting.
>>>>> + */
>>>>> +av_always_inline
>>>>> +static int32_t aptx_qmf_convolution(FilterSignal *signal,
>>>>> + const int32_t coeffs[FILTER_TAPS],
>>>>> + int shift)
>>>>> +{
>>>>> + int32_t *sig = &signal->buffer[signal->pos];
>>>>> + int64_t e = 0;
>>>>> +
>>>>
>>>>> + for (int i = 0; i < FILTER_TAPS; i++)
>>>>
>>>> "for (int" is something we avoided as some comilers didnt like it,
>>>> iam not sure if this is still true but there are none in the codebase
>>>
>>> If you insist on this I will of course change it, but hey, we require
>>> a C99 compiler since a long time and we use so many features of C99
>>> that I really don't get why we couldn't use "for (int".
>>> I can't imagine that any relevant C compiler would not support this
>>> nowadays !
>>> What I propose is to get this in as is, and see if anyone encounter
>>> issue with any compiler. If any issue arise, I will of course send a
>>> patch to fix it.
>>>
>>> Here is an updated patch.
>>>
>>>
>> Send another patch because some people are beyond convincing and its really
>> pissing me off. Really. In particular maybe those compiler writers at
>> Microsoft who seem to think shipping something that doesn't support such a
>> simple yet important feature is important.
>> Or maybe users who don't want to update a 6 year old version of msvc.
>> Or maybe us because we support compiling with msvc at all when its clearly
>> _not_ a C compiler and this project is written in C.
>
> Here you go.
>
> Just for reference, splitting the déclaration out of the for loop added
> 19 lines in this file, which is a 2.3 % increase of the line count.
> (I'm not sure this file is representative of the rest of the code base)
If you combine lines like
+ int channel;
+ int ret;
and
+ int pos, channel, sample;
+ int ret;
You can reduce that a bit.
Don't send a new patch just for that. Only if there's some other change
requested. Whoever commits this can change it before pushing.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
