--- 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]); + 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) { + av_log(s->avctx, AV_LOG_ERROR,"negative number of samples\n"); + return AVERROR_INVALIDDATA; + } } } -- 1.7.7

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@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

W dniu 2012-05-02 21:15, Mashiat Sarker Shakkhar pisze:

On 5/2/2012 11:59 PM, Jakub Stachowski wrote:

...

--- libavcodec/wmalosslessdec.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/libavcodec/wmalosslessdec.c b/libavcodec/wmalosslessdec.c index de5dca3..c1e5480 100644 --- a/libavcodec/wmalosslessdec.c +++ b/libavcodec/wmalosslessdec.c @@ -1173,7 +1173,10 @@ static int decode_packet(AVCodecContext *avctx, void *data, int *got_frame_ptr,

s->frame.nb_samples = 0;

- if (s->packet_done || s->packet_loss) { + if ( !buf&& s->num_saved_bits> get_bits_count(&s->gb)) {

May be don't put a space after the (. Also leave one space before operator and one space after, that's enough.

...

+ s->packet_done = 0; + if (!decode_frame(s)) s->num_saved_bits = 0;

I'd like this line to be broken into two.

Ok to above style fixes.

...

+ } else if (s->packet_done || s->packet_loss) { s->packet_done = 0;

/* sanity check for the buffer length */

To me it appears that this patch creates a special case for the last packet. Could you please explain why are you setting s->num_saved_bits to 0? i.e. what difference does it make? Correct me if I am wrong.

Because after decode_frame returns 0, there are still saved bits left. However if I continue to run decode_frame on those, it returns errors (like 'empty frame').

Shakkhar _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel