diff mbox

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

Message ID 1481894385-64104-1-git-send-email-rsbultje@gmail.com
State Superseded
Headers show

Commit Message

Ronald S. Bultje Dec. 16, 2016, 1:19 p.m. UTC 
The checked bitstream reader makes this unnecessary.
---
 libavcodec/wmavoice.c | 83 ---------------------------------------------------
 1 file changed, 83 deletions(-)

Comments

Paul B Mahol Dec. 16, 2016, 2:15 p.m. UTC | #1 
On 12/16/16, Ronald S. Bultje <rsbultje@gmail.com> wrote:
> The checked bitstream reader makes this unnecessary.
> ---
>  libavcodec/wmavoice.c | 83
> ---------------------------------------------------
>  1 file changed, 83 deletions(-)
>

lgtm
Michael Niedermayer Dec. 16, 2016, 2:49 p.m. UTC | #2 
On Fri, Dec 16, 2016 at 08:19:44AM -0500, Ronald S. Bultje wrote:
> The checked bitstream reader makes this unnecessary.
> ---
>  libavcodec/wmavoice.c | 83 ---------------------------------------------------
>  1 file changed, 83 deletions(-)

this breaks fate

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

[...]
Ronald S. Bultje Dec. 16, 2016, 2:51 p.m. UTC | #3 
Hi,

On Fri, Dec 16, 2016 at 9:49 AM, Michael Niedermayer <michael@niedermayer.cc
> wrote:

> On Fri, Dec 16, 2016 at 08:19:44AM -0500, Ronald S. Bultje wrote:
> > The checked bitstream reader makes this unnecessary.
> > ---
> >  libavcodec/wmavoice.c | 83 ------------------------------
> ---------------------
> >  1 file changed, 83 deletions(-)
>
> this breaks fate
>
> TEST    wmavoice-19k


I noticed, I'm looking, consider this a proof of concept rather than
something meant for committing for now...

Ronald
Michael Niedermayer Dec. 16, 2016, 3:04 p.m. UTC | #4 
On Fri, Dec 16, 2016 at 09:51:07AM -0500, Ronald S. Bultje wrote:
> Hi,
> 
> On Fri, Dec 16, 2016 at 9:49 AM, Michael Niedermayer <michael@niedermayer.cc
> > wrote:
> 
> > On Fri, Dec 16, 2016 at 08:19:44AM -0500, Ronald S. Bultje wrote:
> > > The checked bitstream reader makes this unnecessary.
> > > ---
> > >  libavcodec/wmavoice.c | 83 ------------------------------
> > ---------------------
> > >  1 file changed, 83 deletions(-)
> >
> > this breaks fate
> >
> > TEST    wmavoice-19k
> 
> 
> I noticed, I'm looking, consider this a proof of concept rather than
> something meant for committing for now...

ahh, ok

sorry for the noise in that case


[...]
diff mbox

Patch

diff --git a/libavcodec/wmavoice.c b/libavcodec/wmavoice.c
index ceac61f..0f29bdd 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