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 623388 - [audio encoders] Wrong output timestamps when receiving big input buffers
[audio encoders] Wrong output timestamps when receiving big input buffers
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-libav
git master
Other Linux
: Normal normal
: 0.10.11
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2010-07-02 13:36 UTC by Edward Hervey
Modified: 2010-07-05 09:13 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
render.log (30.10 KB, application/x-bzip)
2010-07-02 16:52 UTC, Edward Hervey
  Details
ffmpegenc: Fix timestamp resyncing (3.98 KB, patch)
2010-07-02 21:43 UTC, Thiago Sousa Santos
none Details | Review
ffmpegenc: Fix timestamp resyncing (3.63 KB, patch)
2010-07-02 22:38 UTC, Thiago Sousa Santos
needs-work Details | Review
ffmpegenc: fix timestamp resyncing some more (1.23 KB, patch)
2010-07-05 08:36 UTC, Mark Nauwelaerts
accepted-commit_now Details | Review

Description Edward Hervey 2010-07-02 13:36:33 UTC
Muxing AAC alone: duration ok
Muxing AAC with any other video stream : wrong duration (Seems to be doubled)

Different aac encoders report the same behaviour, therefore it only happens at that level.
Comment 1 Edward Hervey 2010-07-02 13:42:23 UTC
with gst-convenience/tests/examples/encoding/encoding:

* ./encoding -o aaconly.mov -f video/quicktime -a audio/mpeg,mpegversion=4 myaudioonlyfile.mp3
* ./encoding -o aac-h264.mov -f -video/quicktime -a audio/mpeg,mpegversion=4 -v video/x-h264 myaudiovideofile.avi

The format of the input file (last argument) doesn't matter, provided the first one is audio only and the second is audio+video.
Just compare the duration of your input files with the produced files.
Comment 2 Edward Hervey 2010-07-02 14:36:47 UTC
* You can of course replace video/x-h264 by any other video codec
* It seems the same problem also happens when using ALAC instead of AAC
Comment 3 Edward Hervey 2010-07-02 16:51:56 UTC
I went too quickly, it's actually only with ffenc_aac that it fails. With faac it reports the proper duration.
Comment 4 Edward Hervey 2010-07-02 16:52:50 UTC
Created attachment 165126 [details]
render.log

GST_DEBUG=2,encod*:5,qtmux:5 ./encoding -f video/quicktime,variant=3gpp -v video/x-h264 -a audio/mpeg,mpegversion=4 -o file:///home/bilboed/work/devel/gst-convenience/tests/examples/encoding/3gpp-h264-aac.3gp /media/data/medias/02\ Carrera\ -\ plan\ large.avi > render.log 2>&1
Comment 5 Edward Hervey 2010-07-02 17:06:46 UTC
Moving to gst-ffmpeg, looks like audio encoding is made of fail with big audio chunks :(
Comment 6 Thiago Sousa Santos 2010-07-02 17:24:04 UTC
To reproduce it I did:

gst-launch audiotestsrc num-buffers=10 samplesperbuffer=320000 !
"audio/x-raw-int, endianness=(int)1234, channels=(int)2, width=(int)16,
depth=(int)16, signed=(boolean)true, rate=(int)32000" ! qtmux ! filesink
location=test_raw.mov

and then inserted ffenc_aac before qtmux and the the duration went up a lot.
Comment 7 Edward Hervey 2010-07-02 17:46:17 UTC
the encoder does output the proper overall duration... it's just that it sets
the wrong timestamps.
Comment 8 Edward Hervey 2010-07-02 17:47:10 UTC
in the example Thiago gave, the timestamps go:
0s ----- 10s
20s ---- 30s
20s ---- 30s
40s ---- 50s
40s ---- 50s
.....
Comment 9 Thiago Sousa Santos 2010-07-02 18:19:41 UTC
gst-launch audiotestsrc num-buffers=10 samplesperbuffer=320000 !
"audio/x-raw-int, endianness=(int)1234, channels=(int)2, width=(int)16,
depth=(int)16, signed=(boolean)true, rate=(int)32000" ! ffenc_aac ! fakesink -v

Piece of log to help debugging:

/GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = "chain   ******* < 
(  220 bytes, timestamp: 0:00:09.920000000, duration: 0:00:00.032000000, offset:
 -1, offset_end: -1, flags: 0) 0x8703de0"

*** There is not enough data in the adapter for another aac buffer ***
0:00:00.798176512 32215  0x858b2f8 LOG                   ffmpeg gstffmpegenc.c:1
010:gst_ffmpegenc_chain_audio:<ffenc_aac0> 2048 bytes left in the adapter

0:00:00.798332327 32215  0x858b2f8 LOG             audiotestsrc gstaudiotestsrc.
c:1083:gst_audio_test_src_create:<audiotestsrc0> samplerate 32000
0:00:00.798378114 32215  0x858b2f8 LOG             audiotestsrc gstaudiotestsrc.
c:1085:gst_audio_test_src_create:<audiotestsrc0> next_sample 640000, ts 0:00:20.
000000000
0:00:00.798421211 32215  0x858b2f8 LOG             audiotestsrc gstaudiotestsrc.c:1105:gst_audio_test_src_create:<audiotestsrc0> generating 320000 samples at ts 0:00:10.000000000
0:00:00.825475921 32215  0x858b2f8 DEBUG                 ffmpeg gstffmpegenc.c:910:gst_ffmpegenc_chain_audio:<ffenc_aac0> Received time 0:00:10.000000000, duration 0:00:10.000000000, size 1280000

*** The adapter timestamp is 0, because it has not a whole buffer after the last _take ***
0:00:00.827315935 32215  0x858b2f8 LOG                   ffmpeg gstffmpegenc.c:937:gst_ffmpegenc_chain_audio:<ffenc_aac0> taking adapter timestamp 0:00:00.000000000

*** OOPS ***
0:00:00.827390281 32215  0x858b2f8 DEBUG                 ffmpeg gstffmpegenc.c:957:gst_ffmpegenc_chain_audio:<ffenc_aac0> adapter timestamp drifting, taking upstream timestamp 0:00:39.936000000

0:00:00.827432396 32215  0x858b2f8 LOG                   ffmpeg gstffmpegenc.c:967:gst_ffmpegenc_chain_audio:<ffenc_aac0> pushing buffer in adapter
0:00:00.827471731 32215  0x858b2f8 LOG                   ffmpeg gstffmpegenc.c:974:gst_ffmpegenc_chain_audio:<ffenc_aac0> frame_bytes 4096, avail 1282048
0:00:00.827507803 32215  0x858b2f8 LOG                   ffmpeg gstffmpegenc.c:979:gst_ffmpegenc_chain_audio:<ffenc_aac0> taking 4096 bytes from the adapter
0:00:00.827547512 32215  0x858b2f8 LOG                   ffmpeg gstffmpegenc.c:856:gst_ffmpegenc_encode_audio:<ffenc_aac0> encoding buffer of max size 32768
0:00:00.828958862 32215  0x858b2f8 LOG                   ffmpeg gstffmpegenc.c:867:gst_ffmpegenc_encode_audio:<ffenc_aac0> got output size 176
0:00:00.829015146 32215  0x858b2f8 LOG                   ffmpeg gstffmpegenc.c:877:gst_ffmpegenc_encode_audio:<ffenc_aac0> pushing size 176, timestamp 0:00:39.936000000
/GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = "chain   ******* < (  374 bytes, timestamp: 0:00:09.952000000, duration: 0:00:00.032000000, offset: -1, offset_end: -1, flags: 0) 0x8703d90"
0:00:00.829578846 32215  0x858b2f8 LOG                   ffmpeg gstffmpegenc.c:979:gst_ffmpegenc_chain_audio:<ffenc_aac0> taking 4096 bytes from the adapter
0:00:00.829578910 32215  0x858b2f8 LOG                   ffmpeg gstffmpegenc.c:856:gst_ffmpegenc_encode_audio:<ffenc_aac0> encoding buffer of max size 32768
0:00:00.831124398 32215  0x858b2f8 LOG                   ffmpeg gstffmpegenc.c:867:gst_ffmpegenc_encode_audio:<ffenc_aac0> got output size 309
0:00:00.831158329 32215  0x858b2f8 LOG                   ffmpeg gstffmpegenc.c:877:gst_ffmpegenc_encode_audio:<ffenc_aac0> pushing size 309, timestamp 0:00:39.968000000
/GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = "chain   ******* < (  176 bytes, timestamp: 0:00:39.936000000, duration: 0:00:00.032000000, offset: -1, offset_end: -1, flags: 32) 0x8703cf0"
Comment 10 Thiago Sousa Santos 2010-07-02 18:36:00 UTC
(In reply to comment #9)
> *** The adapter timestamp is 0, because it has not a whole buffer after the
> last _take ***
> 0:00:00.827315935 32215  0x858b2f8 LOG                   ffmpeg
> gstffmpegenc.c:937:gst_ffmpegenc_chain_audio:<ffenc_aac0> taking adapter
> timestamp 0:00:00.000000000
> 

Actually it is probably because this enormous buffer had a 0 timestamp.
Comment 11 Thiago Sousa Santos 2010-07-02 19:41:42 UTC
Think I understood it.

There is a bug on the resyncing code using the prev_timestamp results.

I'm preparing a patch with further details.
Comment 12 Thiago Sousa Santos 2010-07-02 21:43:50 UTC
Created attachment 165151 [details] [review]
ffmpegenc: Fix timestamp resyncing

Properly convert bytes into time using sample size, sample rate
and channels number, instead of sample rate only.

This can cause huge timestamp discontinuities (even though the
durations remain correct) and might cause problems to muxers.

Fixes #623388
Comment 13 Thiago Sousa Santos 2010-07-02 22:38:36 UTC
Created attachment 165153 [details] [review]
ffmpegenc: Fix timestamp resyncing

This is a simplified version of the previous patch.
Comment 14 Edward Hervey 2010-07-03 06:43:21 UTC
Review of attachment 165153 [details] [review]:

I still need to test it over all the use-cases that were previously failing, but the fundamental fix makes complete sense.

::: ext/ffmpeg/gstffmpegenc.c
@@ +918,3 @@
 
+    frame_bytes = frame_size * osize * ctx->channels;
+

This line doesn't need to be moved from where it was previously

@@ +944,3 @@
+          " and adding consumed time %" GST_TIME_FORMAT,
+          GST_TIME_ARGS (ffmpegenc->adapter_ts),
+          GST_TIME_ARGS (consumed_time), GST_TIME_ARGS (timestamp));

OK to leave this new variable and debug line, it does indeed make debugging easier for the future and doesn't cost anything when debugging is disabled.

@@ +956,3 @@
         upstream_time +=
+            gst_util_uint64_scale (bytes, GST_SECOND,
+            ctx->sample_rate * osize * ctx->channels);

This is the fundamental fix, and only required part to fix the issue.

@@ +987,3 @@
+      /* Note that we take frame_bytes and add frame_size.
+       * Makes sense when resyncing because you don't have to count channels
+       * or samplesize to divide by the samplerate */

OK to leave this comment.
Comment 15 Edward Hervey 2010-07-03 07:00:14 UTC
Does fix the issue when using ffenc_aac or ffenc_alac.
Comment 16 Edward Hervey 2010-07-03 07:00:58 UTC
Comment on attachment 165153 [details] [review]
ffmpegenc: Fix timestamp resyncing

Please commit with the fixes mentionned in the review.
Comment 17 Thiago Sousa Santos 2010-07-03 14:59:08 UTC
Pushed.

commit b8f556a566483758be82797421cb9593bba1ac43
Author: Thiago Santos <thiago.sousa.santos@collabora.co.uk>
Date:   Fri Jul 2 18:38:06 2010 -0300

    ffmpegenc: Fix timestamp resyncing
    
    Properly convert bytes into time using sample size, sample rate
    and channels number, instead of sample rate only.
    
    This can cause huge timestamp discontinuities (even though the
    durations remain correct) and might cause problems to muxers.
    
    Fixes #623388
Comment 18 Mark Nauwelaerts 2010-07-05 08:36:18 UTC
Created attachment 165260 [details] [review]
ffmpegenc: fix timestamp resyncing some more

Still puzzled how I got this in in the first place.

Anyway, seems there is still one bytes <-> samples mix-up, and the attached patch should take care of that.
Comment 19 Edward Hervey 2010-07-05 09:07:49 UTC
Comment on attachment 165260 [details] [review]
ffmpegenc: fix timestamp resyncing some more

Ok, please commit
Comment 20 Mark Nauwelaerts 2010-07-05 09:13:46 UTC
commit 1d0b29414a936bf07431be940c1b38f0dfb2f920
Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk>
Date:   Mon Jul 5 10:32:42 2010 +0200

    ffmpegenc: fix timestamp resyncing some more
    
    Convert bytes to samples in remaining occurrence.
    
    See #623388.