Closed Bug 1528265 Opened 5 years ago Closed 4 years ago

Flac decoder reproducibly hangs at specific times in some files

Categories

(Core :: Audio/Video: Playback, defect, P2)

65 Branch
defect

Tracking

()

VERIFIED FIXED
mozilla76
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- wontfix
firefox65 --- wontfix
firefox66 --- wontfix
firefox67 --- wontfix
firefox68 --- wontfix
firefox69 --- wontfix
firefox73 --- wontfix
firefox74 --- wontfix
firefox75 - wontfix
firefox76 --- verified

People

(Reporter: hi, Assigned: jya)

References

(Regression, )

Details

(Keywords: parity-chrome, regression, testcase, Whiteboard: [sci-exclude])

Attachments

(5 files, 1 obsolete file)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Firefox/60.0

Steps to reproduce:

  1. Verify the attached flac with the flac binary and -t flag. It should register as ok
  2. Open the file with Firefox
  3. Play or seek to 2:05 and press play
  4. Let it play to 2:08 (GetMediaTime=127616869)

(Could not find an openly available flac file to verify bug and had to pick one from my personal library. Apologies, feel free to make private)

Actual results:

The decoder will reliably hang at 2:07-2:08 which stops playback. Playback will pick up a few seconds later.

Tested on:

  • Ubuntu 19.10 + Firefox 65.0 (AMD64)
  • Windows10 + Firefox 65.0 (AMD64)
  • MacOS 10.14 + Firefox 65.0

Expected results:

Playback should have continued without an interruption at that specific point. File plays ok in Chrome and VLC.

Edit: File upload was too large. Find affected file here: https://www.dropbox.com/s/q7bwqqpfwekevkt/test.flac

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:67.0) Gecko/20100101 Firefox/67.0
20190215095516

Status: UNCONFIRMED → NEW
Has STR: --- → yes
Component: Untriaged → Audio/Video: Playback
Ever confirmed: true
OS: Unspecified → All
Product: Firefox → Core
Hardware: Unspecified → All

I have identified two of my own music files that exhibit this exact same issue, and have provided links for download. Hopefully this will help narrow down root cause:

  • "sampleA.flac": issue occurs at 10 seconds and firefox hangs for approx. 2 mins 30 secs before resuming playback
  • "sampleB.flac": issue occurs at 2 min 41 sec, I have waited 10+ minutes and playback never resumed.
    • https://openload.co/f/Hs_iZ2zWjeU/sampleB.flac
    • This particular file also seems to affect Firefox more severely. For example, after reproducing the hang, playback does not always resume even if I move the slider to track to a different point in the music file. This particular symptom is 80 - 90% reproducible with this file, in my estimation.

Other comments:

  • I have deleted tags (music metadata) from the .flacs using Mp3Tag (https://www.mp3tag.de/en/), however the issue persists on the same point in time of the file.
  • Although I do not have an exact date, I estimate that I have first encountered this bug around Feb 1 2019.
Blocks: 1487797
Has Regression Range: --- → yes
Flags: needinfo?(cchang)
Keywords: regression

There is an irregular frame inserted in the test.flac. The frame blocksize is expected to be fixed value 4096, but one frame with variable blocksize 4608 is inserted in that file. As a result, the index of the next frame will become a larger number than expected by our calculation.

The index is calculated by

mIndex = Header().mVariableBlockSize
                 ? Header().mFrameOrSampleNum
                 : Header().mFrameOrSampleNum *
                       std::max(Header().mBlocksize, aPreviousBlocksize);

Suppose the blocksize is fixed to 4096 for mFrameOrSampleNum in [0, 98], the blocksize becomes variable to 4608 for mFrameOrSampleNum = 99 (actually it might be a random number), then the blocksize becomes fixed to 4096 again for the reset of the files, then

  1. mIndex(i) = i * 4096 for i in [0, 98]
  2. mIndex(99) = 99
  3. mIndex(100) = 100 * 4608 = 460800
  4. mIndex(j) = j * 4096 for j >= 101

As a result, the mIndex(101) = 101 * 4096 = 413696 < mIndex(100).

I guess that's why the playback hangs in the middle. The file is poorly muxed.

Flags: needinfo?(cchang)
Priority: -- → P2

The frame blocksize is expected to be fixed value 4096

The file is poorly muxed.

FYI, it looks to me like the block size within a FLAC frame can be variable in size, and not just limited to 4096. Per the FLAC format specification sourced from: https://xiph.org/flac/format.html#frame_header

Block size in inter-channel samples:

0000 : reserved
0001 : 192 samples
0010-0101 : 576 * (2^(n-2)) samples, i.e. 576/1152/2304/4608
0110 : get 8 bit (blocksize-1) from end of header
0111 : get 16 bit (blocksize-1) from end of header
1000-1111 : 256 * (2^(n-8)) samples, i.e. 256/512/1024/2048/4096/8192/16384/32768

Just want to make sure that any software changes made will continue to adhere to the official FLAC specifications.

(In reply to Remi from comment #6)

Just want to make sure that any software changes made will continue to adhere to the official FLAC specifications.

Currently, the flac demuxer works fine when all of the blocksize is fixed(and it's not limited to 4096), or all of the blocksize is variable.

The problem in that file is, all the blocksize is fixed(with value 4096) except one block whose size is variable(with value 4608). That is, the blocksizes are 4096, 4096, 4096, ..., 4096, 4608, 4096, ..., 4096.

Per https://xiph.org/flac/format.html#format_overview:

The audio data is composed of one or more audio frames. Each frame consists of a frame header, which contains (...) information about the frame like the block size, (...)

The statement above demonstrates that the block size can change between each frame within a FLAC file. Thus, the blocksize sequence as previously described by you is also legal per the FLAC specification:

4096, 4096, 4096, ..., 4096, 4608, 4096, ..., 4096.

(In reply to Remi from comment #8)

IIRC, the sample number reported by the header in that file is incorrect, so we cannot rely on that value.

(In reply to Remi from comment #6)

The frame blocksize is expected to be fixed value 4096

The file is poorly muxed.

FYI, it looks to me like the block size within a FLAC frame can be variable in size, and not just limited to 4096. Per the FLAC format specification sourced from: https://xiph.org/flac/format.html#frame_header

Block size in inter-channel samples:

0000 : reserved
0001 : 192 samples
0010-0101 : 576 * (2^(n-2)) samples, i.e. 576/1152/2304/4608
0110 : get 8 bit (blocksize-1) from end of header
0111 : get 16 bit (blocksize-1) from end of header
1000-1111 : 256 * (2^(n-8)) samples, i.e. 256/512/1024/2048/4096/8192/16384/32768

Just want to make sure that any software changes made will continue to adhere to the official FLAC specifications.

It can be variable, or it can be fixed. If variable the header contains the frame number, if fixed it's the block number.

You can't have a combination of both, that will break seeking.

Otherwise the alternative would be to parse every single packet linearly when seeking, so if you seek to the end, it would take forever. Right now, if fixed block we know exactly where to seek instantly, if variable we can do a bisecting search.
If we can't assume that packet type are consistent, then we can't assume that the packet index is correct.
This is needed to determine what the time is. This is what's happening here, we assume we seek to a point, then we see a sample actually saying "actually no, I'm ahead in the future". So we play silence until that time is actually reached

(In reply to Jean-Yves Avenard [:jya] from comment #10)

So we play silence until that time is actually reached

Since reporting, I continued using Firefox to play flac files in my library and note down whenever I hit a file that hangs during playback. The problem is fairly common and affects an estimate of around ~5% of files in my library from various sources, encoded by various flac encoder versions. All of them test ok with flac -t and play back without issues in any other media player.

From my point of view, all of this points to this assumption / optimization being the cause for what I would consider broken playback. As far as I know, Flac is a format that is encoded with a variable bitrate. I know that there were a lot of issues with VBR encoded mp3s and seeking behavior historically. Could this be the culprit here?

Unfortunately, I cannot contribute more to the technical discussion. If it would be helpful to have access to affected files, let me know. I'm glad to help out with sample data.

(In reply to Felix from comment #11)

From my point of view, all of this points to this assumption / optimization being the cause for what I would consider broken playback. As far as I know, Flac is a format that is encoded with a variable bitrate. I know that there were a lot of issues with VBR encoded mp3s and seeking behavior historically. Could this be the culprit here?

Unfortunately, I cannot contribute more to the technical discussion. If it would be helpful to have access to affected files, let me know. I'm glad to help out with sample data.

variable bit rate is different to variable packet size. That flac properly tell you how many frames there are per packet or right up front that it will vary makes for much easier seek. mp3 doesn't have that at all, at best you have an "average" bitrate and when you seek you simply jump there. You can't accurately seek.

Which flac encoder did you use that generated such file?

Flac versions 1.30-1.32 downloaded from https://xiph.org/flac/download.html. I also noticed the issue on some files purchased from Bandcamp too and cannot comment on the flac encoder that were used there.

The official flac encoder generates such file? i find that hard to believe

(In reply to Jean-Yves Avenard [:jya] from comment #14)

The official flac encoder generates such file? i find that hard to believe

I find it hard to believe that a broken flac would pass flac -t and do not understand your argument. I am also not aware of any other encoder than the official flac binary

Attached file ffmpeg_remux_test.txt

(In reply to Jean-Yves Avenard [:jya] from comment #14)

Apologies for the snappy response earlier, it was not warranted. I tried remuxing the files marked as affected with ffmpeg and a high log level. I also tried remuxing a known good file. All files play without issues after remuxing with ffmpeg which is interesting.

Analyzing the log:

sample/frame number mismatch in adjacent frames

Occurs in all files and can probably be ignored: See https://trac.ffmpeg.org/ticket/5937

[NULL @ 0x55c84dea5240] blocking strategy change detected in adjacent frames
[NULL @ 0x55c84dea5240] sample/frame number mismatch in adjacent frames
[NULL @ 0x55c84dea5240] crc check failed from offset 83405 (frame 20) to 108972 (frame 1371)

These only occur in the affected files.

What stood out to me was this line present for both affected files:

[NULL @ 0x55c84dea5240] dropping low score -31 frame header from offset 1135 to 13467
[NULL @ 0x55d7e96c7240] dropping low score -45 frame header from offset 3682 to 10085

From: https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/flac_parser.c#L554

Hi Nils, is this still a P2? (or when do you think it will get fixed?) Thanks!

Flags: needinfo?(drno)

I'm also experiencing the same issue on several files of my music library.
Here's one of my affected files, in case it helps: http://srv.isomorphis.me/sample.flac . The issue occurs at 19 seconds.

Remuxing using ffmpeg seems to fix the file in that case as well.

Whiteboard: [sci-exclude]

@armael have you ever encountered any crashes related to this issue? I did, but they don't show up in about:crashes, and I can't seem to reproduce them in an empty profile.

I am also experiencing this issue with a couple songs. Both are flac and stop at the same spot every time. If you skip past the spot they will continue to play. This issue is not in other browsers

I also have this issue. Pretty much every single one of my FLAC files (all of them muxed using ffmpeg) will stop playing or pause at the same spot every time.

Transcoding the flac to mp3 with ffmpeg does not reveal any issues.

@armael Your file indeed triggered this bug at 19s, and the file continued playing ~5-10s later.

Hi,
just want to cofirm this bug. I use the Qobuz streaming service ( a European thing...) and here with every other flac file I stream using their player running with firefox, the file hangs and will resume playing only after (more or less) minutes. The point in time the playback, i.e. decoding the stream, stops is absolutely reproduceable. Since Qobuz is a high quality service and the bug appears in so many differnt files, it is obvious to me, that there is a problem with the decoder invoked by firefox. E.g. with Chrome this problem does not appear at all.
regards

I can confirm what Felix is reporting. This consistently happens on about 10% of my VBR FLAC files. The location that it occurs in are consistent and reproducible always (I have many of them memorized now sadly).

One of the files is a recording for which I personally own the copyright (as the performer in the track) and so I can share that here if that's of any use in debugging the cause of this though it sounds from the conversations above that :chunmin that the cause is variable block size

The official flac encoder generates such file? i find that hard to believe

:jya I can confirm that the official FLAC encoder generates these files as well. Is there any data I can provide to validate this?

I can also confirm the Chromium does not exhibit this problem. I've not taken the time to dig into the Chromium code that renders FLAC audio to find how they deal with variable length blocks.

As a Mozilla employee I can also provide any Mozilla folks with deeper access to debugging this on Firefox stable (share files, we can jump on zoom video call and I can screenshare anything, etc.)

This bug is a bummer as it means I have to either deal with these hangs or run Chrome as well to listen to music.

Flags: needinfo?(jyavenard)

In bug 1565786 I mentioned that the official encoder produces these files at compression level 8 or --best.

Could you be resubmit some files I can experiment with? and where you have to seek to reproduce the issue.

Flags: needinfo?(jyavenard)
Flags: needinfo?(hi)
Flags: needinfo?(drno)

I'm particularly interested with this one: https://openload.co/f/Aym2LY_rj-4/sampleA.flac as it is caused by a regression with bug 1487797

Flags: needinfo?(reminiscence927)
Assignee: nobody → jyavenard

re-uploading my "sampleA.flac"

https://we.tl/t-z4PzmWVcYX

Flags: needinfo?(reminiscence927)

Here's a file that consistently stops about 5 seconds in, in case it helps solve this problem.

https://drive.google.com/open?id=1CUqC3UjnFrMkhLxLAsNVjyRe_mqKNPuM

Is there an update on a fix for this? It's driving me nuts and I'd rather not swap to another browsrr after years of Firefox use on WIndows.

I've recently set up a home network Plex serer on WIndows and have been ripping all my CDsand playing with the Plex web client in FIrefox on the same PC. I keep getting silent hangs. I had assumed it was Plex but it appears not.

  • Latest WIndows 10 with all updates
  • CD ripped with Exact Audio Copy
  • flac level 8

Can I do anyhting to help? I'm a dev (used to build Firefox and built a complex XUL extensions). Do you need more example files?

If more example files are needed, I can provide dozens.

I've created a shared folder on Google Drive, I'll add problematic songs to it as I find them. The titles include the exact time the files hang.
Of the 4 songs in there now, 1 is from Bandcamp, 2 are from Qobuz, 1 is from a CD I ripped. It's definitely not a source issue.

If I can help in any other way, please let me know.

https://drive.google.com/open?id=1CsOZ4joL-LHScwCmVG5yuaH9Nvi-4cHD

(In reply to atmur from comment #36)

https://drive.google.com/open?id=1CsOZ4joL-LHScwCmVG5yuaH9Nvi-4cHD

All eight of those tracks play correctly before Bug 1487797 (2018-10-29 build) so it is the same regression.

Running firefox with firefox --MOZ_LOG=MediaDemuxer:4 you can see the playback time jump forward when the hang happens (all of the problematic flac files from that Google Drive have this in common):

[Child 28301: MediaPDecoder #2]: D/MediaDemuxer FlacTrackDemuxer[0x7f88913ac5f0] GetNextFrame() Begin(time=23.312834 offset=2081922 size=12700)
[Child 28301: MediaPDecoder #2]: D/MediaDemuxer FlacTrackDemuxer[0x7f88913ac5f0] GetNextFrame() Begin(time=23.405714 offset=2094622 size=12615)
[Child 28301: MediaPDecoder #1]: D/MediaDemuxer FlacTrackDemuxer[0x7f88913ac5f0] GetNextFrame() Begin(time=23.498594 offset=2107237 size=12421)
[Child 28301: MediaPDecoder #1]: D/MediaDemuxer FlacTrackDemuxer[0x7f88913ac5f0] GetNextFrame() Begin(time=23.591473 offset=2119658 size=12439)
[Child 28301: MediaPDecoder #1]: D/MediaDemuxer FlacTrackDemuxer[0x7f88913ac5f0] GetNextFrame() Begin(time=23.684353 offset=2132097 size=12501)
[Child 28301: MediaPDecoder #1]: D/MediaDemuxer FlacTrackDemuxer[0x7f88913ac5f0] GetNextFrame() Begin(time=190.217868 offset=2144598 size=12478)

Eventually playback resumes (I don't get the crashes some other people have reported) but the time reported on Firefox's audio player is now out of sync (the progress bar progresses very quickly to the end whilst the audio continues playing long after this).

[Tracking Requested - why for this release]: affecting a few people and getting in the way of their workflow

It would cause the following frame to have an abnormally high timestamp causing audio to be apparently stalled.

for the record the issue happens if a flac file is made of fixed block interleaved with variable ones.

The file "Fix Stevenson - Cavalier = 1:41.flac" still doesn't play properly.

There's something wierd this file, the sample index goes like 965, 966, 107, 967

the duration of a frame being calculated by diffing the next block from the earlier one, we end up with negative times (so very big actually due to overflow)

Flags: needinfo?(hi)
Attachment #9133510 - Attachment is obsolete: true

It didn't take into account that there could be a mixture of variable block size and fixed block size and could use the start time of an invalid block to determine the end of the previous frame as the code relied on Frame::FindNext() to have always returning a valid frame; but this isn't always there case.

We also want to keep the scope of each classes separate, so that it is up to the FrameParser to correct a Frame should it be invalid or not set properly.

The produce the same outcome as originally done in bug 1487797. However, we only do so on the last block ever retrieved.

Ideally, we should calculate the start time based on the number of frames previously returned. It would avoid such inconsistency and while we had one file presenting such symptom, I'm not sure we can always rely for the frame_or_sample_num value to be valid in reference to the first frame block size.

Depends on D66989

This broke 18 months ago I don't think I need to track.

Pushed by jyavenard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c59c5bf65e10
P1. Revert changes introduced by bug 1487797. r=chunmin
https://hg.mozilla.org/integration/autoland/rev/275d9e52d155
P2. Adjust time of last block if needed. r=chunmin
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla76

Where can we get a build to help test this? I don't fancy doing a full Firefox build myself :)

(In reply to Steve Lee from comment #49)

Where can we get a build to help test this? I don't fancy doing a full Firefox build myself :)

https://nightly.mozilla.org in a couple of hours or so.

(In reply to Julien Cristau [:jcristau] from comment #50)

https://nightly.mozilla.org in a couple of hours or so.

\0/

Comment on attachment 9133581 [details]
Bug 1528265 - P1. Revert changes introduced by bug 1487797. r?chunmin

Beta/Release Uplift Approval Request

  • User impact if declined: some audio files can't play without stalling.
    this is a long running bug but only because no one bothered to look into it before, it's been affecting a lot of people
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: Open https://drive.google.com/drive/folders/1CsOZ4joL-LHScwCmVG5yuaH9Nvi-4cHD each file has in its title a time. Seek to about 5s prior that time and let it play.
    It will stall once you reach that time if the fix isn't applied
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): There was a fix introduced in bug 1487797 to get around a very peculiar file, we pretty much removed that fix and back to the old code.
  • String changes made/needed: none
Attachment #9133581 - Flags: approval-mozilla-beta?
Attachment #9133582 - Flags: approval-mozilla-beta?
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

Reproduced the initial issue in Release version 74 using Windows 10.
Verified - Fixed in latest Nightly build 76.0a1 (Build id: 20200318213346) using Windows 10 and Ubuntu 18.04.

Comment on attachment 9133581 [details]
Bug 1528265 - P1. Revert changes introduced by bug 1487797. r?chunmin

P2, we broke on 65, the fix landed only yesterday on nightly and we have no automated tests. I don't think there is any emergency to uplift it and potentially cause other regressions, let's have it ride the 76 train.

Attachment #9133581 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Attachment #9133582 - Flags: approval-mozilla-beta? → approval-mozilla-beta-

Based on comment 54, I will change/remove the flags accordingly.
Thanks.

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-triaged]
Flags: qe-verify+

I've been running lots of music through this and not had a problem yet! FTW

let's have it ride the 76 train.

When is the ETA for that?

If it's a way off, this is really dissapointing as this bug is old and would love a fix.

I know I can carry on using nightly but that is in an indeterminate state and so not safe for everyday use.

Steve Lee,
The fix for this will go out with Firefox 76.0 which will be released May 5th

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: