AAC decoding leaves a gap at the start/end |
||||||
Issue descriptionhttps://jakearchibald.github.io/aac-decode-bug/ Decoding an AAC leaves a gap at the end of the buffer. It used to leave a gap at the start too (bug in stable, fixed in Canary). I tried to bisect this, but I guess we don't support AAC in Chromium? Firefox and Edge have the same bug, so I guess it's a common component. Safari gets it right.
,
Nov 28 2016
The last time I looked at this, ffmpeg does this. In some special cases ffmpeg will remove the initial and trailing gaps, but not always. In particular, if the LAME encoder is used enough information is left in the encoded file for ffmpeg to remove the initial gap (pre-roll or priming frames) but not the trailing (remainder) frames. There's very little WebAudio can do about this since we depend on ffmpeg for the decoding. (Note that Android's media decoder removed these, but WebAudio no longer uses that; ffmpeg is used everywhere now.)
,
Nov 28 2016
Oh. Just tried the test URL using today's ToT chromium. AAC decoding no longer has the priming frames (but the remainder frames are still there). And Opus and FLAC are supported now too.
,
Nov 29 2016
There's still a small non-zero gap at the start in ToT. Anyway, I think there's more to discuss here, so I've opened an issue with the spec https://github.com/WebAudio/web-audio-api/issues/1091
,
Jan 17 2017
,
Sep 13 2017
Closing as WontFix(WAI). WebAudio takes whatever ffmpeg decodes. If ffmpeg fixes the decoders to remove the priming frames and the trailing frames, this will be fixed. But until then, we don't plan on doing anything about this.
,
Sep 18 2017
Huh, this seems pretty bad for cross-browser compatibility. Have we reached out to ffmpeg about it?
,
Apr 12 2018
Can we at least track this?
,
Jun 20 2018
Hmm, this seems like ffmpeg is actually eating valid data since it's present in the .wav file. I feel like we would be seeing more issues if that was happening everywhere, so I wonder if it's specific to this file. Have you tried re-encoding+remuxing the .wav into a .m4a file with a more recent ffmpeg version?
,
Jun 21 2018
I didn't use ffmpeg, I used "afconvert -s 3 -f mp4f tiny-clip.wav" on mac. I just tried it with a more recent version and Chrome still sees a gap that Safari doesn't. I've updated https://jakearchibald.github.io/aac-decode-bug/ so you can now drag & drop your own audio files into it.
,
Jun 21 2018
Thanks will take a look to make sure we understand what's going awry here.
,
Jun 22 2018
Last packet is < 1024 samples, but ffmpeg still outputs the full 1024:
[PACKET]
codec_type=audio
stream_index=0
pts=29632
pts_time=0.617333
dts=29632
dts_time=0.617333
duration=379
duration_time=0.007896
convergence_duration=N/A
convergence_duration_time=N/A
size=6
pos=15263
flags=K_
[/PACKET]
[mov,mp4,m4a,3gp,3g2,mj2 @ 0x1263574c4200] stream 0, sample 31, dts 617333
[1:3:0622/134917.862536:ERROR:audio_file_reader.cc(221)] OnNewFrame: 1024, 0.379ms
This patch clipping the frames read value to the packet duration works, but feels a bit risky since I'm not sure it's always valid. I'll double check with the ffmpeg folks if the aac decoder is expected to always output 1024 samples regardless of duration. I think some encoders specify a trim value for the end in such cases.
diff --git a/media/filters/audio_file_reader.cc b/media/filters/audio_file_reader.cc
index ae37ab7b3753..b95d55002448 100644
--- a/media/filters/audio_file_reader.cc
+++ b/media/filters/audio_file_reader.cc
@@ -215,10 +215,13 @@ bool AudioFileReader::OnNewFrame(
int* total_frames,
std::vector<std::unique_ptr<AudioBus>>* decoded_audio_packets,
AVFrame* frame) {
- const int frames_read = frame->nb_samples;
+ int frames_read = frame->nb_samples;
if (frames_read < 0)
return false;
+ if (audio_codec_ == kCodecAAC && frame->pkt_duration < frames_read)
+ frames_read = frame->pkt_duration;
+
const int channels = frame->channels;
if (frame->sample_rate != sample_rate_ || channels != channels_ ||
frame->format != av_sample_format_) {
,
Jun 26 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/75fe9b33ef2ec997b32582ef801ccc15c0fb34d7 commit 75fe9b33ef2ec997b32582ef801ccc15c0fb34d7 Author: Dale Curtis <dalecurtis@chromium.org> Date: Tue Jun 26 18:01:30 2018 Avoid trailing silence in AAC decodes with WebAudio. FFmpeg won't strip any trailing audio data without end discard being specified in the container, even if the container says the duration of the last packet is smaller than what's decoded. In fact it mostly just has hard coded values for the number samples coming out of the decoding process unfortunately. This change uses the packet duration information for AAC decodes to drop the last few samples. Hopefully at some point in the future it is possible for ffmpeg to do something smarter internally. There is some risk to this approach. Media with invalid duration information may now end up losing data. We may need to restrict this workaround to just the last packet in a given stream. This doesn't fix AAC audio decoded via <audio> or <video> tags, only WebAudio's decodeAudioData(). FFmpegDemuxer (src=) and ChunkDemuxer (MSE) don't agree on how packet duration is reported when applying discard padding, so we can't use that duration to trim the packet. Which isn't really a problem since plain src= tags can't splice streams and MSE requires an explicit appendWindow for correctness. BUG= 668999 TEST=updated unittest expectations. Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel Change-Id: I68d579a68ae476407b89f8398a892090d17c9edd Reviewed-on: https://chromium-review.googlesource.com/1114094 Commit-Queue: Dale Curtis <dalecurtis@chromium.org> Reviewed-by: Raymond Toy <rtoy@chromium.org> Cr-Commit-Position: refs/heads/master@{#570460} [modify] https://crrev.com/75fe9b33ef2ec997b32582ef801ccc15c0fb34d7/media/filters/audio_file_reader.cc [modify] https://crrev.com/75fe9b33ef2ec997b32582ef801ccc15c0fb34d7/media/filters/audio_file_reader_unittest.cc [add] https://crrev.com/75fe9b33ef2ec997b32582ef801ccc15c0fb34d7/media/test/data/440hz-10ms.m4a [modify] https://crrev.com/75fe9b33ef2ec997b32582ef801ccc15c0fb34d7/third_party/WebKit/LayoutTests/webaudio/codec-tests/aac/m4a-short-duration-44khz-expected.wav
,
Jun 26 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7e898f02e3b55adb617f9f63193491267201bd8c commit 7e898f02e3b55adb617f9f63193491267201bd8c Author: Dale Curtis <dalecurtis@chromium.org> Date: Tue Jun 26 19:27:48 2018 Fix front discard when the discard padding spans multiple packets. FFmpeg both specifies a multi-packet discard and marks packets for complete discard. When we overwrite the discard padding value for complete discard, we lose information about furture packets which may need bits discarded from them. This resulted in us discarding 2048 out of 2112 frames instead of the full 2112 when playing an AAC file encoded by Apple. All of our existing tests use a 1024 frame padding instead of Apple's much larger padding, so this patch brings in a new test case. As noted in the test though, the hash isn't quite 100% correct, since we don't discard post-decode silence. This isn't a huge deal since plain src= tags can't splice streams and MSE requires an explicit append window for correctness. BUG= 668999 TEST=new unittest. Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel Change-Id: I6dad531ab986c3deb7e855dddad3011b98e6d9b6 Reviewed-on: https://chromium-review.googlesource.com/1114289 Commit-Queue: Dale Curtis <dalecurtis@chromium.org> Reviewed-by: Thomas Guilbert <tguilbert@chromium.org> Cr-Commit-Position: refs/heads/master@{#570493} [modify] https://crrev.com/7e898f02e3b55adb617f9f63193491267201bd8c/media/filters/ffmpeg_demuxer.cc [modify] https://crrev.com/7e898f02e3b55adb617f9f63193491267201bd8c/media/test/pipeline_integration_test.cc
,
Jun 26 2018
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by jakearchibald@chromium.org
, Nov 28 2016