GNOME Bugzilla – Bug 542054
[patch] dshowdecwrapper audio decoders + misc fixes
Last modified: 2008-11-18 18:24:09 UTC
I'm attaching two patches. The first one fixes random small bugs in dshowadec and adds codec entries for aac and ac3. It also fixes mp3 playback under vista using the MP3 Decoder DMO filter (the default). The second patch modifies libgstdshow to make it possible to specify a comma separated list of preferred filter names in the CodecEntry structure. It also fixes a crasher in CDshowFakeOutputPin::PushBuffer and changes slightly the way media types are negotiated in CDshowFakeSink::CheckMediaType so that the mp3 DMO filter works.
Created attachment 114185 [details] [review] dshowadec patch
Created attachment 114186 [details] [review] libgstdshow patch
The dshowadec patch seems to mostly be applied already. I think some of the debug output is improved, but that's all? Can you provide a patch against cvs? The other patch looks new, but a superficial glance makes it look pretty wrong - it's doing format-specific checks in generic routines, etc. This needs refactoring.
Created attachment 114196 [details] [review] libgstdshow patch
Created attachment 114197 [details] [review] dshowadec patch
Ok, the new patches are actually against cvs. In the dshowadec patch debug is improved and aac and ac3 codec entries are added. The dshowadec_mpeg1 codec entry has a change to make it work on xp and vista with the default mp3 filters. Plus there's a fix for EVENT_FLUSH. The libgstdshow patch does a format specific change, yes. But it's needed to make mp3 playback work on vista with the native decoder and doesn't make the current situation any worse. It also makes it possible to specify multiple preferred filters, which are needed for mp3 to work on xp and vista at the same time. I agree that libgstdshow needs to be refactored, but can we do it after this lands? I have other patches for dshowvdec and with those gstreamer can play 99% of the media testsuite that we have on windows. Once I have this working, I can finally move on and we can think together how to fix or remove libgstdshow.
I committed the audiodec patch, along with some minor fixes (mostly to typos) I had lying around locally. I'm not reviewing patches to libgstdshow; I do not believe that hacking things into that is an appropriate approach, as I've said previously.
Reverted; apparently these weren't independent patches at all.
Even if we don't agree on how to fix libgstdshow _now_ and you don't want to review it, I still think that the patch is correct and reasonable, so I don't see why it should be marked as rejected.
Mike changed dshowdecwrapper significantly in CVS so the patch doesn't apply and the changes are probably not needed anymore.