[Ffmpeg-devel] [PATCH] MP3 decoding from RTP stream. Getting "invalid new backstep"

[Michael Niedermayer]

Hi

On Fri, Nov 03, 2006 at 03:20:04PM +0100, Martin wrote:
> Michael Niedermayer wrote:
> > Hi
> > 
> > On Fri, Nov 03, 2006 at 12:25:30PM +0100, Martin wrote:
> > 
> >>Hi,
> >>
> >>
> >>>>right before returning:
> >>>>
> >>>> if (discard_frame) {
> >>>>   //dicard frame is set if an backstep or overread occurs.
> >>>>   for(i=0;i<nb_frames;i++) {
> >>>>     for(ch=0;ch<s->nb_channels;ch++) {
> >>>>       int j;
> >>>>       for(j=0;j<SBLIMIT;j++) {
> >>>>         s->sb_samples[ch][i][j] = 0;
> >>>>       }
> >>>>     }
> >>>>   }
> >>>> }
> >>>>
> >>>>
> >>>>
> >>>>This didn't worked as expected. Also I found another problem caused by
> >>>>this point in the huffman_decode() function:
> >>>>
> >>>> if (pos > end_pos2 && last_pos){
> >>>>               /* some encoders generate an incorrect size for this
> >>>>                  part. We must go back into the data */
> >>>>               s_index -= 4;
> >>>>               skip_bits_long(&s->gb, last_pos - pos);
> >>>>               av_log(NULL, AV_LOG_INFO, "overread, skip %d enddists:
> >>>>%d %d\n", last_pos - pos, end_pos-pos, end_pos2-pos);
> >>>>               s->frame_distorted = 1;
> >>>>
> >>>>               break;
> >>>>           }
> >>>>
> >>>>
> >>>>My idea was now to introduce a new flag into the MPADecodeContext:
> >>>>int frame_distorted;
> >>>>
> >>>>Now I set this flag to 0 at the start of decode_frame()
> >>>>and set it to 1 when backstep or overread occurs. At the end of
> >>>>decode_frame() I check the flag and return -1 if it is set. This is
> >>>>working for me. If I send a patch with the new flag, would you include it?
> >>>
> >>>
> >>>not without understanding why setting sb_samples to 0 doesnt work
> >>>
> >>>[...]
> >>
> >>It seems to work now, with setting sb_frames to 0. I did it after the
> >>synthesis filter which didn't work. How should I transmit the fact that
> >>a distortion occured in huffmann_decode() to the function
> >>mp_decode_frame()? Can I use the flag for this purpose?
> >>I'm not sure whether I break something if I set sb_samples to 0 directly
> >>at the point where the distortion occurs in huffmann_decode().
> > 
> > 
> > huffmann_decode() should return -1 if theres an error
> > 
> > [...]
> 
> Thanks for the help!
> So here is a patch which sets buffers to 0 if 'overread' or 'backstep'
> occurs which prior resulted in distorted sound.
> 
> Martin

> --- mpegaudiodec.c_bak	2006-11-03 14:38:48.263103729 +0100
> +++ mpegaudiodec.c	2006-11-03 14:43:02.153241601 +0100
> @@ -1705,7 +1705,8 @@ static int huffman_decode(MPADecodeConte
>                  s_index -= 4;
>                  skip_bits_long(&s->gb, last_pos - pos);
>                  av_log(NULL, AV_LOG_INFO, "overread, skip %d enddists: %d %d\n", last_pos - pos, end_pos-pos, end_pos2-pos);
> -                break;
> +                //break;
> +                return -1;
>              }

this one will break decoding of some mp3 files (from broken encoders ...)


[...]
>          assert(i <= buf_size - HEADER_SIZE && i>= 0);
>          memcpy(s->last_buf + s->last_buf_size, s->gb.buffer + buf_size - HEADER_SIZE - i, i);
> @@ -2503,6 +2508,19 @@ static int mp_decode_frame(MPADecodeCont
>          }
>      }
>  #endif
> +
> +    // zero samples if frame is distorted by overread or backstep
> +    if (discard_frame) {
> +        for(i=0;i<nb_frames;i++) {
> +            for(ch=0;ch<s->nb_channels;ch++) {
> +                int j;
> +                for(j=0;j<SBLIMIT;j++) {
> +                    s->sb_samples[ch][i][j] = 0;
> +                }
> +            }
> +        }
> +    }

memset(0);

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In the past you could go to a library and read, borrow or copy any book
Today you'd get arrested for mere telling someone where the library is