[MPlayer-dev-eng] [PATCH] shared memory stream - part 1

[Diego Biurrun]

On Wed, Aug 08, 2007 at 12:55:59AM +0200, Ötvös Attila wrote:
> 
> --- stream/stream_sharedmem.c	(revision 0)
> +++ stream/stream_sharedmem.c	(revision 0)
> @@ -0,0 +1,476 @@
> +/*
> + *  stream_sharedmem.c
> + *
> + *	Copyright (C) ??tv??s, Attila <attila at onebithq.com> - 08/2007

Copyright (C) 2007 Attila Ötvös <attila at onebithq.com>

> + *  You should have received a copy of the GNU General Public License
> + *  along with GNU Make; see the file COPYING.  If not, write to
> + *  the Free Software Foundation, 675 Mass Ave, Cambridge, MA 02139, USA. 

This is MPlayer, not GNU Make.

> +#include "config.h"
> +
> +#ifdef HAVE_SHAREDMEM

Do not surround complete files by #ifdefs, conditional compilation is
handled by the Makefiles.

> +void set_smem_opts(int type, int size, int demuxer_type, int format, int width, int height) {

Keeping lines below 80 characters is a plus.

> +static int open_smem(int smemid, int *shmid, int *smemmaxsize, char **shmptr, smem_t *smem, int num) {
> +    smem_t *ptr;
> +  
> +    *shmid=shmget(smemid, sizeof(smem_t), 0666);                                // alloctes a shared memory segment
> +    if (*shmid < 0) {
> +        mp_msg(MSGT_OPEN,MSGL_ERR, "Can't open shared memory (%d)\n",smemid);
> +        return 0;
> +    }
> +    *shmptr=shmat(*shmid,0,0);                                                  // attach shared memory segment
> +    if (*shmptr==(char*)-1) {
> +        mp_msg(MSGT_OPEN,MSGL_ERR, "Can't attach shared memory (%d)\n",smemid);
> +        return 0;
> +    }
> +    ptr=(smem_t*)*shmptr;
> +    if (ptr->mplayerid[0]!='M' || ptr->mplayerid[1]!='P') {                     // check memory area of MPlayer
> +        shmdt(*shmptr);
> +        mp_msg(MSGT_OPEN,MSGL_ERR, "Illegal shared memory area (%d)\n",smemid);
> +        return 0;
> +    }
> +    if (ptr->type!=0) {                                                         // check valid types
> +        shmdt(*shmptr);
> +        mp_msg(MSGT_OPEN,MSGL_ERR, "Illegal shared memory type (%d)\n",smemid);
> +        return 0;
> +    }
> +    if (num==0) {                                                               // first?
> +        smem->size=ptr->size;

Similar problem here: The comments are so far to the right that I find
it hard to tell which line they belong to.

> --- configure	(revision 24014)
> +++ configure	(working copy)
> @@ -281,6 +281,8 @@
>    --disable-w32threads   disable Win32 threads support [autodetect]
>    --disable-ass          disable internal SSA/ASS subtitle support [autodetect]
>    --enable-rpath         enable runtime linker path for extra libs [disabled]
> +  --enable-sharedmem	 Enable put readed frame to shared memory area in mencoder and
> + 			             shared memory reader stream [autodetect]

Tabs and wild indentation.  Also, I don't understand the comment.

> @@ -2263,6 +2268,24 @@
>  
> +echocheck "sharedmem"
> +if test "$_sharedmem" = auto ; then
> +  cat > $TMPC << EOF
> +#include <sys/types.h>
> +#include <sys/shm.h>
> +int main(void) { shmget(0, 0, 0); shmat(0, 0, 0); shmctl(0, 0, 0); return 0; }
> +EOF
> +  _sharedmem=no
> +  cc_check && _sharedmem=yes
> +fi
> +if test "$_sharedmem" = yes ; then
> +  _def_sharedmem='#define HAVE_SHAREDMEM 1'
> +  _inputmodules="sharedmem $_inputmodules"
> +else
> +  _def_sharedmem='#undef HAVE_SHAREDMEM'
> +  _noinputmodules="sharedmem $_noinputmodules"
> +fi
> +echores "$_sharedmem"

Add this along with the other input modules, not at a random place
please.

> @@ -7879,6 +7903,9 @@
>  /* Define this if your system has pthreads */
>  $_def_pthreads
>  
> +/* Define this if use sharedmem  */
> +$_def_sharedmem
> +
>  /* Define this if you enabled thread support for libavcodec */
>  $_def_threads
>  #ifdef HAVE_THREADS

Same here, don't add this between two thread sections.

> --- libmpdemux/demux_rawvideo.c	(revision 24014)
> +++ libmpdemux/demux_rawvideo.c	(working copy)
> @@ -55,6 +55,12 @@
>  static demuxer_t* demux_rawvideo_open(demuxer_t* demuxer) {
>    sh_video_t* sh_video;
>  
> +#ifdef HAVE_SHAREDMEM
> +  if (width==0) stream_control(demuxer->stream, STREAM_CTRL_GET_WIDTH, &width);
> +  if (height==0) stream_control(demuxer->stream, STREAM_CTRL_GET_HEIGHT, &height);
> +  if (format==0) stream_control(demuxer->stream, STREAM_CTRL_GET_FORMAT, &format);
> +#endif

This could be vertically aligned.

Diego