[vlc-devel] [PATCH] sout: add operations structure
Alexandre Janniaux
ajanni at videolabs.io
Mon Apr 20 13:49:19 CEST 2020
Hi,
On Mon, Apr 20, 2020 at 01:11:53PM +0200, Thomas Guillem wrote:
>
> On Mon, Apr 20, 2020, at 12:43, Rémi Denis-Courmont wrote:
> > And then if the first patch is rejected, I have wasted many hours of work on all the other un-mergeable patches. Thanks but no thanks.
>
> That's what I always did in the past. I can't count the number of hours lost because I had to rewrite a branch. Anyway, I don't regret it, as Steve said, it's easier to review when you can see the change induced by an API change.
To me, having all the changes is not necessary, but it is
certainly helpful to have the reason why you will need this
in the commit message in that case.
But for this patch, it seems like appling the same pattern
everywhere, putting all function pointers in a virtual table
instead of the object itself, so it doesn't feel out of a
patchset although it could explain it.
In bonus, it helps explaining what is module API and what is
core API in objects having both (like vout_window_t).
Regards,
--
Alexandre Janniaux
Videolabs
>
> >
> > Le 20 avril 2020 10:18:04 GMT+03:00, Thomas Guillem <thomas at gllm.fr> a écrit :
> >>
> >> On Mon, Apr 20, 2020, at 09:10, Rémi Denis-Courmont wrote:
> >>> That's a catch-22 requirement. I can't do the transition without this patch.
> >>
> >> I don't get your catch-22, you can do the transition on your private branch and propose the whole branch then.
> >>
> >> Having said that, I'm OK with this patch, it can be pushed now.
> >>
> >>>
> >>> Le 20 avril 2020 08:49:23 GMT+03:00, Steve Lhomme <robux4 at ycbcr.xyz> a écrit :
> >>>> It seems to just double the code to do something. What is the use for that ?
> >>>> Maybe it's better to wait until we have some real examples that use it
> >>>> before merging.
> >>>>
> >>>> On 2020-04-19 13:27, Rémi Denis-Courmont wrote:
> >>>>> include/vlc_sout.h | 42 ++++++++++++++++++++++++++-----
> >>>>> src/stream_output/stream_output.c | 1 +
> >>>>> 2 files changed, 37 insertions(+), 6 deletions(-)
> >>>>>
> >>>>> diff --git a/include/vlc_sout.h b/include/vlc_sout.h
> >>>>> index 9b9c898cfc..a6fcf2b028 100644
> >>>>> --- a/include/vlc_sout.h
> >>>>> +++ b/include/vlc_sout.h
> >>>>> @@ -188,6 +188,14 @@ enum sout_stream_query_e {
> >>>>> SOUT_STREAM_ID_SPU_HIGHLIGHT, /* arg1=void *, arg2=const vlc_spu_highlight_t *, res=can fail */
> >>>>> };
> >>>>>
> >>>>> +struct sout_stream_operations {
> >>>>> + void *(*add)(sout_stream_t *, const es_format_t *);
> >>>>> + void (*del)(sout_stream_t *, void *);
> >>>>> + int (*send)(sout_stream_t *, void *, block_t *);
> >>>>> + int (*control)( sout_stream_t *, int, va_list );
> >>>>> + void (*flush)( sout_stream_t *, void *);
> >>>>> +};
> >>>>> +
> >>>>> struct sout_stream_t
> >>>>> {
> >>>>> struct vlc_object_t obj;
> >>>>> @@ -199,6 +207,8 @@ struct sout_stream_t
> >>>>> config_chain_t *p_cfg;
> >>>>> sout_stream_t *p_next;
> >>>>>
> >>>>> + const struct sout_stream_operations *ops;
> >>>>> +
> >>>>> /* add, remove a stream */
> >>>>> void *(*pf_add)( sout_stream_t *, const es_format_t * );
> >>>>> void (*pf_del)( sout_stream_t *, void * );
> >>>>> @@ -218,31 +228,51 @@ VLC_API sout_stream_t *sout_StreamChainNew(sout_instance_t *p_sout,
> >>>>> static inline void *sout_StreamIdAdd( sout_stream_t *s,
> >>>>> const es_format_t *fmt )
> >>>>> {
> >>>>> - return s->pf_add( s, fmt );
> >>>>> + if (s->ops == NULL)
> >>>>> + return s->pf_add(s, fmt);
> >>>>> + return s->ops->add(s, fmt);
> >>>>> }
> >>>>>
> >>>>> static inline void sout_StreamIdDel( sout_stream_t *s,
> >>>>> void *id )
> >>>>> {
> >>>>> - s->pf_del( s, id );
> >>>>> + if (s->ops == NULL) {
> >>>>> + s->pf_del(s, id);
> >>>>> + return;
> >>>>> + }
> >>>>> + s->ops->del(s, id);
> >>>>> }
> >>>>>
> >>>>> static inline int sout_StreamIdSend( sout_stream_t *s,
> >>>>> void *id, block_t *b )
> >>>>> {
> >>>>> - return s->pf_send( s, id, b );
> >>>>> + if (s->ops == NULL)
> >>>>> + return s->pf_send(s, id, b);
> >>>>> + return s->ops->send(s, id, b);
> >>>>> }
> >>>>>
> >>>>> static inline void sout_StreamFlush( sout_stream_t *s,
> >>>>> void *id )
> >>>>> {
> >>>>> - if (s->pf_flush)
> >>>>> - s->pf_flush( s, id );
> >>>>> + if (s->ops == NULL) {
> >>>>> + if (s->pf_flush != NULL)
> >>>>> + s->pf_flush(s, id);
> >>>>> + return;
> >>>>> + }
> >>>>> + if (s->ops->flush != NULL)
> >>>>> + s->ops->flush(s, id);
> >>>>> }
> >>>>>
> >>>>> static inline int sout_StreamControlVa( sout_stream_t *s, int i_query, va_list args )
> >>>>> {
> >>>>> - return s->pf_control ? s->pf_control( s, i_query, args ) : VLC_EGENERIC;
> >>>>> + if (s->ops == NULL) {
> >>>>> + if (s->pf_control == NULL)
> >>>>> + return VLC_EGENERIC;
> >>>>> + return s->pf_control(s, i_query, args);
> >>>>> + }
> >>>>> + if (s->ops->control == NULL)
> >>>>> + return VLC_EGENERIC;
> >>>>> + return s->ops->control(s, i_query, args);
> >>>>> }
> >>>>>
> >>>>> static inline int sout_StreamControl( sout_stream_t *s, int i_query, ... )
> >>>>> diff --git a/src/stream_output/stream_output.c b/src/stream_output/stream_output.c
> >>>>> index a89b6601c5..524b1a65ca 100644
> >>>>> --- a/src/stream_output/stream_output.c
> >>>>> +++ b/src/stream_output/stream_output.c
> >>>>> @@ -810,6 +810,7 @@ static sout_stream_t *sout_StreamNew( sout_instance_t *p_sout, char *psz_name,
> >>>>> p_stream->psz_name = psz_name;
> >>>>> p_stream->p_cfg = p_cfg;
> >>>>> p_stream->p_next = p_next;
> >>>>> + p_stream->ops = NULL;
> >>>>> p_stream->pf_flush = NULL;
> >>>>> p_stream->pf_control = NULL;
> >>>>> p_stream->pace_nocontrol = false;
> >>>>> --
> >>>>> 2.26.1vlc-devel mailing list
> >>>>> To unsubscribe or modify your subscription options:
> >>>>> https://mailman.videolan.org/listinfo/vlc-devel
> >>>> vlc-devel mailing list
> >>>> To unsubscribe or modify your subscription options:
> >>>> https://mailman.videolan.org/listinfo/vlc-devel
> >>>
> >>> --
> >>> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
> >>> _______________________________________________
> >>> vlc-devel mailing list
> >>> To unsubscribe or modify your subscription options:
> >>> https://mailman.videolan.org/listinfo/vlc-devel
> >>
> >
> > --
> > Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
> > _______________________________________________
> > vlc-devel mailing list
> > To unsubscribe or modify your subscription options:
> > https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
More information about the vlc-devel
mailing list