[vlc-devel] VLC Workshop - 2022-04-02/03

[Simon Latapie]

These notes can also be found here: https://code.videolan.org/videolan/vlc/-/issues/26825.


# Previous workshop overview

## Progress

- Ancillaries: merged.
- Decoder drain: still in at inthewings 's TODO.
- PCR in sout: WIP by @alaric, tests (chromecast mainly) are going well.
- Separating Dialogues vs Notifications: still in @chub 's TODO.
- Medialibrary Preparse: WIP by @louis, using the previous (external program) strategy discussed, no real problem so far.
- gapless: WIP by @tguillem, cf. [audio discussion](#audio).

## Unfinished business

### Vout

- SPU scaling & SPU in black bars
- gestures/keyboard events extraction from vout
- `update_format` (dimensions/chroma/context), cf. [discussion below](#update_format-in-push-model)
- display lock/resize, cf. [discussion below](#display-lockresize)

### libVLC

- Main loop & libvlc interactions, cf. [discussion below](#main-loop-libvlc-interactions)


# VLC 4.0 status

The goal is to get a rough idea of the remaining work to reach an acceptable 4.0 stability/feature map.

This does not take into account all the bugs that have been introduced since 3.0 and still need to be fixed.

## New Clock

- Fix scenarios that trigger audio burst issues. Cf [discussion below](#audio-buffer-time-get)
- Master clock input: we need this for backward compatibility, but we have to implement a PCR smoothing strategy, that takes jitter and drift into account. @TypX can provide some inspiration document.
- clock & sout: live access output is broken (there is no pacing right now in 4.0). Cf [discussion below](#sout-live-pacing-decoder-thread-in-sout)

## Vout & push model

- Filters:
  - drain is still missing
  - filter creation and decoder context management. cf. [discussion below](#sw-decoding-and-hw-vout)
- `update_format` in push model, and interaction between decoders output, filters, and vout, cf. [discussion below](#update_format-in-push-model)
- resize & display lock. Cf [discussion below](#display-lockresize).
- black bars in vout
  - remains android display (all the other main displays should be ok now)
  - WIP on wayland
- Missing: subtitles in black bars

## Audio

- gapless: the feature was not in the initial roadmap, and it is not the real "100% guaranteed gapless" that the input rework will provide (nor crossfade). However:
  - the proposition from @tguillem is almost finished and should work in most cases
  - it has some limitations (300ms minimum buffering), but it includes a lot of safeguards to avoid next media playback failure.

As long as it is explicitly announced as a *non crossfade* feature, it can be included in 4.0.

## Input

- Hardware context for input devices (Dxgi, V4L): right now, there is no way to provide a HW context (no API)
  - Note: should it be postponed to 5.0 ?
- "next frame" feature is not working correctly in 4.0:
  - this triggers the demux for each frame, that is only able to demux a period of time. You end up having too much data demuxed after multiple next frame action
  - the next frame should not trigger demux automatically. this should be managed by the vout buffer level (if should trigger the demux only if empty or almost empty)
  - for the "previous frame" feature, it is supposed to be working right now, but has always been a hack (seeking for an arbitrary amount before the supposed position and decoding until getting the right frame). It should remain a hack for 4.0.

## Developer tools

- modules "listing": VLC3 has a convenient "module tree" view that was a fast access to the player "setup" (summarizing input/decoder/vout configuration for player).
  - having a full list of "loaded" modules is either unusable (module loading persists), and not really a "tree".
  - however, having some information about the player "configuration" would be convenient.
  - A [replacement proposition](https://mailman.videolan.org/pipermail/vlc-devel/2019-June/125774.html) was submitted, but the implementation is not okay.
  - Proposition: get inspiration from the previous proposition, but instead, store information with `info_category_AddInfo` in the `input/es_out`
- thread naming: this has been tested in custom branches, and this has been proven very, very convenient, especially with IDEs/debug tools
  - everyone seems to be okay with it
  - However, there are some limitations depending on the OS (ie name length on Linux limited to... [15 characters?](https://man7.org/linux/man-pages/man3/pthread_setname_np.3.html))
  - It could also be a good idea to remove the thread priority setup, as it is probably not used anymore.

## Sout

- `update_format` on encoders still has to be discussed.

## libVLC

- interaction with main loop (especially on Darwin platforms)
- event management rework, cf. [discussion below](#libvlc-event-management-rework)

## Desktop UIs

- Finalizing new Qt UI
- Working on macOS UI
- New Web UI: there is a lot of progress, but the last big question is the building and packaging part
  - we do not want to pull a complete npm stack in contribs for every build
  - linux distributions tends to force dependencies to javascript packages if we embed stock libraries
  - Agreement: vlc will ship its js files, and the "compiled" artefact will be added in the prebuilt archives

## Other

- vlm refactor and backward compatibility 




# Discussions

## Sout live pacing & decoder thread in sout

Two strategies seem valid to fix the sout live pacing issue:

- either implement a pacing component (module?) for udp, rtp, srt, sdi, and any live sout output
- or change every live sout output to pace itself, and trust the natural data flow in sout to pace the upstream components 

The second strategy would change more code, but appear to have advantages:

- each protocol would like to manage their pacing differently (like udp VS rtp)
- having to guess whether to add a global pacing component or not could be a messy algorithm, given the complexity a sout chain can generate.

A related work revolves around removing the decoder thread usage in sout:

- the goal would be to simplify the 4.0 sout workflow, allowing the use of packetizers and decoders in sout without having to spawn a decoder thread
- this would simplify the workflow, making it a bit "more" synchronous (although there would still be some asynchronicity to deal with).

## SW decoding and HW vout

This is related to @robux4 MRs:

- [MR 1705](https://code.videolan.org/videolan/vlc/-/merge_requests/1705)
- [MR 330](https://code.videolan.org/videolan/vlc/-/merge_requests/330)

In the current implement, when a SW decoder is used, no decoder device is created. However it should be, as a "gpu context/gpu device".

- decoder device is mainly used to manage the GPU RAM right now.
- to reduce to confusion, it should be renamed to not limit the usage to decoding.
- proposition is to rename "decoder device" to "graphics context"
- in case of SW decoding, the "graphics context" will be created at the first call of `update_format`
- A "surface allocator", that is able to provide RAM CPU video contexts, should also be implemented.
- These changes should also imply a creation of a new decoder device "type".

## Audio buffer, time get

Context: see [@tguillem MR](https://code.videolan.org/videolan/vlc/-/merge_requests/264)

At the beginning of playback:

- The input pushes a lot of data at the beginning of the playback
- But the audio `time_get` might return an error since the audio has not started yet.
- Therefore, it is not possible to have a valid master audio clock during that first burst.

The problem is that the next data burst might come later (after more than 1 seconds), resulting on the audio master to not be updated.

The only valid solution we see is to call the audio output `time_get` callback when a slave clock is trying to convert its pts, to update the master clock offset ASAP.

This strategy could be triggered only:

- when starting the playback
- and if the audio clock has not been initialized.

## `update_format` in push model

Context: see [@rom1v MR](https://code.videolan.org/videolan/vlc/-/merge_requests/1021)

The basic idea is to determine if format can be changed between the filters and the vout (without recreating the vout)

- this is valid for resolution
- this is valid for chroma
- But changing video context seems problematic, as:
  - some vout would not be able to detect that (although they just have to fail if there is some ambiguity)
  - But how to identify video context properly ?
- the real solution would be to create filters *before* display, but changing the workflow seems to imply significant amount of work

## display lock/resize

Context: see:

- [last discussion/proposal](https://code.videolan.org/videolan/vlc/-/merge_requests/1226)
- main previous discussions:
  - [MR 324](https://code.videolan.org/videolan/vlc/-/merge_requests/324)
  - [Issue 25925](https://code.videolan.org/videolan/vlc/-/issues/25925)

The global issue revolves around two incorrect behaviors:

- the display lock is unnecessary kept for too long, so we need to break the wait and display ASAP in case of resize
- there is no priority management on the display lock, so that sometimes it is never released for the resize event

Both issues need to be fixed to have an acceptable resize behavior.

@rom1v started a first (not 100% elligible) version of this strategy, but then switched to a "cond wait" strategy, storing changes to be made during the lock until it is released, and then applying the last "state" changes.
The strategy seems slightly better as a UX experience (it is a bit more "reactive"), however:
- this looks only *slightly* better (no real extreme higher performance in term of interaction)
- this changes the behavior of the current vout, and as the vout strategy will be significantly modified in 5.0, it could lead to more painful changes
- this looks more complex than just adding some basic lock priority pattern, which could be implemented by one lock and one cond wait (note: wrongly identified as a "RWLock" strategy during the workshop).

If the lock priority pattern can be done with minimal changes, the @rom1v will propose a MR based on that solution.

Note: @rom1v created [a new issue](https://code.videolan.org/videolan/vlc/-/issues/26792) to follow up the discussion.

## use/share EGL display across modules

Context: see [Issue 23572](https://code.videolan.org/videolan/vlc/-/issues/23572)

As there is no refcount on EGL, this is very difficult to determine when to clear it.

One of the solution could be to:
- NOT use GL on wayland directly
- force using EGL gbm/drm only.

## Main loop & libVLC interactions

The issues regarding libVLC interaction with resources that need to be managed by the main thread (ie on Darwin systems) are still opened, but the relevant strategy is not 100% clear yet.

@alexandre-janniaux thinks he can propose an alternative by adding some "visibility" status management, to break the potential deadlocks, but the implementation needs to be more stable to be discussed.

## libVLC event management rework

Context: cf. [this discussion](https://code.videolan.org/videolan/vlc/-/merge_requests/1584#note_312850)

The objective of the event management rework is to:
- simplify the current API, especially in term of memory management
- avoid wrong event listening mistakes (listening to events that would never be triggered on a given object type)

### Unique VS multiple callbacks registry

First, it is proposed to remove the ability to register multiple callbacks per event.
If an application using libVLC needs to handle multiple callbacks per event:
- it should register only one to libVLC
- then use this unique libVLC callback to dispatch internally.

Proposition seems reasonable, and accepted.

### Callback signature

Now, there are a lot of possible design patterns for callbacks:
- a callback table (type safety possible)
- a unique callback + union (not type safe)
- one callback per (object + event type) (not type safe)
- one callback per object type for all the possible events of the object

The global consensus is to go with one callback per object type for all events.

This could also be a good idea to add a filter parameter to the callback, represented by a mask.

### Event identification

The proposition is to split the current unique enum, to have one enum per object type.
This way, the list of all the available events per object type will be more explicit.

### Summary

```
object_type_set_listener(object, callback, mask)
```

with `mask` set to all by default (-1)


### External/custom events

There should be a way to add arbitrary events without changing libVLC API.

This way, weird (and numerous) custom events could be forwarded to the applications using libVLC.

The discussions have not been 100% completed, but the last proposition would be like:

```
object_type_set_custom_event_listener(namespace, event_id, callback)
```

With:

- `namespace` would be a string to categorize the custom event
- `event_id` would be an integer identifying the custom event

### Underrun announcement

We could use the new API to have some distinction between "buffering" and "underrun" playback status:

- "buffering" means the player is accumulating data in its buffers. This can be an expected status (ie at the beginning of the playback, after a seek, etc.)
- "underrun" means the player suffers from an unexpected lack of data.

The accepted proposition is to keep the existing "buffering" status, but trigger an "underrun" event.

## SMBv1 VS SMBv2

Context: see [MR 1596](https://code.videolan.org/videolan/vlc/-/merge_requests/1596).

Although SMBv1 is known as an insecure protocol, there are still a significant amount of VLC users using it for their local data access.

To avoid having to leak some confidential information (credentials), the accepted proposition is to:
- Allow SMBv1 in Anonymous mode only
- For any password protected service, force SMBv2

@tguillem also proposed that after the first anonymous auth fail, force a warning dialog to ensure the user is aware he/she will potentially leak information from an insecure connection.
This could be valid for HTTP, FTP, and SMBv1.
No objection on this other proposition.

## Controlling VLC remotely

The problematic is, as usual, to find a way to initiate some secured connection between the remote system and vlc, both in term of identification and communication.

For identification, a simple pairing system (ask for some digit code that would be displayed through another way, like a dialog) would be sufficient.

BUT, creating a secure connection afterwards is still problematic, especially with a browser on a LAN network.

Discussions did not ended to a complete solution, but some ideas could be worth the investigation:
- for non-browser clients, generate a certificate per local user (in `AppData` or next to `vlcrc` file?), self-signed, on any IP wildcard would be sufficient
- for browser clients, maybe using webrtc data channels, with a remote valid secure STUN server hosted by VideoLAN could provide a proper connection initialization (throuhg DTLS + srtp ?)

## CI optimization

The MergeRequest process is a big success, but now that a lot of contributors are using it, the CI infrastructure is under stress, and new optimizations seem mandatory,

Contribs are probably the next priority

### Contrib sources download takes a lot of time

The accepted proposition is to create a nightly "contrib sources prebuilt" tarball.
This archive could then be either cached/proxy cached on a server "near" the CI machines, or layered/volume mounted inside job containers.

### Contrib dependencies

Contrib dependencies management can be improved.

Right now, there is no dependency management related to the additional patches applied on the contrib sources.

The accepted proposition is to modify each target that uses APPLY rule in all rules.mak and add the patches to target dependencies. This seems to be the only way to use the Makefile dependency mechanism to trigger the build in case of changes.

-- 
Simon Latapie
garf at videolan.org