[vlc-devel] [RFC PATCH 00/13] Expose picture attachment through libvlc

[Hugo Beauzée-Luyssen]

On Fri, Nov 6, 2020, at 1:47 PM, Thomas Guillem wrote:
> Hello,
> 
> This set looks very good to me. Just one point.
> 
> We need to ensure that demux modules don't update an attachment 
> (asserts or document it).
> 
> Indeed, attachments are now read via VLC CORE API from any threads, so 
> they can't be updated unless you add a mutex for each attachment.
> 
> On Fri, Nov 6, 2020, at 10:22, Hugo Beauzée-Luyssen wrote:
> > This patchset is an RFC that aims at exposing picture attachments (at
> > least) to users through libvlc
> > This is initially motivated by the fact that we need to be able to
> > handle various tracks belonging to the same album, but with different
> > embedded artwork.
> > Currently, VLC will cache embedded artworks as they are found in
> > cachefolder/artist/date/album meaning that the first track artwork 
> > will be cached, and later ones will be considered already cached, even
> > though they are different.
> > 
> > Additionally, this introduces refcounting to the attachments, allowing
> > us to save a deep copy every time the attachments are fetched. This
> > could also allow us to fetch attachments using a unique identifier
> > instead of their names, as the name unicity is currently no guaranteed
> > 
> > Hugo Beauzée-Luyssen (13):
> >   input: Move attachment functions in their own source file
> >   input: attachment: Add a private structure
> >   input: Refcount attachments
> >   mkv: Use input_attachment_t directly
> >   mp4: Extract attachments once
> >   input: Simplify AppendAttachment prototype
> >   input: Compare attachments themselves when updating them
> >   input: Remove now unused attachment_demux
> >   input: Move attachment to input_item_t
> >   input: Manage attachment through the item
> >   core: Expose input_item_GetAttachments
> >   lib: Add a picture from attachment constructor
> >   lib: media: Expose attached pictures
> > 
> >  include/vlc/libvlc_media.h                   |   5 +
> >  include/vlc_input.h                          |  49 +------
> >  include/vlc_input_item.h                     |   7 +
> >  lib/libvlc.sym                               |   1 +
> >  lib/media.c                                  |  38 ++++++
> >  lib/picture.c                                |  51 +++++++
> >  lib/picture_internal.h                       |   2 +
> >  modules/access/bluray.c                      |   6 +-
> >  modules/codec/libass.c                       |   2 +-
> >  modules/codec/subsusf.c                      |   2 +-
> >  modules/demux/avformat/demux.c               |   6 +-
> >  modules/demux/avi/avi.c                      |   4 +-
> >  modules/demux/flac.c                         |   4 +-
> >  modules/demux/mkv/demux.cpp                  |   2 -
> >  modules/demux/mkv/demux.hpp                  |   5 +-
> >  modules/demux/mkv/matroska_segment_parse.cpp |  37 +++---
> >  modules/demux/mkv/mkv.cpp                    |   9 +-
> >  modules/demux/mkv/mkv.hpp                    |  30 -----
> >  modules/demux/mp4/attachments.c              |   5 +-
> >  modules/demux/mp4/attachments.h              |   2 +-
> >  modules/demux/mp4/mp4.c                      |  18 ++-
> >  modules/demux/mpeg/ts.c                      |   4 +-
> >  modules/demux/ogg.c                          |   4 +-
> >  modules/text_renderer/freetype/freetype.c    |   8 +-
> >  src/Makefile.am                              |   1 +
> >  src/input/access.c                           |   5 +-
> >  src/input/attachment.c                       |  90 +++++++++++++
> >  src/input/es_out.c                           |   4 +-
> >  src/input/input.c                            | 132 +++++--------------
> >  src/input/input_internal.h                   |   9 --
> >  src/input/item.c                             |  45 +++++++
> >  src/input/meta.c                             |   4 +-
> >  src/input/stream_memory.c                    |   2 +-
> >  src/libvlccore.sym                           |   4 +
> >  src/video_output/vout_subpictures.c          |   3 +-
> >  35 files changed, 356 insertions(+), 244 deletions(-)
> >  create mode 100644 src/input/attachment.c
> > 
> > -- 
> > 2.28.0
> > 
> > _______________________________________________
> > 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

Good point, I'll add some documentation.

I thought of enforcing the const-ness of attachments but that would mean hold/release would also be passed a const input_attachment_t*, which seems wrong, while technically doable.
Also, I suppose input_attachment_t should be renamed input_item_attachment(_t) ?

-- 
  Hugo Beauzée-Luyssen
  hugo at beauzee.fr