After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 340027 - [patch] wavparse fails for several files
[patch] wavparse fails for several files
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other All
: Normal normal
: 0.10.4
Assigned To: Stefan Sauer (gstreamer, gtkdoc dev)
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2006-04-28 14:46 UTC by Stefan Sauer (gstreamer, gtkdoc dev)
Modified: 2006-07-24 13:42 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
ugly hack to contunue (450 bytes, patch)
2006-04-28 14:50 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
none Details | Review
add a new chunk type (560 bytes, patch)
2006-05-03 12:45 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
none Details | Review
add parsing for the fact-chunk (2.97 KB, patch)
2006-05-03 12:56 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
none Details | Review
better fact handling, fix crashers, reflow some code (9.51 KB, patch)
2006-05-15 14:37 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
none Details | Review
better fact handling, fix crashers, reflow some code (10.47 KB, patch)
2006-05-15 15:48 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
none Details | Review
add a new chunk type (703 bytes, patch)
2006-05-17 08:33 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
committed Details | Review
better fact handling, fix crashers, bps calculation, reflow some code (10.85 KB, patch)
2006-05-17 08:37 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
none Details | Review
updated: better fact handling, fix crashers, bps calculation, reflow some code (11.73 KB, patch)
2006-07-19 12:42 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
committed Details | Review

Description Stefan Sauer (gstreamer, gtkdoc dev) 2006-04-28 14:46:12 UTC
wavparse fails when 'bps' is 0 in the header. if he files contains mp2/3 we still can get the bps from there.
Comment 1 Stefan Sauer (gstreamer, gtkdoc dev) 2006-04-28 14:50:43 UTC
Created attachment 64470 [details] [review]
ugly hack to contunue

this of course is ugly. any idea about how to handle this?
Comment 2 Stefan Sauer (gstreamer, gtkdoc dev) 2006-05-03 12:45:24 UTC
Created attachment 64740 [details] [review]
add a new chunk type
Comment 3 Stefan Sauer (gstreamer, gtkdoc dev) 2006-05-03 12:53:35 UTC
the "ugly hack" patch is still needed :(
Comment 4 Stefan Sauer (gstreamer, gtkdoc dev) 2006-05-03 12:56:19 UTC
Created attachment 64741 [details] [review]
add parsing for the fact-chunk
Comment 5 Stefan Sauer (gstreamer, gtkdoc dev) 2006-05-15 14:37:04 UTC
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
Comment 6 Stefan Sauer (gstreamer, gtkdoc dev) 2006-05-15 15:48:58 UTC
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)
Comment 8 Michael Smith 2006-05-16 18:57:26 UTC
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.

Comment 9 Stefan Sauer (gstreamer, gtkdoc dev) 2006-05-17 08:33:10 UTC
Created attachment 65649 [details] [review]
add a new chunk type

correct placement of new chunk type
Comment 10 Stefan Sauer (gstreamer, gtkdoc dev) 2006-05-17 08:37:43 UTC
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.
Comment 11 Wim Taymans 2006-07-18 09:57:59 UTC
can you make a new patch against CVS version for the gstwavparse bits? 
You're free to add the fact tag. 
Comment 12 Stefan Sauer (gstreamer, gtkdoc dev) 2006-07-19 12:42:03 UTC
Created attachment 69171 [details] [review]
updated: better fact handling, fix crashers, bps calculation, reflow some code
Comment 13 Stefan Sauer (gstreamer, gtkdoc dev) 2006-07-21 21:54:12 UTC
fact has been committed, is the other patch okay as well?
Comment 14 Stefan Sauer (gstreamer, gtkdoc dev) 2006-07-24 13:42:04 UTC
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).