[vlc-devel] [PATCH 3/6] vlc_stream: add vlc_stream_NewMRL
filip at atch.se
Tue Mar 28 20:01:56 CEST 2017
Thanks for the very fast feedback, as always much appreciated!
On 2017-03-28 20:56, Rémi Denis-Courmont wrote:
> Le perjantaina 17. maaliskuuta 2017, 3.22.33 EEST Filip Roséen a écrit :
> > ---
> > include/vlc_stream.h | 16 ++++++++++++++++
> > src/input/stream.c | 27 +++++++++++++++++++++++++++
> > src/input/stream_extractor.c | 6 ------
> > src/libvlccore.sym | 1 +
> > 4 files changed, 44 insertions(+), 6 deletions(-)
> > diff --git a/include/vlc_stream.h b/include/vlc_stream.h
> > index 3a6d6ccd0e..55350d6b30 100644
> > --- a/include/vlc_stream.h
> > +++ b/include/vlc_stream.h
> > @@ -350,6 +350,22 @@ VLC_USED;
> > #define vlc_stream_NewURL(a, b) vlc_stream_NewURL(VLC_OBJECT(a), b)
> > /**
> > + * Create a stream_t reading from a \ref MRL.
> > + * You must delete it using vlc_stream_Delete.
> > + *
> > + * \warning This function shall only be used when MRL functionality is
> > + * explicitly needed (such as when referring to items within an
> > + * archive). \ref vlc_stream_NewURL shall be used where
> > applicable.
> Frankly, *I* did not understand what that distinction meant until I read the
> code below. Now, I can see the logic, and I don´t think that my own
> documentation is great either. But I find this text very unhelpful.
Yes, I was myself struggling quite a bit to come up with a wording
that didn't span too many lines that actually reflected what the
I was hoping that the reference to the MRL-documentation would be a
good way of signifying the difference, but I will try to come up with
a better wording (and might try it out on a few non-VLC developers
just to see if it works).
> > + *
> > + * \param obj the parent of the requested stream
> > + * \param mrl the mrl for which the stream_t should be created
> > + * \return NULL on error, pointer to stream_t on success
> > + */
> > +VLC_API stream_t * vlc_stream_NewMRL(vlc_object_t *obj, const char *mrl)
> > +VLC_USED;
> > +#define vlc_stream_NewMRL(a, b) vlc_stream_NewMRL(VLC_OBJECT(a), b)
> And regardless of how much you manage to improve the documentation, if you
> leave this function in vlc_stream.h, people will (mis)use it, because they
> cultivate cargo instead of reading the documentation.
> It´s not your fault, but I think you should put it in the extractor header.
That seems like an excellent idea, though the reason I put it in
`include/vlc_stream.h` was due to the return-type as well as the
But the more I think about it, the more I think you are correct; I
will move it to `include/vlc_stream_extractor.h` as it is tightly
coupled with *stream-extractors* in either case.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the vlc-devel