[FFmpeg-devel] wmavoice: don't check if we have enough bits to read a superframe.

Submitted by Ronald S. Bultje on Dec. 16, 2016, 2:41 p.m.

Details

Message ID 1481899288-69470-1-git-send-email-rsbultje@gmail.com
State New
Headers show

Commit Message

Ronald S. Bultje Dec. 16, 2016, 2:41 p.m.
The checked bitstream reader makes this unnecessary.
---
 libavcodec/wmavoice.c | 86 ++-------------------------------------------------
 1 file changed, 2 insertions(+), 84 deletions(-)

Comments

Michael Niedermayer Dec. 16, 2016, 2:57 p.m.
On Fri, Dec 16, 2016 at 09:41:28AM -0500, Ronald S. Bultje wrote:
> The checked bitstream reader makes this unnecessary.
> ---
>  libavcodec/wmavoice.c | 86 ++-------------------------------------------------
>  1 file changed, 2 insertions(+), 84 deletions(-)

This breaks fate too

TEST    wmavoice-19k
stddev: 2973.99 PSNR: 26.86 MAXDIFF:37092 bytes:   527906/   527906
stddev: |2973.99 - 0| >= 3
Test wmavoice-19k failed. Look at tests/data/fate/wmavoice-19k.err for details.
make: *** [fate-wmavoice-19k] Error 1

[...]

Patch hide | download patch | download mbox

diff --git a/libavcodec/wmavoice.c b/libavcodec/wmavoice.c
index ba02c7d..e4ea128 100644
--- a/libavcodec/wmavoice.c
+++ b/libavcodec/wmavoice.c
@@ -1654,83 +1654,6 @@  static void stabilize_lsps(double *lsps, int num)
 }
 
 /**
- * Test if there's enough bits to read 1 superframe.
- *
- * @param orig_gb bit I/O context used for reading. This function
- *                does not modify the state of the bitreader; it
- *                only uses it to copy the current stream position
- * @param s WMA Voice decoding context private data
- * @return < 0 on error, 1 on not enough bits or 0 if OK.
- */
-static int check_bits_for_superframe(GetBitContext *orig_gb,
-                                     WMAVoiceContext *s)
-{
-    GetBitContext s_gb, *gb = &s_gb;
-    int n, need_bits, bd_idx;
-    const struct frame_type_desc *frame_desc;
-
-    /* initialize a copy */
-    init_get_bits(gb, orig_gb->buffer, orig_gb->size_in_bits);
-    skip_bits_long(gb, get_bits_count(orig_gb));
-    av_assert1(get_bits_left(gb) == get_bits_left(orig_gb));
-
-    /* superframe header */
-    if (get_bits_left(gb) < 14)
-        return 1;
-    if (!get_bits1(gb))
-        return AVERROR(ENOSYS);           // WMAPro-in-WMAVoice superframe
-    if (get_bits1(gb)) skip_bits(gb, 12); // number of  samples in superframe
-    if (s->has_residual_lsps) {           // residual LSPs (for all frames)
-        if (get_bits_left(gb) < s->sframe_lsp_bitsize)
-            return 1;
-        skip_bits_long(gb, s->sframe_lsp_bitsize);
-    }
-
-    /* frames */
-    for (n = 0; n < MAX_FRAMES; n++) {
-        int aw_idx_is_ext = 0;
-
-        if (!s->has_residual_lsps) {     // independent LSPs (per-frame)
-           if (get_bits_left(gb) < s->frame_lsp_bitsize) return 1;
-           skip_bits_long(gb, s->frame_lsp_bitsize);
-        }
-        bd_idx = s->vbm_tree[get_vlc2(gb, frame_type_vlc.table, 6, 3)];
-        if (bd_idx < 0)
-            return AVERROR_INVALIDDATA; // invalid frame type VLC code
-        frame_desc = &frame_descs[bd_idx];
-        if (frame_desc->acb_type == ACB_TYPE_ASYMMETRIC) {
-            if (get_bits_left(gb) < s->pitch_nbits)
-                return 1;
-            skip_bits_long(gb, s->pitch_nbits);
-        }
-        if (frame_desc->fcb_type == FCB_TYPE_SILENCE) {
-            skip_bits(gb, 8);
-        } else if (frame_desc->fcb_type == FCB_TYPE_AW_PULSES) {
-            int tmp = get_bits(gb, 6);
-            if (tmp >= 0x36) {
-                skip_bits(gb, 2);
-                aw_idx_is_ext = 1;
-            }
-        }
-
-        /* blocks */
-        if (frame_desc->acb_type == ACB_TYPE_HAMMING) {
-            need_bits = s->block_pitch_nbits +
-                (frame_desc->n_blocks - 1) * s->block_delta_pitch_nbits;
-        } else if (frame_desc->fcb_type == FCB_TYPE_AW_PULSES) {
-            need_bits = 2 * !aw_idx_is_ext;
-        } else
-            need_bits = 0;
-        need_bits += frame_desc->frame_size;
-        if (get_bits_left(gb) < need_bits)
-            return 1;
-        skip_bits_long(gb, need_bits);
-    }
-
-    return 0;
-}
-
-/**
  * Synthesize output samples for a single superframe. If we have any data
  * cached in s->sframe_cache, that will be used instead of whatever is loaded
  * in s->gb.
@@ -1771,12 +1694,6 @@  static int synth_superframe(AVCodecContext *ctx, AVFrame *frame,
         s->sframe_cache_size = 0;
     }
 
-    if ((res = check_bits_for_superframe(gb, s)) == 1) {
-        *got_frame_ptr = 0;
-        return 1;
-    } else if (res < 0)
-        return res;
-
     /* First bit is speech/music bit, it differentiates between WMAVoice
      * speech samples (the actual codec) and WMAVoice music samples, which
      * are really WMAPro-in-WMAVoice-superframes. I've never seen those in
@@ -1999,7 +1916,7 @@  static int wmavoice_decode_packet(AVCodecContext *ctx, void *data,
     pos = get_bits_left(gb);
     if ((res = synth_superframe(ctx, data, got_frame_ptr)) < 0) {
         return res;
-    } else if (*got_frame_ptr) {
+    } else if (*got_frame_ptr && get_bits_left(&s->gb) >= 0) {
         int cnt = get_bits_count(gb);
         s->skip_bits_next = cnt & 7;
         res = cnt >> 3;
@@ -2015,6 +1932,7 @@  static int wmavoice_decode_packet(AVCodecContext *ctx, void *data,
         copy_bits(&s->pb, avpkt->data, size, gb, s->sframe_cache_size);
         // FIXME bad - just copy bytes as whole and add use the
         // skip_bits_next field
+        *got_frame_ptr = 0;
     }
 
     return size;