After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 724536 - Infinite loop while encoding audio to aac
Infinite loop while encoding audio to aac
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-libav
git master
Other All
: Normal normal
: 1.3.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
: 728139 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2014-02-17 13:09 UTC by Dmitry
Modified: 2015-06-19 11:05 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
audioencoder: add flag to avoid stalling when subclass misbehaves (1.73 KB, patch)
2014-02-18 14:28 UTC, Thiago Sousa Santos
rejected Details | Review
avaudenc: consider eos if input is null and output has 0 duration (2.07 KB, patch)
2014-02-20 04:19 UTC, Thiago Sousa Santos
needs-work Details | Review
avaudenc: fix audio encoder flushing according to libav docs (6.26 KB, patch)
2014-02-20 20:36 UTC, Thiago Sousa Santos
committed Details | Review

Description Dmitry 2014-02-17 13:09:57 UTC
AAC encoder doesn't stop when source is exhausted and continuously encodes 0 sized frame

Problem may be reproduced using given pipeline:

gst-launch-1.0 audiotestsrc num-buffers=16 ! audio/x-raw, rate=(int)44100,\
layout=(string)interleaved, format=(string)F32LE, channels=(int)2,\
channel-mask=(bitmask)0x3 ! avenc_aac compliance=-2 ! fakesink
Comment 1 Thiago Sousa Santos 2014-02-18 14:20:32 UTC
Any reason you are using libav's aac instead of faac from gst-plugins-bad? Faac has a higher rank and is recommended on top of avenc_aac.

It seems that the aac encoder keeps producing some output even though we feed it a 0 sized input. It looks like a libav issue to me.

What we can do is patch gst-libav to at least abort encoding data when the user sets the state to READY to avoid a stall of the pipeline.
Comment 2 Thiago Sousa Santos 2014-02-18 14:28:58 UTC
Created attachment 269561 [details] [review]
audioencoder: add flag to avoid stalling when subclass misbehaves

If the subclass gets stuck at pushing buffers uninterruptly, use a flag
to abort in case the state was changed to READY and avoid stalling
the whole application
Comment 3 Sebastian Dröge (slomo) 2014-02-19 20:30:05 UTC
Comment on attachment 269561 [details] [review]
audioencoder: add flag to avoid stalling when subclass misbehaves

I think we should not work around bugs in the subclass inside the base class.
Comment 4 Thiago Sousa Santos 2014-02-19 21:04:09 UTC
I guess there is nothing we can do here, the bug seems to be in libav. We feed it empty input and it keeps producing output. So, report it there and close this?
Comment 5 Sebastian Dröge (slomo) 2014-02-19 21:18:17 UTC
Yes, especially because this encoder needs to have special flags passed to it to work at all (it's marked experimental). If it can get fixed fast enough in libav that should be good enough, otherwise work around it in avvidenc :)
Comment 6 Thiago Sousa Santos 2014-02-20 04:19:23 UTC
Created attachment 269756 [details] [review]
avaudenc: consider eos if input is null and output has 0 duration

Some audio codecs don't error out when fed null/0 input. In this
case we detect it is time to EOS by comparing the input and output
sizes. When both are 0 it returns EOS to the base class
Comment 7 Sebastian Dröge (slomo) 2014-02-20 19:12:08 UTC
Comment on attachment 269756 [details] [review]
avaudenc: consider eos if input is null and output has 0 duration

I don't think this looks correct... we push 0 sized buffers to the codec for flushing it IIRC.

Also can you file a bug in libav bugtracker about that and link it here?
Comment 8 Thiago Sousa Santos 2014-02-20 19:17:49 UTC
(In reply to comment #7)
> (From update of attachment 269756 [details] [review])
> I don't think this looks correct... we push 0 sized buffers to the codec for
> flushing it IIRC.

But it checks that input has 0 bytes and the output has duration 0. Isn't this enough to detect that libav has nothing more to produce/flush?

> 
> Also can you file a bug in libav bugtracker about that and link it here?

https://bugzilla.libav.org/show_bug.cgi?id=642
Comment 9 Sebastian Dröge (slomo) 2014-02-20 19:57:03 UTC
(In reply to comment #8)
> (In reply to comment #7)
> > (From update of attachment 269756 [details] [review] [details])
> > I don't think this looks correct... we push 0 sized buffers to the codec for
> > flushing it IIRC.
> 
> But it checks that input has 0 bytes and the output has duration 0. Isn't this
> enough to detect that libav has nothing more to produce/flush?

Hm, yes should be.
Comment 10 Thiago Sousa Santos 2014-02-20 20:36:58 UTC
Created attachment 269836 [details] [review]
avaudenc: fix audio encoder flushing according to libav docs

* @param[in] frame AVFrame containing the raw audio data to be encoded.
 *                  May be NULL when flushing an encoder that has the
 *                  CODEC_CAP_DELAY capability set.

The AVFrame itself should be null, not the frame.data pointer

Patch updated after talking to libav developers on how to properly flush
an encoder.
Comment 11 Sebastian Dröge (slomo) 2014-02-20 21:15:55 UTC
Comment on attachment 269836 [details] [review]
avaudenc: fix audio encoder flushing according to libav docs

Hah, good catch :) We probably do the same mistake in the audio decoder and the video encoder/decoder
Comment 12 Thiago Sousa Santos 2014-02-20 21:43:29 UTC
Seems that we do it correctly for the other cases.

For video we already use NULL for draining the encoder.

For decoding the correct way is using pkt.data = NULL and pkt.size = 0 and we already do that.

commit 845b874575480800e0bdb3209ef7dcc0337a0a16
Author: Thiago Santos <ts.santos@sisa.samsung.com>
Date:   Thu Feb 20 17:25:35 2014 -0300

    avaudenc: fix audio encoder flushing according to libav docs
    
     * @param[in] frame AVFrame containing the raw audio data to be encoded.
     *                  May be NULL when flushing an encoder that has the
     *                  CODEC_CAP_DELAY capability set.
    
    The AVFrame itself should be null, not the frame.data pointer
    
    https://bugzilla.gnome.org/show_bug.cgi?id=724536
Comment 13 Thiago Sousa Santos 2014-05-14 21:13:06 UTC
*** Bug 728139 has been marked as a duplicate of this bug. ***