Am 05.02.2015 um 10:03 schrieb Diego Biurrun:
> On Wed, Feb 04, 2015 at 09:27:41AM +0100, Oleksij Rempel wrote:
>> --- a/Changelog
>> +++ b/Changelog
>> @@ -12,6 +12,7 @@ version <next>:
>>  - VP8 in Ogg demuxing
>>  - OpenH264 encoder wrapper
>>  - Support DNx100 (960x720@8)
>> +- DSS_SP decoder
> 
> "DSS SP" I guess.
> 
>> --- a/libavcodec/codec_desc.c
>> +++ b/libavcodec/codec_desc.c
>> @@ -2202,6 +2202,13 @@ static const AVCodecDescriptor codec_descriptors[] = {
>>      {
>> +        .id        = AV_CODEC_ID_DSS_SP,
>> +        .type      = AVMEDIA_TYPE_AUDIO,
>> +        .name      = "dss_sp",
>> +        .long_name = NULL_IF_CONFIG_SMALL("DSS SP"),
> 
> This should spell out the abbreviation IMO.
> 
> What does SP stand for anyway?

http://www.olympusamerica.com/cpg_section/cpg_support_faqs.asp?id=1400

"A QP (Quality Play) quality mode recording uses more memory than an SP
(Standard Play) quality mode recording because the QP mode makes a
more-detailed digital record and uses more memory. "

In this case complete description would be: "Digital Speech Standard -
Standard Play Mode"

> 
>> --- /dev/null
>> +++ b/libavcodec/dss_sp.c
>> @@ -0,0 +1,784 @@
>> +/*
>> + * dss_sp audio decoder
> 
> same
> 
>> +#include "libavutil/common.h"
>> +#include "libavutil/channel_layout.h"
>> +#include "libavutil/mem.h"
>> +#include "libavutil/opt.h"
>> +
>> +#include "avcodec.h"
>> +#include "internal.h"
>> +#include "get_bits.h"
> 
> nit: order
> 
>> +typedef struct LPCData {
>> +    int32_t filter[14];
>> +} LPCData;
> 
> Why do you wrap the array in a struct?
> 
>> +typedef struct DssSpContext {
>> +    int32_t excitation[288 + 6];
>> +    int32_t history[187];
>> +    DssSpFrame fparam;
>> +    int32_t working_buffer[SUBFRAMES][72];
>> +    int32_t audio_buf[15];
>> +    int32_t err_buf1[15];
>> +    LPCData lpc;
>> +    int32_t filter[15];
>> +    int32_t vector_buf[72];
>> +    int noise_state;
>> +    int32_t err_buf2[15];
> 
> These array sizes seem slightly magicky to me.
> 
>> +/*
>> + * Used for the coding/decoding of the pulses positions for the MP-MLQ 
>> codebook.
> 
> pulse positions
> 
>> +static const int32_t binary_decreasing_array[] = {
>> +    32767, 16384, 8192, 4096, 2048, 1024, 512, 256,
>> +    128, 64, 32, 16, 8, 4, 2,
>> +};
>> +
>> +static const int32_t dss_sp_unc_decreasing_array[] = {
>> +    32767, 26214, 20972, 16777, 13422, 10737, 8590, 6872,
>> +    5498, 4398, 3518, 2815, 2252, 1801, 1441,
>> +};
> 
> Both of these would fit in uint16_t.
> 
>> +static const int32_t dss_sp_sinc[67] = {
>> +      262,   293,   323,   348,   356,   336,   269,   139,
>> +      -67,  -358,  -733, -1178, -1668, -2162, -2607, -2940,
>> +    -3090, -2986, -2562, -1760,  -541,  1110,  3187,  5651,
>> +     8435, 11446, 14568, 17670, 20611, 23251, 25460, 27125,
>> +    28160, 28512, 28160,
>> +    27125, 25460, 23251, 20611, 17670, 14568, 11446,  8435,
>> +     5651,  3187,  1110,  -541, -1760, -2562, -2986, -3090,
>> +    -2940, -2607, -2162, -1668, -1178,  -733,  -358,   -67,
>> +      139,   269,   336,   356,   348,   323,   293,   262,
>> +};
> 
> This in int16_t.
> 
>> +static int dss_sp_decode_one_frame(DssSpContext *p,
>> +                                   int16_t *abuf_dst, const uint8_t 
>> *abuff_src)
> 
> abuf_src
> 
> Keep the number of 'f' consistent ;)
> 
>> +AVCodec ff_dss_sp_decoder = {
>> +    .name           = "dss_sp",
>> +    .long_name      = NULL_IF_CONFIG_SMALL("DSS SP"),
> 
> I'd spell out the abbreviation.
> 
> Diego
> _______________________________________________
> libav-devel mailing list
> [email protected]
> https://lists.libav.org/mailman/listinfo/libav-devel
> 


-- 
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