Bugzilla – Bug 7857
Player crashes when playing an Ogg file with large headers
Last modified: 2009-06-17 09:35:07 UTC
When playing certain ogg files, there's no audio output and the file keeps restarting after +/- 30 seconds of playing. After that there's no output until I powercycle the reciever, then everything is working OK until I try playing a known-to-freeze file. An example file can be downloaded from: http://wdz5.xs4all.nl/~hendrik/mmw-deadzy.ogg I've only encountered this with certain ogg files so far.
Created attachment 3261 [details] file to reproduce Attaching file from URL above.
Created attachment 3263 [details] Results of trying to play this file Yikes, this picture is what happened to my SB2 trying to play this file. I wonder if it is related to the large amount of tag data, including an invalid base64 cover image.
Not the cause of the firmware issues, but it looks like our Ogg parser doesn't read the tags properly in here, it truncates the image and doesn't read the rest of the tags after it.
I created two bugreports about the header parsing. Header truncation bug #7895 Header in multiple oggpages bug #7896
Also retry this after bug 4418 is fixed.
I am not at all up to speed on ogg... I know Richard has a lot on his plate but he might be a better person to look at this. Does anyone have exact specs on how these headers differ from the ones that we handle OK?
I looked at this a bit, the problem is that the Vorbis code allocates too much data memory while reading the large comment header pages. If we could figure out a way to skip reading the comment header it should work OK.
This is not going to make it into 7.1, unfortunately, despite it being a real and critical bug.
Andy to follow up with the OGG 'guy'
Some interest from Monty, he's looking for a proper way to fix this in the API, not a dirty hack like we need.
From bug 4418 discussion (since the two are unrelated) Are you guys using Tremor or libvorbis? I know Monty knows about the issue, but I can't find anything in Xiph's Trac. I'd like to see a record on their end. I agree that a 'dirty hack' is not the correct approach. That doesn't mean the slimdevices/Logitech has to sit around and lose customers while the bug is "properly" squashed upstream. I'd love to know a little more about how/where the bug is causing the receiver to fail. IE: has it been narrowed down to a specific API call? Vorbis is a big open source effort, and there should certainly be a bug report open upstream with testers/developers working on it. You could _at least_ handle this in squeezecenter; it seems like you rescale/resample the cover art and send it apart from the actual ogg file. If that is the case, why not use SqueezeCenter (which is already reading these headers) to detect a coverart (or comment, whatever) header field that is going to crash the API on the receiver and stream an OGG file with the offending header truncated. It will allow all these irritated users to play their ogg files until Xiph developers can fix the API. Ogg:Vorbis::Header should be able to deal with this, even if it was necessary to creat an "Ogg-Patch" branch and post it on the SqueezeCenter wiki. (If you don't want to include Ogg::Vorbis::Header CPAN stuff in the main release)
Here's a link to the Tremor mailing list where there was a small amount of discussion about the problem: http://lists.xiph.org/pipermail/tremor/2008-September/thread.html http://lists.xiph.org/pipermail/tremor/2008-October/thread.html You sound like you know something about the Ogg decoder. Do you want to help with a patch? Both Felix and myself have tried to fix this problem and weren't able to get anywhere. That's why I asked for help on the mailing list. A patch to SC to get it to dynamically strip out the large comment is not the best solution (it won't help with files on remote sources like MP3tunes) but would be better than nothing. I am not sure it's as easy as you think it is though.
Created attachment 4302 [details] Calculates header length in directory tree
(In reply to comment #12) > Here's a link to the Tremor mailing list where there was a small amount of > discussion about the problem: > > http://lists.xiph.org/pipermail/tremor/2008-September/thread.html > http://lists.xiph.org/pipermail/tremor/2008-October/thread.html > > You sound like you know something about the Ogg decoder. Do you want to help > with a patch? Both Felix and myself have tried to fix this problem and weren't > able to get anywhere. That's why I asked for help on the mailing list. > > A patch to SC to get it to dynamically strip out the large comment is not the > best solution (it won't help with files on remote sources like MP3tunes) but > would be better than nothing. I am not sure it's as easy as you think it is > though. > Hi Andy, I'm going to do some work on tracing this bug a bit better. I created the attached small c program last night to dump out a list of the length of every comment header (including nulls) for every ogg file in my directory tree. I'm going to dump these values into a database and try to find a specific threshold at which the SBR crashes and needs to be reset. Once I have that info I'll proceed with trying to step through the decode routines in API and see if I can trace this memory problem. It's certainly not a sure shot, and if I don't come up with anything I'm simply going to write a separate application that will pull the coverart comment, base64 decode the jpeg, rescale/resample, and reencode it so that it falls below the error threshold. At least that will give me (and potentially other users) playable Ogg files with embedded coverart for the time being. I looked at the squeezecenter stuff and it does seem possible to flag offending Ogg files, but logically the quickest way I could think of to solve the problem was to pipe the binary ogg file through another small c program that strips out the coverart or resamples it and then pipes the ogg back into SC. However if that's whats necessary I might as well go ahead and resample all of my Ogg files one time, since the coverart on working files is fully sufficient for my needs with the SBR, and picard could just retag everything if I was so inclined.
Thanks. But keep in mind that what we need is a patch to Tremor to completely skip over the comment header without allocating extra memory. If you just want to fix your files a simple call to vorbiscomment can strip out the artwork comment.
(In reply to comment #15) > Thanks. But keep in mind that what we need is a patch to Tremor to completely > skip over the comment header without allocating extra memory. > > If you just want to fix your files a simple call to vorbiscomment can strip out > the artwork comment. > Yes... I haven't looked specifically but I'm making this assumption, tell me if I am right: the SBR actually doesn't read anything from the Ogg comments, metadata and cover art are passed to the duet controller/etc independently. The receiver/transporter/etc just reads the Ogg headers for decode info and then proceeds on decoding the track and forwarding the PCM stream to the digital outs / DAC for analog out? It seems like the Tremor API _should_ be able to skip the comments without allocating memory. Obviously that would be much simpler than trying to find out where a buffer or something is being overrun or massively over allocated.
Tremor reads all comments and allocates memory for them. In our case the player does not care what those comments are and should skip over them. If you're looking at the Tremor source, note that we are using the lowmem branch.
(In reply to comment #17) > Tremor reads all comments and allocates memory for them. In our case the > player does not care what those comments are and should skip over them. > > If you're looking at the Tremor source, note that we are using the lowmem > branch. > Thanks for the heads up, I'll start looking at tremor now. As an ancillary note (other users could test): I've determined that what causes the SBR to crash is either an overall comment length greater than somewhere in the range of 46793 - 46937 characters (thresh=46900?) *or* a coverart comment that is between 45854 - 46210 characters (thresh=46000?).
There's no exact number where it will crash, it depends on the overall memory usage. For example if you use Ethernet instead of wireless the number will be larger.
(In reply to comment #19) > There's no exact number where it will crash, it depends on the overall memory > usage. For example if you use Ethernet instead of wireless the number will be > larger. > Andy, Can you verify for me that the failure happens at a call to ov_open() on the receiver? [implicitly, ov_open would be the logical API call unless you have a custom routine written. If it is, logically the receiver's memory would be overrun via the call path ov_open()->_ov_open1()->_fetch_headers()->vorbis_comment_init(). Either way, fundamentally I want to make sure that it is _fetch_headers() or something that eventually calls _fetch_headers() that is crashing the receiver] -----Nate
(In reply to comment #20) > If it is, logically the receiver's memory would be > overrun via the call path > ov_open()->_ov_open1()->_fetch_headers()->vorbis_comment_init(). Sorry for spamming this bug report. vorbis_comment_init() should be vorbis_dsp_headerin(). If you'd prefer I'd be happy to discuss this with you over email instead of the bug, unless you don't mind.
It's fine to use this bug for discussion. We actually use the ov_open_callbacks interface, not ov_open. But yes you are right, the problem is this: while(i<3){ ogg_stream_pagein(vf->os,og_ptr); while(i<3){ int result=ogg_stream_packetout(vf->os,&op); if(result==0)break; if(result==-1){ ret=OV_EBADHEADER; goto bail_header; } if((ret=vorbis_dsp_headerin(vi,vc,&op))){ goto bail_header; } i++; } if(i<3) if(_get_next_page(vf,og_ptr,CHUNKSIZE)<0){ ret=OV_EBADHEADER; goto bail_header; } } If the comment is large and spans multiple pages, all pages will be read in and lots of memory is allocated before vorbis_dsp_headerin is able to parse the full comment. The player crashes during this page memory allocation.
Andy, I believe I've come up with an effective way to skip the comment header (second header packet, spanning N page(s)) without buffering any of the body of the page(s) in Tremor (obviously buffering resumes on the last page if the third header packet shares it with the second). I've got a lot of debugging output, commenting, and extraneous code in place that I'll need to strip and I'm going to need do some more rigorous testing (esp run it through it's paces with valgrind). That said I do hope to have a solution implemented by this weekend though (classwork permitting) that I can run by you and Monty. If that is the case, do you suspected this fix will be implemented before 7.3.1? -----Nate
Great! If you can get me a patch I will try it out right away. Can't say yet if it would make it into 7.3 or 7.3.1.
(In reply to comment #24) > Great! If you can get me a patch I will try it out right away. Can't say yet > if it would make it into 7.3 or 7.3.1. > I'll work on refinement and cleaning it up, hopefully you'll have a patch to test Saturday. Right now I'm trying to minimize the amount of memory allocated to buffer the data to ensure we're actually hitting OggS at the beginning of a new page. I'd like to keep memory usage under 1kb for skipping the comment header, if possible. That said, do you using _anything_ from the comment header in the decoder of the audio players (vendor_string, for example)?
No, the player decoder does not use anything from the comment header.
i only recently discovered how to *easily* add albumart to .ogg files (using jaikoz audio tagger). seemed to work fine until i tried to FF/REW... as soon as i let go of the FF/REW button, my SB3 says "problem: cant open file for..." and stops playing. this only seems to happen on .ogg files with albumart embedded, on .oggs without albumart FF/REW seems to work fine. all albumart i have used was so far significantly smaller than 64KByte, ususally between 20-40 KByte. my system setup: SqueezeCenter Version: 7.2 - 22900 @ Tue Aug 26 10:59:02 PDT 2008 - Linux - EN - utf8 Server IP address: 192.168.0.102 Perl Version: 5.8.8 armv5tejl-linux-thread-multi MySQL Version: 5.0.27 Platform Architecture: armv5tejl-linux Hostname: Turbonas Server Port Number: 9001 Total Players Recognized: 2 Player InformationName: Bedroom Model: Squeezebox v3 Firmware: 112 The IP address for this player is: 192.168.0.5:47747 The Ethernet MAC address for this player is: 00:04:20:07:4e:dc Wireless Signal Strength: 49 i am running SSOTS 3.15 and turbonas FW 2.1.0 Build 0904T. could this be related to this bug?
Could be. Nicholas Vinen and I are working on two separate implementations to selectively skip packets in the ogg stream (effectively to skip the oversized comment headers that seem to be causing the problem). Nicholas' is done and in a patch form, I think it hits the Tremor list soon. Mine is still very much a work in progress because I'm *trying* to be meticulous about covering every scenario. Unfortunately I've found there are alot of different angles, developing testing oggs is tedious work, and the API is very sparsely documented. Granted it follows fairly logically once you grind into it... I sure hope we can squash this bug within the next week or so. If so, we'll be able to see if your issue is related.
*** Bug 10128 has been marked as a duplicate of this bug. ***
Btw this not, I repeat, not an ogg file but a FLAC file.
(In reply to comment #30) > Btw this not, I repeat, not an ogg file but a FLAC file. > If FLAC files do the same thing, I imagine the decoder has a similar problem. Being an Xiph.org product as well, it wouldn't surprise me. None the less, it's probably reason enough for a separate bug report. (Unless that is what you intended to comment in, because your comment seems somewhat out of place here) BTW Andy, I've been utterly swamped with schoolwork and haven't gotten much done on Tremor. I have six papers due Friday, so I have no idea when I am going to be able to get to it.
Sorry Nate, My comment is abit out of place as it was refering to #10206 (or #10207) which I opened for a similar case with a FLAC file which I did not want to induce in this Ogg Vorbis case. Anyway with F/W 120 a message is displayed suggesting that the file cannot be played at all. Which probably is preferable over a "stuck" SB3.
Changing target to next release
It hasn't been mentioned yet so I said I may as well say that a temporary way around this is to remove native ogg encoding in the squeezecentre settings and have it transcode to FLAC and stream to the receiver that way. Thanks to funkstar for pointing that out to me in this thread... http://forums.slimdevices.com/showthread.php?p=384917 I know it doesn't help get the problem resolved, but it might be of use to others like me who currently have this problem and stumble across this bug report. Folks probably won't need it, but I have files that are guaranteed to reproduce this problem, just say the word if anyone wants them and I'll send them on. In my case it's likely to be the artwork that's causing the receiver to fail.
Andy, I have been reading through your correspondence with Nicholas and Ethan on the Tremor list. I think the approach you are currently pursuing is probably the most logical and simple to implement. I don't think what I had worked on would be particularly useful to you if you do want to read the comment packet out (and bypass comments who's length values exceed a specific threshold). Instead I was keeping the dynamic allocation routines in place but simply bypassing them and discarding the comment header. I was also working on implementing the ability to skip over excessively large packets later in the stream (from a preset threshold) but again, not particularly useful given your current goals. I'll keep my nose in the Tremor list but it looks like you are on the right track.
Fixed, firmware coming soon. Player should at least fail gracefully with large headers.
Good news, fixed a bug in the truncation code and it now appears files with any size comments will play just fine!
(In reply to comment #37) > Good news, fixed a bug in the truncation code and it now appears files with any > size comments will play just fine! > Awesome. I am thrilled to hear you were able to hash it out!
New firmware is now available, please test!
Verified fixed in 7.3.3 r25732 Boom r45 SB2/3 r125 TP r75
This bug has been fixed in the 7.3.3 release version of SqueezeCenter! If you haven't already. please download the new version from http://www.logitechsqueezebox.com/support/download-squeezecenter.html If you are still experiencing this problem, feel free to reopen the bug with your new comments and we'll have another look.