[FFmpeg-devel] [PATCH 2/2] Skip last samples in frame if needed

[Jakub Stachowski]

W dniu 2012-05-02 21:22, Mashiat Sarker Shakkhar pisze:
> On 5/2/2012 11:59 PM, Jakub Stachowski wrote:
>> ---
>> libavcodec/wmalosslessdec.c | 15 ++++++++++++++-
>> 1 files changed, 14 insertions(+), 1 deletions(-)
>>
>> diff --git a/libavcodec/wmalosslessdec.c b/libavcodec/wmalosslessdec.c
>> index c1e5480..b77d576 100644
>> --- a/libavcodec/wmalosslessdec.c
>> +++ b/libavcodec/wmalosslessdec.c
>> @@ -977,8 +977,16 @@ static int decode_subframe(WmallDecodeCtx *s)
>>
>> /* Write to proper output buffer depending on bit-depth */
>> for (i = 0; i< s->channels_for_cur_subframe; i++) {
>> + int already_written;
>> int c = s->channel_indexes_for_cur_subframe[i];
>> - int subframe_len =
>> s->channel[c].subframe_len[s->channel[c].cur_subframe];
>> +
>> + if (s->bits_per_sample == 16)
>> + already_written = (s->samples_16[c] - ((int16_t *)s->frame.data[0] +
>> c)) / s->num_channels;
>> + else
>> + already_written = (s->samples_32[c] - ((int32_t *)s->frame.data[0] +
>> c)) / s->num_channels;
>> +
>> + int subframe_len = FFMIN(s->frame.nb_samples - already_written,
>> s->channel[c].subframe_len[s->channel[c].cur_subframe]);
>> +
>
> I am doubtful about this part, especially the way you calculate the
> number of samples already written. That's all I can say. Please try to
> get expert opinion from someone who know WMA / ASF.

The number of samples are only for current frame. When computing it I 
don't need to think about anything WMA specific - I just count how many 
samples got copied from channel_residues to output buffer. In previous 
version of the patch I used counter that was increased in copy loop in 
decode_subframe, I changed it current way of computing number of already 
written samples in order to avoid doing extra work (even if it is just 
single increment) in inner loop.
Or do you mean that my assumption that encoder should write 
(samples_per_frame-skip) samples is suspect?

>
>>
>> for (j = 0; j< subframe_len; j++) {
>> if (s->bits_per_sample == 16) {
>> @@ -1056,6 +1064,11 @@ static int decode_frame(WmallDecodeCtx *s)
>> if (get_bits1(gb)) {
>> skip = get_bits(gb, av_log2(s->samples_per_frame * 2));
>> av_dlog(s->avctx, "end skip: %i\n", skip);
>> + s->frame.nb_samples -= skip;
>> + if (s->frame.nb_samples<0) {
>
> As I have said before, one space each before and after operator is
> recommended.
>
>> + av_log(s->avctx, AV_LOG_ERROR,"negative number of samples\n");
>
> I don't think this error message is accurate. After all, there's no such
> thing as "negative number of samples". A more accurate error messege
> IMHO would be something like - "Invalid value for end skip (Corrupted
> input?)". Others might disagree though.
>
> Also we recommend a space after every comma.

Ok.

>
>> + return AVERROR_INVALIDDATA;
>> + }
>> }
>>
>> }
>
> Regards
> Shakkhar
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>