[vlc-devel] [PATCH] demux: json: add json_reader context

[Alexandre Janniaux]

Hi,

On Wed, Oct 21, 2020 at 05:31:10PM +0300, Rémi Denis-Courmont wrote:
> Le keskiviikkona 21. lokakuuta 2020, 15.47.24 EEST Francois Cartegnie a écrit
> :
> > Le 14/10/2020 à 19:38, Rémi Denis-Courmont a écrit :
> > >>>> -size_t json_read(void *data, void *buf, size_t max);
> > >>>> -void json_parse_error(void *log, const char *msg);
> > >>>> +struct json_reader {
> > >>>> +    void *priv;
> > >>>> +    ssize_t (*read)(void *, void *, size_t);
> > >>>> +    void (*logger)(void *, const struct json_object *, const char *);
> > >>>> +};
> > >>>
> > >>> I don't see the point in having a log callback. The VLC logger is
> > >>> already
> > >>> essentially a log callback by itself. We don't need two indirection
> > >>> layers.
> >
> > Not having another indirection means to have includes inside gammar code
> > to perform vlc_error() on the log callback.
>
> That's kind of the point of using link-time indirection. We don't have any
> single module that would need two or more sets of JSON parser callbacks.

Yes, but honestly I find this quite ugly to use the build
system to solve an API problem. It makes it harder to
understand at the first look and it constrains the build
system to support this, though it works with fewer lines.

Having the indirection at the API level is much easier to
read and understand, and is just an additional hop for error
cases which should not happen in the nominal case anyway.

That's quite the same anyway, so I'm no sure I see why a
link-time indirection should be valid with calling VLC
logger inside, whereas API-level indirection doing the
same would not be valid.

Regards,
--
Alexandre Janniaux
Videolabs