Opened 5 years ago

Closed 5 years ago

Last modified 3 years ago

#420 closed defect (fixed)

Sound fragments after seeking

Reported by: kaptnole 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 kaptnole 5 years ago.
Sound bug demonstrating sourcecode
audioBitStream.jpg (18.3 KB) - added by prunkdump 5 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 5 years ago.
Another bug demonstrating sourcecode (GNU)

Download all attachments as: .zip

Change History (19)

Changed 5 years ago by kaptnole

Sound bug demonstrating sourcecode

Changed 5 years ago by prunkdump

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

Changed 5 years ago by prunkdump

Another bug demonstrating sourcecode (GNU)

comment:1 Changed 5 years ago by prunkdump

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

Open the dump with audacity

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

comment:2 Changed 5 years ago by michael

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 Changed 5 years ago by kaptnole

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 Changed 5 years ago by michael

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 Changed 5 years ago by kaptnole

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 Changed 5 years ago by michael

  • Resolution set to fixed
  • Status changed from new to closed

Thanks, ive added the code

comment:7 Changed 5 years ago by cehoyos

  • Component changed from undetermined to avcodec
  • Keywords aac added
  • Version changed from unspecified to git-master

comment:8 Changed 5 years ago by JHawkZZ

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 5 years ago by JHawkZZ (previous) (diff)

comment:9 Changed 5 years ago by JHawkZZ

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:10 Changed 5 years ago by cehoyos

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 Changed 5 years ago by kaptnole

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 Changed 5 years ago by cehoyos

Could you send a patch to ffmpeg-devel?

comment:13 Changed 5 years ago by kaptnole

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

comment:14 Changed 5 years ago by cehoyos

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 Changed 5 years ago by cehoyos

  • Keywords latm added
  • Resolution set to fixed
  • Status changed from reopened to closed

Fixed by you.

comment:16 Changed 3 years ago by DonMoir

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 3 years ago by DonMoir (previous) (diff)
Note: See TracTickets for help on using tickets.