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 348973 - New plugins: MVE muxer/demuxer
New plugins: MVE muxer/demuxer
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other All
: Normal enhancement
: 0.10.5
Assigned To: Tim-Philipp Müller
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2006-07-27 19:05 UTC by Jens Granseuer
Modified: 2007-01-11 16:31 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
MVE typefinding for -base (1.13 KB, patch)
2006-07-30 19:56 UTC, Jens Granseuer
committed Details | Review
add MVE plugins to -bad (229.52 KB, patch)
2006-07-30 19:57 UTC, Jens Granseuer
committed Details | Review

Description Jens Granseuer 2006-07-27 19:05:14 UTC
I'd like to submit a muxer and demuxer for Interplay's MVE format (used in various games, e.g. Baldur's Gate, Freespace, Descent, Fallout) for review.

The plugins are available here:

http://www.lanipage.de/gst-mvedemux-0.10.0.1.tar.bz2

If they are deemed good enough for inclusion I'll create proper patches against -bad. A sampling of MVE movie files can be found at ftp://www.mplayerhq.hu/MPlayer/samples/game-formats/interplay-mve/
Comment 1 Tim-Philipp Müller 2006-07-28 09:04:48 UTC
> I'd like to submit a muxer and demuxer for Interplay's MVE format (...)
> for review. http://www.lanipage.de/gst-mvedemux-0.10.0.1.tar.bz2
> 
> If they are deemed good enough for inclusion I'll create proper
> patches against -bad.

Code looks great, license too, I don't see any reason not to add this to -bad. Please create proper patches against -bad so that it goes into gst/mve/ :)

It would be great if you could also create a patch against gst-plugins-base/gst/typefind/gsttypefindfunctions.c and move the typefinding code there.

> ftp://www.mplayerhq.hu/MPlayer/samples/game-formats/interplay-mve/

Can't get to the sample files, it asks for a username/password ...

PS: Muxer and demuxer? I can only see a demuxer (not that that's a problem, just wondering)
Comment 2 Jens Granseuer 2006-07-28 09:15:38 UTC
> Can't get to the sample files, it asks for a username/password ...

Works fine here...

> PS: Muxer and demuxer? I can only see a demuxer (not that that's a problem,
> just wondering)

Urgs, sorry, I messed up. The link should have read:

http://www.lanipage.de/gst-mve-0.10.1.tar.bz2

That's change management for ya...
Comment 3 Tim-Philipp Müller 2006-07-28 09:40:20 UTC
Ah, thanks. A patch against -bad would still be good.


Out of curiosity:

 - why are the encoders/decoders built into the muxer/demuxer instead
   of being stand-alone elements? (not really a problem, just wondering;
   it's probably easier to implement things like QoS into stand-alone
   decoders for example).

 - the decoder/encoder code - is that more or less directly copied
   from ffmpeg? If so, why don't we use use the decoders/encoders
   from gst-ffmpeg instead? (ffdec_interplayvideo,
   ffdec_interplay_dcpm)
Comment 4 Jens Granseuer 2006-07-28 11:03:04 UTC
> A patch against -bad would still be good.

Will do.

> why are the encoders/decoders built into the muxer/demuxer instead
> of being stand-alone elements?

Mostly because separating them seemed an order more complex to me, and I couldn't see much use for stand-alone encoders/decoders for this format.

> the decoder/encoder code - is that more or less directly copied
> from ffmpeg? If so, why don't we use use the decoders/encoders
> from gst-ffmpeg instead?

The 8-bit decoder is more or less copied from ffmpeg, but
a) the ffmpeg decoder crashes for me (haven't investigated much, though)
b) the ffmpeg decoder does not support 16-bit movies, or chaining (basically
   several movies in one file)
c) ffmpeg does not have an encoder. That was written from scratch.
Comment 5 Jens Granseuer 2006-07-30 19:56:44 UTC
Created attachment 69919 [details] [review]
MVE typefinding for -base
Comment 6 Jens Granseuer 2006-07-30 19:57:27 UTC
Created attachment 69920 [details] [review]
add MVE plugins to -bad
Comment 7 Jens Granseuer 2007-01-06 14:50:47 UTC
Tim, any specific reasons why this is still on hold?
Comment 8 Tim-Philipp Müller 2007-01-07 19:54:22 UTC
Not really, just slipped off my radar. I'm not entirely happy with the decoders/encoders not being separate elements, but if there are no other formats that can embed this, then it's okay I guess. Also wanted to check if others had objections to adding actual encoding/decoding code to the repository, but that doesn't seem to be an issue. Will look at it again this week.
Comment 9 Tim-Philipp Müller 2007-01-11 11:41:33 UTC
Committed with some minor changes:
 - put all into one single mve plugin instead of having two plugins
 - the finalize function might be called twice, can't free stuff
   there unconditionally (mutex)
 - use GST_ELEMENT_ERROR() more often when returning a FLOW_ERROR
   instead of GST_ERROR_OBJECT (in demuxer at least).
 - initialised some variables to NULL/0 in the encoders/decoders in
   otder to avoid compiler warnings, better double-check these:
     $ grep -e 'best = 0' -e 'best = NULL' -e 'mean = 0' -bad/gst/mve/*c


  2007-01-11  Tim-Philipp Müller  <tim at centricular dot net>

	Patch by: Jens Granseuer  <jensgr at gmx net>

	* configure.ac:
	* gst/mve/Makefile.am:
	* gst/mve/TODO:
	* gst/mve/gstmve.c:
	* gst/mve/gstmvedemux.c:
	* gst/mve/gstmvedemux.h:
	* gst/mve/gstmvemux.c:
	* gst/mve/gstmvemux.h:
	* gst/mve/mve.h:
	* gst/mve/mveaudiodec.c:
	* gst/mve/mveaudioenc.c:
	* gst/mve/mvevideodec16.c:
	* gst/mve/mvevideodec8.c:
	* gst/mve/mvevideoenc16.c:
	* gst/mve/mvevideoenc8.c:
	  Add Interplay MVE format demuxer/decoder and muxer/encoder. Demuxer
	  doesn't support seeking yet, but seems to work fine otherwise.
	  Closes #348973.

Comment 10 Jens Granseuer 2007-01-11 16:31:29 UTC
(In reply to comment #9)
>  - initialised some variables to NULL/0 in the encoders/decoders in
>    otder to avoid compiler warnings, better double-check these:
>      $ grep -e 'best = 0' -e 'best = NULL' -e 'mean = 0' -bad/gst/mve/*c

The warnings were bogus anyway, so initializing to whatever doesn't make any difference. ;-)

Thanks.