Re: [FFmpeg-devel] [PATCH 02/14] libavcodec: Implementation of AAC_fixed_decoder (LC-module) [2/5]
>On Thu, Oct 09, 2014 at 12:02:26PM +, Nedeljko Babic wrote: >> > >> >softfloat uses "if (a.mant + 0x4000 < 0)" to normalize >> >0x4000U + 0x4000U is < 0 for int32 and thus not part of the >> >range though -1 would be, is that a problem ? >> >we could use a.mant + 0x4000 <= 0 in that case >> > >> >the main difference i see to aac is that it shifts up if its too small >> >while softfloat shift down when its too large >> >> The main problem is exactly in difference of handling too large and too small >> values. >> If we must use code from softfloat as is, it will demand a lot of changes in >> aac implementation. >> >> And there are other problems with softfloat: >> - Normalization done for av_add_sf (and av_sub_sf), av_normalize1_sf, is not >> ok. >> It's the same normalization used in multiplication and it will not handle >> normalization of mantissa and exponent correctly after addition. >> On the other hand, av_normalize_sf can not be used as is since it would >> have >> infinite loop for a.mant == 0. >> - I guess we would have a problem with av_div_sf since it needs for b to be >> normalized and it doesn't handle division by zero (also I am not sure about >> precision). >> >> We need functions that are implemented just in our emulation (like sincos >> for example) and not in softfloat and I guess (but am not sure atm) that some >> of them would also need to be changed if we need to use assumptions from >> softfloat. >> >> > >> >both dont implement rounding correctly >> >> We use this rounding for simplicity. On our test cases we do not have >> significant problems with it (overall max 3 bit difference with floating >> point code), although we can change it if it proves to be necessary. >> > >> My question still remains: should we use our float emulation and just >> integrate it in softfloat adjusting it where necessary, or should we use >> softfloat and adjust our aac implementation? > >its ok to integrate your code in sf / change sf as long as it doesnt >remove optimizations or make future optimizations hard > >thanks Ok. I'll will then recreate this patch and I'm thinking of rebasing all the patches since I see there were some changes that affect our implementation. I will resend entire patch set for review afterwards. Thanks -Nedeljko ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 02/14] libavcodec: Implementation of AAC_fixed_decoder (LC-module) [2/5]
On Thu, Oct 09, 2014 at 12:02:26PM +, Nedeljko Babic wrote: > > > >softfloat uses "if (a.mant + 0x4000 < 0)" to normalize > >0x4000U + 0x4000U is < 0 for int32 and thus not part of the > >range though -1 would be, is that a problem ? > >we could use a.mant + 0x4000 <= 0 in that case > > > >the main difference i see to aac is that it shifts up if its too small > >while softfloat shift down when its too large > > The main problem is exactly in difference of handling too large and too small > values. > If we must use code from softfloat as is, it will demand a lot of changes in > aac implementation. > > And there are other problems with softfloat: > - Normalization done for av_add_sf (and av_sub_sf), av_normalize1_sf, is not > ok. > It's the same normalization used in multiplication and it will not handle > normalization of mantissa and exponent correctly after addition. > On the other hand, av_normalize_sf can not be used as is since it would > have > infinite loop for a.mant == 0. > - I guess we would have a problem with av_div_sf since it needs for b to be > normalized and it doesn't handle division by zero (also I am not sure about > precision). > > We need functions that are implemented just in our emulation (like sincos > for example) and not in softfloat and I guess (but am not sure atm) that some > of them would also need to be changed if we need to use assumptions from > softfloat. > > > > >both dont implement rounding correctly > > We use this rounding for simplicity. On our test cases we do not have > significant problems with it (overall max 3 bit difference with floating > point code), although we can change it if it proves to be necessary. > > My question still remains: should we use our float emulation and just > integrate it in softfloat adjusting it where necessary, or should we use > softfloat and adjust our aac implementation? its ok to integrate your code in sf / change sf as long as it doesnt remove optimizations or make future optimizations hard thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Avoid a single point of failure, be that a person or equipment. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 02/14] libavcodec: Implementation of AAC_fixed_decoder (LC-module) [2/5]
> >softfloat uses "if (a.mant + 0x4000 < 0)" to normalize >0x4000U + 0x4000U is < 0 for int32 and thus not part of the >range though -1 would be, is that a problem ? >we could use a.mant + 0x4000 <= 0 in that case > >the main difference i see to aac is that it shifts up if its too small >while softfloat shift down when its too large The main problem is exactly in difference of handling too large and too small values. If we must use code from softfloat as is, it will demand a lot of changes in aac implementation. And there are other problems with softfloat: - Normalization done for av_add_sf (and av_sub_sf), av_normalize1_sf, is not ok. It's the same normalization used in multiplication and it will not handle normalization of mantissa and exponent correctly after addition. On the other hand, av_normalize_sf can not be used as is since it would have infinite loop for a.mant == 0. - I guess we would have a problem with av_div_sf since it needs for b to be normalized and it doesn't handle division by zero (also I am not sure about precision). We need functions that are implemented just in our emulation (like sincos for example) and not in softfloat and I guess (but am not sure atm) that some of them would also need to be changed if we need to use assumptions from softfloat. > >both dont implement rounding correctly We use this rounding for simplicity. On our test cases we do not have significant problems with it (overall max 3 bit difference with floating point code), although we can change it if it proves to be necessary. My question still remains: should we use our float emulation and just integrate it in softfloat adjusting it where necessary, or should we use softfloat and adjust our aac implementation? Btw, sorry for me replying on my posts instead on yours. I have some problem receiving mails from this thread... -Nedeljko ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 02/14] libavcodec: Implementation of AAC_fixed_decoder (LC-module) [2/5]
On Wed, Oct 08, 2014 at 09:46:56AM +, Nedeljko Babic wrote: > Hi again, > > >> but I do agree that it is more maintainable to have one float emulation > >> and I > >> am willing to integrate our emulation in softfloat. > >> > >> However, there is a difference in some conventions used (for example is it > >> more > >> important to represent exactly 0.5 or 1, order of fields in struct that > >> represents emulated float, etc.) and our aac implementation is tailored to > >> use > >> our float emulation. > > > >i dont understand what you mean by 0.5 vs 1 ? > >floats are base 2 so both 0.5 and 1 can be represented exactly > > > > I was a little unclear here, sorry. > > It is true that both 0.5 and 1 can be represented exactly. > > I was referring to a range of normalized mantissas. > > We are using [0.5 - 1) and softfloat uses different range. > We do not have 1 in our range. > > Please look at difference between av_normalize1_sf that is used in softfloat > version of mul and normalization used in our float_mul for example. softfloat uses "if (a.mant + 0x4000 < 0)" to normalize 0x4000U + 0x4000U is < 0 for int32 and thus not part of the range though -1 would be, is that a problem ? we could use a.mant + 0x4000 <= 0 in that case the main difference i see to aac is that it shifts up if its too small while softfloat shift down when its too large both dont implement rounding correctly > > If we need to use range from softfloat, all calculations in aac fixed point > implementation that depend on it need to be changed, so I am in favor of using > our range (of course :)), especially since softfloat is not being used > anywhere > currently and changing the range of mantissas in it would not cause any > disturbance in ffmpeg. > > - Nedeljko > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Those who are best at talking, realize last or never when they are wrong. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 02/14] libavcodec: Implementation of AAC_fixed_decoder (LC-module) [2/5]
Hi again, >> but I do agree that it is more maintainable to have one float emulation and I >> am willing to integrate our emulation in softfloat. >> >> However, there is a difference in some conventions used (for example is it >> more >> important to represent exactly 0.5 or 1, order of fields in struct that >> represents emulated float, etc.) and our aac implementation is tailored to >> use >> our float emulation. > >i dont understand what you mean by 0.5 vs 1 ? >floats are base 2 so both 0.5 and 1 can be represented exactly > I was a little unclear here, sorry. It is true that both 0.5 and 1 can be represented exactly. I was referring to a range of normalized mantissas. We are using [0.5 - 1) and softfloat uses different range. We do not have 1 in our range. Please look at difference between av_normalize1_sf that is used in softfloat version of mul and normalization used in our float_mul for example. If we need to use range from softfloat, all calculations in aac fixed point implementation that depend on it need to be changed, so I am in favor of using our range (of course :)), especially since softfloat is not being used anywhere currently and changing the range of mantissas in it would not cause any disturbance in ffmpeg. - Nedeljko ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 02/14] libavcodec: Implementation of AAC_fixed_decoder (LC-module) [2/5]
Hi On Mon, Oct 06, 2014 at 10:40:51AM +, Nedeljko Babic wrote: > Hi and sorry for such a late response. > > It looks that I misplaced this mail... > > > From: Djordje Pesut > >> > >> Add float emulation > >> > >> Signed-off-by: Nedeljko Babic > >> --- > >> libavcodec/float_emu.h | 295 > >> + > >> libavcodec/float_emu_tab.c | 293 > >> > >> 2 files changed, 588 insertions(+) > >> create mode 100644 libavcodec/float_emu.h > >> create mode 100644 libavcodec/float_emu_tab.c > > > >theres libavutil/softfloat > > > >I dont see why this should be re implemented in each decoder > >If you can improve softfloat, please do > > > >[...] > > > > If I am not mistaking, there is one (possibly two) more implementations of > float emulation besides softfloat in ffmpeg (dca encoder and lagarith > decoder), lagarith is a lossless codec, it needs a "bitexact to the reference" implementation, so it cannot easily be shared > but I do agree that it is more maintainable to have one float emulation and I > am willing to integrate our emulation in softfloat. > > However, there is a difference in some conventions used (for example is it > more > important to represent exactly 0.5 or 1, order of fields in struct that > represents emulated float, etc.) and our aac implementation is tailored to use > our float emulation. i dont understand what you mean by 0.5 vs 1 ? floats are base 2 so both 0.5 and 1 can be represented exactly > > Could I assume that in such cases I can change convention that is used in > softfloat with convection that we use, since I don't see that softfloat is > used > anywhere in ffmpeg currently? > > That way we could integrate our code without need to change aac implementation > and without need for changes in parts of float emulation that are not > supported > in softfloat currently (sqrt and sincos). > > Also, there is a question that Reimar initiated in his review of whether it's > more important to optimize the case where an operand is 0 or to reduce the > number of branches. this is hard to awnser without having code in place that uses the softfloat stuff so that we are able to benchmark but id say if the cost for a 0 check is very small and its gain are very big then its likely a good idea also theres the question why add/mul would be called with trivial arguments, this is potentially a sign that the calling code is poorly written and performs avoidable operations [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I do not agree with what you have to say, but I'll defend to the death your right to say it. -- Voltaire signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 02/14] libavcodec: Implementation of AAC_fixed_decoder (LC-module) [2/5]
Hi and sorry for such a late response.
It looks that I misplaced this mail...
> From: Djordje Pesut
>>
>> Add float emulation
>>
>> Signed-off-by: Nedeljko Babic
>> ---
>> libavcodec/float_emu.h | 295
>> +
>> libavcodec/float_emu_tab.c | 293
>>
>> 2 files changed, 588 insertions(+)
>> create mode 100644 libavcodec/float_emu.h
>> create mode 100644 libavcodec/float_emu_tab.c
>
>theres libavutil/softfloat
>
>I dont see why this should be re implemented in each decoder
>If you can improve softfloat, please do
>
>[...]
>
If I am not mistaking, there is one (possibly two) more implementations of
float emulation besides softfloat in ffmpeg (dca encoder and lagarith decoder),
but I do agree that it is more maintainable to have one float emulation and I
am willing to integrate our emulation in softfloat.
However, there is a difference in some conventions used (for example is it more
important to represent exactly 0.5 or 1, order of fields in struct that
represents emulated float, etc.) and our aac implementation is tailored to use
our float emulation.
Could I assume that in such cases I can change convention that is used in
softfloat with convection that we use, since I don't see that softfloat is used
anywhere in ffmpeg currently?
That way we could integrate our code without need to change aac implementation
and without need for changes in parts of float emulation that are not supported
in softfloat currently (sqrt and sincos).
Also, there is a question that Reimar initiated in his review of whether it's
more important to optimize the case where an operand is 0 or to reduce the
number of branches.
We have optimized cases when operands are zero for some operations. Should
I include this in softfloat, or should this kind of optimizations be left for
architecture/codec specific optimizations?
>> +typedef struct aac_float_t {
>> +int mant;
>> +int expo;
>> +} aac_float_t;
>
>float "emulation" isnt aac specific and *_t are reserved in POSIX
>IIRC
>
This float emulation was sent as part of implementation of aac fixed point
decoder. It is currently used only here and we did not know if/when it will be
used in any other codec.
On the other hand, I do agree that it is not aac specific and it will be
renamed in the new patch (when we integrate it with softfloat).
>
>[...]
>> diff --git a/libavcodec/float_emu_tab.c b/libavcodec/float_emu_tab.c
>> new file mode 100644
>> index 000..396a7a3
>> --- /dev/null
>> +++ b/libavcodec/float_emu_tab.c
>> @@ -0,0 +1,293 @@
>> +/*
>> + * Copyright (c) 2012
>> + * MIPS Technologies, Inc., California.
>> + *
>> + * Redistribution and use in source and binary forms, with or without
>> + * modification, are permitted provided that the following conditions
>> + * are met:
>> + * 1. Redistributions of source code must retain the above copyright
>> + *notice, this list of conditions and the following disclaimer.
>> + * 2. Redistributions in binary form must reproduce the above copyright
>> + *notice, this list of conditions and the following disclaimer in the
>> + *documentation and/or other materials provided with the distribution.
>> + * 3. Neither the name of the MIPS Technologies, Inc., nor the names of is
>> + *contributors may be used to endorse or promote products derived from
>> + *this software without specific prior written permission.
>> + *
>> + * THIS SOFTWARE IS PROVIDED BY THE MIPS TECHNOLOGIES, INC. ``AS IS'' AND
>> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
>> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
>> PURPOSE
>> + * ARE DISCLAIMED. IN NO EVENT SHALL THE MIPS TECHNOLOGIES, INC. BE LIABLE
>> + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
>> CONSEQUENTIAL
>> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
>> + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
>> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
>> STRICT
>> + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
>> + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
>> + * SUCH DAMAGE.
>> + *
>> + * Author: Stanislav Ocovaj (stanislav.ocovaj imgtec com)
>> + *
>> + * This file is part of FFmpeg.
>> + *
>> + * FFmpeg is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * FFmpeg is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have r
Re: [FFmpeg-devel] [PATCH 02/14] libavcodec: Implementation of AAC_fixed_decoder (LC-module) [2/5]
On Mon, Sep 01, 2014 at 07:55:40PM +0200, Nedeljko Babic wrote:
> From: Djordje Pesut
>
> Add float emulation
>
> Signed-off-by: Nedeljko Babic
> ---
> libavcodec/float_emu.h | 295
> +
> libavcodec/float_emu_tab.c | 293
> 2 files changed, 588 insertions(+)
> create mode 100644 libavcodec/float_emu.h
> create mode 100644 libavcodec/float_emu_tab.c
theres libavutil/softfloat
I dont see why this should be re implemented in each decoder
If you can improve softfloat, please do
[...]
> +typedef struct aac_float_t {
> +int mant;
> +int expo;
> +} aac_float_t;
float "emulation" isnt aac specific and *_t are reserved in POSIX
IIRC
[...]
> diff --git a/libavcodec/float_emu_tab.c b/libavcodec/float_emu_tab.c
> new file mode 100644
> index 000..396a7a3
> --- /dev/null
> +++ b/libavcodec/float_emu_tab.c
> @@ -0,0 +1,293 @@
> +/*
> + * Copyright (c) 2012
> + * MIPS Technologies, Inc., California.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + *notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + *notice, this list of conditions and the following disclaimer in the
> + *documentation and/or other materials provided with the distribution.
> + * 3. Neither the name of the MIPS Technologies, Inc., nor the names of is
> + *contributors may be used to endorse or promote products derived from
> + *this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE MIPS TECHNOLOGIES, INC. ``AS IS'' AND
> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE MIPS TECHNOLOGIES, INC. BE LIABLE
> + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> + * SUCH DAMAGE.
> + *
> + * Author: Stanislav Ocovaj (stanislav.ocovaj imgtec com)
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
> USA
> + */
> +
> +const int divTable[128] = {
> + 0x3fff,0x3f80fe04,0x3f03f03f,0x3e88cb3d,
> + 0x3e0f83e1,0x3d980f66,0x3d226358,0x3cae7592,
> + 0x3c3c3c3c,0x3bcbadc8,0x3b5cc0ed,0x3aef6ca9,
> + 0x3a83a83b,0x3a196b1f,0x39b0ad12,0x3949660b,
tabs are forbidden in git, we would not be able to push this
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The greatest way to live with honor in this world is to be what we pretend
to be. -- Socrates
signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 02/14] libavcodec: Implementation of AAC_fixed_decoder (LC-module) [2/5]
>> +/* Rounding to zero used for simplicity */
>> +static av_always_inline aac_float_t float_add(aac_float_t a, aac_float_t b)
>> +{
>> +int diff;
>> +
>> +if (a.mant == 0)
>> +return b;
>> +
>> +if (b.mant == 0)
>> +return a;
>> +
>> +diff = a.expo - b.expo;
>> +
>> +if (diff < 0) // a.expo < b.expo
>> +{
>> +diff = -diff;
>> +if (diff >= 31)
>> +a.mant = 0;
>> +else
>> +a.mant >>= diff;
>> +a.expo = b.expo;
>> +}
>> +else // a.expo >= b.expo
>> +{
>> +if (diff >= 31)
>> +b.mant = 0;
>> +else
>> +b.mant >>= diff;
>> +}
>
>In addition to the comments I had before, if you care only
>about (mostly) matching single-precision IEEE you can
>save some cycles by changing to diff > 24 instead
>of >= 31.
>Obviously that means you need to change the
>code to directly return e.g. a in the else
>path instead of insisting on explicitly
>adding 0 to a, otherwise it won't get any faster of course.
The idea for this entire emulation was not to make IEEE compliant emulation,
but
to create a tool that is used in aac fixed point decoder on places where
dynamical
range is to large for fixed point operations to be used.
Probably our mistake was in that we didn't put a comment regarding this in the
file,
but it was easily overlooked since this patch is part of the patch set that
implements
fixed point AAC decoder and not stand alone patch for implementing float
emulation.
Also name for this file should probably be changed back to aac_float_emu.h
instead
float_emu.h to emphasize this.
Having this in mind, changing diff >= 31 with > 24 here is not an option unless
entire
algorithm is changed.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 02/14] libavcodec: Implementation of AAC_fixed_decoder (LC-module) [2/5]
On Mon, Sep 01, 2014 at 07:55:40PM +0200, Nedeljko Babic wrote:
> +/* Rounding to zero used for simplicity */
> +static av_always_inline aac_float_t float_add(aac_float_t a, aac_float_t b)
> +{
> +int diff;
> +
> +if (a.mant == 0)
> +return b;
> +
> +if (b.mant == 0)
> +return a;
> +
> +diff = a.expo - b.expo;
> +
> +if (diff < 0) // a.expo < b.expo
> +{
> +diff = -diff;
> +if (diff >= 31)
> +a.mant = 0;
> +else
> +a.mant >>= diff;
> +a.expo = b.expo;
> +}
> +else // a.expo >= b.expo
> +{
> +if (diff >= 31)
> +b.mant = 0;
> +else
> +b.mant >>= diff;
> +}
In addition to the comments I had before, if you care only
about (mostly) matching single-precision IEEE you can
save some cycles by changing to diff > 24 instead
of >= 31.
Obviously that means you need to change the
code to directly return e.g. a in the else
path instead of insisting on explicitly
adding 0 to a, otherwise it won't get any faster of course.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 02/14] libavcodec: Implementation of AAC_fixed_decoder (LC-module) [2/5]
From: Djordje Pesut
Add float emulation
Signed-off-by: Nedeljko Babic
---
libavcodec/float_emu.h | 295 +
libavcodec/float_emu_tab.c | 293
2 files changed, 588 insertions(+)
create mode 100644 libavcodec/float_emu.h
create mode 100644 libavcodec/float_emu_tab.c
diff --git a/libavcodec/float_emu.h b/libavcodec/float_emu.h
new file mode 100644
index 000..8dc975a
--- /dev/null
+++ b/libavcodec/float_emu.h
@@ -0,0 +1,295 @@
+/*
+ * Copyright (c) 2012
+ * MIPS Technologies, Inc., California.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *notice, this list of conditions and the following disclaimer in the
+ *documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of the MIPS Technologies, Inc., nor the names of is
+ *contributors may be used to endorse or promote products derived from
+ *this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE MIPS TECHNOLOGIES, INC. ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE MIPS TECHNOLOGIES, INC. BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ *
+ * Author: Stanislav Ocovaj (stanislav.ocovaj imgtec com)
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ * Note: Rounding-to-nearest used unless otherwise stated
+ */
+
+#ifndef AVCODEC_FLOAT_EMU_H
+#define AVCODEC_FLOAT_EMU_H
+
+#include "libavutil/common.h"
+#include "libavutil/intmath.h"
+
+extern const int divTable[128];
+extern const int sqrtTab[513];
+extern const int sqrExpMultTab[2];
+extern const int aac_costbl_1[16];
+extern const int aac_costbl_2[32];
+extern const int aac_sintbl_2[32];
+extern const int aac_costbl_3[32];
+extern const int aac_sintbl_3[32];
+extern const int aac_costbl_4[33];
+extern const int aac_sintbl_4[33];
+
+typedef struct aac_float_t {
+int mant;
+int expo;
+} aac_float_t;
+
+static const aac_float_t FLOAT_0 = { 0, 0};
+static const aac_float_t FLOAT_05 = { 536870912, 0};
+static const aac_float_t FLOAT_1 = { 536870912, 1};
+static const aac_float_t FLOAT_EPSILON= { 703687442, -16};
+static const aac_float_t FLOAT_1584893192 = { 850883053, 1};
+static const aac_float_t FLOAT_10 = { 81920, 17};
+static const aac_float_t FLOAT_099= {1073740750, 0};
+
+static av_always_inline aac_float_t int2float(int x, int exp)
+{
+aac_float_t ret;
+
+if (x == 0)
+{
+ret.mant = 0;
+ret.expo = 0;
+}
+else
+{
+int nz = 29 - ff_log2(FFABS(x));
+ret.mant = x << nz;
+ret.expo = exp - nz;
+}
+
+return ret;
+}
+
+/* Rounding to zero used for simplicity */
+static av_always_inline aac_float_t float_add(aac_float_t a, aac_float_t b)
+{
+int diff;
+
+if (a.mant == 0)
+return b;
+
+if (b.mant == 0)
+return a;
+
+diff = a.expo - b.expo;
+
+if (diff < 0) // a.expo < b.expo
+{
+diff = -diff;
+if (diff >= 31)
+a.mant = 0;
+else
+a.mant >>= diff;
+a.expo = b.expo;
+}
+else // a.expo >= b.expo
+{
+if (diff >= 31)
+b.mant = 0;
+else
+b.mant >>= diff;
+}
+
+a.mant = a.mant + b.mant;
+
+if (a
Re: [FFmpeg-devel] [PATCH 02/14] libavcodec: Implementation of AAC_fixed_decoder (LC-module) [2/5]
I agree with most of your suggestions and they will be incorporated in the next
patch.
Thanks again on your review.
- Nedeljko
Od: ffmpeg-devel-boun...@ffmpeg.org [ffmpeg-devel-boun...@ffmpeg.org] u ime
korisnika Reimar Döffinger [reimar.doeffin...@gmx.de]
Poslato: 7. avgust 2014 22:12
Za: FFmpeg development discussions and patches
Tema: Re: [FFmpeg-devel] [PATCH 02/14] libavcodec: Implementation of
AAC_fixed_decoder (LC-module) [2/5]
On Thu, Aug 07, 2014 at 09:51:36AM +, Nedeljko Babic wrote:
> >> +Q30(0.95), Q30(0.9922480620), Q30(0.9846153846),
> >> Q30(0.9770992366),
> >
> >I'm a bit unsure btw. if this makes more sense than coding the converted
> >numbers.
> >It feels like it combines the disadvantages, since it neither avoids
> >code duplication with the float tables nor does it compiler/architecture
> >dependence completely (I think).
> >
>
> I am not sure what disadvantages are combined (there is no code duplication
> for example),
I assumed the same tables might already exist somewhere, but I might
just misunderstand.
> but since only advantage is in that it can be seen what fixed point
> representation is used,
> the values will be hard coded in hex format.
To be honest, I flagged this more as a point of discussion.
In some ways, this variant is more "readable/verifiable", but
on the other hand bit-exact decoding between compilers/platforms
would be even nicer. Yet again, it might be a purely theoretical issue.
> >> +aac_float_t ret;
> >> +int nz;
> >> +
> >> +if (x == 0)
> >> +{
> >> +ret.mant = 0;
> >> +ret.expo = 0;
> >> +}
> >> +else
> >> +{
> >> +ret.expo = exp;
> >> +ret.mant = x;
> >> +nz = 29 - av_log2_c_emu(FFABS(ret.mant));
> >> +ret.mant <<= nz;
> >> +ret.expo -= nz;
> >
> >nz is only used here, so it should be declared here.
> >Also
> >int nz = 29 - av_log2_c_emu(FFABS(x));
>
> Since ISO C90 forbids mixing declarations and code, warnings will be
> generated if nz is declared here.
> On the other hand, it can (and will) be declared at the start of the else
> block.
Yes, I meant at the start of the else block by "here", though with my proposed
change that will
also be the first use :)
> >> +if (manta == 0)
> >> +return b;
> >> +
> >> +if (mantb == 0)
> >> +return a;
> >> +
> >> +diff = expa - expb;
> >> +if (diff < 0) // expa < expb
> >> +{
> >> +diff = -diff;
> >> +if (diff >= 31)
> >> +manta = 0;
> >> +else if (diff != 0)
> >> +manta >>= diff;
> >> +expa = expb;
> >> +}
> >> +else // expa >= expb
> >> +{
> >> +if (diff >= 31)
> >> +mantb = 0;
> >> +else if (diff != 0)
> >> +mantb >>= diff;
> >> +}
> >> +
> >> +manta = manta + mantb;
> >> +if (manta == 0)
> >> +expa = 0;
> >> +else
> >> +{
> >> +nz = 30 - av_log2_c_emu(FFABS(manta));
> >> +manta <<= nz;
> >> +manta >>= 1;
> >> +expa -= (nz-1);
> >> +}
> >
> >I think this needs a good thinking over.
> >If diff was calculated first, the >= 31/ <= -31 checks
> >could be combined with the checks for 0.
>
> diff is calculated right after checking if mantissas are zero.
> Mantissas are checked first because if they are zero,
> it is a waste of time to do other calculations.
>
> On the other hand, I am not sure what would be gain in combining
> >= 31/ <= -31 checks with the checks for 0 (for diff).
As far as I can tell, if the difference is <= 31 we will always
return either a or b, i.e. whether one is at least 31 exponents
smaller or it is 0 is the same thing.
Assuming I understood the code right, I guess you can also just replace
> >> +if (diff >= 31)
> >> +manta = 0;
by
> >> +if (diff >= 31)
> >> +return b;
Though the question is whether it's more important performance wise
to optimize the case where one operand is 0 or to reduce the number of
branches. Or it might not matter at all.
But I admit I haven't looked at it closely enough to be 100% sure about
any of this, thus the statement that I believe someone needs to think
through
Re: [FFmpeg-devel] [PATCH 02/14] libavcodec: Implementation of AAC_fixed_decoder (LC-module) [2/5]
On Thu, Aug 07, 2014 at 09:51:36AM +, Nedeljko Babic wrote:
> >> +Q30(0.95), Q30(0.9922480620), Q30(0.9846153846),
> >> Q30(0.9770992366),
> >
> >I'm a bit unsure btw. if this makes more sense than coding the converted
> >numbers.
> >It feels like it combines the disadvantages, since it neither avoids
> >code duplication with the float tables nor does it compiler/architecture
> >dependence completely (I think).
> >
>
> I am not sure what disadvantages are combined (there is no code duplication
> for example),
I assumed the same tables might already exist somewhere, but I might
just misunderstand.
> but since only advantage is in that it can be seen what fixed point
> representation is used,
> the values will be hard coded in hex format.
To be honest, I flagged this more as a point of discussion.
In some ways, this variant is more "readable/verifiable", but
on the other hand bit-exact decoding between compilers/platforms
would be even nicer. Yet again, it might be a purely theoretical issue.
> >> +aac_float_t ret;
> >> +int nz;
> >> +
> >> +if (x == 0)
> >> +{
> >> +ret.mant = 0;
> >> +ret.expo = 0;
> >> +}
> >> +else
> >> +{
> >> +ret.expo = exp;
> >> +ret.mant = x;
> >> +nz = 29 - av_log2_c_emu(FFABS(ret.mant));
> >> +ret.mant <<= nz;
> >> +ret.expo -= nz;
> >
> >nz is only used here, so it should be declared here.
> >Also
> >int nz = 29 - av_log2_c_emu(FFABS(x));
>
> Since ISO C90 forbids mixing declarations and code, warnings will be
> generated if nz is declared here.
> On the other hand, it can (and will) be declared at the start of the else
> block.
Yes, I meant at the start of the else block by "here", though with my proposed
change that will
also be the first use :)
> >> +if (manta == 0)
> >> +return b;
> >> +
> >> +if (mantb == 0)
> >> +return a;
> >> +
> >> +diff = expa - expb;
> >> +if (diff < 0) // expa < expb
> >> +{
> >> +diff = -diff;
> >> +if (diff >= 31)
> >> +manta = 0;
> >> +else if (diff != 0)
> >> +manta >>= diff;
> >> +expa = expb;
> >> +}
> >> +else // expa >= expb
> >> +{
> >> +if (diff >= 31)
> >> +mantb = 0;
> >> +else if (diff != 0)
> >> +mantb >>= diff;
> >> +}
> >> +
> >> +manta = manta + mantb;
> >> +if (manta == 0)
> >> +expa = 0;
> >> +else
> >> +{
> >> +nz = 30 - av_log2_c_emu(FFABS(manta));
> >> +manta <<= nz;
> >> +manta >>= 1;
> >> +expa -= (nz-1);
> >> +}
> >
> >I think this needs a good thinking over.
> >If diff was calculated first, the >= 31/ <= -31 checks
> >could be combined with the checks for 0.
>
> diff is calculated right after checking if mantissas are zero.
> Mantissas are checked first because if they are zero,
> it is a waste of time to do other calculations.
>
> On the other hand, I am not sure what would be gain in combining
> >= 31/ <= -31 checks with the checks for 0 (for diff).
As far as I can tell, if the difference is <= 31 we will always
return either a or b, i.e. whether one is at least 31 exponents
smaller or it is 0 is the same thing.
Assuming I understood the code right, I guess you can also just replace
> >> +if (diff >= 31)
> >> +manta = 0;
by
> >> +if (diff >= 31)
> >> +return b;
Though the question is whether it's more important performance wise
to optimize the case where one operand is 0 or to reduce the number of
branches. Or it might not matter at all.
But I admit I haven't looked at it closely enough to be 100% sure about
any of this, thus the statement that I believe someone needs to think
through this code once more, because I strongly suspect it's needlessly
confusing and slow.
> >also the != 0 check can certainly only make performance worse.
>
> I agree with this in if part, since it is not needed as you pointed out,
> but this check is needed in the else part.
If the statement is
if (s != 0) a >>= s;
how can the if _ever_ be _needed_ for correctness?
a >>= 0 is a NOP, so no need to explicitly skip it for s == 0.
> >For all functions, I think there should be a look at what rounding
> >mode they end up using in the end and document that.
> >This one looks like round to 0.
> >
>
> This is rounding to 0. It can be changed to rounding to nearest without loss
> in precision.
>
> Do you suggest that we should make comment like:
> /* Rounding to zero used */
> for every function?
Well, if every function used round to zero that, it should be written once e.g.
at the top of the file.
However that does not seem to be the case.
Ideally, I think there should be a statement that all functions unless
otherwise stated use round to 0 for simplicity.
In addition, for those using a different rounding this should be
documented.
> >> +static av_always_inline aac_float_t float_sub(aac_float_t a, aa
Re: [FFmpeg-devel] [PATCH 02/14] libavcodec: Implementation of AAC_fixed_decoder (LC-module) [2/5]
Thanks on your review,
>On Fri, Aug 01, 2014 at 03:53:08PM +0200, Nedeljko Babic wrote:
>> +#if !defined(_AAC_FLOAT_EMU_)
>> +#define _AAC_FLOAT_EMU_
>
>That is not out usual style
>1) We use #ifndef
>2) Identifiers starting with _ + upper case letter are reserved by POSIX
>and never should be used
>3) The naming convention is like e.g. AVCODEC_AAC_FLOAT_EMU_H
>
This file will be removed.
Instead float_emu_tab.c and float_emu.h will be used.
>> +static const uint8_t ff_log2_tab[256] = {
>
>One with the same name exists in libavutil, I am not sure that is a good
>idea.
>
The one from the libavutil will be used instead of this one.
>> +static int divTable[128] = {
>
>These and the following ones seem to lack "const".
>
The versions from float_emu_tab.c will be used and they do have "const"
>> +Q30(0.95), Q30(0.9922480620), Q30(0.9846153846),
>> Q30(0.9770992366),
>
>I'm a bit unsure btw. if this makes more sense than coding the converted
>numbers.
>It feels like it combines the disadvantages, since it neither avoids
>code duplication with the float tables nor does it compiler/architecture
>dependence completely (I think).
>
I am not sure what disadvantages are combined (there is no code duplication for
example),
but since only advantage is in that it can be seen what fixed point
representation is used,
the values will be hard coded in hex format.
>> +static av_always_inline av_const int av_log2_c_emu(unsigned int v)
>
>Since this duplicates existing code it definitely should have a comment.
>Also if performance is the reason there is a question whether we should
>at least internally have libavutil provide it instead, to at least avoid
>duplication.
>Though to be honest on most architectures the solution to it being slow
>should be to provide a ASM version.
>
The version from intmath.h will be used so no code duplication will occur.
>> +static av_always_inline aac_float_t int2float(const int x, const int exp)
>
>Those "const" are kind of pointless IMHO.
>
You are correct and they will be removed.
>> +aac_float_t ret;
>> +int nz;
>> +
>> +if (x == 0)
>> +{
>> +ret.mant = 0;
>> +ret.expo = 0;
>> +}
>> +else
>> +{
>> +ret.expo = exp;
>> +ret.mant = x;
>> +nz = 29 - av_log2_c_emu(FFABS(ret.mant));
>> +ret.mant <<= nz;
>> +ret.expo -= nz;
>
>nz is only used here, so it should be declared here.
>Also
>int nz = 29 - av_log2_c_emu(FFABS(x));
Since ISO C90 forbids mixing declarations and code, warnings will be generated
if nz is declared here.
On the other hand, it can (and will) be declared at the start of the else block.
>ret.expo = exp - nz;
>ret.mant = x << nz;
>seems more readable to me.
>
Ok. It will be changed.
>> +static av_always_inline aac_float_t float_add(aac_float_t a, aac_float_t b)
>> +{
>> +int diff, nz;
>> +int expa = a.expo;
>> +int expb = b.expo;
>> +int manta = a.mant;
>> +int mantb = b.mant;
>
>I don't think those local copies are a good idea, at least for
>readability.
>
Ok. This will be changed.
>> +if (manta == 0)
>> +return b;
>> +
>> +if (mantb == 0)
>> +return a;
>> +
>> +diff = expa - expb;
>> +if (diff < 0) // expa < expb
>> +{
>> +diff = -diff;
>> +if (diff >= 31)
>> +manta = 0;
>> +else if (diff != 0)
>> +manta >>= diff;
>> +expa = expb;
>> +}
>> +else // expa >= expb
>> +{
>> +if (diff >= 31)
>> +mantb = 0;
>> +else if (diff != 0)
>> +mantb >>= diff;
>> +}
>> +
>> +manta = manta + mantb;
>> +if (manta == 0)
>> +expa = 0;
>> +else
>> +{
>> +nz = 30 - av_log2_c_emu(FFABS(manta));
>> +manta <<= nz;
>> +manta >>= 1;
>> +expa -= (nz-1);
>> +}
>
>I think this needs a good thinking over.
>If diff was calculated first, the >= 31/ <= -31 checks
>could be combined with the checks for 0.
diff is calculated right after checking if mantissas are zero.
Mantissas are checked first because if they are zero,
it is a waste of time to do other calculations.
On the other hand, I am not sure what would be gain in combining
>= 31/ <= -31 checks with the checks for 0 (for diff).
>In the upper if, diff can obviously not be 0,
You are correct and != 0 check will be removed here.
>also the != 0 check can certainly only make performance worse.
I agree with this in if part, since it is not needed as you pointed out,
but this check is needed in the else part.
>For all functions, I think there should be a look at what rounding
>mode they end up using in the end and document that.
>This one looks like round to 0.
>
This is rounding to 0. It can be changed to rounding to nearest without loss in
precision.
Do you suggest that we should make comment like:
/* Rounding to zero used */
for every function?
>> +static av_always_inline aac_float_t float_sub(aac_float_t a, aac_flo
Re: [FFmpeg-devel] [PATCH 02/14] libavcodec: Implementation of AAC_fixed_decoder (LC-module) [2/5]
On Fri, Aug 01, 2014 at 03:53:08PM +0200, Nedeljko Babic wrote:
> +#if !defined(_AAC_FLOAT_EMU_)
> +#define _AAC_FLOAT_EMU_
That is not out usual style
1) We use #ifndef
2) Identifiers starting with _ + upper case letter are reserved by POSIX
and never should be used
3) The naming convention is like e.g. AVCODEC_AAC_FLOAT_EMU_H
> +static const uint8_t ff_log2_tab[256] = {
One with the same name exists in libavutil, I am not sure that is a good
idea.
> +static int divTable[128] = {
These and the following ones seem to lack "const".
> +Q30(0.95), Q30(0.9922480620), Q30(0.9846153846),
> Q30(0.9770992366),
I'm a bit unsure btw. if this makes more sense than coding the converted
numbers.
It feels like it combines the disadvantages, since it neither avoids
code duplication with the float tables nor does it compiler/architecture
dependence completely (I think).
> +static av_always_inline av_const int av_log2_c_emu(unsigned int v)
Since this duplicates existing code it definitely should have a comment.
Also if performance is the reason there is a question whether we should
at least internally have libavutil provide it instead, to at least avoid
duplication.
Though to be honest on most architectures the solution to it being slow
should be to provide a ASM version.
> +static av_always_inline aac_float_t int2float(const int x, const int exp)
Those "const" are kind of pointless IMHO.
> +aac_float_t ret;
> +int nz;
> +
> +if (x == 0)
> +{
> +ret.mant = 0;
> +ret.expo = 0;
> +}
> +else
> +{
> +ret.expo = exp;
> +ret.mant = x;
> +nz = 29 - av_log2_c_emu(FFABS(ret.mant));
> +ret.mant <<= nz;
> +ret.expo -= nz;
nz is only used here, so it should be declared here.
Also
int nz = 29 - av_log2_c_emu(FFABS(x));
ret.expo = exp - nz;
ret.mant = x << nz;
seems more readable to me.
> +static av_always_inline aac_float_t float_add(aac_float_t a, aac_float_t b)
> +{
> +int diff, nz;
> +int expa = a.expo;
> +int expb = b.expo;
> +int manta = a.mant;
> +int mantb = b.mant;
I don't think those local copies are a good idea, at least for
readability.
> +if (manta == 0)
> +return b;
> +
> +if (mantb == 0)
> +return a;
> +
> +diff = expa - expb;
> +if (diff < 0) // expa < expb
> +{
> +diff = -diff;
> +if (diff >= 31)
> +manta = 0;
> +else if (diff != 0)
> +manta >>= diff;
> +expa = expb;
> +}
> +else // expa >= expb
> +{
> +if (diff >= 31)
> +mantb = 0;
> +else if (diff != 0)
> +mantb >>= diff;
> +}
> +
> +manta = manta + mantb;
> +if (manta == 0)
> +expa = 0;
> +else
> +{
> +nz = 30 - av_log2_c_emu(FFABS(manta));
> +manta <<= nz;
> +manta >>= 1;
> +expa -= (nz-1);
> +}
I think this needs a good thinking over.
If diff was calculated first, the >= 31/ <= -31 checks
could be combined with the checks for 0.
In the upper if, diff can obviously not be 0,
also the != 0 check can certainly only make performance worse.
For all functions, I think there should be a look at what rounding
mode they end up using in the end and document that.
This one looks like round to 0.
> +static av_always_inline aac_float_t float_sub(aac_float_t a, aac_float_t b)
Why would this need a whole separate implementation instead
of changing the sign of b and calling add?
> +static av_always_inline aac_float_t float_mul(aac_float_t a, aac_float_t b)
> +{
> +aac_float_t res;
> +int mant;
> +int expa = a.expo;
> +int expb = b.expo;
> +long long accu;
> +
> +expa = expa + expb;
> +accu = (long long)a.mant * b.mant;
I'd suggest using types like int64_t.
> +mant = (int)((accu + 0x2000) >> 30);
This suddenly seems to use round-to-nearest-away as rounding mode/
> +if (mant == 0)
> +expa = 0;
> +else if (mant < 536870912 && mant > -536870912)
> +{
These constants would be vastly more readable in hex.
> +mant <<= 1;
> +expa = expa - 1;
expa--;
> +if (mantb != 0)
> +{
> +iB = float_recip(b);
> +// newton iteration to double precision
> +tmp = float_sub(FLOAT_1, float_mul(b, iB));
> +iB = float_add(iB, float_mul(iB, tmp));
> +res = float_mul(a, iB);
> +}
> +else
> +{
> +res.mant = 1;
> +res.expo = 2147483647;
Hexadecimal would be nicer.
Also, this seems to be the only function so far that handles
infinity correctly.
I have some doubts of the value of that, as long as it is the only one.
If there's a particular reason why it only needs to be supported here,
that should be documented.
> +static av_always_inline int float_gt(aac_float_t a, aac_float_t b)
> +{
> +int expa = a.expo;
> +int expb = b.expo;
> +int manta = a.mant;
> +int mantb = b.mant;
> +
> +if (manta == 0)
> +expa =
[FFmpeg-devel] [PATCH 02/14] libavcodec: Implementation of AAC_fixed_decoder (LC-module) [2/5]
From: Djordje Pesut
Add float emulation
Signed-off-by: Nedeljko Babic
---
libavcodec/aac_float_emu.h | 698 +
libavcodec/float_emu.h | 400 ++
libavcodec/float_emu_tab.c | 296 +++
3 files changed, 1394 insertions(+)
create mode 100644 libavcodec/aac_float_emu.h
create mode 100644 libavcodec/float_emu.h
create mode 100644 libavcodec/float_emu_tab.c
diff --git a/libavcodec/aac_float_emu.h b/libavcodec/aac_float_emu.h
new file mode 100644
index 000..f7a4fa0
--- /dev/null
+++ b/libavcodec/aac_float_emu.h
@@ -0,0 +1,698 @@
+/*
+ * Copyright (c) 2012
+ * MIPS Technologies, Inc., California.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *notice, this list of conditions and the following disclaimer in the
+ *documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of the MIPS Technologies, Inc., nor the names of is
+ *contributors may be used to endorse or promote products derived from
+ *this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE MIPS TECHNOLOGIES, INC. ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE MIPS TECHNOLOGIES, INC. BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ *
+ * Author: Stanislav Ocovaj (stanislav.ocovaj imgtec com)
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#if !defined(_AAC_FLOAT_EMU_)
+#define _AAC_FLOAT_EMU_
+
+#include "libavutil/common.h"
+
+typedef struct aac_float_t {
+int mant;
+int expo;
+} aac_float_t;
+
+#define ADD_SUFFIX(a) a ## _fixed
+#define Q30(x) (int)((x)*1073741824.0 + 0.5)
+#define Q31(x) (int)((x)*2147483648.0 + 0.5)
+
+#define ff_log2_tab ADD_SUFFIX(ff_log2_tab)
+#define divTable ADD_SUFFIX(divTable)
+#define sqrtTab ADD_SUFFIX(sqrtTab)
+#define sqrExpMultTab ADD_SUFFIX(sqrExpMultTab)
+#define aac_costbl_1 ADD_SUFFIX(aac_costbl_1)
+#define aac_costbl_2 ADD_SUFFIX(aac_costbl_2)
+#define aac_sintbl_2 ADD_SUFFIX(aac_sintbl_2)
+#define aac_costbl_3 ADD_SUFFIX(aac_costbl_3)
+#define aac_sintbl_3 ADD_SUFFIX(aac_sintbl_3)
+#define aac_costbl_4 ADD_SUFFIX(aac_costbl_4)
+#define aac_sintbl_4 ADD_SUFFIX(aac_sintbl_4)
+
+static const aac_float_t FLOAT_0 = { 0, 0};
+static const aac_float_t FLOAT_05 = { 536870912, 0};
+static const aac_float_t FLOAT_1 = { 536870912, 1};
+static const aac_float_t FLOAT_EPSILON= { 703687442, -16};
+static const aac_float_t FLOAT_1584893192 = { 850883053, 1};
+static const aac_float_t FLOAT_10 = { 81920, 17};
+static const aac_float_t FLOAT_099= {1073740750, 0};
+
+static const uint8_t ff_log2_tab[256] = {
+0, 0, 1, 1, 2, 2, 2, 2, 3, 3, 3, 3, 3, 3, 3, 3,
+4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4,
+5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5,
+5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5,
+6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6,
+6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6,
+6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6,
+6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6,
+7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7,
+7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7,
+7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7,
+7, 7, 7, 7, 7, 7, 7, 7, 7, 7,
