MNG support for MPlayer
Hello MPlayer developers, MNG animations do not seem to be supported by MPlayer 1.0rc2. Because I wanted to play back MNG animations using MPlayer, I tried to implement a demuxer in libmpdemux using libmng 1.0.9 (on Debian etch). It worked out and I've been able to play the MNG animations using the new demuxer. Perhaps the code is useful for other people, so I'm sending the patchfile. Best regards, Stefan
On Mon, 15 Sep 2008 11:42:06 +0200, Stefan Schuermans wrote:
Hello MPlayer developers,
MNG animations do not seem to be supported by MPlayer 1.0rc2. Because I wanted to play back MNG animations using MPlayer, I tried to implement a demuxer in libmpdemux using libmng 1.0.9 (on Debian etch).
It worked out and I've been able to play the MNG animations using the new demuxer. Perhaps the code is useful for other people, so I'm sending the patchfile.
Best regards, Stefan
nice work. please wait for our demuxer maintainer. libmpdemux is being slowly obsoleted, but rxt said he wouldnt reject demuxers that passed review. nico reviewed your patch in july when you first sent it. http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/2008-July/058145.html could you address those small issues?
[MPlayer-1.0rc2-mng20080914.diff text/x-patch (19.0KB)] diff -r -N -U 3 MPlayer-1.0rc2/configure MPlayer-1.0rc2-mng20080914/configure --- MPlayer-1.0rc2/configure 2008-08-30 19:41:35.000000000 +0200 +++ MPlayer-1.0rc2-mng20080914/configure 2008-08-30
can you please diff this against current svn ?
+/* + * MNG file parser for MPlayer + * by Stefan Schuermans <stefan@blinkenarea.org> + */ +
missing standard mplayer license header (please copy and paste from any mplayer file). the amount of source comments is tremendous! i think your code could be used as a tutorial of how to add a demuxer into mplayer. similar to the ffmpeg demuxer howto in the multimedia wiki. -compn
compn wrote:
On Mon, 15 Sep 2008 11:42:06 +0200, Stefan Schuermans wrote:
MNG animations do not seem to be supported by MPlayer 1.0rc2. Because I wanted to play back MNG animations using MPlayer, I tried to implement a demuxer in libmpdemux using libmng 1.0.9 (on Debian etch).
It worked out and I've been able to play the MNG animations using the new demuxer. Perhaps the code is useful for other people, so I'm sending the patchfile.
nice work. please wait for our demuxer maintainer. libmpdemux is being slowly obsoleted, but rxt said he wouldnt reject demuxers that passed review.
nico reviewed your patch in july when you first sent it. http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/2008-July/058145.html could you address those small issues?
Oh, I wasn't aware the patch actually made it to the list. I recevied a notification in reply that my mail was on hold for approval and did not hear anything else. So today, I decided to join the list and try again. Sorry for posting it twice. I'll fix most of those issues. However, I'm not sure what to do aginst the comment to demux_mng_refresh() and the static framerate yet. The demux_mng_refresh() function may be called for parts of the display only by libmng. So it might happen that there are two invocatiosn of demug_mng_refresh() for the same frame, once for the upper half and once for the lower half. However, if only a part of the image changes between two frames, it is also possible that debug_mng_refresh() is only called for this part and the rest of the frame has to use the data from the last frame. MNG has a variable framerate with the time of a frame ranging from 1ms to multiple seconds. I don't know how to change the framerate MPlayer uses in the middle of playing. Both issues boil down to the same "problem": libmng wants to decide when to take an action regarding the display and MPlayer also wants to decide whan to take such an action (i.e. display the next frame) by calling demux_mng_fill_buffer( ). At least, I've understood it this way...
can you please diff this against current svn ?
Yes, I'll try to do that.
missing standard mplayer license header (please copy and paste from any mplayer file).
Okay. Regards, Stefan
On Mon, 2008-09-15 at 11:42 +0200, Stefan Schuermans wrote:
Hello MPlayer developers,
MNG animations do not seem to be supported by MPlayer 1.0rc2. Because I wanted to play back MNG animations using MPlayer, I tried to implement a demuxer in libmpdemux using libmng 1.0.9 (on Debian etch).
It worked out and I've been able to play the MNG animations using the new demuxer. Perhaps the code is useful for other people, so I'm sending the patchfile.
Why does this demuxer have a hardcoded framerate? There is the settimer callback which I assume libmng uses to give the real point at which frame content changes. Why are you not using the values from that to determine when to return a frame?
Uoti Urpala wrote:
On Mon, 2008-09-15 at 11:42 +0200, Stefan Schuermans wrote:
MNG animations do not seem to be supported by MPlayer 1.0rc2. Because I wanted to play back MNG animations using MPlayer, I tried to implement a demuxer in libmpdemux using libmng 1.0.9 (on Debian etch).
It worked out and I've been able to play the MNG animations using the new demuxer. Perhaps the code is useful for other people, so I'm sending the patchfile.
Why does this demuxer have a hardcoded framerate? There is the settimer callback which I assume libmng uses to give the real point at which frame content changes. Why are you not using the values from that to determine when to return a frame?
MNG has got a variable framerate (1ms up to multiple seconds per frame) and I thought the framerate in MPlayer is fixed once the demuxer has been opened. With other words: I did not find out how to change the interval after which MPlayer calls the fill_buffer callback. Can you point me at some documentation regarding this or at another demuxer that does that?
On Mon, 2008-09-15 at 15:44 +0200, Stefan Schuermans wrote:
Uoti Urpala wrote:
Why does this demuxer have a hardcoded framerate? There is the settimer callback which I assume libmng uses to give the real point at which frame content changes. Why are you not using the values from that to determine when to return a frame?
MNG has got a variable framerate (1ms up to multiple seconds per frame) and I thought the framerate in MPlayer is fixed once the demuxer has been opened. With other words: I did not find out how to change the interval after which MPlayer calls the fill_buffer callback. Can you point me at some documentation regarding this or at another demuxer that does that?
A demuxer stream does not need to have a frame rate. You only need to set the correct timestamp for the frames (the pts field of demuxer packets). The changes between timestamps can be arbitrary. Calls to the fill_buffer callback are not really timed (though they of course depend somewhat on how often new frames are required). MPlayer calls it at some point to get the next frame, and then later times the display of that frame according to the timestamps.
Uoti Urpala wrote:
On Mon, 2008-09-15 at 15:44 +0200, Stefan Schuermans wrote:
With other words: I did not find out how to change the interval after which MPlayer calls the fill_buffer callback. Can you point me at some documentation regarding this or at another demuxer that does that?
A demuxer stream does not need to have a frame rate. You only need to set the correct timestamp for the frames (the pts field of demuxer packets). The changes between timestamps can be arbitrary.
Calls to the fill_buffer callback are not really timed (though they of course depend somewhat on how often new frames are required). MPlayer calls it at some point to get the next frame, and then later times the display of that frame according to the timestamps.
Thank you for this tip. I've now found the "DEMUXER_CTRL_CORRECT_PTS" control call I've been missing so far and I think I was able to convert the MNG demuxer to dynamic framerate. But I've implemented a minimum framerate of 5 fps because output shows some odd behaviour if the framerate is much slower, e.g. when moving X11 output window. (Btw: This control call seems to be missing in the GIF demuxer?!?!) The other issues mentioned so far are also fixed now, except I'm still not using the MNG library refresh callback. I've added more comment text why I did it another way. The diff attached has been done against current svn version.
On Mon, 2008-09-15 at 20:48 +0200, Stefan Schuermans wrote:
I've now found the "DEMUXER_CTRL_CORRECT_PTS" control call I've been missing so far and I think I was able to convert the MNG demuxer to dynamic framerate.
I'm not sure exactly what you expect it to do, but it's not strictly related to variable frame duration. I think it shouldn't affect behavior much in this case as there is no decoder delay (unless without it some brokenness in the old version happens to trigger).
But I've implemented a minimum framerate of 5 fps because output shows some odd behaviour if the framerate is much slower, e.g. when moving X11 output window.
This is a limitation elsewhere in MPlayer and you should not add such workarounds to individual demuxers. If a partial workaround is desired I think a two-line change in mplayer.c to limit the amount of time slept at once would be enough.
(Btw: This control call seems to be missing in the GIF demuxer?!?!)
As described above correct-pts shouldn't matter much in this case.
The other issues mentioned so far are also fixed now, except I'm still not using the MNG library refresh callback. I've added more comment text why I did it another way.
The diff attached has been done against current svn version.
A couple of random comments, I haven't read all of the diff: + --enable-mng enable MNG input/output support [autodetect] Isn't this input only? + mng_priv->canvas = (unsigned char *)malloc(height * width * 3); There's no need to cast void * pointers (multiple cases in the patch). +// MNG libarary callback: (a part of) the canvas should be shown typo
Uoti Urpala wrote:
On Mon, 2008-09-15 at 20:48 +0200, Stefan Schuermans wrote:
I've now found the "DEMUXER_CTRL_CORRECT_PTS" control call I've been missing so far and I think I was able to convert the MNG demuxer to dynamic framerate.
I'm not sure exactly what you expect it to do, but it's not strictly related to variable frame duration. I think it shouldn't affect behavior much in this case as there is no decoder delay (unless without it some brokenness in the old version happens to trigger).
Okay, so I misunderstood this one. I've taken another look at the GIF demuxer. This one works fine without this control code. However, when I take this control code out of the MNG demuxer, the MNG file is always played with the fixed framerate specified when creating the new video stream header. I think the following code in the GIF demuxer is responsible for setting the frame times: dp->pts = ((float)priv->current_pts) / 100; dp->pos = stream_tell(demuxer->stream); ds_add_packet(demuxer->video, dp); The pts member speicifies the time of the frame in seconds (measured from beginning), right? I'm using the same code in the MNG demuxer now, but it does not work. Can somebody give me a tip regarding this or point me to the correct place in the documentation, please?
But I've implemented a minimum framerate of 5 fps because output shows some odd behaviour if the framerate is much slower, e.g. when moving X11 output window.
This is a limitation elsewhere in MPlayer and you should not add such workarounds to individual demuxers. If a partial workaround is desired I think a two-line change in mplayer.c to limit the amount of time slept at once would be enough.
Okay, I understand. I removed this workaround from the MNG demuxer. However, now it is a little bit funny to use, e.g. when an MNG frame is 20 seconds long and a seek is done. Nothing happens until the frame is over (e.g. for 15 seconds), then the seek is done. Is this intended behaviour, a known issue or another bug in my demuxer?
A couple of random comments, I haven't read all of the diff:
Thanks, I've fixed those and the issues found by Diego Biurrun in the attached version of my patch.
On Tue, Sep 16, 2008 at 09:36:42PM +0200, Stefan Schuermans wrote:
I'm using the same code in the MNG demuxer now, but it does not work. Can somebody give me a tip regarding this or point me to the correct place in the documentation, please?
There is a hack in libmpdemux/video.c, around line 562 for gif and matroska demuxers. Note that the gcc-specific __attribute__ stuff without ifdefs etc. is not acceptable, if in any way possible the code should compile with any compiler that implements C99 (preferably it should also work with C89, but that is not important). Does libmng indeed require that you set the refresh callback even if it is of no use for us (and the same for alloc and free, I would expect a somewhat sanely designed library to provide defaults for cases that probably 99% of users will want the same way)? Is the seeking feature actually useful for you? It seems like a lot of code for something that is likely to not work too well. Greetings, Reimar Döffinger
Reimar Döffinger wrote:
On Tue, Sep 16, 2008 at 09:36:42PM +0200, Stefan Schuermans wrote:
I'm using the same code in the MNG demuxer now, but it does not work. Can somebody give me a tip regarding this or point me to the correct place in the documentation, please?
There is a hack in libmpdemux/video.c, around line 562 for gif and matroska demuxers.
Thank you, that helped me a lot. I think it's a litle bit odd that one has to put the end time of the current frame in the pts member od the demuxer packet to make the frames last for the correct durations, but it works now. I'm not sure what's better now: Adding the MNG demuxer to video.c or re-adding the DEMUXER_CTRL_CORRECT_PTS conrol code to demux_mng.c. Both solutions seem to work. What's your preference?
Note that the gcc-specific __attribute__ stuff without ifdefs etc. is not acceptable, if in any way possible the code should compile with any compiler that implements C99 (preferably it should also work with C89, but that is not important).
I saw this somewhere in the source (e.g. in vidix/ivtv_vid.c), so I assumed this was okay to get rid of the warnings about unused parameters. I can simply remove the attribute stuff, or construct something with #ifdef. What's the MPlayer way of dealing with unused parameters?
Does libmng indeed require that you set the refresh callback even if it is of no use for us (and the same for alloc and free, [...])?
Yes, it's needed. The error MNG_NOCALLBACK is reported from the first function doing real work if it has not been set. The alloc and free can be switched to internal memory management by a (libmng) compile time define MNG_INTERNAL_MEMMNGMT. However, this does not seem to be set in standard libmng packages. (Even if this is set, mng_initialize() still takes the two parameters for alloc and free, but seems to ignore them. So nothing should break if the libmng package is compiled with this option set.)
Is the seeking feature actually useful for you? It seems like a lot of code for something that is likely to not work too well.
Seeking works (not reacting to input until next frame seems to be an issue elsewhere in MPlayer). Removing the seeking feature would reduce code size only by about 40 lines. I'd like to leave this in, because it is quite useful when working with long MNG animations and trying to only look at certain parts. Regards, Stefan
On Tue, 2008-09-16 at 23:55 +0200, Stefan Schuermans wrote:
I saw this somewhere in the source (e.g. in vidix/ivtv_vid.c), so I assumed this was okay to get rid of the warnings about unused parameters. I can simply remove the attribute stuff, or construct
Do you get warnings about those parameters without the attributes? If so, with what compiler/flags? GCC 4.3 does not warn about unused function parameters unless you use -Wunused-parameter or -Wextra, which are not normally set by MPlayer.
Uoti Urpala wrote:
On Tue, 2008-09-16 at 23:55 +0200, Stefan Schuermans wrote:
I saw this somewhere in the source (e.g. in vidix/ivtv_vid.c), so I assumed this was okay to get rid of the warnings about unused parameters. I can simply remove the attribute stuff, or construct
Do you get warnings about those parameters without the attributes? If so, with what compiler/flags? GCC 4.3 does not warn about unused function parameters unless you use -Wunused-parameter or -Wextra, which are not normally set by MPlayer.
I get those warnings due to the "-W" flag used after "configure --enable-debug". The compiler is "cc (GCC) 4.1.2 20061115 (prerelease) (Debian 4.1.1-21)" as compiler. cc -Wdisabled-optimization -Wno-pointer-sign -Wdeclaration-after-statement -std=gnu99 -W -Wall -O2 -march=k8 -mtune=k8 -pipe -g -D_REENTRANT -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE64_SOURCE -I. -I/usr/openwin/include -I/usr/include/kde/artsc -pthread -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/freetype2 -c -o libmpdemux/demux_mng.o libmpdemux/demux_mng.c libmpdemux/demux_mng.c: In function 'demux_mng_free': libmpdemux/demux_mng.c:77: warning: unused parameter 'size' libmpdemux/demux_mng.c: In function 'demux_mng_closestream': libmpdemux/demux_mng.c:95: warning: unused parameter 'h_mng' libmpdemux/demux_mng.c: In function 'demux_mng_refresh': libmpdemux/demux_mng.c:148: warning: unused parameter 'h_mng' libmpdemux/demux_mng.c:149: warning: unused parameter 'x' libmpdemux/demux_mng.c:150: warning: unused parameter 'y' libmpdemux/demux_mng.c:151: warning: unused parameter 'width' libmpdemux/demux_mng.c:152: warning: unused parameter 'height' libmpdemux/demux_mng.c: In function 'demux_mng_fill_buffer': libmpdemux/demux_mng.c:197: warning: unused parameter 'ds' libmpdemux/demux_mng.c: In function 'demux_mng_seek': libmpdemux/demux_mng.c:368: warning: unused parameter 'audio_delay'
On Wed, Sep 17, 2008 at 09:50:46AM +0200, Stefan Schuermans wrote:
Uoti Urpala wrote:
On Tue, 2008-09-16 at 23:55 +0200, Stefan Schuermans wrote:
I saw this somewhere in the source (e.g. in vidix/ivtv_vid.c), so I assumed this was okay to get rid of the warnings about unused parameters. I can simply remove the attribute stuff, or construct
Do you get warnings about those parameters without the attributes? If so, with what compiler/flags? GCC 4.3 does not warn about unused function parameters unless you use -Wunused-parameter or -Wextra, which are not normally set by MPlayer.
I get those warnings due to the "-W" flag used after "configure --enable-debug".
I guess that -Wnounused-parameter should be added then. Anybody against? Diego
On Wed, 2008-09-17 at 23:48 +0200, Diego Biurrun wrote:
On Wed, Sep 17, 2008 at 09:50:46AM +0200, Stefan Schuermans wrote:
Uoti Urpala wrote:
Do you get warnings about those parameters without the attributes? If so, with what compiler/flags? GCC 4.3 does not warn about unused function parameters unless you use -Wunused-parameter or -Wextra, which are not normally set by MPlayer.
I get those warnings due to the "-W" flag used after "configure --enable-debug".
Yes -W is the old name for -Wextra. I didn't remember the configure debug switch enabled extra warnings as I've been doing all debug compiles without using it.
I guess that -Wnounused-parameter should be added then. Anybody against?
I think it should be added if -Wextra is used (and I think the option spelling should be changed from -W to -Wextra at the same time). Is there much reason for the warning flags to be different for debug builds?
On Thu, Sep 18, 2008 at 01:00:14AM +0300, Uoti Urpala wrote:
On Wed, 2008-09-17 at 23:48 +0200, Diego Biurrun wrote:
On Wed, Sep 17, 2008 at 09:50:46AM +0200, Stefan Schuermans wrote:
Uoti Urpala wrote:
Do you get warnings about those parameters without the attributes? If so, with what compiler/flags? GCC 4.3 does not warn about unused function parameters unless you use -Wunused-parameter or -Wextra, which are not normally set by MPlayer.
I get those warnings due to the "-W" flag used after "configure --enable-debug". I guess that -Wnounused-parameter should be added then. Anybody against?
I think it should be added if -Wextra is used (and I think the option spelling should be changed from -W to -Wextra at the same time).
gcc 2.95 does not support -Wextra, just -W.
Is there much reason for the warning flags to be different for debug builds?
I have always wondered myself. The CFLAG handling is in for an overhaul. Maybe this will finally motivate me enough to do it... Diego
On Tue, 16 Sep 2008 21:36:42 +0200, Stefan Schuermans wrote:
Okay, I understand. I removed this workaround from the MNG demuxer. However, now it is a little bit funny to use, e.g. when an MNG frame is 20 seconds long and a seek is done. Nothing happens until the frame is over (e.g. for 15 seconds), then the seek is done. Is this intended behaviour, a known issue or another bug in my demuxer?
its a known issue with mplayer that it will not respond to input until the next frame is displayed. you can run into it while trying to play a big file on a slow network. -compn
On Tue, Sep 16, 2008 at 09:36:42PM +0200, Stefan Schuermans wrote:
Thanks, I've fixed those and the issues found by Diego Biurrun in the attached version of my patch.
--- trunk/configure 2008-09-16 16:41:32.000000000 +0200 +++ mng/configure 2008-09-16 16:40:32.000000000 +0200 @@ -4675,6 +4679,39 @@
+echocheck "MNG support" +if test "$_mng" = auto ; then + _mng=no + if irix ; then + # Don't check for -lmng on irix + _res_comment="disabled on irix"
Did you verify this or is it just copied and pasted (and thus guesswork) from the PNG test?
+cat > $TMPC << EOF +#include <libmng.h> +#include <string.h> +int main(void) { + const char * p_ver = mng_version_text() ?: ""; + printf("libmng: %s\n", p_ver); + return p_ver[0] == 0; +}
You do not seem to be using string.h.
--- trunk/libmpdemux/demux_mng.c 1970-01-01 01:00:00.000000000 +0100 +++ mng/libmpdemux/demux_mng.c 2008-09-16 21:23:49.000000000 +0200 @@ -0,0 +1,481 @@ + + mng_handle h_mng; // handle of MNG library image
MNG library image handle
+ mng_uint32 width; // width of MNG image
MNG image width
+ mng_uint32 height; // height of MNG image
MNG image height
+ int timer_msecs; // number of milliseconds after which the MNG library want to be called again
wants
+// MNG library callback: (a part of) the canvas should be shown +static mng_bool demux_mng_refresh(mng_handle __attribute__((unused)) h_mng, + mng_uint32 __attribute__((unused)) x, + mng_uint32 __attribute__((unused)) y, + mng_uint32 __attribute__((unused)) width, + mng_uint32 __attribute__((unused)) height)
Hmm, are you sure these __attribute__((unused)) work without problems in non-gcc compilers?
+ mng_priv->displaying = 1; // remember that mng_display() has been called now + mng_priv->finished = mng_ret == 0; // remember if animation is finished (i.e. mng_display() returned 0) +
trailing whitespace
+ // shutdown MNG inmage instance
image
+static void demux_mng_seek(demuxer_t * demuxer, float rel_seek_secs, float __attribute__((unused)) audio_delay, int flags)
awfully long line In general you will earn extra good karma by keeping line length below 80 characters.
+ if (mng_ret) { + mp_msg(MSGT_DEMUX, MSGL_ERR, "demux_mng: could not reset MNG display state: mng_retcode %d\n", mng_ret); + return;
stray tab Diego
Diego Biurrun wrote:
On Tue, Sep 16, 2008 at 09:36:42PM +0200, Stefan Schuermans wrote:
--- trunk/configure 2008-09-16 16:41:32.000000000 +0200 +++ mng/configure 2008-09-16 16:40:32.000000000 +0200 @@ -4675,6 +4679,39 @@
+echocheck "MNG support" +if test "$_mng" = auto ; then + _mng=no + if irix ; then + # Don't check for -lmng on irix + _res_comment="disabled on irix"
Did you verify this or is it just copied and pasted (and thus guesswork) from the PNG test?
You got me, I copied this from the PNG auto detection, but I spent a (short) thought on the irix stuff. I was not sure how to deal with MNG on irix, so I first thought to not break anything on irix and just skip MNG because MNG is based on PNG and PNG seems to be problematic on irix. But now I've checked the libmng documentation again in more detail. It states states that libmng does not use libpng, so it's not sure there are problems with libmng on irix. I removed the "if" skipping the test for irix.
+cat > $TMPC << EOF +#include <libmng.h> +#include <string.h> +int main(void) { + const char * p_ver = mng_version_text() ?: ""; + printf("libmng: %s\n", p_ver); + return p_ver[0] == 0; +}
You do not seem to be using string.h.
You're right, I removed that. I also removed the __attribute__((unused)), converted the code to not contain any lines longer than 78 characters and corrected the typos. Regarding the issue with variable framerate, I now understood how the GIF demuxer does the variable framerate and switched the MNG demuxer to the same mechanism. Stefan
On Thu, Sep 18, 2008 at 10:47:00PM +0200, Stefan Schuermans wrote:
I also removed the __attribute__((unused)), converted the code to not contain any lines longer than 78 characters and corrected the typos.
Regarding the issue with variable framerate, I now understood how the GIF demuxer does the variable framerate and switched the MNG demuxer to the same mechanism.
--- configure (revision 27638) +++ configure (working copy) @@ -4683,6 +4687,33 @@
+echocheck "MNG support" +if test "$_mng" = auto ; then + _mng=no + cat > $TMPC << EOF +#include <libmng.h> +int main(void) { + const char * p_ver = mng_version_text() ?: ""; + printf("libmng: %s\n", p_ver); + return p_ver[0] == 0; +} +EOF + if cc_check -lmng -lz $_ld_lm ; then + if tmp_run ; then + _mng=yes + else + _res_comment="no library version" + fi + fi +fi
I wonder why tmp_run is necessary? And yes, I know that the libpng test does it as well...
--- libmpdemux/demux_mng.c (revision 0) +++ libmpdemux/demux_mng.c (revision 0) @@ -0,0 +1,528 @@ + + int header_processed; // if header of MNG image is processed
if MNG image header is processed
+ // whenever the libmng tells us to wait for some time
whenever libmng tells us Your code is laboriously commented. Possibly some of it could be doxygenized. Diego
On Fri, 19 Sep 2008 00:11:05 +0200, Diego Biurrun wrote:
On Thu, Sep 18, 2008 at 10:47:00PM +0200, Stefan Schuermans wrote: Your code is laboriously commented. Possibly some of it could be doxygenized.
Diego
should make it a template for future libmpcodecs. -compn
Diego Biurrun wrote:
On Thu, Sep 18, 2008 at 10:47:00PM +0200, Stefan Schuermans wrote:
--- configure (revision 27638) +++ configure (working copy) @@ -4683,6 +4687,33 @@
+echocheck "MNG support" +if test "$_mng" = auto ; then + _mng=no + cat > $TMPC << EOF +#include <libmng.h> +int main(void) { + const char * p_ver = mng_version_text() ?: ""; + printf("libmng: %s\n", p_ver); + return p_ver[0] == 0; +} +EOF + if cc_check -lmng -lz $_ld_lm ; then + if tmp_run ; then + _mng=yes + else + _res_comment="no library version" + fi + fi +fi
I wonder why tmp_run is necessary? And yes, I know that the libpng test does it as well...
If I've understood this correctly, cc_check tries to compile and link the C source and tmp_run tries to run the result. I think it is neccessary to run the code to detect if linking against libmng works. I've seen it at least once (with some broken libmng installation) that compiling and linking worked, but the binary did not run. I'm not sure any more if it crashed or did not load due to not finding libmng.so.
Your code is laboriously commented. Possibly some of it could be doxygenized.
I did that and hope the style of the the doxygen comments is okay. Greetings, Stefan
Stefan Schuermans wrote:
Diego Biurrun wrote:
On Thu, Sep 18, 2008 at 10:47:00PM +0200, Stefan Schuermans wrote:
--- configure (revision 27638) +++ configure (working copy) @@ -4683,6 +4687,33 @@
+echocheck "MNG support" [...]
I wonder why tmp_run is necessary? And yes, I know that the libpng test does it as well...
If I've understood this correctly, cc_check tries to compile and link the C source and tmp_run tries to run the result. I think it is neccessary to run the code to detect if linking against libmng works. I've seen it at least once (with some broken libmng installation) that compiling and linking worked, but the binary did not run. I'm not sure any more if it crashed or did not load due to not finding libmng.so.
Your code is laboriously commented. Possibly some of it could be doxygenized.
I did that and hope the style of the the doxygen comments is okay.
Any comments regarding the patch I sent last week? If there are no further suggestions for improvement please apply it. Greetings, Stefan
On Thu, Sep 25, 2008 at 01:22:20PM +0200, Stefan Schuermans wrote:
Stefan Schuermans wrote:
Diego Biurrun wrote:
On Thu, Sep 18, 2008 at 10:47:00PM +0200, Stefan Schuermans wrote:
--- configure (revision 27638) +++ configure (working copy) @@ -4683,6 +4687,33 @@
+echocheck "MNG support" [...]
I wonder why tmp_run is necessary? And yes, I know that the libpng test does it as well...
If I've understood this correctly, cc_check tries to compile and link the C source and tmp_run tries to run the result. I think it is neccessary to run the code to detect if linking against libmng works. I've seen it at least once (with some broken libmng installation) that compiling and linking worked, but the binary did not run. I'm not sure any more if it crashed or did not load due to not finding libmng.so.
Your code is laboriously commented. Possibly some of it could be doxygenized.
I did that and hope the style of the the doxygen comments is okay.
Any comments regarding the patch I sent last week? If there are no further suggestions for improvement please apply it.
Well, there are some minor things I do not really like (sometimes excessive comments, that would have the pointer to the whole demuxer in the priv struct when the stream would be enough, and I also wonder what happens to the alpha channel, does MNG not support it, or just libmng or?), but I leave that to you if you have the time and motivation to look into that. What IMO is not acceptable though is the configure check: AFAICT it tries to execute the binary, which means it will always fail when cross-compiling. I think there is no real need for that. Greetings, Reimar Döffinger
Reimar Döffinger wrote:
On Thu, Sep 25, 2008 at 01:22:20PM +0200, Stefan Schuermans wrote:
Stefan Schuermans wrote:
Diego Biurrun wrote:
Your code is laboriously commented. Possibly some of it could be doxygenized. I did that and hope the style of the the doxygen comments is okay. Any comments regarding the patch I sent last week? If there are no further suggestions for improvement please apply it.
Well, there are some minor things I do not really like (sometimes excessive comments, that would have the pointer to the whole demuxer in the priv struct when the stream would be enough, and I also wonder what happens to the alpha channel, does MNG not support it, or just libmng or?),
I'll try to fix the "whole demuxer / just the stream" issue and I would also like to try to implement the alpha channel (it is supported by MNG and libmng) although I've not yet used that. Can you point me to the correct place in the tech docs of MPlayer that describe how to set up the fields in the video stream header to use the alpha channel, i.e. to make MPlayer expect RGBA instead of RGB in the demuxer packets?
but I leave that to you if you have the time and motivation to look into that. What IMO is not acceptable though is the configure check: AFAICT it tries to execute the binary, which means it will always fail when cross-compiling. I think there is no real need for that.
Okay, cross-compiling is a point I did not think of. I understand that running the binary in this case is very problematic, I'll remove that. Greetings, Stefan
Hello, On Thu, Sep 25, 2008 at 09:22:44PM +0200, Stefan Schuermans wrote:
I'll try to fix the "whole demuxer / just the stream" issue and I would also like to try to implement the alpha channel (it is supported by MNG and libmng) although I've not yet used that. Can you point me to the correct place in the tech docs of MPlayer that describe how to set up the fields in the video stream header to use the alpha channel, i.e. to make MPlayer expect RGBA instead of RGB in the demuxer packets?
Set biBitCount to 32 and biCompression either to mmioFOURCC(32, 'B', 'G', 'R') or to 0. Note that (most of) MPlayer can not actually make use of it, though it might work with -vo png (never tried).
On Fri, Sep 19, 2008 at 03:39:20PM +0200, Stefan Schuermans wrote:
Diego Biurrun wrote:
On Thu, Sep 18, 2008 at 10:47:00PM +0200, Stefan Schuermans wrote:
--- configure (revision 27638) +++ configure (working copy) @@ -4683,6 +4687,33 @@ +echocheck "MNG support" +if test "$_mng" = auto ; then + _mng=no + cat > $TMPC << EOF +#include <libmng.h> +int main(void) { + const char * p_ver = mng_version_text() ?: ""; + printf("libmng: %s\n", p_ver); + return p_ver[0] == 0; +} +EOF + if cc_check -lmng -lz $_ld_lm ; then + if tmp_run ; then + _mng=yes + else + _res_comment="no library version" + fi + fi +fi
I wonder why tmp_run is necessary? And yes, I know that the libpng test does it as well...
If I've understood this correctly, cc_check tries to compile and link the C source and tmp_run tries to run the result. I think it is neccessary to run the code to detect if linking against libmng works. I've seen it at least once (with some broken libmng installation) that compiling and linking worked, but the binary did not run. I'm not sure any more if it crashed or did not load due to not finding libmng.so.
I think the the test in the libpng test is already suspicious. Just drop it. As Reimar said, it will be problematic if cross-compiling.
--- configure (revision 27638) +++ configure (working copy) @@ -4683,6 +4687,32 @@
+if test "$_mng" = yes ; then + _def_mng='#define HAVE_MNG 1'
CONFIG_MNG
--- libmpdemux/demux_mng.c (revision 0) +++ libmpdemux/demux_mng.c (revision 0) @@ -0,0 +1,622 @@ + + * start time must be > 0 for the variable frame time mechanism + * (GIF, MATROSKA, MNG) in video.c to work for the first frame
When you have complete sentences like this, you should ideally capitalize the first word and end in a period, same below.
+ * This structure is used as private data for MPlayer demuxer + * als also as private data for the MNG library.
als also?
+/** + * \brief MNG library callback: (a part of) the canvas should be shown + * \param[in] h_mng MNG library image handle + * \param[in] x rectangle's left edge + * \param[in] y rectangle's top edge + * \param[in] width rectangle's width + * \param[in] height rectangle's heigt + * \return \p MNG_TRUE on success, \p MNG_FALSE on error (never happens)
What are all those rectangles? I may be missing something obvious here..
+ // save number of milliseconds after which to call MNG library again
the MNG
+ * \return demuxer sructure on success, \p NULL on error
sTructure Diego
Diego Biurrun wrote:
On Fri, Sep 19, 2008 at 03:39:20PM +0200, Stefan Schuermans wrote:
On Thu, Sep 18, 2008 at 10:47:00PM +0200, Stefan Schuermans wrote:
+ if cc_check -lmng -lz $_ld_lm ; then + if tmp_run ; then I wonder why tmp_run is necessary? And yes, I know that the libpng test does it as well... If I've understood this correctly, cc_check tries to compile and link
Diego Biurrun wrote: the C source and tmp_run tries to run the result. I think it is neccessary to run the code to detect if linking against libmng works. I've seen it at least once (with some broken libmng installation) that compiling and linking worked, but the binary did not run. I'm not sure any more if it crashed or did not load due to not finding libmng.so.
I think the the test in the libpng test is already suspicious. Just drop it. As Reimar said, it will be problematic if cross-compiling.
I've dropped it now.
+/** + * \brief MNG library callback: (a part of) the canvas should be shown + * \param[in] h_mng MNG library image handle + * \param[in] x rectangle's left edge + * \param[in] y rectangle's top edge + * \param[in] width rectangle's width + * \param[in] height rectangle's heigt + * \return \p MNG_TRUE on success, \p MNG_FALSE on error (never happens)
What are all those rectangles? I may be missing something obvious here..
There's only one rectangle, which has got a left edge, a top edge, a width and a height. I've moved the long description out of the function into the doxygen part to make it clear. Your other comments and Reimar's comments (alpha channel and the "demuxer/stream" issue) are also integrated in the new patch. Greetings, Stefan
Stefan Schuermans wrote:
[...] Your other comments and Reimar's comments (alpha channel and the "demuxer/stream" issue) are also integrated in the new patch.
[...] --enable-png enable PNG input/output support [autodetect] + --enable-mng enable MNG input support [autodetect] --enable-jpeg enable JPEG input/output support [autodetect] [...] +const demuxer_desc_t demuxer_desc_mng = { + "MNG demuxer", [...]
What about the last version of my MNG patch I sent? I have not heared anything for a bit more than two weeks now...!? Greetings, Stefan
On Sat, Oct 18, 2008 at 06:02:03PM +0200, Stefan Schuermans wrote:
Stefan Schuermans wrote:
[...] Your other comments and Reimar's comments (alpha channel and the "demuxer/stream" issue) are also integrated in the new patch.
[...] --enable-png enable PNG input/output support [autodetect] + --enable-mng enable MNG input support [autodetect] --enable-jpeg enable JPEG input/output support [autodetect] [...] +const demuxer_desc_t demuxer_desc_mng = { + "MNG demuxer", [...]
What about the last version of my MNG patch I sent? I have not heared anything for a bit more than two weeks now...!?
It looks OK from my side. Any further comments? Reimar? If I hear nothing for a week or so I think I will apply this. Diego
On Mon, Sep 29, 2008 at 12:45:58PM +0200, Stefan Schuermans wrote:
+echocheck "MNG support" +if test "$_mng" = auto ; then + _mng=no + cat > $TMPC << EOF +#include <libmng.h> +int main(void) { + const char * p_ver = mng_version_text() ?: "";
This is not valid C, though gcc unfortunately only warns.
+ return p_ver[0] == 0; +}
Change it to e.g. const char * p_ver = mng_version_text(); return !p_ver || p_ver[0] == 0; and IMO it is okay to apply. I'd be happy for any volunteer to apply it, otherwise I will try to remember to do it "somewhen". Greetings, Reimar Döffinger
Reimar Döffinger wrote:
On Mon, Sep 29, 2008 at 12:45:58PM +0200, Stefan Schuermans wrote:
+echocheck "MNG support" +if test "$_mng" = auto ; then + _mng=no + cat > $TMPC << EOF +#include <libmng.h> +int main(void) { + const char * p_ver = mng_version_text() ?: "";
This is not valid C, though gcc unfortunately only warns.
+ return p_ver[0] == 0; +}
Change it to e.g. const char * p_ver = mng_version_text(); return !p_ver || p_ver[0] == 0;
Done. New patch based on current SVN is attached.
and IMO it is okay to apply.
Thank you for your quick answer. Greetings, Stefan
On Wed, 26 Nov 2008 21:28:53 +0100, Stefan Schuermans wrote:
Reimar Döffinger wrote:
On Mon, Sep 29, 2008 at 12:45:58PM +0200, Stefan Schuermans wrote: Done. New patch based on current SVN is attached.
and IMO it is okay to apply.
Thank you for your quick answer.
something about the configure check is broken. i dont have libmng installed and compilation fails with your patch. Administrator@BEAGH /f/mplayer $ make makefile:880: warning: overriding commands for target `TAGS' makefile:877: warning: ignoring old commands for target `TAGS' gcc -Wdisabled-optimization -Wdeclaration-after-statement -std=gnu99 -W -Wall -O2 -march=pentium4 -mtune=pentium4 -pipe -g3 -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE64_SOURCE -I. -c -o libmpdemux/demux_mng.o libmpdemux/demux_mng.c libmpdemux/demux_mng.c:38:20: libmng.h: No such file or directory libmpdemux/demux_mng.c:58: error: syntax error before "mng_handle" libmpdemux/demux_mng.c:58: warning: no semicolon at end of struct or union libmpdemux/demux_mng.c:60: error: syntax error before "width" libmpdemux/demux_mng.c:60: warning: type defaults to `int' in declaration of `width' libmpdemux/demux_mng.c:60: warning: data definition has no type or storage class libmpdemux/demux_mng.c:61: error: syntax error before "height" libmpdemux/demux_mng.c:61: warning: type defaults to `int' in declaration of `height' libmpdemux/demux_mng.c:61: warning: data definition has no type or storage class libmpdemux/demux_mng.c:88: error: syntax error before '}' token libmpdemux/demux_mng.c:88: warning: type defaults to `int' in declaration of `mng_priv_t' libmpdemux/demux_mng.c:88: warning: data definition has no type or storage class libmpdemux/demux_mng.c:95: error: syntax error before "demux_mng_alloc" libmpdemux/demux_mng.c:95: error: syntax error before "size" libmpdemux/demux_mng.c:96: warning: return type defaults to `int' libmpdemux/demux_mng.c: In function `demux_mng_alloc': libmpdemux/demux_mng.c:97: error: `size' undeclared (first use in this function) libmpdemux/demux_mng.c:97: error: (Each undeclared identifier is reported only once libmpdemux/demux_mng.c:97: error: for each function it appears in.) libmpdemux/demux_mng.c:97: warning: return makes integer from pointer without a cast libmpdemux/demux_mng.c: At top level: libmpdemux/demux_mng.c:105: error: syntax error before "ptr" libmpdemux/demux_mng.c: In function `demux_mng_free': libmpdemux/demux_mng.c:107: error: `ptr' undeclared (first use in this function) libmpdemux/demux_mng.c: At top level: libmpdemux/demux_mng.c:115: error: syntax error before "demux_mng_openstream" libmpdemux/demux_mng.c:115: error: syntax error before "h_mng" libmpdemux/demux_mng.c:116: warning: return type defaults to `int' libmpdemux/demux_mng.c: In function `demux_mng_openstream': libmpdemux/demux_mng.c:117: error: `mng_priv' undeclared (first use in this function) libmpdemux/demux_mng.c:117: warning: implicit declaration of function `mng_get_userdata' libmpdemux/demux_mng.c:117: error: `h_mng' undeclared (first use in this function) libmpdemux/demux_mng.c:118: warning: ISO C90 forbids mixed declarations and code libmpdemux/demux_mng.c:123: error: `MNG_TRUE' undeclared (first use in this function) libmpdemux/demux_mng.c: At top level: libmpdemux/demux_mng.c:131: error: syntax error before "demux_mng_closestream" libmpdemux/demux_mng.c:131: error: syntax error before "h_mng" libmpdemux/demux_mng.c:132: warning: return type defaults to `int' libmpdemux/demux_mng.c: In function `demux_mng_closestream': libmpdemux/demux_mng.c:133: error: `MNG_TRUE' undeclared (first use in this function) libmpdemux/demux_mng.c: At top level: libmpdemux/demux_mng.c:144: error: syntax error before "demux_mng_readdata" libmpdemux/demux_mng.c:144: error: syntax error before "h_mng" libmpdemux/demux_mng.c:146: warning: return type defaults to `int' libmpdemux/demux_mng.c: In function `demux_mng_readdata': libmpdemux/demux_mng.c:147: error: `mng_priv' undeclared (first use in this function) libmpdemux/demux_mng.c:147: error: `h_mng' undeclared (first use in this function) libmpdemux/demux_mng.c:148: warning: ISO C90 forbids mixed declarations and code libmpdemux/demux_mng.c:151: error: `buf' undeclared (first use in this function) libmpdemux/demux_mng.c:151: error: `size' undeclared (first use in this function) libmpdemux/demux_mng.c:151: error: invalid lvalue in assignment libmpdemux/demux_mng.c:153: error: `MNG_TRUE' undeclared (first use in this function) libmpdemux/demux_mng.c: At top level: libmpdemux/demux_mng.c:163: error: syntax error before "demux_mng_processheader" libmpdemux/demux_mng.c:163: error: syntax error before "h_mng" libmpdemux/demux_mng.c:165: warning: return type defaults to `int' libmpdemux/demux_mng.c: In function `demux_mng_processheader': libmpdemux/demux_mng.c:166: error: `mng_priv' undeclared (first use in this function) libmpdemux/demux_mng.c:166: error: `h_mng' undeclared (first use in this function) libmpdemux/demux_mng.c:174: warning: implicit declaration of function `mng_get_playtime' libmpdemux/demux_mng.c:182: error: `MNG_FALSE' undeclared (first use in this function) libmpdemux/demux_mng.c:185: error: `MNG_TRUE' undeclared (first use in this function) libmpdemux/demux_mng.c: At top level: libmpdemux/demux_mng.c:194: error: syntax error before "demux_mng_getcanvasline" libmpdemux/demux_mng.c:194: error: syntax error before "h_mng" libmpdemux/demux_mng.c:195: warning: return type defaults to `int' libmpdemux/demux_mng.c: In function `demux_mng_getcanvasline': libmpdemux/demux_mng.c:196: error: `mng_priv' undeclared (first use in this function) libmpdemux/demux_mng.c:196: error: `h_mng' undeclared (first use in this function) libmpdemux/demux_mng.c:199: error: `line' undeclared (first use in this function) libmpdemux/demux_mng.c:200: error: `mng_ptr' undeclared (first use in this function) libmpdemux/demux_mng.c:202: error: syntax error before "MNG_NULL" libmpdemux/demux_mng.c:202: error: `MNG_NULL' undeclared (first use in this function) libmpdemux/demux_mng.c: At top level: libmpdemux/demux_mng.c:229: error: syntax error before "demux_mng_refresh" libmpdemux/demux_mng.c:229: error: syntax error before "h_mng" libmpdemux/demux_mng.c:234: warning: return type defaults to `int' libmpdemux/demux_mng.c: In function `demux_mng_refresh': libmpdemux/demux_mng.c:236: error: `MNG_TRUE' undeclared (first use in this function) libmpdemux/demux_mng.c: At top level: libmpdemux/demux_mng.c:244: error: syntax error before "demux_mng_gettickcount" libmpdemux/demux_mng.c:244: error: syntax error before "h_mng" libmpdemux/demux_mng.c:245: warning: return type defaults to `int' libmpdemux/demux_mng.c:258: error: syntax error before "demux_mng_settimer" libmpdemux/demux_mng.c:258: error: syntax error before "h_mng" libmpdemux/demux_mng.c:259: warning: return type defaults to `int' libmpdemux/demux_mng.c: In function `demux_mng_settimer': libmpdemux/demux_mng.c:264: error: `msecs' undeclared (first use in this function) libmpdemux/demux_mng.c:265: error: `MNG_TRUE' undeclared (first use in this function) libmpdemux/demux_mng.c: In function `demux_mng_fill_buffer': libmpdemux/demux_mng.c:293: error: `mng_handle' undeclared (first use in this function) libmpdemux/demux_mng.c:293: error: syntax error before "h_mng" libmpdemux/demux_mng.c:294: error: `mng_retcode' undeclared (first use in this function) libmpdemux/demux_mng.c:315: error: `mng_ret' undeclared (first use in this function) libmpdemux/demux_mng.c:315: warning: implicit declaration of function `mng_display_resume' libmpdemux/demux_mng.c:318: warning: implicit declaration of function `mng_display' libmpdemux/demux_mng.c:319: error: `MNG_NEEDTIMERWAIT' undeclared (first use in this function) libmpdemux/demux_mng.c: In function `demux_mng_open': libmpdemux/demux_mng.c:368: error: `mng_handle' undeclared (first use in this function) libmpdemux/demux_mng.c:368: error: syntax error before "h_mng" libmpdemux/demux_mng.c:369: error: `mng_retcode' undeclared (first use in this function) libmpdemux/demux_mng.c:379: warning: implicit declaration of function `mng_initialize' libmpdemux/demux_mng.c:379: error: syntax error before "mng_priv" libmpdemux/demux_mng.c:392: warning: implicit declaration of function `mng_setcb_openstream' libmpdemux/demux_mng.c:393: warning: implicit declaration of function `mng_setcb_closestream' libmpdemux/demux_mng.c:394: warning: implicit declaration of function `mng_setcb_readdata' libmpdemux/demux_mng.c:395: warning: implicit declaration of function `mng_setcb_processheader' libmpdemux/demux_mng.c:396: warning: implicit declaration of function `mng_setcb_getcanvasline' libmpdemux/demux_mng.c:397: warning: implicit declaration of function `mng_setcb_refresh' libmpdemux/demux_mng.c:398: warning: implicit declaration of function `mng_setcb_gettickcount' libmpdemux/demux_mng.c:399: warning: implicit declaration of function `mng_setcb_settimer' libmpdemux/demux_mng.c:400: warning: implicit declaration of function `mng_set_canvasstyle' libmpdemux/demux_mng.c:400: error: `MNG_CANVAS_RGBA8' undeclared (first use in this function) libmpdemux/demux_mng.c:403: warning: implicit declaration of function `mng_cleanup' libmpdemux/demux_mng.c:409: error: `mng_ret' undeclared (first use in this function) libmpdemux/demux_mng.c:409: warning: implicit declaration of function `mng_read' libmpdemux/demux_mng.c: In function `demux_mng_seek': libmpdemux/demux_mng.c:500: error: `mng_handle' undeclared (first use in this function) libmpdemux/demux_mng.c:500: error: syntax error before "h_mng" libmpdemux/demux_mng.c:501: error: `mng_retcode' undeclared (first use in this function) libmpdemux/demux_mng.c:551: error: `mng_ret' undeclared (first use in this function) libmpdemux/demux_mng.c:551: warning: implicit declaration of function `mng_display_reset' libmpdemux/demux_mng.c:552: error: `MNG_NEEDTIMERWAIT' undeclared (first use in this function) libmpdemux/demux_mng.c:613: confused by earlier errors, bailing out make: *** [libmpdemux/demux_mng.o] Error 1 -compn
On Wed, 2008-11-26 at 19:29 -0500, compn wrote:
something about the configure check is broken. i dont have libmng installed and compilation fails with your patch.
The configure check is probably OK, but the Makefile change is wrong (it compiles libmpdemux/demux_mng.c unconditionally whatever result the configure test gave).
Uoti Urpala wrote:
On Wed, 2008-11-26 at 19:29 -0500, compn wrote:
something about the configure check is broken. i dont have libmng installed and compilation fails with your patch.
The configure check is probably OK, but the Makefile change is wrong (it compiles libmpdemux/demux_mng.c unconditionally whatever result the configure test gave).
Thanks for testing. The improved patch is attached. Greetings, Stefan
On Fri, Nov 28, 2008 at 10:03:58PM +0100, Stefan Schuermans wrote:
Uoti Urpala wrote:
On Wed, 2008-11-26 at 19:29 -0500, compn wrote:
something about the configure check is broken. i dont have libmng installed and compilation fails with your patch.
The configure check is probably OK, but the Makefile change is wrong (it compiles libmpdemux/demux_mng.c unconditionally whatever result the configure test gave).
Thanks for testing. The improved patch is attached.
Applied. I reformatted demux_mng.c to fit FFmpeg coding style. Diego
Diego Biurrun wrote:
On Fri, Nov 28, 2008 at 10:03:58PM +0100, Stefan Schuermans wrote:
Uoti Urpala wrote:
[wrong Makefile change] Thanks for testing. The improved patch is attached.
Applied. I reformatted demux_mng.c to fit FFmpeg coding style.
Thank you. And also thanks to everybody for reviewing and commenting. Stefan
Hello, On Thu, Sep 25, 2008 at 11:30:29PM +0200, Diego Biurrun wrote:
+/** + * \brief MNG library callback: (a part of) the canvas should be shown + * \param[in] h_mng MNG library image handle + * \param[in] x rectangle's left edge + * \param[in] y rectangle's top edge + * \param[in] width rectangle's width + * \param[in] height rectangle's heigt + * \return \p MNG_TRUE on success, \p MNG_FALSE on error (never happens)
What are all those rectangles? I may be missing something obvious here..
Not worth changing it, but that was one of those comments I considered pointless, since it basically explains the libmng API - not something that really belongs in MPlayer. And while as always there are things that could be done better (with a lot of extra effort), IMO you can go ahead and apply the latest version. Greetings, Reimar Döffinger
Hello Reimar and Diego, Reimar Döffinger wrote:
On Thu, Sep 25, 2008 at 11:30:29PM +0200, Diego Biurrun wrote:
+/** + * \brief MNG library callback: (a part of) the canvas should be shown + * \param[in] h_mng MNG library image handle + * \param[in] x rectangle's left edge + * \param[in] y rectangle's top edge + * \param[in] width rectangle's width + * \param[in] height rectangle's heigt + * \return \p MNG_TRUE on success, \p MNG_FALSE on error (never happens) What are all those rectangles? I may be missing something obvious here..
Not worth changing it, but that was one of those comments I considered pointless, since it basically explains the libmng API - not something that really belongs in MPlayer. And while as always there are things that could be done better (with a lot of extra effort), IMO you can go ahead and apply the latest version.
may I ask what's the state of my MNG patch? Greetings, Stefan
On Wed, Nov 26, 2008 at 08:46:55PM +0100, Stefan Schuermans wrote:
Reimar Döffinger wrote:
On Thu, Sep 25, 2008 at 11:30:29PM +0200, Diego Biurrun wrote:
+/** + * \brief MNG library callback: (a part of) the canvas should be shown + * \param[in] h_mng MNG library image handle + * \param[in] x rectangle's left edge + * \param[in] y rectangle's top edge + * \param[in] width rectangle's width + * \param[in] height rectangle's heigt + * \return \p MNG_TRUE on success, \p MNG_FALSE on error (never happens) What are all those rectangles? I may be missing something obvious here..
Not worth changing it, but that was one of those comments I considered pointless, since it basically explains the libmng API - not something that really belongs in MPlayer. And while as always there are things that could be done better (with a lot of extra effort), IMO you can go ahead and apply the latest version.
may I ask what's the state of my MNG patch?
It does not compile, the Makefile change is wrong. Diego
On Mon, Sep 15, 2008 at 08:48:07PM +0200, Stefan Schuermans wrote:
The diff attached has been done against current svn version.
--- trunk/configure 2008-09-15 20:26:57.000000000 +0200 +++ mng-20080915/configure 2008-09-15 20:33:05.000000000 +0200 @@ -7686,6 +7723,7 @@ else $_dvdnavconfig --version --minilibs >> $TMPLOG 2>&1 || _dvdnav=no fi +MNG = $_mng fi if test "$_dvdnav" = auto ; then cat > $TMPC <<EOF
This hunk seems to have been misplaced.
--- trunk/libmpdemux/demux_mng.c 1970-01-01 01:00:00.000000000 +0100 +++ mng-20080915/libmpdemux/demux_mng.c 2008-09-15 20:33:05.000000000 +0200 @@ -0,0 +1,487 @@ +/* + * MNG file parser for MPlayer
We usually call this demuxer, not parser.
+// private context structure +// - used as private data in MPlayer demuxer +// - used as private data in MNG library +typedef struct { + demuxer_t * demuxer; // pointer to MPlayer demuxer object + mng_handle h_mng; // handle of MNG library image + int header_processed; // if header of MNG image is processed + mng_uint32 width; // width of MNG image + mng_uint32 height; // height of MNG image + int total_time_ms; // total time of MNG animation (in milliseconds) + unsigned char * canvas; // canvas to draw the image onto + // - lines top-down with pixels left-to-right + // with channels RGB, no padding + // - NULL if no canvas yet + int displaying; // if displaying already (i.e. if mng_display has already been called) + int finished; // if animation is finished + int global_time_ms; // current global time for MNG library (in milliseconds) + int anim_cur_time_ms; // time of current frame in MNG animation (in milliseconds) + int anim_frame_duration_ms; // duration of current frame in MNG animation (in milliseconds) + int show_cur_time_ms; // current time in the show process, i.e. time of last demux packet created (in milliseconds) + int show_next_time_ms; // next time in the show process, i.e. time of next demux packet to create (in milliseconds) + int timer_msecs; // number of milliseconds after which the MNG library want to be called again +} mng_priv_t;
The comments could be aligned for better readability.
+// MNG library callback: allocate a new memmory block and fill it with zeros
memory
+ // get totel time of animation
total
+ // clear varibale MNG library will write number of milliseconds to (via settimer callback)
varibale?
+ // clear varibale MNG library will write number of milliseconds to (via settimer callback)
ditto
+// descriptior of this MNG demuxer
I think this comment is useless and it should be "descriptor". Diego
participants (5)
-
compn -
Diego Biurrun -
Reimar Döffinger -
Stefan Schuermans -
Uoti Urpala