On Mon, Nov 24, 2014 at 5:21 PM, Oleksij Rempel <[email protected]> wrote:
> Am 24.11.2014 um 17:33 schrieb Vittorio Giovara:
>> On Mon, Nov 24, 2014 at 6:52 AM, Oleksij Rempel <[email protected]> 
>> wrote:
>>> Am 23.11.2014 um 23:59 schrieb Vittorio Giovara:
>>>> From: Oleksij Rempel <[email protected]>
>>>>
>>>> Signed-off-by: Oleksij Rempel <[email protected]>
>>>> Signed-off-by: Luca Barbato <[email protected]>
>>>> Signed-off-by: Vittorio Giovara <[email protected]>
>>>> ---
>>>> Updated patch with version and changelog entry and placed the public entry
>>>> in the right place (at the end of the list).
>>>>
>>>> One question for the author, the decoder supports SP decoding but I tried 
>>>> it
>>>> on an LP sample and it seems to work too. If that is the case, would it 
>>>> make
>>>> sense to name it just 'dss', given that an hq layer could be added too?
>>>
>>> Damn... i tested absolutely wrong path before sending patches :(
>>>
>>> We have here 3 parts: demuxer and two codecs. LP mode codec is G723.1;
>>> SP mode codec i called DSS_SP. For two years i did some notices about
>>> demuxer:
>>> https://bugzilla.libav.org/show_bug.cgi?id=334
>>> Now i have some doubts about block format in the container. I think
>>> first part is a frame offset in then block, and second part is probably
>>> mode magic.
>>>
>>> This work has only core support of this format. Bookmarks are not
>>> supported - i don't what is the best way to integrate it.
>>>
>>> I assume HQ mode will be absolutely different codec. May be some thing
>>> what we already have.
>>>
>>> Are both modes working for you correctly?
>>
>> Well, I haven't checked the hash sums thoroughly but the dss and the
>> wav you uploaded to https://github.com/olerem/redss sounded pretty
>> similar. If you say that dss_hq could be a completely different codec,
>> and since this version can handle sp and lp, I'd tend to name the
>> current decoder just "DSS". What do you think?
>>
>> Also your decoder triggers two (simple) coverity warnings, I am
>> attaching them here.
>>
>>
>> ______________________________
>> __________________________________________________________________________
>> *** CID 1255926:  Uninitialized scalar variable  (UNINIT)
>> /libavcodec/dss_sp_dec.c: 654 in dss_sp_sf_synthesis()
>> 648         dss_sp_scale_vector(p->err_buf1, -normalize_bits, 15);
>> 649
>> 650         if (size > 0)
>> 651             vsum_2 = dss_sp_vector_sum(p, size);
>> 652
>> 653         if (vsum_2 >= 0x40)
>>>>>     CID 1255926:  Uninitialized scalar variable  (UNINIT)
>>>>>     Using uninitialized value "vsum_1".
>> 654             tmp = (vsum_1 << 11) / vsum_2;
>> 655         else
>> 656             tmp = 1;
>> 657
>> 658         bias     = 409 * tmp >> 15 << 15;
>> 659         tmp      = (bias + 32358 * p->noise_state) >> 15;
>>
>> ________________________________________________________________________________________________________
>> *** CID 1255927:  Uninitialized scalar variable  (UNINIT)
>> /libavcodec/dss_sp_dec.c: 653 in dss_sp_sf_synthesis()
>> 647         dss_sp_scale_vector(p->audio_buf, -normalize_bits, 15);
>> 648         dss_sp_scale_vector(p->err_buf1, -normalize_bits, 15);
>> 649
>> 650         if (size > 0)
>> 651             vsum_2 = dss_sp_vector_sum(p, size);
>> 652
>>>>>     CID 1255927:  Uninitialized scalar variable  (UNINIT)
>>>>>     Using uninitialized value "vsum_2".
>> 653         if (vsum_2 >= 0x40)
>> 654             tmp = (vsum_1 << 11) / vsum_2;
>> 655         else
>> 656             tmp = 1;
>> 657
>> 658         bias     = 409 * tmp >> 15 << 15;
>>
>> Could you address those and the comments you received based on this version?
>> Thank you
>>
>
> Ok, do you have some public git with this patches? So i'll provide patch
> on top of it. I hope it will keep reviewing easier.

You can always get the latest patches from our patchwork
https://patches.libav.org/patch/55804/
https://patches.libav.org/patch/55805/ and
https://patches.libav.org/patch/55806/

I've uploaded them for your convenience here
https://github.com/kodabb/libav/tree/dss
-- 
Vittorio
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to