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

[Martin]

Hi

Michael Niedermayer wrote:
> Hi
> 
> On Mon, Nov 06, 2006 at 09:19:17AM +0100, Martin Thielen wrote:
> 
>>Hi
>>
>>Michael Niedermayer wrote:
>>
>>>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 ...)
>>
>>So ahould I set the samples to 0 here as well instead of return -1?
> 
> 
> no this case is not an error

Ok, so I removed this return -1 from the patch.

> 
> 
> 
>>>
>>>[...]
>>>
>>>
>>>>        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);
>>>
>>>[...]
>>
>>Is the rest of the patch ok?
> 
> 
> i think so, but thats just from memory ...
> 
> [...]

The new patch which sets buffers to 0 if 'backstep'occurs which prior
resulted in distorted sound is attached.

Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ffmpeg-6883-patch-discard-distorted-frames.diff
Type: text/x-patch
Size: 1779 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20061106/df4b5699/attachment.bin>