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

[Martin Simmons]

>>>>> On Sun, 12 Aug 2007 01:56:42 +0200, Ötvös Attila said:
> 2007. augusztus 12. 01.01 dátummal Martin Simmons ezt írta:
> > >>>>> On Sat, 11 Aug 2007 23:50:55 +0200, Ötvös Attila said:
> > >
> > > +    p->shmemmaxsize=0;
> > > +    for(i=0;i<p->shmemcount;i++) {               // open shared memory
> > > +        if (!open_shmem(shmemids[i],
> > > +                &(p->shmems[i].shmid),
> > > +                &(p->shmems[i].shmemmaxsize),
> > > +                &(p->shmems[i].shmptr),
> > > +                &(p->shmem),i)) {
> > > +            close_shmem(p->shmemcount,p->shmems);
> > > +            free(p->shmems);
> > > +            m_struct_free(&stream_opts,opts);
> > > +            return STREAM_ERROR;
> > > +        }
> > > +        if (p->shmems[i].shmemmaxsize>p->shmemmaxsize)
> > > +            p->shmemmaxsize=p->shmems[i].shmemmaxsize;
> > > +    }
> > > +    free(shmemids);
> > > +    mp_msg(MSGT_OPEN,MSGL_INFO, "Shared memory stream info:"
> > > +        " format=0x%x width=%d
> > > height=%d\n",p->shmem.format,p->shmem.width, +        p->shmem.height);
> > > +    p->shmemrecno = 0;
> > > +    p->shmid = p->shmems[p->shmemrecno].shmid;
> > > +    p->shmemmaxsize = p->shmems[p->shmemrecno].shmemmaxsize;
> >
> > Why set p->shmemmaxsize here after computing it in the loop?
> 
> Yes, My mistake:
> 
> @@ -452,6 +452,7 @@
>              p->shmemmaxsize=p->shmems[i].shmemmaxsize;
>      }
>      free(shmemids);
> +    p->data = malloc(p->shmemmaxsize);
>      mp_msg(MSGT_OPEN,MSGL_INFO, "Shared memory stream info:"
>          " format=0x%x width=%d height=%d\n",p->shmem.format,p->shmem.width,
>          p->shmem.height);
> @@ -460,7 +461,6 @@
>      p->shmemmaxsize = p->shmems[p->shmemrecno].shmemmaxsize;
>      p->shmptr = p->shmems[p->shmemrecno].shmptr;
>  
> -    p->data = malloc(p->shmemmaxsize);
>      *file_format = DEMUXER_TYPE_RAWVIDEO;
>      stream->flags = 0;
>      stream->sector_size = 2048;

Looks better.  Using a normal variable in the for loop instead of
p->shmemmaxsize would be clearer IMHO.


> 
> 
> > Also, I'm confused about how the shmem reading in fill_buffer is
> > synchronized with the writing in write_buffer.
> 
> Now shmem is real time stream (similar TV) therefore don't direct sync from 
> writer to reader.
> Lather we will make direct sync (with pts and rpts) to new type of shmem:
> 
> #define SHMEM_SYNC	1
> set_shmem_opts(SHMEM_SYNC, ... )
> open_output_stream(...)

I think the TV code does sync between the hardware writing and mplayer reading
(it happens in the kernel).  It also uses threads to buffer the frames and a
lock in grab_video_frame to sync access to the next frame for the stream.

Another problem is out-of-order execution.  Memory barriers will be needed to
make the shmem stream work on multi-core machines (see
http://en.wikipedia.org/wiki/Memory_barrier).

__Martin