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.


-- 
Regards,
Oleksij

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to