[FFmpeg-devel] [PATCH] QCELP decoder

[Reynaldo H. Verdejo Pinochet]

Hello

Michael Niedermayer wrote:
> 
> also this belongs in a file seperate from the qcelp decoder so it can be
> shared with other decoders.

Now I understand why Kenan splitted it. do you have a
preferred name for it? The proposed qcelp_lsp.c seems
odd.

>> +    case SILENCE:
>> +        av_log_missing_feature(avctx, "Blank frame", 1);
>> +    default:
>> +        q->framerate = I_F_Q;
> 
> is it intended that SILENCE sets IFQ ?

Not in my code, this is the kind of changes I'd like
to see out of the 'merging' attempt and back as
patches against the SoC code, I don't have your gifted
sight, I get lost. Merging the soc code with cosmetic
fixes into HEAD and latter working on these functional
changes would be OK too. Is that something you'd be
willing to accept? I'm OK with either way, I just
don't want to handle both tasks in the same patchset.

>> +static int qcelp_decode_frame(AVCodecContext *avctx,
> 
> qcelp_ prefixes shouldnt be needed for static functions unless you/Reynaldo
> want them

I like them but don't have any strong feelings about it,
maybe its better to remove the prefix and avoid a few
too-long lines while doing it.

>> +        memset(q->cbsign, 0, 72);
> 
> iam not sure if C gurantees that these variables will be packed in 72 byte

Same here. Neither on my code.

Bests
--
Reynaldo H. Verdejo Pinochet