GNOME Bugzilla – Bug 340027
[patch] wavparse fails for several files
Last modified: 2006-07-24 13:42:04 UTC
wavparse fails when 'bps' is 0 in the header. if he files contains mp2/3 we still can get the bps from there.
Created attachment 64470 [details] [review] ugly hack to contunue this of course is ugly. any idea about how to handle this?
Created attachment 64740 [details] [review] add a new chunk type
the "ugly hack" patch is still needed :(
Created attachment 64741 [details] [review] add parsing for the fact-chunk
Created attachment 65506 [details] [review] better fact handling, fix crashers, reflow some code some error cases are handled better, biggest problem is that late src-pad addition
Created attachment 65508 [details] [review] better fact handling, fix crashers, reflow some code now also fix odd stream size problems (resulting in data flow errors)
Testfiles: http://www.andrew.cmu.edu/user/lpinelis/Multimedia%20Authoring%20II/hw3/WickedLaugh.wav http://wavthis.com/HWeen/psycho.zip http://wavthis.com/HWeen/WickedLaugh.zip http://wavthis.com/HWeen/witch.zip
C++ comments. No docs on what wav->fact is; this is totally obscure! Why not add fact parsing to gst-libs/gst/riff/? That would make more sense to me. I can't see where GST_RIFF_TAG_fact is defined at all? Rather than the hack to set ->bps = 1, why not just allow bps == 0, and not use it in any of the places where it's currently used? I can't imagine this is answering queries sensibly at the moment. The pad_unlinked stuff doesn't look right? You should just return appropriate flow returns when you try to push things on that pad if neccesary. This seems to confuse width with block alignment, which is admittedly a confusing bit of handling wav.
Created attachment 65649 [details] [review] add a new chunk type correct placement of new chunk type
Created attachment 65650 [details] [review] better fact handling, fix crashers, bps calculation, reflow some code c++ comments transformed fact-chunk defines the number of sample when format!=pcm. i've added a comment to the source. imho there is no need to add something of riff-lib, as the chnk is just one dword. I reworked the bps calculation. the bps=1 hack is gone. bps are calculated from available data (incl fact). If it can't e done, it erorrs out, as then we can't properly seek or timestamp buffers. removed the pad_unlinked stuff again. this was working around other bugs.
can you make a new patch against CVS version for the gstwavparse bits? You're free to add the fact tag.
Created attachment 69171 [details] [review] updated: better fact handling, fix crashers, bps calculation, reflow some code
fact has been committed, is the other patch okay as well?
2006-07-24 Stefan Kost,,, <set EMAIL_ADDRESS environment variable> * gst/wavparse/gstwavparse.c: (gst_wavparse_reset), (gst_wavparse_other), (gst_wavparse_perform_seek), (gst_wavparse_get_upstream_size), (gst_wavparse_stream_headers), (gst_wavparse_add_src_pad), (gst_wavparse_stream_data), (gst_wavparse_pad_query): * gst/wavparse/gstwavparse.h: Use information from 'fact' chunk for length calculation of compressed samples. Calculate bps if bogus value is found in wav header (embeded mp2/mp3).