Opened 10 years ago

Closed 9 years ago

Last modified 8 years ago

#420 closed defect (fixed)

Sound fragments after seeking

Reported by: Ole Owned by:
Priority: important Component: avcodec
Version: git-master Keywords: aac latm
Cc: Blocked By:
Blocking: Reproduced by developer: yes
Analyzed by developer: yes

Description

While trying to use FFmpeg libs as a developer I found the following:

When decoding sound from some compressed audio formats and doing a seek, the first decoded packet after seeking is buggy.

I had this with some mp4 files with aac audio codec and some wmv files with wma audio codec.
Looks like some leftovers from internal buffers - but I am using 'avcodec_flush_buffers'!
That of course leads to nasty sound noise/clicks.

See attached sourcecode.
Basically it shows the first few bytes of the first decoded sound packet then seeks back and shows the first packets decoding again.
If you use it with some wmv or mp4 file with sound you should see see effect that both outputs at 0 are different but should be equal.
Just make sure the sound is not only silence at the first 10 packets.

Or am I missing some other flush...whatever function here?

I am developing under windows using the actual ffmpeg git version.
Confirmed for 32 and 64 bit version, self compiled and downloaded autobuilds.

Attachments (3)

SoundFragmentsAfterSeeking.txt (3.5 KB ) - added by Ole 10 years ago.
Sound bug demonstrating sourcecode
audioBitStream.jpg (18.3 KB ) - added by prunkdump 10 years ago.
Audacity screenshot of the bug. stream1 : seek while playing sound. stream 2 : seek while playing silence
seekbug.tar.bz2 (1.5 MB ) - added by prunkdump 10 years ago.
Another bug demonstrating sourcecode (GNU)

Download all attachments as: .zip

Change History (19)

by Ole, 10 years ago

Sound bug demonstrating sourcecode

by prunkdump, 10 years ago

Attachment: audioBitStream.jpg added

Audacity screenshot of the bug. stream1 : seek while playing sound. stream 2 : seek while playing silence

by prunkdump, 10 years ago

Attachment: seekbug.tar.bz2 added

Another bug demonstrating sourcecode (GNU)

comment:1 by prunkdump, 10 years ago

I noticed the same problem. See audioBitStream.jpg to see what append on the audio streams. On the first stream I seek while playing strong sound, on the second I seek while playing very low sound (close to silence)

So I send my bug demonstration.

To compile

tar -xvjf seekbug.tar.bz2
cd seekbug
make

Usage

./seekbug music.mp3

right arrow -> play
down arrow -> pause
left arrow -> seek to start

escape -> quit

To hear the problem

  • play the audio
  • pause while playing strong song
  • seek to start
  • play again while listening carefully

To see the bit stream
Debug with gdb

gdb seekbug
: set args music.mp3
: break seekBugBreak
: run 

Do the seek when you want

  • play
  • pause (when you want)
  • seek to start

Enable the break call

  • type the "d" key
  • play again (gdb break)

Dump memory

: dump memory dump.raw bufferStart bufferEnd 
: quit
yes

Open the dump with audacity

  • file -> import -> raw data
  • dump.raw
  • signed 16bit, default endianness, stereo
Last edited 10 years ago by prunkdump (previous) (diff)

comment:2 by Michael Niedermayer, 10 years ago

Ive failed to find a good testcase for this but
you could try something like: (in aacdec.c)

static void flush(AVCodecContext *avctx)
{
    AACContext *ac= avctx->priv_data;
    int type, i, ch;

    for (type = 3; type >= 0; type--) {
        for (i = 0; i < MAX_ELEM_ID; i++) {
            ChannelElement *che = ac->che[type][i];
            if (che) {
                memset(che, 0, sizeof(*che));
            }
        }
    }
}
...
.flush          = flush,

and if it works find out which fields actually need to be reset, id be happy to apply a patch that would fix this

comment:3 by Ole, 10 years ago

Hi, Michael,

Your proposal works!

For my testcase it is sufficient to clear the element with type 1 and id 0.
But it did not hurt to clear them all.

Thank you.

comment:4 by Michael Niedermayer, 10 years ago

I was more wondering which fields of the element have to be cleared, clearing them all seems a bit strong even if it works. I mean for example something like SingleChannelElement.coeffs/saved

comment:5 by Ole, 10 years ago

Its the "saved" field, the following works:

static void flush(AVCodecContext *avctx)
{
    AACContext *ac= avctx->priv_data;
    int type, i, j;

    for (type = 3; type >= 0; type--) {
        for (i = 0; i < MAX_ELEM_ID; i++) {
            ChannelElement *che = ac->che[type][i];
            if (che) {
		for (j = 0; j <= 1; j++) {
                    // memset(che->ch[j].coeffs, 0, sizeof(che->ch[j].coeffs));
                    memset(che->ch[j].saved, 0, sizeof(che->ch[j].saved));
                }
            }
        }
    }
}

comment:6 by Michael Niedermayer, 10 years ago

Resolution: fixed
Status: newclosed

Thanks, ive added the code

comment:7 by Carl Eugen Hoyos, 10 years ago

Component: undeterminedavcodec
Keywords: aac added
Version: unspecifiedgit-master

comment:8 by Jered, 9 years ago

Hi guys. I ran into this exact problem using a December 26th snapshot of ffmpeg, which surprised me because it includes AAC's static flush function.

It turns out that in aacdec.h, "AVCodec ff_aac_decoder"s flush function pointer was not assigned. Only "AVCodec ff_aac_latm_decoder"s pointer was.

I verified it fixed by setting the pointer and rebuilding ffmpeg, but I wanted to post here so it can hopefully be included in the main branch.

Last edited 9 years ago by Jered (previous) (diff)

comment:9 by Jered, 9 years ago

Resolution: fixed
Status: closedreopened

comment:10 by Carl Eugen Hoyos, 9 years ago

Please test current git head, if the problem is still reproducible and you have a patch to fix it, please send the patch to ffmpeg-devel.

comment:11 by Ole, 9 years ago

JHawkZZ is absolutely right.
.../libavcodec/aacdec.c defines two decoders "ff_aac_decoder" and "ff_aac_latm_decoder". This whole thread was about "ff_aac_decoder".
To be honest, I do not even know what "ff_aac_latm_decoder" is for.
Unfortunately the patching "flush" function was falsely assigned only to the latter one.
I do not know If it also needs it but in any case it is missing in "ff_aac_decoder".
If you add a line:

flush           = flush,

in the initialization of "ff_aac_decoder" the bug is fixed.

comment:12 by Carl Eugen Hoyos, 9 years ago

Could you send a patch to ffmpeg-devel?

comment:13 by Ole, 9 years ago

How do you define "patch"?
Sending a post to ffmpeg-devel that links here?

comment:14 by Carl Eugen Hoyos, 9 years ago

No, please do a git checkout, make your change, run git diff>patchfile.diff and send the resulting patch as an attachment together with an explanation to ffmpeg-devel.

comment:15 by Carl Eugen Hoyos, 9 years ago

Keywords: latm added
Resolution: fixed
Status: reopenedclosed

Fixed by you.

comment:16 by DonMoir, 8 years ago

Not sure if the flush here is sufficient. After numerous seeks on audio with acc fltp I am seeing a constant ramp up of memory. During normal playback it all looks good but somehow many seeks cause ramp of memory and quite a bit.

This is file I am using for testing. It's not the video but audio memory that increases a lot after numerous seeks. I tested by turning off the video or audio to check results.

http://sms.pangolin.com/temp/anoha_0.ts

Closing the audio context and then reopening fixes the problem but then thats overkill. Flush seems to only clear memory and not release any that may be accumulating elsewhere in this case.

Last edited 8 years ago by DonMoir (previous) (diff)
Note: See TracTickets for help on using tickets.