Fatih Uzunoğlu authored 2 months ago and Steve Lhomme committed 1 month ago

Delegate images are meant to be cached, and put in the
atlas. We have to use a reasonable size otherwise they
can easily spoil the cache.

Currently source size is used as worst case (the largest
the image can be), but most of the time the interface
window size is wide enough to display multiple items,
where in that case item size is not as large as the
worst case.

Granted this will make images have poorer quality when
the interface has such a size that only one/two items are
visible per row, as then the items would scale up to use
the all available width. However, it needs to be balanced.

I propose using the average case and not the worst case
for the source size. I propose that we determine the
average (nbItemPerRow) to be 3 for the moment. This means
that we target the optimal quality when there are at least
three items displayed per row and not less.

I used the formula provided in the comments, one line above.

Fatih Uzunoğlu authored 2 months ago and Steve Lhomme committed 1 month ago

This causes the cursor to remain busy forever.

Fatih Uzunoğlu authored 3 months ago and Steve Lhomme committed 1 month ago

Partially changing a value type changes the whole value. In
image case, this means that the images can get re-loaded with
indeterminate size just to discard them after the size is
determined.

Fortunately this does not happen currently, because the sizes
are determined before the component is complete and QQuickImage
only loads if the component is complete.

However, due to the declarative nature of QML, bindings are
evaluated and assigned one by one, this means that source size
would be changed two times even if .width and .height are
pending re-evaluation at the same time (such as both depend on
DPR). At the same time, the order is also not defined, so
with such a setup as following:

sourceSize.width: width * eDPR
sourceSize.height: height * eDPR

When eDPR changes, Qt evaluates the binding for width or height,
adjusts sourceSize, sourceSize changes and change signal is signalled,
then evaluates the other sub-part of the value type (width or height),
adjusts sourceSize, sourceSize changes again and change signal is
signalled. Qt could technically optimize this, but as of Qt 6.8.1
it is not the case.

Meanwhile with the following:

sourceSize: Qt.size(width * eDPR, height * eDPR)

When eDPR changes, Qt evaluates the binding and adjusts the source
size, sourceSize changes and change signal is signalled. Source size
does not change two times, and the image would not be loaded two
times.

Fatih Uzunoğlu authored 3 months ago and Steve Lhomme committed 1 month ago

Partially changing a value type changes the whole value. In
image case, this means that the images can get re-loaded with
indeterminate size just to discard them after the size is
determined.

Fortunately this does not happen currently, because the sizes
are determined before the component is complete and QQuickImage
only loads if the component is complete.

However, due to the declarative nature of QML, bindings are
evaluated and assigned one by one, this means that source size
would be changed two times even if .width and .height are
pending re-evaluation at the same time (such as both depend on
DPR). At the same time, the order is also not defined, so
with such a setup as following:

sourceSize.width: width * eDPR
sourceSize.height: height * eDPR

When eDPR changes, Qt evaluates the binding for width or height,
adjusts sourceSize, sourceSize changes and change signal is signalled,
then evaluates the other sub-part of the value type (width or height),
adjusts sourceSize, sourceSize changes again and change signal is
signalled. Qt could technically optimize this, but as of Qt 6.8.1
it is not the case.

Meanwhile with the following:

sourceSize: Qt.size(width * eDPR, height * eDPR)

When eDPR changes, Qt evaluates the binding and adjusts the source
size, sourceSize changes and change signal is signalled. Source size
does not change two times, and the image would not be loaded two
times.

Fatih Uzunoğlu authored 3 months ago and Steve Lhomme committed 1 month ago

Partially changing a value type changes the whole value. In
image case, this means that the images can get re-loaded with
indeterminate size just to discard them after the size is
determined.

Fortunately this does not happen currently, because the sizes
are determined before the component is complete and QQuickImage
only loads if the component is complete.

However, due to the declarative nature of QML, bindings are
evaluated and assigned one by one, this means that source size
would be changed two times even if .width and .height are
pending re-evaluation at the same time (such as both depend on
DPR). At the same time, the order is also not defined, so
with such a setup as following:

sourceSize.width: width * eDPR
sourceSize.height: height * eDPR

When eDPR changes, Qt evaluates the binding for width or height,
adjusts sourceSize, sourceSize changes and change signal is signalled,
then evaluates the other sub-part of the value type (width or height),
adjusts sourceSize, sourceSize changes again and change signal is
signalled. Qt could technically optimize this, but as of Qt 6.8.1
it is not the case.

Meanwhile with the following:

sourceSize: Qt.size(width * eDPR, height * eDPR)

When eDPR changes, Qt evaluates the binding and adjusts the source
size, sourceSize changes and change signal is signalled. Source size
does not change two times, and the image would not be loaded two
times.

Fatih Uzunoğlu authored 3 months ago and Steve Lhomme committed 1 month ago

Partially changing a value type changes the whole value. In
image case, this means that the images can get re-loaded with
indeterminate size just to discard them after the size is
determined.

Fortunately this does not happen currently, because the sizes
are determined before the component is complete and QQuickImage
only loads if the component is complete.

However, due to the declarative nature of QML, bindings are
evaluated and assigned one by one, this means that source size
would be changed two times even if .width and .height are
pending re-evaluation at the same time (such as both depend on
DPR). At the same time, the order is also not defined, so
with such a setup as following:

sourceSize.width: width * eDPR
sourceSize.height: height * eDPR

When eDPR changes, Qt evaluates the binding for width or height,
adjusts sourceSize, sourceSize changes and change signal is signalled,
then evaluates the other sub-part of the value type (width or height),
adjusts sourceSize, sourceSize changes again and change signal is
signalled. Qt could technically optimize this, but as of Qt 6.8.1
it is not the case.

Meanwhile with the following:

sourceSize: Qt.size(width * eDPR, height * eDPR)

When eDPR changes, Qt evaluates the binding and adjusts the source
size, sourceSize changes and change signal is signalled. Source size
does not change two times, and the image would not be loaded two
times.

Fatih Uzunoğlu authored 3 months ago and Steve Lhomme committed 1 month ago

Partially changing a value type changes the whole value. In
image case, this means that the images can get re-loaded with
indeterminate size just to discard them after the size is
determined.

Fortunately this does not happen currently, because the sizes
are determined before the component is complete and QQuickImage
only loads if the component is complete.

However, due to the declarative nature of QML, bindings are
evaluated and assigned one by one, this means that source size
would be changed two times even if .width and .height are
pending re-evaluation at the same time (such as both depend on
DPR). At the same time, the order is also not defined, so
with such a setup as following:

sourceSize.width: width * eDPR
sourceSize.height: height * eDPR

When eDPR changes, Qt evaluates the binding for width or height,
adjusts sourceSize, sourceSize changes and change signal is signalled,
then evaluates the other sub-part of the value type (width or height),
adjusts sourceSize, sourceSize changes again and change signal is
signalled. Qt could technically optimize this, but as of Qt 6.8.1
it is not the case.

Meanwhile with the following:

sourceSize: Qt.size(width * eDPR, height * eDPR)

When eDPR changes, Qt evaluates the binding and adjusts the source
size, sourceSize changes and change signal is signalled. Source size
does not change two times, and the image would not be loaded two
times.

Fatih Uzunoğlu authored 3 months ago and Steve Lhomme committed 1 month ago

Partially changing a value type changes the whole value. In
image case, this means that the images can get re-loaded with
indeterminate size just to discard them after the size is
determined.

Fortunately this does not happen currently, because the sizes
are determined before the component is complete and QQuickImage
only loads if the component is complete.

However, due to the declarative nature of QML, bindings are
evaluated and assigned one by one, this means that source size
would be changed two times even if .width and .height are
pending re-evaluation at the same time (such as both depend on
DPR). At the same time, the order is also not defined, so
with such a setup as following:

sourceSize.width: width * eDPR
sourceSize.height: height * eDPR

When eDPR changes, Qt evaluates the binding for width or height,
adjusts sourceSize, sourceSize changes and change signal is signalled,
then evaluates the other sub-part of the value type (width or height),
adjusts sourceSize, sourceSize changes again and change signal is
signalled. Qt could technically optimize this, but as of Qt 6.8.1
it is not the case.

Meanwhile with the following:

sourceSize: Qt.size(width * eDPR, height * eDPR)

When eDPR changes, Qt evaluates the binding and adjusts the source
size, sourceSize changes and change signal is signalled. Source size
does not change two times, and the image would not be loaded two
times.

Fatih Uzunoğlu authored 3 months ago and Steve Lhomme committed 1 month ago

Partially changing a value type changes the whole value. In
image case, this means that the images can get re-loaded with
indeterminate size just to discard them after the size is
determined.

Fortunately this does not happen currently, because the sizes
are determined before the component is complete and QQuickImage
only loads if the component is complete.

However, due to the declarative nature of QML, bindings are
evaluated and assigned one by one, this means that source size
would be changed two times even if .width and .height are
pending re-evaluation at the same time (such as both depend on
DPR). At the same time, the order is also not defined, so
with such a setup as following:

sourceSize.width: width * eDPR
sourceSize.height: height * eDPR

When eDPR changes, Qt evaluates the binding for width or height,
adjusts sourceSize, sourceSize changes and change signal is signalled,
then evaluates the other sub-part of the value type (width or height),
adjusts sourceSize, sourceSize changes again and change signal is
signalled. Qt could technically optimize this, but as of Qt 6.8.1
it is not the case.

Meanwhile with the following:

sourceSize: Qt.size(width * eDPR, height * eDPR)

When eDPR changes, Qt evaluates the binding and adjusts the source
size, sourceSize changes and change signal is signalled. Source size
does not change two times, and the image would not be loaded two
times.

Fatih Uzunoğlu authored 2 months ago and Steve Lhomme committed 1 month ago

This fixes the layouting issue where the cone bar overlaps with the
menu bar when the page changes (such as, switching to player view).

Fatih Uzunoğlu authored 2 months ago and Steve Lhomme committed 1 month ago

This setting allows disabling picture-in-picture mode.

Alexandre Janniaux authored 5 months ago and Steve Lhomme committed 2 months ago

This adds the clock context id metric which allows tracking some of the
lipsync issues by checking every contexts are switching to the new
context reliably. In this patch, clock->context can be null, but in
future work it will never be NULL.

This also setup a new tag on traces exposed by TraceRender to separate
the metrics where ts is the current time and metrics where ts is a time
provided by the tracer user. The rationale is that some systems cannot
render non-monotonic metrics and the time at which the event appear is
more important than the value of the timestamp, which should be traced
separately.

Alexandre Janniaux authored 5 months ago and Steve Lhomme committed 2 months ago

This reports the buffering level (in ms) from the input clock in the
traces, which is useful to analyze artifacts generated by the access or
the input mainloop, as well as the startup condition.

Alexandre Janniaux authored 5 months ago and Steve Lhomme committed 2 months ago

This reports the buffering level (in ms) from the input clock in the
es_out, so that the core can track the buffering level in the current
playback.

The es_out is currently only using the value if positive to trigger a
rebuffering. Previous <= 0 values were discarded.

Alexandre Janniaux authored 1 year ago and Steve Lhomme committed 2 months ago

The events are tracing when decoders are ready for the input to start,
providing details on why the playback startup is being delayed.

Co-authored-by: Thomas Guillem <thomas@gllm.fr>

Alexandre Janniaux authored 2 months ago and Steve Lhomme committed 2 months ago

If the p_extra is freed in the previous branch, it is also freed in this
branch without being reset to NULL before.

Regression from a95213e2.

Alaric Senat authored 2 months ago and Steve Lhomme committed 2 months ago

Whenever an ES selection is forced, the algorithm taking care of default
selecting the tracks at ES creation should take it into account.
Otherwise, tracks with highest priority will always be selected when
created after an user selected one. This happens a lot in DVDs since PS
sets track priority.

Fixes #24815

Alexandre Janniaux authored 4 months ago and Steve Lhomme committed 2 months ago

GL_EXT_YUV_target[^1] provides a way to access an external texture
without applying a conversion to RGBA texture, which can be used to
supply our own matrices for conversion.

This can be used to workaround bogus RGBA conversion for some devices
and can help integrating better with deinterlacing, or in general plane
filtering, and some YUV pipeline like encoding.

[^1]: https://registry.khronos.org/OpenGL/extensions/EXT/EXT_YUV_target.txt

Alexandre Janniaux authored 4 months ago and Steve Lhomme committed 2 months ago

When a filter could not load because of a sampler creation failure,
there were no messages indicating that it came from the sampler, nor
information on what failed in the sampler.

Alexandre Janniaux authored 4 months ago and Steve Lhomme committed 2 months ago

A new YUV list is created to avoid adding the new chroma to YUV
fallbacks.

It's different between C and C++, we should use the proper type no matter what.
See https://learn.microsoft.com/en-us/windows/win32/api/dxgidebug/nf-dxgidebug-dxgigetdebuginterface

This reverts commit fbbd68f8.

There are in fact the "discard" and "mean" deinterlace filters that use half
height.

Marvin Scholz authored 2 months ago and Jean-Baptiste Kempf committed 2 months ago

This is used in other places in the script and if unset leads to the
`-arch` argument being passed without any architecture, which is invalid
and causes compiler invocations to fail.

The reason this worked in CI is that the build script overwrites the
ACTUAL_HOST_ARCH as it can be changed by an argument. However the
env.build.sh script is used also outside the build.sh script in
manual build scenarios, which is the whole reason it is a separate
script to begin with.

The tracer is designed to send the metrics towards a telegraf server
using this kind of configuration:

    [[inputs.socket_listener]]
    service_address = "tcp://localhost:8094"

The telegraf server can then forward the metrics towards an influxdb
server for monitoring or directly to grafana live server[^1] for
introspection.

The influxdb database can also be used to query the metrics after
they've been indexed.

The application using the tracer can use the VLC_TELEGRAF_ENDPOINT
environment variable (eg. VLC_TELEGRAF_ENDPOINT=tcp://127.0.0.1:8094)
to set where the tracer will output the traces to.

A bunch of notes and improvement left for later:

 - Unsafe code is still used for accessing the fields data since an
   union is used and union access is unsafe. It could probably be
   wrapped from the binding implementation.

 - There is no way to specify the address using the configuration
   since vlc_variable is not bound to the Rust bindings and no
   unsafe extern "C" code is done from the plugin.

 - The tracer currently panic as the telegraf server dies because it
   doesn't handle reconnection. Proper reconnection logic with maybe
   size-limited temporary storing might be a proper workaround for that.

 - The tracer doesn't use the timestamp provided by VLC right now,
   proper time conversion is required for it to work.

 - There's no proper tags for the metrics being sent, which is an
   issue given that tags are providing indexing on the data. The
   tags should be generated from the string values of the tracer.
   Maybe an additional "Role" should be added in the trace entry for
   that purpose.

[^1]: https://grafana.com/blog/2021/08/16/streaming-real-time-telegraf-metrics-using-grafana-live/

This commit allows rust code to use the tracer API, through the trace!()
macro, and also exposes a TracerCapability and TracerModuleLoader to
create new tracer modules in Rust.

Pierre Lamot authored 2 months ago and Steve Lhomme committed 2 months ago

As configure.ac is patched

Thomas Guillem authored 2 months ago and Steve Lhomme committed 2 months ago

Forgot to remove this leftover from 709610a2

EsOutGetFromID() was made obsolete in a88c34e6

Alaric Senat authored 2 months ago and Steve Lhomme committed 2 months ago

Emscripten is known to deal badly with function casts to incompatible
types [^1]. This patch prevent us to accidentally merge harmful UB casts
to wasm enabled code. The warning is really strict but is compliant to the
standard. Code that wants to use function cast should be disabled from
the wasm platform.

[^1]: https://emscripten.org/docs/porting/guidelines/function_pointer_issues.html

Alaric Senat authored 2 months ago and Steve Lhomme committed 2 months ago

The function signature does not match the libplacebo callback (gl_t*
instead of void*). While this is supported by many platforms and
compilers, it's undefined behavior strictly by the standard [1] and it's
known to often fail with emscripten [^2].

[1]:
Section 6.3.2.3, paragraph 8:

> A pointer to a function of one type may be converted to a pointer to a
> function of another type and back again; the result shall compare
> equal to the original pointer. If a converted pointer is used to call
> a function whose type is not compatible with the pointed-to type, the
> behavior is undefined.

Quoting section 6.7.5.1, paragraph 2 for pointer compatibility:

> For two pointer types to be compatible, both shall be identically
> qualified and both shall be pointers to compatible types.


[^2]: https://emscripten.org/docs/porting/guidelines/function_pointer_issues.html