On 01/06/2012 06:03 AM, Martin Storsjö wrote:

> Hi,
> 
> On Fri, 6 Jan 2012, Justin Ruggles wrote:
> 
>> The fate reference is updated because the previous test skipped a sample in
>> each encode() call due each input frame having an odd number of samples.
>> ---
>> libavcodec/g722enc.c  |   62 +++++++++++++++++++++++++++++++++---------------
>> tests/ref/acodec/g722 |    8 +++---
>> 2 files changed, 46 insertions(+), 24 deletions(-)
> 
> Looks sensible in general, but can you split it in two steps - first 
> refactoring encode_trellis/encode_no_trellis/encode_byte, then later doing 
> the behavioural change?

sure, i'll do that

>> diff --git a/libavcodec/g722enc.c b/libavcodec/g722enc.c
>> index 1eed784..7b89955 100644
>> --- a/libavcodec/g722enc.c
>> +++ b/libavcodec/g722enc.c
>> @@ -56,6 +56,8 @@ static av_cold int g722_encode_init(AVCodecContext * avctx)
>>         }
>>     }
>>
>> +    avctx->frame_size = PREV_SAMPLES_BUF_SIZE - 22;
>> +
>>     return 0;
>> }
> 
> I guess this is as good a default as any, but IMO this really doesn't have 
> anything to do with the prev_samples buffer size, so perhaps it's less 
> confusing to just declare it as some constant value. (For "normal" voip 
> use, 320 perhaps would be a more common default, giving 20 ms packets.) A 
> comment saying that it is arbitrary should at least be added.


yeah, 320 sounds good.

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

Reply via email to