[FFmpeg-devel] [PATCH 2/2] avformat: add protocol_whitelist

[wm4]

On Sun, 24 Jan 2016 12:47:59 +0100
Nicolas George <george at nsup.org> wrote:

> Le quintidi 5 pluviôse, an CCXXIV, Michael Niedermayer a écrit :
> > --- a/libavformat/avio.h
> > +++ b/libavformat/avio.h
> > @@ -595,6 +595,10 @@ int avio_open(AVIOContext **s, const char *url, int flags);
> >  int avio_open2(AVIOContext **s, const char *url, int flags,
> >                 const AVIOInterruptCB *int_cb, AVDictionary **options);
> >  
> > +int avio_open_whitelist(AVIOContext **s, const char *url, int flags,
> > +                         const AVIOInterruptCB *int_cb, AVDictionary **options,
> > +                         const char *whitelist);
> > +  
> 
> Please no! Adding new arguments to a function that already has too many and
> using a string as a structured data structure: two things that are already
> present way too much in the code base and should be avoided for future
> design.
> 
> I suggest to put it in a structure, maybe AVIOSettings (or directly
> AVGlobalSettings), as an array of protocols, and with int_cb while we are at
> it:
> 
> typedef struct AVIOSettings {
>     AVIOInterruptCB *int_cb;
>     struct URLProtocol **protocols;
>     unsigned nb_protocols;
> }

+1

In general, there might be even more things that have to be passed down
than these. I'm also wondering if proper event-based interruption could
be added later using this mechanism (AVIOInterruptCB can't be extended
due to ABI issues).

> 
> Then, if someone really like strings for user interface, they can do:
> 
> int avio_settings_set_protocols_whitelist_string(AVIOSettings *settings,
>                                                  const char *whitelist);
> 
> Regards,
>