[FFmpeg-devel] [PATCH] avcodec/alsdec: implement floating point decoding

[Thilo Borgmann]

Am 21.07.16 um 04:17 schrieb Umair Khan:
> On Tue, Jul 19, 2016 at 11:17 PM, Umair Khan <omerjerk at gmail.com> wrote:
>>
>> Updated patch.
>>
>> On Tue, Jul 19, 2016 at 8:19 PM, Thilo Borgmann <thilo.borgmann at mail.de> wrote:
>>> Am 19.07.16 um 15:42 schrieb Umair Khan:
>>>> On Tue, Jul 19, 2016 at 5:20 PM, Thilo Borgmann <thilo.borgmann at mail.de> wrote:
>>>>> Am 19.07.16 um 09:22 schrieb Umair Khan:
>>>>>> On Tue, Jul 19, 2016 at 12:29 PM, Paul B Mahol <onemda at gmail.com> wrote:
>>>>>>>
>>>>>>> On 7/19/16, Umair Khan <omerjerk at gmail.com> wrote:
>>>>>>>> On Tue, Jul 19, 2016 at 3:08 AM, Michael Niedermayer
>>>>>>>> <michael at niedermayer.cc> wrote:
>>>>>>>>> On Mon, Jul 18, 2016 at 11:42:48PM +0530, Umair Khan wrote:
>>>>>>>>>> On Sun, Jul 17, 2016 at 3:38 PM, Michael Niedermayer
>>>>>>>>>> <michael at niedermayer.cc> wrote:
>>>>>>>>>>> On Sun, Jul 17, 2016 at 11:54:49AM +0200, Michael Niedermayer wrote:
>>>>>>>>>>>> On Sun, Jul 17, 2016 at 09:00:48AM +0530, Umair Khan wrote:
>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Sun, Jul 17, 2016 at 12:25 AM, Thilo Borgmann
>>>>>>>>>>>>> <thilo.borgmann at mail.de> wrote:
>>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> From 70e65b26cc3f84c9c664c30808b43a5e1cf16eaa Mon Sep 17 00:00:00
>>>>>>>>>>>>>>> 2001
>>>>>>>>>>>>>>> From: Umair Khan <omerjerk at gmail.com>
>>>>>>>>>>>>>>> Date: Sat, 16 Jul 2016 23:52:39 +0530
>>>>>>>>>>>>>>> Subject: [PATCH 1/1] avcodec/alsdec: implement floating point
>>>>>>>>>>>>>>> decoding
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> It conforms to RM22 version of the reference codec.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Signed-off-by: Umair Khan <omerjerk at gmail.com>
>>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>>  libavcodec/Makefile           |   2 +-
>>>>>>>>>>>>>>>  libavcodec/alsdec.c           | 284
>>>>>>>>>>>>>>> +++++++++++++++++++++++++++++++++++++++++-
>>>>>>>>>>>>>>>  libavcodec/mlz.c              | 171 +++++++++++++++++++++++++
>>>>>>>>>>>>>>>  libavcodec/mlz.h              |  69 ++++++++++
>>>>>>>>>>>>>>>  libavutil/softfloat_ieee754.h | 115 +++++++++++++++++
>>>>>>>>>>>>>>>  5 files changed, 638 insertions(+), 3 deletions(-)
>>>>>>>>>>>>>>>  create mode 100644 libavcodec/mlz.c
>>>>>>>>>>>>>>>  create mode 100644 libavcodec/mlz.h
>>>>>>>>>>>>>>>  create mode 100644 libavutil/softfloat_ieee754.h
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
>>>>>>>>>>>>>>> index abef19e..a03adf5 100644
>>>>>>>>>>>>>>> --- a/libavcodec/Makefile
>>>>>>>>>>>>>>> +++ b/libavcodec/Makefile
>>>>>>>>>>>>>>> @@ -163,7 +163,7 @@ OBJS-$(CONFIG_ALAC_DECODER)            +=
>>>>>>>>>>>>>>> alac.o alac_data.o alacdsp.o
>>>>>>>>>>>>>>>  OBJS-$(CONFIG_ALAC_ENCODER)            += alacenc.o alac_data.o
>>>>>>>>>>>>>>>  OBJS-$(CONFIG_ALIAS_PIX_DECODER)       += aliaspixdec.o
>>>>>>>>>>>>>>>  OBJS-$(CONFIG_ALIAS_PIX_ENCODER)       += aliaspixenc.o
>>>>>>>>>>>>>>> -OBJS-$(CONFIG_ALS_DECODER)             += alsdec.o bgmc.o
>>>>>>>>>>>>>>> mpeg4audio.o
>>>>>>>>>>>>>>> +OBJS-$(CONFIG_ALS_DECODER)             += alsdec.o bgmc.o mlz.o
>>>>>>>>>>>>>>> mpeg4audio.o
>>>>>>>>>>>>>>>  OBJS-$(CONFIG_AMRNB_DECODER)           += amrnbdec.o
>>>>>>>>>>>>>>> celp_filters.o   \
>>>>>>>>>>>>>>>                                            celp_math.o
>>>>>>>>>>>>>>> acelp_filters.o \
>>>>>>>>>>>>>>>                                            acelp_vectors.o
>>>>>>>>>>>>>>>      \
>>>>>>>>>>>>>>> diff --git a/libavcodec/alsdec.c b/libavcodec/alsdec.c
>>>>>>>>>>>>>>> index a7e58a2..c710fc3 100644
>>>>>>>>>>>>>>> --- a/libavcodec/alsdec.c
>>>>>>>>>>>>>>> +++ b/libavcodec/alsdec.c
>>>>>>>>>>>>>>> @@ -35,8 +35,11 @@
>>>>>>>>>>>>>>>  [...]
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> +/** multiply two softfloats and handle the rounding off
>>>>>>>>>>>>>>> + */
>>>>>>>>>>>>>>> +static SoftFloat_IEEE754 multiply(SoftFloat_IEEE754 a,
>>>>>>>>>>>>>>> SoftFloat_IEEE754 b) {
>>>>>>>>>>>>>>> [...]
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Why is this in alsdec.c?
>>>>>>>>>>>>>
>>>>>>>>>>>>> This is not the actual IEEE 754 multiplication. It is as is
>>>>>>>>>>>>> mentioned
>>>>>>>>>>>>> in the reference spec.
>>>>>>>>>>>>> The typical one for 754 floats, I've implemented here separately -
>>>>>>>>>>>>> https://github.com/omerjerk/FFmpeg/commit/d6cd4bf66b9da46dd87580d7d974ce44abdcfba2#diff-4dd4b2d8d523f336fbefa96e9252187cR93
>>>>>>>>>>>>>
>>>>>>>>>>>>>> [...]
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> diff --git a/libavcodec/mlz.c b/libavcodec/mlz.c
>>>>>>>>>>>>>>> new file mode 100644
>>>>>>>>>>>>>>> index 0000000..cb2ed6a
>>>>>>>>>>>>>>> --- /dev/null
>>>>>>>>>>>>>>> +++ b/libavcodec/mlz.c
>>>>>>>>>>>>>>> [...]
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> +static int input_code(GetBitContext* gb, int len) {
>>>>>>>>>>>>>>> +    int tmp_code = 0;
>>>>>>>>>>>>>>> +    int i;
>>>>>>>>>>>>>>> +    for (i = 0; i < len; ++i) {
>>>>>>>>>>>>>>> +        tmp_code += get_bits1(gb) << i;
>>>>>                              |=
>>>>> should be better.
>>>>>
>>>>>>>>>>>>>>> +    }
>>>>>>>>>>>>>>> +    return tmp_code;
>>>>>>>>>>>>>>> +}
>>>>>
>>>>> Also, you could revert it byte-wise using ff_revert[]... if len is often bigger
>>>>> than 8 that might be worth it.
>>>>
>>>> I didn't get you.
>>>
>>> tmp_code |= get_bits1(gb) << i;
>>>
>>>
>>>> I tried logging that variable and len is always greater than 8. So
>>>> what should we go with?
>>>> It was 9 or 10 for the file I ran. And going with the logic of lzw, it
>>>> will hardly be greater than 11 ever.
>>>
>>> I think it would not benefit enough from ff_reverse then. Keep the loop.
> 
> I'm really sorry for top posting. But anything else with the patch left?

Nothing yet, more reviewers might be working on it though... thus another week
should be spent to wait for more comments.

-Thilo