- Jan 10, 2022
-
-
After this, groups will be hidden until we bring them back in VideoAllDisplay.
-
and remove a pointless stray obj-c-ism (the `nil`).
-
-
-
-
-
-
the options seem to relate strongly to options provided by the aomenc tool, and their current documentation for this option states that the default they currently use is 19 ([1]). i don't see any harm in adjusting ours to match. --- note, i'm curious about the fact that our max range is 70, while their documentation states 35, though there is conflicting information later on in their documentation. the author of our module has commented `MAX_LAG_BUFFERS + MAX_LAP_BUFFERS` besides the range, which correspods to this in ([2]), but doesn't help me understand at all: ``` #define MAX_LAG_BUFFERS 35 #define MAX_LAP_BUFFERS 35 #define MAX_TOTAL_BUFFERS (MAX_LAG_BUFFERS + MAX_LAP_BUFFERS) ``` is aomenc maybe taking the user value (0-35) and adding `MAX_LAP_BUFFERS` (35) to it, whilst we are expecting the user to account for this?? [1]: https://github.com/master-of-zen/Av1an/blob/master/docs/Encoders/aomenc.md [2]: https://aomedia.googlesource.com/aom/+/refs/heads/applejack/av1/encoder/lookahead.h
-
this almost entirely reverts 0f9bb18b, leaving just the updated larger upper range bound. there were two problems with that change: 1. it is problematic to use such library version detection in plugin descriptors. 2. it is pointless to have introduced just this one very specific warning and clamping, while ignoring the existing fact that for the usage mode of good-quality the value should be 1-5, while for realtime it should be 6-8, now 6-10 with libaom >= v3.2.0. (and as of a few commits back, 7-9 for mode all-intra). either we should clip and warn fully, or not bother and let libaom just accept/reject/clip as appropriate. implementing clipping/warning handling for the different appliable ranges just adds unnecessary extra burden to keeping this module up to date with libaom changes. it is problematic to use library version detection in the plugin descriptor to set the corresponding upper bound, since the generated config set built by executing the descriptor function is saved in a cache to make loading vlc more efficient. this may work fine for builds that bundle libaom as a contrib, if updates re-build the plugin cache, but does not work fine on systems where libaom is a separate system library, which could at any time be upgraded or downgraded without a rebuild of the vlc plugin cache, which could leave the bound too low or too high depending upon the version installed at the time of loading vlc. let's just specify the new upper bound and leave it at that. if someone is using an older system library and happens to use a value of 9 or 10, then we can just let libaom clip the value or reject it.
-
to explain what ranges of values go with what --sout-aom-cpu-used modes. this ignores for the moment the current dynamic upper bound difference depending upon whether or not libaom is >= v3.2.0. it also introduces documentation for values corresponding to the new all-intra mode that we now allow to be used.
-
gives better UX than trying to explain the values in the label text. - gives a drop list in gui prefs rather than a numberic control. - improves consistency in how such mappings are placed in help output. note that it is correct to pair a range with a choice list, since only ranges are enforced upon setting values.
-
the author of this module seems to have aligned the usage of this option with the 'usage' param of `aom_codec_enc_config_default()`, which takes one of the following three values: - AOM_USAGE_GOOD_QUALITY=0 - AOM_USAGE_REALTIME=1 - AOM_USAGE_ALL_INTRA=2 the 3rd is a recent addition from libaom v3.1.0. i don't see any harm in expanding this to allow this additional new value to be passed through to libaom.
-
and clarify the label (from [1]). gives better UX than trying to explain the values in the label text. - gives a drop list in gui prefs rather than a numberic control. - improves consistency in how such mappings are placed in help output. note that it is correct to pair a range with a choice list, since only ranges are enforced upon setting values. [1]: https://github.com/master-of-zen/Av1an/blob/master/docs/Encoders/aomenc.md
-
and add longtext (taken from the comments in the switch block). this gives better UX, with a drop list in gui prefs rather than a numberic control, a mapping of values in help output, and the longtext provides additional details to further explain the differences. note that it is correct to pair a range with a choice list, since only ranges are enforced upon setting values.
-
there are four variants in the enum it corresponds to, thus should be 0-3.
-
only values 0-2 map to actual cases in the module's switch block. (corresponding to 0=main, 1=high, and 2=professional respectively).
-
-
-
`vlc_custom_create()` uses `calloc()` to allocate the memory, so initialising all of the attributes individually to null/0/false is unnecessary. the initialisation was not even complete, missing all of these: - s->psz_name - s->psz_location - s->b_preparsing - s->out - s->pf_demux
-
if in "any" mode and falling back to an extension based match, and if the `asprintf()` call fails to create the extension based shortcut search string, the consensus is that it should give up rather than proceed to try an "any" based demux module load. note that the `modbuf = NULL` statement was just resetting the undefined state of the variable from the failed `asprintf()` call to ensure that the later `free(modbuf)` was valid. it is not needed now that we go down the error path. note, the init of `s->psz_filepath` in `vlc_stream_CustomNew()` is not strictly necessary to avoid `free()` on an initialised attribute, since `calloc()` is actually involved in the object creation - much of the initialisation done in `vlc_stream_CustomNew()` is actually redundant.
-
upon `asprintf()` failure, the pointer variable used to capture the result is left in an undefined state. here, should such failure occur, the later `free(modbuf)` call would be invalid. we must reset this variable to null upon failure to fix this. note that `modbuf` is not the same variable as `module`, which seems to be causing some confusion in review. this patch has absolutely no impact upon the operation of this function other than fixing the bug just described only. it **does not** cause the module search to operate upon a null search string. it does not change the search string at all. note that `module` initially holds a search string owned by the caller of the function. the function may though need to replace this with a new string that it must allocate on the heap. for the purposes of ensuring that the function later only releases the new string, a copy of the allocated string pointer is kept in the `modbuf` variable. in fact the string pointer in `module` is only replaced with the newly allocated one upon success of `asprintf()`. thus, should the allocation fail, `module` will remain as `"any"` (the allocation only occurs under an `"any"` based search). the patch thus does not alter the search string, and thus does not impact module loading in any way.
-
- Jan 09, 2022
-
-
The mapping code is copy/pasted from the libavutil<->libplacebo helpers found in <libplacebo/utils/libav_internal.h>. This is better than pulling in an ffmpeg dependency, and is reasonably forward-compatible (the updated libplacebo helper will just have to be copy/pasted in the unlikely event that this metadata ever gets extended).
-
-
This three-way allocation dance is rather annoying/verbose to write. So give it a name and make sure nobody ever has to write it ever again.
-
This shouldn't need to mutate picture_t. Marking it const avoids warning when passing it a const picture_t from elsewhere.
-
Struct is made to be as close-as-possible to a carbon copy of the libavutil struct as is reasonable, in order to make everybody's life easier while still avoiding a direct dependency from the vout onto libavutil.
-
-
- Jan 08, 2022
-
-
Rémi Denis-Courmont authored
-
Rémi Denis-Courmont authored
This reverts commit 6218609a. This broke the build: ../../modules/video_output/opengl/egl_display_gbm.c: In function ‘OpenDeviceFd’: ../../modules/video_output/opengl/egl_display_gbm.c:72:26: error: expected ‘}’ before ‘DRM_RENDER_MINOR_NAME’ 72 | DRM_DIR_NAME "/" DRM_RENDER_MINOR_NAME "128", | ^~~~~~~~~~~~~~~~~~~~~ ../../modules/video_output/opengl/egl_display_gbm.c:71:53: note: to match this ‘{’ 71 | static const char *default_drm_device_paths[] = { | ^
-
Reports the different available connectors when opening the KMS vout window module, with naming like most Wayland compositors and DRM info tools. This does not implement hotplug support though.
-
DRM_DIR_NAME can be different, for instance on OpenBSD.
-
The path to card* is different on Linux and on other systems like some BSD ones supporting the DRM infrastructure.
-
Not all planes can be used with every CRTC, and especially when there are multiple connectors plugged. Check that the corresponding CRTC is compatible with the plane we probed before selecting it. This avoids drmModeSetPlane failing with invalid CRTC -EINVAL errors.
-
Shifts is more usual and easier to read here.
-
This is not using drmAvailable() which only checks against minor=0, since card0 might not be available or not support DRM while card1 might support it and be selected by --kms-device. In addition, print the debug information about the current underlying DRM driver implementation used. Note that according to NVidia's DRM documentation[^1]: DRM-NVDC does not populate the drmVersionPtr structure's date, major, minor, and patchlevel fields. [^1]: https://docs.nvidia.com/jetson/l4t-graphics/group__direct__rendering__manager.html#gaabb52f28bfc81b9af64866f88131d513
-
Add an option, --kms-connector, to target an arbitrary connector instead of using the first suitable connector available. This is an equivalent to selecting the output screen in the VLC API.
-
-
-
Apply the settings from the video output configuration, since KMS is able to do rescaling and cropping.
-
Avoid additional indentation level by using early exit on error.
-