[vlc-devel] Libnfs API changed

[ronnie sahlberg]

In your change, you can improve it a little when the V2 API is used
since V2 will write it into the buffer for you so you don't have to do
the memcpy:

This could be a suggestion :
---
   else
    {
        p_sys->res.read.i_len = i_status;
        if (p_sys->res.read.p_buf) {
              memcpy(p_sys->res.read.p_buf, p_data, i_status);
        }
    }
}
---


and

---

#ifdef LIBNFS_API_V2
p_sys->res.read.p_buf = NULL;
if (nfs_read_async(p_sys->p_nfs, p_sys->p_nfsfh, p_buf, i_len,
nfs_read_cb, p_access) < 0)
#else
p_sys->res.read.p_buf = p_buf;
if (nfs_read_async(p_sys->p_nfs, p_sys->p_nfsfh, i_len, nfs_read_cb,
p_access) < 0)
#endif
{
---

On Mon, 16 Dec 2024 at 18:40, Steve Lhomme <robux4 at ycbcr.xyz> wrote:
>
> Hi Ronnie,
>
> Thanks for the heads up.
> I did a Merge Request to fix our usage in VLC:
> https://code.videolan.org/videolan/vlc/-/merge_requests/6527
>
> I tried to update our contribs to the latest version but it fails to
> build properly on Windows.
> I opened some issues:
> - https://github.com/sahlberg/libnfs/issues/506
> - https://github.com/sahlberg/libnfs/issues/507
>
> Thanks,
> Steve
>
> On 15/12/2024 11:11, ronnie sahlberg wrote:
> > Libnfs is now pushing out a new major release.
> > This wll affect VLC as some signatures in the API has changed in order
> > to allow zero-copy for the READ paths.
> > This greatly improves performance on low=end devices such as RPi.
> >
> > The new API is mainly due to nfs_read* cha ngin its signature to make
> > zero-copy possible.
> > But since this breaks pretty much every application, why not clean up
> > and fix all other warts in the API at the same time.
> >
> > VLC as dar as I can tell only uses the high-level APIs so the changes
> > should be fairly trivial.
> > This is an example on how fio added compile time support for either
> > the classic api or the new api:
> >
> > ---
> > diff --git a/engines/nfs.c b/engines/nfs.c
> > index 6bc4af1f..13b55038 100644
> > --- a/engines/nfs.c
> > +++ b/engines/nfs.c
> > @@ -157,16 +157,28 @@ static int queue_write(struct fio_libnfs_options
> > *o, struct io_u *io_u)
> >   {
> >          struct nfs_data *nfs_data = io_u->engine_data;
> >
> > +#ifdef LIBNFS_API_V2
> > +       return nfs_pwrite_async(o->context, nfs_data->nfsfh,
> > +                               io_u->buf, io_u->buflen, io_u->offset,
> > +                               nfs_callback, io_u);
> > +#else
> >          return nfs_pwrite_async(o->context, nfs_data->nfsfh, io_u->offset,
> >                                  io_u->buflen, io_u->buf, nfs_callback, io_u);
> > +#endif
> >   }
> >
> >   static int queue_read(struct fio_libnfs_options *o, struct io_u *io_u)
> >   {
> >          struct nfs_data *nfs_data = io_u->engine_data;
> >
> > +#ifdef LIBNFS_API_V2
> > +       return nfs_pread_async(o->context, nfs_data->nfsfh,
> > +                               io_u->buf, io_u->buflen, io_u->offset,
> > +                               nfs_callback, io_u);
> > +#else
> >          return nfs_pread_async(o->context, nfs_data->nfsfh, io_u->offset,
> >                                  io_u->buflen, nfs_callback, io_u);
> > +#endif
> >   }
> > --
> >
> >
> > VLC will need the same type of ifdef conditionals to ensure it can
> > compile against either API.
> >
> > It is painful to break an API but sometimes you have to. And there is
> > no difference between breaking it a little and breaking it alot so
> > taking the opportunity to fix a lot of mistakes done in the last 20
> > years.
> >
> > In exchange for this breakage I offer significantly improved
> > performance on very low end devices with poor memory bandwidth. Likem
> > the original RPi.
> >
> > regards
> > ronnie sahlberg
> > _______________________________________________
> > vlc-devel mailing list
> > To unsubscribe or modify your subscription options:
> > https://mailman.videolan.org/listinfo/vlc-devel
>