Bug 722667
| Summary: | FLAC to Ogg conversion corrupts audio data stream | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Michael Schwendt <bugs.michael> | ||||||
| Component: | gstreamer-plugins-good | Assignee: | Benjamin Otte <otte> | ||||||
| Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||||
| Severity: | unspecified | Docs Contact: | |||||||
| Priority: | unspecified | ||||||||
| Version: | 15 | CC: | bdpepple, cmontgom, otte | ||||||
| Target Milestone: | --- | ||||||||
| Target Release: | --- | ||||||||
| Hardware: | All | ||||||||
| OS: | Unspecified | ||||||||
| Whiteboard: | |||||||||
| Fixed In Version: | gstreamer-plugins-good-0.10.29-2.fc15 | Doc Type: | Bug Fix | ||||||
| Doc Text: | Story Points: | --- | |||||||
| Clone Of: | Environment: | ||||||||
| Last Closed: | 2011-12-10 19:41:59 UTC | Type: | --- | ||||||
| Regression: | --- | Mount Type: | --- | ||||||
| Documentation: | --- | CRM: | |||||||
| Verified Versions: | Category: | --- | |||||||
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||
| Cloudforms Team: | --- | Target Upstream Version: | |||||||
| Embargoed: | |||||||||
| Attachments: |
|
||||||||
|
Description
Michael Schwendt
2011-07-16 10:43:15 UTC
Just for kicks, if one removes the tags from the .flac file, then converts it to .ogg, there are still errors reported by ogginfo. And at least one interruption can be heard at 0:56. (with the original example the first skip is around 0:30) Another much smaller test-case: $ wget http://mschwendt.fedorapeople.org/tmp/Complex-Intro.ogg $ gst-launch filesrc location="Complex-Intro.ogg" name=src ! decodebin name=decoder ! audioconvert ! flacenc mid-side-stereo=true quality=5 ! filesink location="Complex-Intro.flac" ... $ gst-launch filesrc location="Complex-Intro.flac" name=src ! decodebin name=decoder ! audioconvert ! vorbisenc quality=0.4 ! oggmux ! filesink location="testout.ogg" ... $ ogginfo testout.ogg ... WARNING: sequence number gap in stream 1. Got page 12 when expecting page 11. Indicates missing data. WARNING: discontinuity in stream (1) WARNING: discontinuity in stream (1) ... $ oggdec testout.ogg oggdec from vorbis-tools 1.4.0 Decoding "testout.ogg" to "testout.wav" [ 5.5%]WARNING: hole in data (-3) [ 6.5%]WARNING: hole in data (-3) ... Then trying to convert that "broken" Ogg file back to FLAC: $ gst-launch filesrc location="testout.ogg" name=src ! decodebin name=decoder ! audioconvert ! flacenc mid-side-stereo=true quality=5 ! filesink location="testout.flac" ... WARNING: from element /GstPipeline:pipeline0/GstFlacEnc:flacenc0: The stream is in the wrong format. Additional debug info: gstflacenc.c(1185): gst_flac_enc_check_discont (): /GstPipeline:pipeline0/GstFlacEnc:flacenc0: Stream discontinuity detected (wanted 0:00:00.422312925 got 0:00:00.631247165). The output will have wrong timestamps, consider using audiorate to handle discontinuities ... The output will have wrong timestamps, consider using audiorate to handle discontinuities (gst-launch-0.10:6528): GStreamer-CRITICAL **: gst_segment_set_newsegment_full: assertion `segment->format == format' failed and many more such "stream discontinuity" errors. Somebody with Ubuntu 11.04 i386 has reported it to both SoundConverter and xiph.org, and obviously (since oggenc doesn't suffer from the issue) they closed their ticket as invalid: https://trac.xiph.org/ticket/1807 | Indeed, this Ogg file is corrupt. The program that produced the stream | muxed it improperly. It is skipping pages, which is an error condition. | I suspect the odd playback behavior you're getting is simply due to | different programs handling |the demuxing errors differently (there is | some missing audio data as well, checking the stream by hand here). | | So in short, the program you are using to produce Ogg Vorbis files from | FLAC is buggy and producing corrupt files. There's not much more to it | than that. [...] application appears to have a bug. A corrupt stream will | generally cause errors in playback. | | Converting here using oggenc (our own encoding application) also using the | same/latest encoding libraries produces a correct Ogg Vorbis encode from | the FLAC. I understand this bug and almost have a patch. It is a combination of a bug in gstflacdec and a bad decision in gstvorbisenc. gstflacdec is converting input buffer timestamp to sample number, using that internally, then converting sample number back to timestamp for the output buffer. Unfortunately, it's using gst_util_uint64_scale_int to convert timestamp to sample number (which always rounds by truncating) and since sample numbers aren't usually expressible in an integer number of nanoseconds, the output buffer timestamp is off by a full sample about half the time. gstvorbisenc, on the other side, is hardwired to tolerate only a half-sample of jitter in the input buffer; if it sees more, it treats the input as a discontinuous stream and resets the encode process. If time 'went backwards' due to jittering back a sample, it truncates the buffer as well. The combination of these two behaviors is obviously catastrophic to FLAC->Vorbis transcodes, and worse, the corruption happens without any warning whatsoever. I propose the proper correct fix: flacdec: round the samplenumber conversion so that 898559.999996400 is sample 898560, not 898559. vorbisenc: at a minimum, tolerate a few samples of jitter. I'm not even sure that the way it's implementing discontinuous streams even makes sense, but it definitely needs to stop mistriggering on continuous streams. Created attachment 514574 [details]
proposed patch part 1
This is the patch to the vorbis encoder element to relax jitter action threshold from .5 samples to 3 samples.
Created attachment 514575 [details]
patch for the flacdec jitter bug
this patch corrects the bug in flacdec that was causing a full sample of timestamp jitter
patches are tested here; either one fixes the specific reported bug. The flacdec patch is for a real bug obviously, and I think the vorbisenc patch should be put in place to back away a bit from a potentially bad design decision. I'm not sure what the vorbisenc code thinks it's doing with respect to discontinuous streams, but I am sure it should not be mistriggering on inputs with very minor timestamp jitter. Company, what is the next step? Eg, am I supposed to set review flags or the like... Not want to interfere with a special work-flow you may have, but the next step would be for "company" to apply the patches in Rawhide git, submit builds, and decide on what released products (e.g. Fedora 15) need official updates, too. Company is Benjamin Otte. I was referring to internal workflow, not what to do with the patches. Okay.
> Company is Benjamin Otte
I know :)
gstreamer-plugins-good-0.10.30-5.fc16 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/gstreamer-plugins-good-0.10.30-5.fc16 gstreamer-plugins-good-0.10.29-2.fc15 has been submitted as an update for Fedora 15. https://admin.fedoraproject.org/updates/gstreamer-plugins-good-0.10.29-2.fc15 Package gstreamer-plugins-good-0.10.29-2.fc15: * should fix your issue, * was pushed to the Fedora 15 testing repository, * should be available at your local mirror within two days. Update it with: # su -c 'yum update --enablerepo=updates-testing gstreamer-plugins-good-0.10.29-2.fc15' as soon as you are able to. Please go to the following url: https://admin.fedoraproject.org/updates/FEDORA-2011-16543/gstreamer-plugins-good-0.10.29-2.fc15 then log in and leave karma (feedback). gstreamer-plugins-good-0.10.29-2.fc15 has been pushed to the Fedora 15 stable repository. If problems still persist, please make note of it in this bug report. gstreamer-plugins-good-0.10.30-5.fc16 has been pushed to the Fedora 16 stable repository. If problems still persist, please make note of it in this bug report. |