[MPlayer-dev-eng] [PATCH] vstream client support

[Joey Parrish]

On Tue, Feb 22, 2005 at 07:34:43PM -0600, Joey Parrish wrote:
> On Wed, Feb 23, 2005 at 01:00:36AM +0100, Reimar Döffinger wrote:
> > doxygen-Style comments are missing...
> 
> For new functions, yes.  I don't have any new functions.  These are
> standard interfaces that are the same across all stream modules.  It's
> ludicrous to document them with doxygen.  The function pointers in
> stream.h should be doxygened, if anything.
> 
> > > +  if(!p->host) {
> > > +    mp_msg(MSGT_OPEN, MSGL_ERR, "We need a host name (ex: tivo://hostname/fsid)\n");
> > > +    m_struct_free(&stream_opts, opts);
> > > +    return STREAM_ERROR;
> > 
> > Does this have to be freed here? And if initialization succeed, when
> > should it be freed then? At the end of this function? Or on close?
> 
> stream_netstream does it, which is what I based my module on.
> If I can trust other stream modules, then it should be freed when I
> return with an error.  However, I don't free it on success, because I
> cast it to my private struct and use it later in that fashion.
> It should be freed on close, which in the new version it is.
> 
> > > +  if (!strcmp(p->fsid, "list")) {
> > > +    vstream_list_streams(0);
> > > +    return STREAM_ERROR;
> > > +  } else if (!strcmp(p->fsid, "llist")) {
> > > +    vstream_list_streams(1);
> > > +    return STREAM_ERROR;
> > > +  }
> > 
> > Hmmm. Don't you need a m_struct_free here, too? I actually prefer gotos
> > to have all the freeing in one place (and only one exit point of the
> > function) this helps a lot avoiding such things.
> 
> For consistency, yes, I should.  But this is the code I didn't copy from
> netstream.  :)  I'll add free statements there, too.  Thanks.
> 
> > > +  stream->start_pos = 0;
> > > +  stream->end_pos = vstream_streamsize();
> > > +  mp_msg(MSGT_OPEN, MSGL_DBG2, "Tivo stream size is %d\n", stream->end_pos);
> > 
> > Maybe better MSGL_V, I think _DBG2 is for messages that might be
> > printed several times or output a lot in general...
> 
> Okay, I'll change it to _V.
> 
> Updated patch attached.  I've also incorporated Diego's comments about
> European text style.  (I can't help that American schools raised me to
> be incompatible with the rest of the world.)  It's a hard habit to
> break, as you may notice from this paragraph.  :)

Committed.

--Joey

-- 
"SAKE!!!!" --Tom Cruise