[PATCH 1/7] avutil: add an API to handle 3D Reference Displays Information
From: James Almer <jamrial@gmail.com> As defined in section G.14.3.2.3 of ITU-T H.265, it's required for proper signaling of MV-HEVC. Signed-off-by: James Almer <jamrial@gmail.com> --- libavutil/Makefile | 2 + libavutil/tdrdi.c | 51 ++++++++++++++ libavutil/tdrdi.h | 164 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 217 insertions(+) create mode 100644 libavutil/tdrdi.c create mode 100644 libavutil/tdrdi.h diff --git a/libavutil/Makefile b/libavutil/Makefile index 9ef118016b..94a56bb72f 100644 --- a/libavutil/Makefile +++ b/libavutil/Makefile @@ -85,6 +85,7 @@ HEADERS = adler32.h \ sha512.h \ spherical.h \ stereo3d.h \ + tdrdi.h \ threadmessage.h \ time.h \ timecode.h \ @@ -179,6 +180,7 @@ OBJS = adler32.o \ slicethread.o \ spherical.o \ stereo3d.o \ + tdrdi.o \ threadmessage.o \ time.o \ timecode.o \ diff --git a/libavutil/tdrdi.c b/libavutil/tdrdi.c new file mode 100644 index 0000000000..26192a1d2f --- /dev/null +++ b/libavutil/tdrdi.c @@ -0,0 +1,51 @@ +/* + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * FFmpeg is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with FFmpeg; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include <stddef.h> +#include <stdint.h> + +#include "mem.h" +#include "tdrdi.h" + +AV3DReferenceDisplaysInfo *av_tdrdi_alloc(unsigned int nb_displays, size_t *out_size) +{ + struct TestStruct { + AV3DReferenceDisplaysInfo p; + AV3DReferenceDisplay b; + }; + const size_t entries_offset = offsetof(struct TestStruct, b); + size_t size = entries_offset; + AV3DReferenceDisplaysInfo *tdrdi; + + if (nb_displays > (SIZE_MAX - size) / sizeof(AV3DReferenceDisplay)) + return NULL; + size += sizeof(AV3DReferenceDisplay) * nb_displays; + + tdrdi = av_mallocz(size); + if (!tdrdi) + return NULL; + + tdrdi->num_ref_displays = nb_displays; + tdrdi->entry_size = sizeof(AV3DReferenceDisplay); + tdrdi->entries_offset = entries_offset; + + if (out_size) + *out_size = size; + + return tdrdi; +} diff --git a/libavutil/tdrdi.h b/libavutil/tdrdi.h new file mode 100644 index 0000000000..8dcca42c7b --- /dev/null +++ b/libavutil/tdrdi.h @@ -0,0 +1,164 @@ +/* + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * FFmpeg is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with FFmpeg; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +/** + * @file + * @ingroup lavu_video_3d_reference_displays_info + * Spherical video + */ + +#ifndef AVUTIL_TDRDI_H +#define AVUTIL_TDRDI_H + +#include <stddef.h> +#include <stdint.h> + +#include "libavutil/avassert.h" + +/** + * @defgroup lavu_video_3d_reference_displays_info 3D Reference Displays Information + * @ingroup lavu_video + * + * The 3D Reference Displays Information describes information about the reference display + * width(s) and reference viewing distance(s) as well as information about the corresponding + * reference stereo pair(s). + * @{ + */ + +#define AV_TDRDI_MAX_NUM_REF_DISPLAY 32 + +/** + * This structure describes information about the reference display width(s) and reference + * viewing distance(s) as well as information about the corresponding reference stereo pair(s). + * See section G.14.3.2.3 of ITU-T H.265 for more information. + * + * @note The struct must be allocated with av_tdrdi_alloc() and + * its size is not a part of the public ABI. + */ +typedef struct AV3DReferenceDisplaysInfo { + /** + * The exponent of the maximum allowable truncation error for + * {exponent,mantissa}_ref_display_width as given by 2<sup>(-prec_ref_display_width)</sup>. + */ + uint8_t prec_ref_display_width; + + /** + * A flag to indicate the presence of reference viewing distance. + * If false, the values of prec_ref_viewing_dist, exponent_ref_viewing_distance, + * and mantissa_ref_viewing_distance are undefined. + */ + uint8_t ref_viewing_distance_flag; + + /** + * The exponent of the maximum allowable truncation error for + * {exponent,mantissa}_ref_viewing_distance as given by 2<sup>^(-prec_ref_viewing_dist)</sup>. + * The value of prec_ref_viewing_dist shall be in the range of 0 to 31, inclusive. + */ + uint8_t prec_ref_viewing_dist; + + /** + * The number of reference displays that are signalled in this struct. + * Allowed range is 1 to 32, inclusive. + */ + uint8_t num_ref_displays; + + /** + * Offset in bytes from the beginning of this structure at which the array + * of reference displays starts. + */ + size_t entries_offset; + + /** + * Size of each entry in bytes. May not match sizeof(AV3DReferenceDisplay). + */ + size_t entry_size; +} AV3DReferenceDisplaysInfo; + +/** + * Data structure for single deference display information. + * It is allocated as a part of AV3DReferenceDisplaysInfo and should be retrieved with + * av_tdrdi_get_display(). + * + * sizeof(AV3DReferenceDisplay) is not a part of the ABI and new fields may be + * added to it. +*/ +typedef struct AV3DReferenceDisplay { + /** + * The ViewId of the left view of a stereo pair corresponding to the n-th reference display. + */ + uint16_t left_view_id; + + /** + * The ViewId of the left view of a stereo pair corresponding to the n-th reference display. + */ + uint16_t right_view_id; + + /** + * The exponent part of the reference display width of the n-th reference display. + */ + uint8_t exponent_ref_display_width; + + /** + * The mantissa part of the reference display width of the n-th reference display. + */ + uint8_t mantissa_ref_display_width; + + /** + * Tthe exponent part of the reference viewing distance of the n-th reference display. + */ + uint8_t exponent_ref_viewing_distance; + + /** + * The mantissa part of the reference viewing distance of the n-th reference display. + */ + uint8_t mantissa_ref_viewing_distance; + + /** + * An array of flags to indicates that the information about additional horizontal shift of + * the left and right views for the n-th reference display is present. + */ + uint8_t additional_shift_present_flag; + + /** + * The recommended additional horizontal shift for a stereo pair corresponding to the n-th + * reference baseline and the n-th reference display. + */ + int16_t num_sample_shift; +} AV3DReferenceDisplay; + +static av_always_inline AV3DReferenceDisplay* +av_tdrdi_get_display(AV3DReferenceDisplaysInfo *tdrdi, unsigned int idx) +{ + av_assert0(idx < tdrdi->num_ref_displays); + return (AV3DReferenceDisplay *)((uint8_t *)tdrdi + tdrdi->entries_offset + + idx * tdrdi->entry_size); +} + +/** + * Allocate a AV3DReferenceDisplaysInfo structure and initialize its fields to default + * values. + * + * @return the newly allocated struct or NULL on failure + */ +AV3DReferenceDisplaysInfo *av_tdrdi_alloc(unsigned int nb_displays, size_t *size); + +/** + * @} + */ + +#endif /* AVUTIL_TDRDI_H */ -- 2.49.0
From: James Almer <jamrial@gmail.com> Signed-off-by: James Almer <jamrial@gmail.com> --- libavutil/frame.h | 11 +++++++++++ libavutil/side_data.c | 1 + 2 files changed, 12 insertions(+) diff --git a/libavutil/frame.h b/libavutil/frame.h index 8493233ba2..c50cd263d9 100644 --- a/libavutil/frame.h +++ b/libavutil/frame.h @@ -243,6 +243,17 @@ enum AVFrameSideDataType { * The data is an int storing the view ID. */ AV_FRAME_DATA_VIEW_ID, + + /** + * This side data contains information about the reference display width(s) + * and reference viewing distance(s) as well as information about the + * corresponding reference stereo pair(s), i.e., the pair(s) of views to be + * displayed for the viewer's left and right eyes on the reference display + * at the reference viewing distance. + * The payload is the AV3DReferenceDisplaysInfo struct defined in + * libavutil/tdrdi.h. + */ + AV_FRAME_DATA_3D_REFERENCE_DISPLAYS, }; enum AVActiveFormatDescription { diff --git a/libavutil/side_data.c b/libavutil/side_data.c index 17965f2d3c..fa2a2c2a13 100644 --- a/libavutil/side_data.c +++ b/libavutil/side_data.c @@ -55,6 +55,7 @@ static const AVSideDataDescriptor sd_props[] = { [AV_FRAME_DATA_ICC_PROFILE] = { "ICC profile", AV_SIDE_DATA_PROP_GLOBAL | AV_SIDE_DATA_PROP_COLOR_DEPENDENT }, [AV_FRAME_DATA_SEI_UNREGISTERED] = { "H.26[45] User Data Unregistered SEI message", AV_SIDE_DATA_PROP_MULTI }, [AV_FRAME_DATA_VIDEO_HINT] = { "Encoding video hint", AV_SIDE_DATA_PROP_SIZE_DEPENDENT }, + [AV_FRAME_DATA_3D_REFERENCE_DISPLAYS] = { "3D Reference Displays Information", AV_SIDE_DATA_PROP_GLOBAL }, }; const AVSideDataDescriptor *av_frame_side_data_desc(enum AVFrameSideDataType type) -- 2.49.0
From: James Almer <jamrial@gmail.com> Signed-off-by: James Almer <jamrial@gmail.com> --- libavcodec/avcodec.c | 1 + libavcodec/packet.c | 1 + libavcodec/packet.h | 11 +++++++++++ 3 files changed, 13 insertions(+) diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c index 7bcb0295e5..03c3d09b87 100644 --- a/libavcodec/avcodec.c +++ b/libavcodec/avcodec.c @@ -65,6 +65,7 @@ const SideDataMap ff_sd_global_map[] = { { AV_PKT_DATA_CONTENT_LIGHT_LEVEL, AV_FRAME_DATA_CONTENT_LIGHT_LEVEL }, { AV_PKT_DATA_ICC_PROFILE, AV_FRAME_DATA_ICC_PROFILE }, { AV_PKT_DATA_AMBIENT_VIEWING_ENVIRONMENT,AV_FRAME_DATA_AMBIENT_VIEWING_ENVIRONMENT }, + { AV_PKT_DATA_3D_REFERENCE_DISPLAYS, AV_FRAME_DATA_3D_REFERENCE_DISPLAYS }, { AV_PKT_DATA_NB }, }; diff --git a/libavcodec/packet.c b/libavcodec/packet.c index 5104eb98b1..4003b56223 100644 --- a/libavcodec/packet.c +++ b/libavcodec/packet.c @@ -308,6 +308,7 @@ const char *av_packet_side_data_name(enum AVPacketSideDataType type) case AV_PKT_DATA_IAMF_RECON_GAIN_INFO_PARAM: return "IAMF Recon Gain Info Parameter Data"; case AV_PKT_DATA_FRAME_CROPPING: return "Frame Cropping"; case AV_PKT_DATA_LCEVC: return "LCEVC NAL data"; + case AV_PKT_DATA_3D_REFERENCE_DISPLAYS: return "3D Reference Displays Info"; } return NULL; } diff --git a/libavcodec/packet.h b/libavcodec/packet.h index 71bc2e0575..a3953776fc 100644 --- a/libavcodec/packet.h +++ b/libavcodec/packet.h @@ -345,6 +345,17 @@ enum AVPacketSideDataType { */ AV_PKT_DATA_LCEVC, + /** + * This side data contains information about the reference display width(s) + * and reference viewing distance(s) as well as information about the + * corresponding reference stereo pair(s), i.e., the pair(s) of views to be + * displayed for the viewer's left and right eyes on the reference display + * at the reference viewing distance. + * The payload is the AV3DReferenceDisplaysInfo struct defined in + * libavutil/tdrdi.h. + */ + AV_PKT_DATA_3D_REFERENCE_DISPLAYS, + /** * The number of side data types. * This is not part of the public API/ABI in the sense that it may -- 2.49.0
From: James Almer <jamrial@gmail.com> Signed-off-by: James Almer <jamrial@gmail.com> --- libavformat/dump.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/libavformat/dump.c b/libavformat/dump.c index c263921b99..49506455d3 100644 --- a/libavformat/dump.c +++ b/libavformat/dump.c @@ -37,6 +37,7 @@ #include "libavutil/replaygain.h" #include "libavutil/spherical.h" #include "libavutil/stereo3d.h" +#include "libavutil/tdrdi.h" #include "libavutil/timecode.h" #include "libavcodec/avcodec.h" @@ -461,6 +462,14 @@ static void dump_cropping(void *ctx, const AVPacketSideData *sd) av_log(ctx, AV_LOG_INFO, "%d/%d/%d/%d", left, right, top, bottom); } +static void dump_tdrdi(void *ctx, const AVPacketSideData *sd) +{ + const AV3DReferenceDisplaysInfo *tdrdi = + (const AV3DReferenceDisplaysInfo *)sd->data; + + av_log(ctx, AV_LOG_INFO, "number of reference displays: %u", tdrdi->num_ref_displays); +} + static void dump_sidedata(void *ctx, const AVPacketSideData *side_data, int nb_side_data, int w, int h, AVRational avg_frame_rate, const char *indent, int log_level) @@ -540,6 +549,10 @@ static void dump_sidedata(void *ctx, const AVPacketSideData *side_data, int nb_s av_log(ctx, AV_LOG_INFO, "Frame cropping: "); dump_cropping(ctx, sd); break; + case AV_PKT_DATA_3D_REFERENCE_DISPLAYS: + av_log(ctx, log_level, "3D Reference Displays Information: "); + dump_tdrdi(ctx, sd); + break; default: av_log(ctx, log_level, "unknown side data type %d " "(%"SIZE_SPECIFIER" bytes)", sd->type, sd->size); -- 2.49.0
From: James Almer <jamrial@gmail.com> Signed-off-by: James Almer <jamrial@gmail.com> --- libavfilter/vf_showinfo.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/libavfilter/vf_showinfo.c b/libavfilter/vf_showinfo.c index 8109ca7fce..c706d00c96 100644 --- a/libavfilter/vf_showinfo.c +++ b/libavfilter/vf_showinfo.c @@ -38,6 +38,7 @@ #include "libavutil/pixdesc.h" #include "libavutil/spherical.h" #include "libavutil/stereo3d.h" +#include "libavutil/tdrdi.h" #include "libavutil/timestamp.h" #include "libavutil/timecode.h" #include "libavutil/mastering_display_metadata.h" @@ -152,6 +153,14 @@ static void dump_roi(AVFilterContext *ctx, const AVFrameSideData *sd) } } +static void dump_tdrdi(AVFilterContext *ctx, const AVFrameSideData *sd) +{ + const AV3DReferenceDisplaysInfo *tdrdi = (const AV3DReferenceDisplaysInfo *)sd->data; + + + av_log(ctx, AV_LOG_INFO, "number of reference displays: %u", tdrdi->num_ref_displays); +} + static void dump_detection_bbox(AVFilterContext *ctx, const AVFrameSideData *sd) { int nb_bboxes; @@ -860,6 +869,9 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame) case AV_FRAME_DATA_VIEW_ID: av_log(ctx, AV_LOG_INFO, "view id: %d\n", *(int*)sd->data); break; + case AV_FRAME_DATA_3D_REFERENCE_DISPLAYS: + dump_tdrdi(ctx, sd); + break; default: if (name) av_log(ctx, AV_LOG_INFO, -- 2.49.0
From: James Almer <jamrial@gmail.com> Signed-off-by: James Almer <jamrial@gmail.com> --- libavcodec/hevc/hevcdec.c | 52 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/libavcodec/hevc/hevcdec.c b/libavcodec/hevc/hevcdec.c index 636df5a4e9..797c9c76c9 100644 --- a/libavcodec/hevc/hevcdec.c +++ b/libavcodec/hevc/hevcdec.c @@ -36,6 +36,7 @@ #include "libavutil/opt.h" #include "libavutil/pixdesc.h" #include "libavutil/stereo3d.h" +#include "libavutil/tdrdi.h" #include "libavutil/timecode.h" #include "aom_film_grain.h" @@ -4099,6 +4100,55 @@ static int hevc_update_thread_context(AVCodecContext *dst, } #endif +static int hevc_sei_to_context(AVCodecContext *avctx, HEVCSEI *sei) +{ + int ret; + + if (sei->tdrdi.num_ref_displays) { + AVBufferRef *buf; + size_t size; + AV3DReferenceDisplaysInfo *tdrdi = av_tdrdi_alloc(sei->tdrdi.num_ref_displays, &size); + + if (!tdrdi) + return AVERROR(ENOMEM); + + buf = av_buffer_create((uint8_t *)tdrdi, size, NULL, NULL, 0); + if (!buf) { + av_free(tdrdi); + return AVERROR(ENOMEM); + } + + tdrdi->prec_ref_display_width = sei->tdrdi.prec_ref_display_width; + tdrdi->ref_viewing_distance_flag = sei->tdrdi.ref_viewing_distance_flag; + tdrdi->prec_ref_viewing_dist = sei->tdrdi.prec_ref_viewing_dist; + tdrdi->num_ref_displays = sei->tdrdi.num_ref_displays; + for (int i = 0; i < sei->tdrdi.num_ref_displays; i++) { + AV3DReferenceDisplay *display = av_tdrdi_get_display(tdrdi, i); + + display->left_view_id = sei->tdrdi.left_view_id[i]; + display->right_view_id = sei->tdrdi.right_view_id[i]; + display->exponent_ref_display_width = sei->tdrdi.exponent_ref_display_width[i]; + display->mantissa_ref_display_width = sei->tdrdi.mantissa_ref_display_width[i]; + display->exponent_ref_viewing_distance = sei->tdrdi.exponent_ref_viewing_distance[i]; + display->mantissa_ref_viewing_distance = sei->tdrdi.mantissa_ref_viewing_distance[i]; + display->additional_shift_present_flag = sei->tdrdi.additional_shift_present_flag[i]; + display->num_sample_shift = sei->tdrdi.num_sample_shift[i]; + } + ret = ff_frame_new_side_data_from_buf_ext(avctx, &avctx->decoded_side_data, &avctx->nb_decoded_side_data, + AV_FRAME_DATA_3D_REFERENCE_DISPLAYS, &buf); + if (ret < 0) { + av_buffer_unref(&buf); + return ret; + } + } + + ret = ff_h2645_sei_to_context(avctx, &sei->common); + if (ret < 0) + return ret; + + return 0; +} + static av_cold int hevc_decode_init(AVCodecContext *avctx) { HEVCContext *s = avctx->priv_data; @@ -4122,7 +4172,7 @@ static av_cold int hevc_decode_init(AVCodecContext *avctx) return ret; } - ret = ff_h2645_sei_to_context(avctx, &s->sei.common); + ret = hevc_sei_to_context(avctx, &s->sei); if (ret < 0) return ret; } -- 2.49.0
From: Diego de Souza <ddesouza@nvidia.com> Added support for MV-HEVC encoding for stereoscopic videos (2 views only). Compatible with the framepack filter when using the AV_STEREO3D_FRAMESEQUENCE format. Signed-off-by: Diego de Souza <ddesouza@nvidia.com> --- libavcodec/nvenc.c | 102 ++++++++++++++++++++++++++++++++++++++++ libavcodec/nvenc.h | 9 ++++ libavcodec/nvenc_hevc.c | 5 +- 3 files changed, 115 insertions(+), 1 deletion(-) diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c index 41a4dc55f4..a2457523b1 100644 --- a/libavcodec/nvenc.c +++ b/libavcodec/nvenc.c @@ -37,6 +37,8 @@ #include "libavutil/timecode_internal.h" #include "libavutil/mathematics.h" #include "libavutil/mastering_display_metadata.h" +#include "libavutil/stereo3d.h" +#include "libavutil/tdrdi.h" #include "atsc_a53.h" #include "codec_desc.h" #include "encode.h" @@ -660,6 +662,14 @@ static int nvenc_check_capabilities(AVCodecContext *avctx) ctx->support_dyn_bitrate = nvenc_check_cap(avctx, NV_ENC_CAPS_SUPPORT_DYN_BITRATE_CHANGE); +#ifdef NVENC_HAVE_MVHEVC + ctx->multiview_supported = nvenc_check_cap(avctx, NV_ENC_CAPS_SUPPORT_MVHEVC_ENCODE) > 0; + if(ctx->profile == NV_ENC_HEVC_PROFILE_MULTIVIEW_MAIN && !ctx->multiview_supported) { + av_log(avctx, AV_LOG_WARNING, "Multiview not supported by the device\n"); + return AVERROR(ENOSYS); + } +#endif + return 0; } @@ -1518,6 +1528,26 @@ static av_cold int nvenc_setup_hevc_config(AVCodecContext *avctx) hevc->outputPictureTimingSEI = 1; +#ifdef NVENC_HAVE_MVHEVC + if (ctx->multiview_supported && (ctx->profile == NV_ENC_HEVC_PROFILE_MAIN || ctx->profile == NV_ENC_HEVC_PROFILE_MULTIVIEW_MAIN)) { + const AVFrameSideData *sd_stereo3d = av_frame_side_data_get(avctx->decoded_side_data, avctx->nb_decoded_side_data, AV_FRAME_DATA_STEREO3D); + const AVFrameSideData *sd_tdrdi = av_frame_side_data_get(avctx->decoded_side_data, avctx->nb_decoded_side_data, AV_FRAME_DATA_3D_REFERENCE_DISPLAYS); + const AVStereo3D *stereo3d = sd_stereo3d ? (const AVStereo3D*)sd_stereo3d->data : NULL; + + if (sd_tdrdi && stereo3d && stereo3d->type == AV_STEREO3D_FRAMESEQUENCE) + ctx->profile = NV_ENC_HEVC_PROFILE_MULTIVIEW_MAIN; + + if (ctx->profile == NV_ENC_HEVC_PROFILE_MULTIVIEW_MAIN && stereo3d && + stereo3d->type != AV_STEREO3D_2D && + stereo3d->type != AV_STEREO3D_UNSPEC && + stereo3d->type != AV_STEREO3D_FRAMESEQUENCE) + { + av_log(avctx, AV_LOG_WARNING, "Unsupported multiview input, disabling multiview encoding.\n"); + ctx->profile = NV_ENC_HEVC_PROFILE_MAIN; + } + } +#endif + switch (ctx->profile) { case NV_ENC_HEVC_PROFILE_MAIN: cc->profileGUID = NV_ENC_HEVC_PROFILE_MAIN_GUID; @@ -1531,6 +1561,18 @@ static av_cold int nvenc_setup_hevc_config(AVCodecContext *avctx) cc->profileGUID = NV_ENC_HEVC_PROFILE_FREXT_GUID; avctx->profile = AV_PROFILE_HEVC_REXT; break; +#ifdef NVENC_HAVE_MVHEVC + case NV_ENC_HEVC_PROFILE_MULTIVIEW_MAIN: + cc->profileGUID = NV_ENC_HEVC_PROFILE_MAIN_GUID; + avctx->profile = AV_PROFILE_HEVC_MULTIVIEW_MAIN; + ctx->multiview = 1; + + hevc->enableMVHEVC = 1; + hevc->outputHevc3DReferenceDisplayInfo = 1; + + av_log(avctx, AV_LOG_VERBOSE, "Enabling MV HEVC encoding.\n"); + break; +#endif } // force setting profile as main10 if input is 10 bit or if it should be encoded as 10 bit @@ -1545,6 +1587,13 @@ static av_cold int nvenc_setup_hevc_config(AVCodecContext *avctx) avctx->profile = AV_PROFILE_HEVC_REXT; } +#ifdef NVENC_HAVE_MVHEVC + if (ctx->multiview && avctx->profile != AV_PROFILE_HEVC_MULTIVIEW_MAIN) { + av_log(avctx, AV_LOG_ERROR, "Multiview encoding only works for Main profile content.\n"); + return AVERROR(EINVAL); + } +#endif + hevc->chromaFormatIDC = IS_YUV444(ctx->data_pix_fmt) ? 3 : IS_YUV422(ctx->data_pix_fmt) ? 2 : 1; #ifdef NVENC_HAVE_NEW_BIT_DEPTH_API @@ -2565,6 +2614,9 @@ static int nvenc_set_timestamp(AVCodecContext *avctx, // This can be more than necessary, but we don't know the real reorder delay. delay = FFMAX(ctx->encode_config.frameIntervalP - 1, 0); +#ifdef NVENC_HAVE_MVHEVC + delay *= ctx->multiview ? 2 : 1; +#endif if (ctx->output_frame_num >= delay) { pkt->dts = timestamp_queue_dequeue(ctx->timestamp_list); ctx->output_frame_num++; @@ -3047,6 +3099,9 @@ static int nvenc_send_frame(AVCodecContext *avctx, const AVFrame *frame) MASTERING_DISPLAY_INFO mastering_disp_info = { 0 }; CONTENT_LIGHT_LEVEL content_light_level = { 0 }; #endif +#ifdef NVENC_HAVE_MVHEVC + HEVC_3D_REFERENCE_DISPLAY_INFO ref_disp_info = { 0 }; +#endif NvencContext *ctx = avctx->priv_data; NvencDynLoadFunctions *dl_fn = &ctx->nvenc_dload_funcs; @@ -3117,6 +3172,53 @@ static int nvenc_send_frame(AVCodecContext *avctx, const AVFrame *frame) return res; #endif +#ifdef NVENC_HAVE_MVHEVC + if (ctx->multiview) { + const AVFrameSideData *sd_tdrdi = av_frame_get_side_data(frame, AV_FRAME_DATA_3D_REFERENCE_DISPLAYS); + const AVFrameSideData *sd_view_id = av_frame_get_side_data(frame, AV_FRAME_DATA_VIEW_ID); + + if (sd_view_id) + ctx->next_view_id = *(int*)sd_view_id->data; + + pic_params.codecPicParams.hevcPicParams.viewId = ctx->next_view_id; + + if (sd_tdrdi) { + AV3DReferenceDisplaysInfo *tdrdi = (AV3DReferenceDisplaysInfo*)sd_tdrdi->data; + + ref_disp_info.refViewingDistanceFlag = tdrdi->ref_viewing_distance_flag; + ref_disp_info.precRefViewingDist = tdrdi->prec_ref_viewing_dist; + ref_disp_info.precRefDisplayWidth = tdrdi->prec_ref_display_width; + + ref_disp_info.numRefDisplaysMinus1 = tdrdi->num_ref_displays - 1; + + for (i = 0; i < tdrdi->num_ref_displays && + i < FF_ARRAY_ELEMS(ref_disp_info.leftViewId); i++) { + const AV3DReferenceDisplay *display = av_tdrdi_get_display(tdrdi, i); + ref_disp_info.leftViewId[i] = display->left_view_id; + ref_disp_info.rightViewId[i] = display->right_view_id; + ref_disp_info.exponentRefDisplayWidth[i] = display->exponent_ref_display_width; + ref_disp_info.mantissaRefDisplayWidth[i] = display->mantissa_ref_display_width; + ref_disp_info.exponentRefViewingDistance[i] = display->exponent_ref_viewing_distance; + ref_disp_info.mantissaRefViewingDistance[i] = display->mantissa_ref_viewing_distance; + ref_disp_info.additionalShiftPresentFlag[i] = display->additional_shift_present_flag; + ref_disp_info.numSampleShiftPlus512[i] = display->num_sample_shift + 512; + } + + pic_params.codecPicParams.hevcPicParams.p3DReferenceDisplayInfo = &ref_disp_info; + ctx->display_sei_sent = 1; + } else if (!ctx->display_sei_sent) { + ref_disp_info.precRefDisplayWidth = 31; + ref_disp_info.leftViewId[0] = 0; + ref_disp_info.rightViewId[0] = 1; + + pic_params.codecPicParams.hevcPicParams.p3DReferenceDisplayInfo = &ref_disp_info; + ctx->display_sei_sent = 1; + } + + ctx->next_view_id = !ctx->next_view_id; + } +#endif + res = nvenc_store_frame_data(avctx, &pic_params, frame); if (res < 0) return res; diff --git a/libavcodec/nvenc.h b/libavcodec/nvenc.h index 4b12846ed7..4a4d6730b1 100644 --- a/libavcodec/nvenc.h +++ b/libavcodec/nvenc.h @@ -107,6 +107,7 @@ typedef void ID3D11Device; #define NVENC_HAVE_AV1_UHQ_TUNING #define NVENC_HAVE_H264_AND_AV1_TEMPORAL_FILTER #define NVENC_HAVE_HEVC_AND_AV1_MASTERING_METADATA +#define NVENC_HAVE_MVHEVC #endif typedef struct NvencSurface @@ -180,6 +181,11 @@ enum { NV_ENC_HEVC_PROFILE_MAIN, NV_ENC_HEVC_PROFILE_MAIN_10, NV_ENC_HEVC_PROFILE_REXT, +#ifdef NVENC_HAVE_MVHEVC + NV_ENC_HEVC_PROFILE_MULTIVIEW_MAIN, +#endif + + NV_ENC_HEVC_PROFILE_COUNT }; enum { @@ -253,6 +259,7 @@ typedef struct NvencContext void *nvencoder; uint32_t frame_idx_counter; + uint32_t next_view_id; int preset; int profile; @@ -311,6 +318,8 @@ typedef struct NvencContext int split_encode_mode; int mdm, cll; int cbr_padding; + int multiview, multiview_supported; + int display_sei_sent; } NvencContext; int ff_nvenc_encode_init(AVCodecContext *avctx); diff --git a/libavcodec/nvenc_hevc.c b/libavcodec/nvenc_hevc.c index d74314f245..54e2fe323e 100644 --- a/libavcodec/nvenc_hevc.c +++ b/libavcodec/nvenc_hevc.c @@ -60,10 +60,13 @@ static const AVOption options[] = { { "ull", "Ultra low latency", 0, AV_OPT_TYPE_CONST, { .i64 = NV_ENC_TUNING_INFO_ULTRA_LOW_LATENCY }, 0, 0, VE, .unit = "tune" }, { "lossless", "Lossless", 0, AV_OPT_TYPE_CONST, { .i64 = NV_ENC_TUNING_INFO_LOSSLESS }, 0, 0, VE, .unit = "tune" }, #endif - { "profile", "Set the encoding profile", OFFSET(profile), AV_OPT_TYPE_INT, { .i64 = NV_ENC_HEVC_PROFILE_MAIN }, NV_ENC_HEVC_PROFILE_MAIN, AV_PROFILE_HEVC_REXT, VE, .unit = "profile" }, + { "profile", "Set the encoding profile", OFFSET(profile), AV_OPT_TYPE_INT, { .i64 = NV_ENC_HEVC_PROFILE_MAIN }, NV_ENC_HEVC_PROFILE_MAIN, NV_ENC_HEVC_PROFILE_COUNT - 1, VE, .unit = "profile" }, { "main", "", 0, AV_OPT_TYPE_CONST, { .i64 = NV_ENC_HEVC_PROFILE_MAIN }, 0, 0, VE, .unit = "profile" }, { "main10", "", 0, AV_OPT_TYPE_CONST, { .i64 = NV_ENC_HEVC_PROFILE_MAIN_10 }, 0, 0, VE, .unit = "profile" }, { "rext", "", 0, AV_OPT_TYPE_CONST, { .i64 = NV_ENC_HEVC_PROFILE_REXT }, 0, 0, VE, .unit = "profile" }, +#ifdef NVENC_HAVE_MVHEVC + { "mv", "", 0, AV_OPT_TYPE_CONST, { .i64 = NV_ENC_HEVC_PROFILE_MULTIVIEW_MAIN }, 0, 0, VE, .unit = "profile" }, +#endif { "level", "Set the encoding level restriction", OFFSET(level), AV_OPT_TYPE_INT, { .i64 = NV_ENC_LEVEL_AUTOSELECT }, NV_ENC_LEVEL_AUTOSELECT, NV_ENC_LEVEL_HEVC_62, VE, .unit = "level" }, { "auto", "", 0, AV_OPT_TYPE_CONST, { .i64 = NV_ENC_LEVEL_AUTOSELECT }, 0, 0, VE, .unit = "level" }, { "1", "", 0, AV_OPT_TYPE_CONST, { .i64 = NV_ENC_LEVEL_HEVC_1 }, 0, 0, VE, .unit = "level" }, -- 2.49.0
On 07.06.2025 23:34, Timo Rothenpieler wrote:
From: Diego de Souza <ddesouza@nvidia.com>
Added support for MV-HEVC encoding for stereoscopic videos (2 views only). Compatible with the framepack filter when using the AV_STEREO3D_FRAMESEQUENCE format.
Signed-off-by: Diego de Souza <ddesouza@nvidia.com>
I intend to push this series soon. So if you plan to review it, please do so soon as well, or let me know you plan to at least. All patches had been on the list previously already.
Timo Rothenpieler:
On 07.06.2025 23:34, Timo Rothenpieler wrote:
From: Diego de Souza <ddesouza@nvidia.com>
Added support for MV-HEVC encoding for stereoscopic videos (2 views only). Compatible with the framepack filter when using the AV_STEREO3D_FRAMESEQUENCE format.
Signed-off-by: Diego de Souza <ddesouza@nvidia.com>
I intend to push this series soon. So if you plan to review it, please do so soon as well, or let me know you plan to at least. All patches had been on the list previously already.
When was this on the list? - Andreas
On 08.06.2025 16:17, Andreas Rheinhardt wrote:
Timo Rothenpieler:
On 07.06.2025 23:34, Timo Rothenpieler wrote:
From: Diego de Souza <ddesouza@nvidia.com>
Added support for MV-HEVC encoding for stereoscopic videos (2 views only). Compatible with the framepack filter when using the AV_STEREO3D_FRAMESEQUENCE format.
Signed-off-by: Diego de Souza <ddesouza@nvidia.com>
I intend to push this series soon. So if you plan to review it, please do so soon as well, or let me know you plan to at least. All patches had been on the list previously already.
When was this on the list?
https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=13814 https://patchwork.ffmpeg.org/project/ffmpeg/patch/20250130194124.21836-8-tim...
Timo Rothenpieler:
From: James Almer <jamrial@gmail.com>
As defined in section G.14.3.2.3 of ITU-T H.265, it's required for proper signaling of MV-HEVC.
Signed-off-by: James Almer <jamrial@gmail.com> --- libavutil/Makefile | 2 + libavutil/tdrdi.c | 51 ++++++++++++++ libavutil/tdrdi.h | 164 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 217 insertions(+) create mode 100644 libavutil/tdrdi.c create mode 100644 libavutil/tdrdi.h
diff --git a/libavutil/Makefile b/libavutil/Makefile index 9ef118016b..94a56bb72f 100644 --- a/libavutil/Makefile +++ b/libavutil/Makefile @@ -85,6 +85,7 @@ HEADERS = adler32.h \ sha512.h \ spherical.h \ stereo3d.h \ + tdrdi.h \ threadmessage.h \ time.h \ timecode.h \ @@ -179,6 +180,7 @@ OBJS = adler32.o \ slicethread.o \ spherical.o \ stereo3d.o \ + tdrdi.o \ threadmessage.o \ time.o \ timecode.o \ diff --git a/libavutil/tdrdi.c b/libavutil/tdrdi.c new file mode 100644 index 0000000000..26192a1d2f --- /dev/null +++ b/libavutil/tdrdi.c @@ -0,0 +1,51 @@ +/* + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * FFmpeg is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with FFmpeg; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include <stddef.h> +#include <stdint.h> + +#include "mem.h" +#include "tdrdi.h" + +AV3DReferenceDisplaysInfo *av_tdrdi_alloc(unsigned int nb_displays, size_t *out_size) +{ + struct TestStruct { + AV3DReferenceDisplaysInfo p; + AV3DReferenceDisplay b; + }; + const size_t entries_offset = offsetof(struct TestStruct, b); + size_t size = entries_offset; + AV3DReferenceDisplaysInfo *tdrdi; + + if (nb_displays > (SIZE_MAX - size) / sizeof(AV3DReferenceDisplay)) + return NULL; + size += sizeof(AV3DReferenceDisplay) * nb_displays; + + tdrdi = av_mallocz(size); + if (!tdrdi) + return NULL; + + tdrdi->num_ref_displays = nb_displays; + tdrdi->entry_size = sizeof(AV3DReferenceDisplay); + tdrdi->entries_offset = entries_offset; + + if (out_size) + *out_size = size; + + return tdrdi; +} diff --git a/libavutil/tdrdi.h b/libavutil/tdrdi.h new file mode 100644 index 0000000000..8dcca42c7b --- /dev/null +++ b/libavutil/tdrdi.h @@ -0,0 +1,164 @@ +/* + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * FFmpeg is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with FFmpeg; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +/** + * @file + * @ingroup lavu_video_3d_reference_displays_info + * Spherical video + */ + +#ifndef AVUTIL_TDRDI_H +#define AVUTIL_TDRDI_H + +#include <stddef.h> +#include <stdint.h> + +#include "libavutil/avassert.h" + +/** + * @defgroup lavu_video_3d_reference_displays_info 3D Reference Displays Information + * @ingroup lavu_video + * + * The 3D Reference Displays Information describes information about the reference display + * width(s) and reference viewing distance(s) as well as information about the corresponding + * reference stereo pair(s). + * @{ + */ + +#define AV_TDRDI_MAX_NUM_REF_DISPLAY 32 + +/** + * This structure describes information about the reference display width(s) and reference + * viewing distance(s) as well as information about the corresponding reference stereo pair(s). + * See section G.14.3.2.3 of ITU-T H.265 for more information. + * + * @note The struct must be allocated with av_tdrdi_alloc() and + * its size is not a part of the public ABI. + */ +typedef struct AV3DReferenceDisplaysInfo { + /** + * The exponent of the maximum allowable truncation error for + * {exponent,mantissa}_ref_display_width as given by 2<sup>(-prec_ref_display_width)</sup>. + */ + uint8_t prec_ref_display_width; + + /** + * A flag to indicate the presence of reference viewing distance. + * If false, the values of prec_ref_viewing_dist, exponent_ref_viewing_distance, + * and mantissa_ref_viewing_distance are undefined. + */ + uint8_t ref_viewing_distance_flag; + + /** + * The exponent of the maximum allowable truncation error for + * {exponent,mantissa}_ref_viewing_distance as given by 2<sup>^(-prec_ref_viewing_dist)</sup>. + * The value of prec_ref_viewing_dist shall be in the range of 0 to 31, inclusive. + */ + uint8_t prec_ref_viewing_dist; + + /** + * The number of reference displays that are signalled in this struct. + * Allowed range is 1 to 32, inclusive. + */ + uint8_t num_ref_displays; + + /** + * Offset in bytes from the beginning of this structure at which the array + * of reference displays starts. + */ + size_t entries_offset; + + /** + * Size of each entry in bytes. May not match sizeof(AV3DReferenceDisplay). + */ + size_t entry_size; +} AV3DReferenceDisplaysInfo; + +/** + * Data structure for single deference display information. + * It is allocated as a part of AV3DReferenceDisplaysInfo and should be retrieved with + * av_tdrdi_get_display(). + * + * sizeof(AV3DReferenceDisplay) is not a part of the ABI and new fields may be + * added to it. +*/ +typedef struct AV3DReferenceDisplay { + /** + * The ViewId of the left view of a stereo pair corresponding to the n-th reference display. + */ + uint16_t left_view_id; + + /** + * The ViewId of the left view of a stereo pair corresponding to the n-th reference display. + */ + uint16_t right_view_id; + + /** + * The exponent part of the reference display width of the n-th reference display. + */ + uint8_t exponent_ref_display_width; + + /** + * The mantissa part of the reference display width of the n-th reference display. + */ + uint8_t mantissa_ref_display_width; + + /** + * Tthe exponent part of the reference viewing distance of the n-th reference display. + */ + uint8_t exponent_ref_viewing_distance; + + /** + * The mantissa part of the reference viewing distance of the n-th reference display. + */ + uint8_t mantissa_ref_viewing_distance; + + /** + * An array of flags to indicates that the information about additional horizontal shift of + * the left and right views for the n-th reference display is present. + */ + uint8_t additional_shift_present_flag; + + /** + * The recommended additional horizontal shift for a stereo pair corresponding to the n-th + * reference baseline and the n-th reference display. + */ + int16_t num_sample_shift; +} AV3DReferenceDisplay; + +static av_always_inline AV3DReferenceDisplay* +av_tdrdi_get_display(AV3DReferenceDisplaysInfo *tdrdi, unsigned int idx) +{ + av_assert0(idx < tdrdi->num_ref_displays); + return (AV3DReferenceDisplay *)((uint8_t *)tdrdi + tdrdi->entries_offset + + idx * tdrdi->entry_size); +} + +/** + * Allocate a AV3DReferenceDisplaysInfo structure and initialize its fields to default + * values. + * + * @return the newly allocated struct or NULL on failure + */ +AV3DReferenceDisplaysInfo *av_tdrdi_alloc(unsigned int nb_displays, size_t *size); + +/** + * @} + */ + +#endif /* AVUTIL_TDRDI_H */
I don't like that you add another allocator for this; instead we should add a generic allocator for the frame side-data types. - Andreas
On 6/8/2025 11:29 AM, Andreas Rheinhardt wrote:
Timo Rothenpieler:
From: James Almer <jamrial@gmail.com>
As defined in section G.14.3.2.3 of ITU-T H.265, it's required for proper signaling of MV-HEVC.
Signed-off-by: James Almer <jamrial@gmail.com> --- libavutil/Makefile | 2 + libavutil/tdrdi.c | 51 ++++++++++++++ libavutil/tdrdi.h | 164 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 217 insertions(+) create mode 100644 libavutil/tdrdi.c create mode 100644 libavutil/tdrdi.h
diff --git a/libavutil/Makefile b/libavutil/Makefile index 9ef118016b..94a56bb72f 100644 --- a/libavutil/Makefile +++ b/libavutil/Makefile @@ -85,6 +85,7 @@ HEADERS = adler32.h \ sha512.h \ spherical.h \ stereo3d.h \ + tdrdi.h \ threadmessage.h \ time.h \ timecode.h \ @@ -179,6 +180,7 @@ OBJS = adler32.o \ slicethread.o \ spherical.o \ stereo3d.o \ + tdrdi.o \ threadmessage.o \ time.o \ timecode.o \ diff --git a/libavutil/tdrdi.c b/libavutil/tdrdi.c new file mode 100644 index 0000000000..26192a1d2f --- /dev/null +++ b/libavutil/tdrdi.c @@ -0,0 +1,51 @@ +/* + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * FFmpeg is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with FFmpeg; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include <stddef.h> +#include <stdint.h> + +#include "mem.h" +#include "tdrdi.h" + +AV3DReferenceDisplaysInfo *av_tdrdi_alloc(unsigned int nb_displays, size_t *out_size) +{ + struct TestStruct { + AV3DReferenceDisplaysInfo p; + AV3DReferenceDisplay b; + }; + const size_t entries_offset = offsetof(struct TestStruct, b); + size_t size = entries_offset; + AV3DReferenceDisplaysInfo *tdrdi; + + if (nb_displays > (SIZE_MAX - size) / sizeof(AV3DReferenceDisplay)) + return NULL; + size += sizeof(AV3DReferenceDisplay) * nb_displays; + + tdrdi = av_mallocz(size); + if (!tdrdi) + return NULL; + + tdrdi->num_ref_displays = nb_displays; + tdrdi->entry_size = sizeof(AV3DReferenceDisplay); + tdrdi->entries_offset = entries_offset; + + if (out_size) + *out_size = size; + + return tdrdi; +} diff --git a/libavutil/tdrdi.h b/libavutil/tdrdi.h new file mode 100644 index 0000000000..8dcca42c7b --- /dev/null +++ b/libavutil/tdrdi.h @@ -0,0 +1,164 @@ +/* + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * FFmpeg is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with FFmpeg; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +/** + * @file + * @ingroup lavu_video_3d_reference_displays_info + * Spherical video + */ + +#ifndef AVUTIL_TDRDI_H +#define AVUTIL_TDRDI_H + +#include <stddef.h> +#include <stdint.h> + +#include "libavutil/avassert.h" + +/** + * @defgroup lavu_video_3d_reference_displays_info 3D Reference Displays Information + * @ingroup lavu_video + * + * The 3D Reference Displays Information describes information about the reference display + * width(s) and reference viewing distance(s) as well as information about the corresponding + * reference stereo pair(s). + * @{ + */ + +#define AV_TDRDI_MAX_NUM_REF_DISPLAY 32 + +/** + * This structure describes information about the reference display width(s) and reference + * viewing distance(s) as well as information about the corresponding reference stereo pair(s). + * See section G.14.3.2.3 of ITU-T H.265 for more information. + * + * @note The struct must be allocated with av_tdrdi_alloc() and + * its size is not a part of the public ABI. + */ +typedef struct AV3DReferenceDisplaysInfo { + /** + * The exponent of the maximum allowable truncation error for + * {exponent,mantissa}_ref_display_width as given by 2<sup>(-prec_ref_display_width)</sup>. + */ + uint8_t prec_ref_display_width; + + /** + * A flag to indicate the presence of reference viewing distance. + * If false, the values of prec_ref_viewing_dist, exponent_ref_viewing_distance, + * and mantissa_ref_viewing_distance are undefined. + */ + uint8_t ref_viewing_distance_flag; + + /** + * The exponent of the maximum allowable truncation error for + * {exponent,mantissa}_ref_viewing_distance as given by 2<sup>^(-prec_ref_viewing_dist)</sup>. + * The value of prec_ref_viewing_dist shall be in the range of 0 to 31, inclusive. + */ + uint8_t prec_ref_viewing_dist; + + /** + * The number of reference displays that are signalled in this struct. + * Allowed range is 1 to 32, inclusive. + */ + uint8_t num_ref_displays; + + /** + * Offset in bytes from the beginning of this structure at which the array + * of reference displays starts. + */ + size_t entries_offset; + + /** + * Size of each entry in bytes. May not match sizeof(AV3DReferenceDisplay). + */ + size_t entry_size; +} AV3DReferenceDisplaysInfo; + +/** + * Data structure for single deference display information. + * It is allocated as a part of AV3DReferenceDisplaysInfo and should be retrieved with + * av_tdrdi_get_display(). + * + * sizeof(AV3DReferenceDisplay) is not a part of the ABI and new fields may be + * added to it. +*/ +typedef struct AV3DReferenceDisplay { + /** + * The ViewId of the left view of a stereo pair corresponding to the n-th reference display. + */ + uint16_t left_view_id; + + /** + * The ViewId of the left view of a stereo pair corresponding to the n-th reference display. + */ + uint16_t right_view_id; + + /** + * The exponent part of the reference display width of the n-th reference display. + */ + uint8_t exponent_ref_display_width; + + /** + * The mantissa part of the reference display width of the n-th reference display. + */ + uint8_t mantissa_ref_display_width; + + /** + * Tthe exponent part of the reference viewing distance of the n-th reference display. + */ + uint8_t exponent_ref_viewing_distance; + + /** + * The mantissa part of the reference viewing distance of the n-th reference display. + */ + uint8_t mantissa_ref_viewing_distance; + + /** + * An array of flags to indicates that the information about additional horizontal shift of + * the left and right views for the n-th reference display is present. + */ + uint8_t additional_shift_present_flag; + + /** + * The recommended additional horizontal shift for a stereo pair corresponding to the n-th + * reference baseline and the n-th reference display. + */ + int16_t num_sample_shift; +} AV3DReferenceDisplay; + +static av_always_inline AV3DReferenceDisplay* +av_tdrdi_get_display(AV3DReferenceDisplaysInfo *tdrdi, unsigned int idx) +{ + av_assert0(idx < tdrdi->num_ref_displays); + return (AV3DReferenceDisplay *)((uint8_t *)tdrdi + tdrdi->entries_offset + + idx * tdrdi->entry_size); +} + +/** + * Allocate a AV3DReferenceDisplaysInfo structure and initialize its fields to default + * values. + * + * @return the newly allocated struct or NULL on failure + */ +AV3DReferenceDisplaysInfo *av_tdrdi_alloc(unsigned int nb_displays, size_t *size); + +/** + * @} + */ + +#endif /* AVUTIL_TDRDI_H */
I don't like that you add another allocator for this; instead we should add a generic allocator for the frame side-data types.
Wont work for packet side data. And i purposely didn't add yet another allocator that inserts the result into a frame, like there's in so many other modules, because eventually the generic one would be introduced. You said you wanted to take over my work on the generic allocator, but not sure if you did anything with it. The core issue was handling more complex types that didn't just have an extra nb_blocks argument.
On 08.06.2025 17:45, James Almer wrote:
On 6/8/2025 11:29 AM, Andreas Rheinhardt wrote:
Timo Rothenpieler:
From: James Almer <jamrial@gmail.com>
I don't like that you add another allocator for this; instead we should add a generic allocator for the frame side-data types.
Wont work for packet side data. And i purposely didn't add yet another allocator that inserts the result into a frame, like there's in so many other modules, because eventually the generic one would be introduced.
You said you wanted to take over my work on the generic allocator, but not sure if you did anything with it. The core issue was handling more complex types that didn't just have an extra nb_blocks argument.
So, what is the conclusion here? I'd like to push this set if you can come to an agreement. I haven't looked into it much, but implementing av_tdrdi_alloc() in a generic way does seem a bit hacky. And other types might need even more info for the allocation.
On 6/9/2025 5:59 PM, Timo Rothenpieler wrote:
On 08.06.2025 17:45, James Almer wrote:
On 6/8/2025 11:29 AM, Andreas Rheinhardt wrote:
Timo Rothenpieler:
From: James Almer <jamrial@gmail.com>
I don't like that you add another allocator for this; instead we should add a generic allocator for the frame side-data types.
Wont work for packet side data. And i purposely didn't add yet another allocator that inserts the result into a frame, like there's in so many other modules, because eventually the generic one would be introduced.
You said you wanted to take over my work on the generic allocator, but not sure if you did anything with it. The core issue was handling more complex types that didn't just have an extra nb_blocks argument.
So, what is the conclusion here? I'd like to push this set if you can come to an agreement.
I haven't looked into it much, but implementing av_tdrdi_alloc() in a generic way does seem a bit hacky. And other types might need even more info for the allocation.
The set LGTM. A custom frame side data allocator does not imply an allocator is required for this struct. Frames are not the only user.
James Almer:
On 6/9/2025 5:59 PM, Timo Rothenpieler wrote:
On 08.06.2025 17:45, James Almer wrote:
On 6/8/2025 11:29 AM, Andreas Rheinhardt wrote:
Timo Rothenpieler:
From: James Almer <jamrial@gmail.com>
I don't like that you add another allocator for this; instead we should add a generic allocator for the frame side-data types.
Wont work for packet side data. And i purposely didn't add yet another allocator that inserts the result into a frame, like there's in so many other modules, because eventually the generic one would be introduced.
You said you wanted to take over my work on the generic allocator, but not sure if you did anything with it. The core issue was handling more complex types that didn't just have an extra nb_blocks argument.
So, what is the conclusion here? I'd like to push this set if you can come to an agreement.
I haven't looked into it much, but implementing av_tdrdi_alloc() in a generic way does seem a bit hacky. And other types might need even more info for the allocation.
The set LGTM. A custom frame side data allocator does not imply an allocator is required for this struct. Frames are not the only user.
I was thinking about a generic side data allocator (for the AVFrameSideDataType stuff, but it is not meant to be AVFrame specific), so that it can be used by more than just AVFrames. Will write one tomorrow. - Andreas
On 10/06/2025 00:09, Andreas Rheinhardt wrote:
James Almer:
On 6/9/2025 5:59 PM, Timo Rothenpieler wrote:
On 08.06.2025 17:45, James Almer wrote:
On 6/8/2025 11:29 AM, Andreas Rheinhardt wrote:
Timo Rothenpieler:
From: James Almer <jamrial@gmail.com>
I don't like that you add another allocator for this; instead we should add a generic allocator for the frame side-data types.
Wont work for packet side data. And i purposely didn't add yet another allocator that inserts the result into a frame, like there's in so many other modules, because eventually the generic one would be introduced.
You said you wanted to take over my work on the generic allocator, but not sure if you did anything with it. The core issue was handling more complex types that didn't just have an extra nb_blocks argument.
So, what is the conclusion here? I'd like to push this set if you can come to an agreement.
I haven't looked into it much, but implementing av_tdrdi_alloc() in a generic way does seem a bit hacky. And other types might need even more info for the allocation.
The set LGTM. A custom frame side data allocator does not imply an allocator is required for this struct. Frames are not the only user.
I was thinking about a generic side data allocator (for the AVFrameSideDataType stuff, but it is not meant to be AVFrame specific), so that it can be used by more than just AVFrames. Will write one tomorrow.
Would you be okay to for now pushing this with the API turned into an internal one, so the public API can then be decided and implemented later?
On 13/06/2025 16:07, Timo Rothenpieler wrote:
On 10/06/2025 00:09, Andreas Rheinhardt wrote:
James Almer:
On 6/9/2025 5:59 PM, Timo Rothenpieler wrote:
On 08.06.2025 17:45, James Almer wrote:
On 6/8/2025 11:29 AM, Andreas Rheinhardt wrote:
Timo Rothenpieler: > From: James Almer <jamrial@gmail.com> > I don't like that you add another allocator for this; instead we should add a generic allocator for the frame side-data types.
Wont work for packet side data. And i purposely didn't add yet another allocator that inserts the result into a frame, like there's in so many other modules, because eventually the generic one would be introduced.
You said you wanted to take over my work on the generic allocator, but not sure if you did anything with it. The core issue was handling more complex types that didn't just have an extra nb_blocks argument.
So, what is the conclusion here? I'd like to push this set if you can come to an agreement.
I haven't looked into it much, but implementing av_tdrdi_alloc() in a generic way does seem a bit hacky. And other types might need even more info for the allocation.
The set LGTM. A custom frame side data allocator does not imply an allocator is required for this struct. Frames are not the only user.
I was thinking about a generic side data allocator (for the AVFrameSideDataType stuff, but it is not meant to be AVFrame specific), so that it can be used by more than just AVFrames. Will write one tomorrow.
Would you be okay to for now pushing this with the API turned into an internal one, so the public API can then be decided and implemented later?
Will apply soon with the new public APIs turned into an ff_ symbol.
On 6/16/2025 9:38 AM, Timo Rothenpieler wrote:
On 13/06/2025 16:07, Timo Rothenpieler wrote:
On 10/06/2025 00:09, Andreas Rheinhardt wrote:
James Almer:
On 6/9/2025 5:59 PM, Timo Rothenpieler wrote:
On 08.06.2025 17:45, James Almer wrote:
On 6/8/2025 11:29 AM, Andreas Rheinhardt wrote: > Timo Rothenpieler: >> From: James Almer <jamrial@gmail.com> >> > I don't like that you add another allocator for this; instead we > should > add a generic allocator for the frame side-data types.
Wont work for packet side data. And i purposely didn't add yet another allocator that inserts the result into a frame, like there's in so many other modules, because eventually the generic one would be introduced.
You said you wanted to take over my work on the generic allocator, but not sure if you did anything with it. The core issue was handling more complex types that didn't just have an extra nb_blocks argument.
So, what is the conclusion here? I'd like to push this set if you can come to an agreement.
I haven't looked into it much, but implementing av_tdrdi_alloc() in a generic way does seem a bit hacky. And other types might need even more info for the allocation.
The set LGTM. A custom frame side data allocator does not imply an allocator is required for this struct. Frames are not the only user.
I was thinking about a generic side data allocator (for the AVFrameSideDataType stuff, but it is not meant to be AVFrame specific), so that it can be used by more than just AVFrames. Will write one tomorrow.
Would you be okay to for now pushing this with the API turned into an internal one, so the public API can then be decided and implemented later?
Will apply soon with the new public APIs turned into an ff_ symbol. No, the allocator is required, and an internal function like you suggest would need a separate internal header, and will make the struct virtually unusable outside lavu for non frame side data users.
On 16.06.2025 14:55, James Almer wrote:
On 6/16/2025 9:38 AM, Timo Rothenpieler wrote:
On 13/06/2025 16:07, Timo Rothenpieler wrote:
On 10/06/2025 00:09, Andreas Rheinhardt wrote:
James Almer:
On 6/9/2025 5:59 PM, Timo Rothenpieler wrote:
On 08.06.2025 17:45, James Almer wrote: > On 6/8/2025 11:29 AM, Andreas Rheinhardt wrote: >> Timo Rothenpieler: >>> From: James Almer <jamrial@gmail.com> >>> >> I don't like that you add another allocator for this; instead we >> should >> add a generic allocator for the frame side-data types. > > Wont work for packet side data. And i purposely didn't add yet > another allocator that inserts the result into a frame, like there's > in so many other modules, because eventually the generic one > would be > introduced. > > You said you wanted to take over my work on the generic allocator, > but not sure if you did anything with it. The core issue was > handling > more complex types that didn't just have an extra nb_blocks > argument.
So, what is the conclusion here? I'd like to push this set if you can come to an agreement.
I haven't looked into it much, but implementing av_tdrdi_alloc() in a generic way does seem a bit hacky. And other types might need even more info for the allocation.
The set LGTM. A custom frame side data allocator does not imply an allocator is required for this struct. Frames are not the only user.
I was thinking about a generic side data allocator (for the AVFrameSideDataType stuff, but it is not meant to be AVFrame specific), so that it can be used by more than just AVFrames. Will write one tomorrow.
Would you be okay to for now pushing this with the API turned into an internal one, so the public API can then be decided and implemented later?
Will apply soon with the new public APIs turned into an ff_ symbol. No, the allocator is required, and an internal function like you suggest would need a separate internal header, and will make the struct virtually unusable outside lavu for non frame side data users.
It's only meant as a temporary measure until the final public API is figured out, so the rest can move on.
On 6/16/2025 2:26 PM, Timo Rothenpieler wrote:
On 16.06.2025 14:55, James Almer wrote:
On 6/16/2025 9:38 AM, Timo Rothenpieler wrote:
On 13/06/2025 16:07, Timo Rothenpieler wrote:
On 10/06/2025 00:09, Andreas Rheinhardt wrote:
James Almer:
On 6/9/2025 5:59 PM, Timo Rothenpieler wrote: > On 08.06.2025 17:45, James Almer wrote: >> On 6/8/2025 11:29 AM, Andreas Rheinhardt wrote: >>> Timo Rothenpieler: >>>> From: James Almer <jamrial@gmail.com> >>>> >>> I don't like that you add another allocator for this; instead >>> we should >>> add a generic allocator for the frame side-data types. >> >> Wont work for packet side data. And i purposely didn't add yet >> another allocator that inserts the result into a frame, like >> there's >> in so many other modules, because eventually the generic one >> would be >> introduced. >> >> You said you wanted to take over my work on the generic allocator, >> but not sure if you did anything with it. The core issue was >> handling >> more complex types that didn't just have an extra nb_blocks >> argument. > > So, what is the conclusion here? > I'd like to push this set if you can come to an agreement. > > I haven't looked into it much, but implementing av_tdrdi_alloc() > in a > generic way does seem a bit hacky. And other types might need even > more info for the allocation.
The set LGTM. A custom frame side data allocator does not imply an allocator is required for this struct. Frames are not the only user.
I was thinking about a generic side data allocator (for the AVFrameSideDataType stuff, but it is not meant to be AVFrame specific), so that it can be used by more than just AVFrames. Will write one tomorrow.
Would you be okay to for now pushing this with the API turned into an internal one, so the public API can then be decided and implemented later?
Will apply soon with the new public APIs turned into an ff_ symbol. No, the allocator is required, and an internal function like you suggest would need a separate internal header, and will make the struct virtually unusable outside lavu for non frame side data users.
It's only meant as a temporary measure until the final public API is figured out, so the rest can move on.
Again, what's the point? I'm not trying to block the set, i'm trying to get it pushed as is because the struct needs its own, standalone allocator. Whatever we do with frame side data is out of scope for this set and should not affect it.
On 16.06.2025 14:55, James Almer wrote:
On 6/16/2025 9:38 AM, Timo Rothenpieler wrote:
On 13/06/2025 16:07, Timo Rothenpieler wrote:
On 10/06/2025 00:09, Andreas Rheinhardt wrote:
James Almer:
On 6/9/2025 5:59 PM, Timo Rothenpieler wrote: > On 08.06.2025 17:45, James Almer wrote: >> On 6/8/2025 11:29 AM, Andreas Rheinhardt wrote: >>> Timo Rothenpieler: >>>> From: James Almer <jamrial@gmail.com> >>>> >>> I don't like that you add another allocator for this; instead >>> we should >>> add a generic allocator for the frame side-data types. >> >> Wont work for packet side data. And i purposely didn't add yet >> another allocator that inserts the result into a frame, like >> there's >> in so many other modules, because eventually the generic one >> would be >> introduced. >> >> You said you wanted to take over my work on the generic allocator, >> but not sure if you did anything with it. The core issue was >> handling >> more complex types that didn't just have an extra nb_blocks >> argument. > > So, what is the conclusion here? > I'd like to push this set if you can come to an agreement. > > I haven't looked into it much, but implementing av_tdrdi_alloc() > in a > generic way does seem a bit hacky. And other types might need even > more info for the allocation.
The set LGTM. A custom frame side data allocator does not imply an allocator is required for this struct. Frames are not the only user.
I was thinking about a generic side data allocator (for the AVFrameSideDataType stuff, but it is not meant to be AVFrame specific), so that it can be used by more than just AVFrames. Will write one tomorrow.
Would you be okay to for now pushing this with the API turned into an internal one, so the public API can then be decided and implemented later?
Will apply soon with the new public APIs turned into an ff_ symbol. No, the allocator is required, and an internal function like you suggest would need a separate internal header, and will make the struct virtually unusable outside lavu for non frame side data users.
It's only meant as a temporary measure until the final public API is figured out, so the rest can move on. Sorry for being an asshole and forgetting about this. Will really send a
Timo Rothenpieler: patch for what I intend tomorrow. - Andreas
On 16.06.2025 20:26, Andreas Rheinhardt wrote:
On 16.06.2025 14:55, James Almer wrote:
On 6/16/2025 9:38 AM, Timo Rothenpieler wrote:
On 13/06/2025 16:07, Timo Rothenpieler wrote:
On 10/06/2025 00:09, Andreas Rheinhardt wrote:
James Almer: > On 6/9/2025 5:59 PM, Timo Rothenpieler wrote: >> On 08.06.2025 17:45, James Almer wrote: >>> On 6/8/2025 11:29 AM, Andreas Rheinhardt wrote: >>>> Timo Rothenpieler: >>>>> From: James Almer <jamrial@gmail.com> >>>>> >>>> I don't like that you add another allocator for this; instead >>>> we should >>>> add a generic allocator for the frame side-data types. >>> >>> Wont work for packet side data. And i purposely didn't add yet >>> another allocator that inserts the result into a frame, like >>> there's >>> in so many other modules, because eventually the generic one >>> would be >>> introduced. >>> >>> You said you wanted to take over my work on the generic allocator, >>> but not sure if you did anything with it. The core issue was >>> handling >>> more complex types that didn't just have an extra nb_blocks >>> argument. >> >> So, what is the conclusion here? >> I'd like to push this set if you can come to an agreement. >> >> I haven't looked into it much, but implementing av_tdrdi_alloc() >> in a >> generic way does seem a bit hacky. And other types might need even >> more info for the allocation. > > The set LGTM. A custom frame side data allocator does not imply an > allocator is required for this struct. Frames are not the only user. >
I was thinking about a generic side data allocator (for the AVFrameSideDataType stuff, but it is not meant to be AVFrame specific), so that it can be used by more than just AVFrames. Will write one tomorrow.
Would you be okay to for now pushing this with the API turned into an internal one, so the public API can then be decided and implemented later?
Will apply soon with the new public APIs turned into an ff_ symbol. No, the allocator is required, and an internal function like you suggest would need a separate internal header, and will make the struct virtually unusable outside lavu for non frame side data users.
It's only meant as a temporary measure until the final public API is figured out, so the rest can move on. Sorry for being an asshole and forgetting about this. Will really send a
Timo Rothenpieler: patch for what I intend tomorrow.
ping Or did I simply miss the patch?
On 23.06.2025 17:44, Timo Rothenpieler wrote:
On 16.06.2025 20:26, Andreas Rheinhardt wrote:
On 16.06.2025 14:55, James Almer wrote:
On 6/16/2025 9:38 AM, Timo Rothenpieler wrote:
On 13/06/2025 16:07, Timo Rothenpieler wrote:
On 10/06/2025 00:09, Andreas Rheinhardt wrote: > James Almer: >> On 6/9/2025 5:59 PM, Timo Rothenpieler wrote: >>> On 08.06.2025 17:45, James Almer wrote: >>>> On 6/8/2025 11:29 AM, Andreas Rheinhardt wrote: >>>>> Timo Rothenpieler: >>>>>> From: James Almer <jamrial@gmail.com> >>>>>> >>>>> I don't like that you add another allocator for this; instead >>>>> we should >>>>> add a generic allocator for the frame side-data types. >>>> >>>> Wont work for packet side data. And i purposely didn't add yet >>>> another allocator that inserts the result into a frame, like >>>> there's >>>> in so many other modules, because eventually the generic one >>>> would be >>>> introduced. >>>> >>>> You said you wanted to take over my work on the generic >>>> allocator, >>>> but not sure if you did anything with it. The core issue was >>>> handling >>>> more complex types that didn't just have an extra nb_blocks >>>> argument. >>> >>> So, what is the conclusion here? >>> I'd like to push this set if you can come to an agreement. >>> >>> I haven't looked into it much, but implementing av_tdrdi_alloc() >>> in a >>> generic way does seem a bit hacky. And other types might need even >>> more info for the allocation. >> >> The set LGTM. A custom frame side data allocator does not imply an >> allocator is required for this struct. Frames are not the only >> user. >> > > I was thinking about a generic side data allocator (for the > AVFrameSideDataType stuff, but it is not meant to be AVFrame > specific), > so that it can be used by more than just AVFrames. Will write one > tomorrow.
Would you be okay to for now pushing this with the API turned into an internal one, so the public API can then be decided and implemented later?
Will apply soon with the new public APIs turned into an ff_ symbol. No, the allocator is required, and an internal function like you suggest would need a separate internal header, and will make the struct virtually unusable outside lavu for non frame side data users.
It's only meant as a temporary measure until the final public API is figured out, so the rest can move on. Sorry for being an asshole and forgetting about this. Will really send a
Timo Rothenpieler: patch for what I intend tomorrow.
ping
Or did I simply miss the patch?
ping
On 7/1/2025 3:49 PM, Timo Rothenpieler wrote:
On 23.06.2025 17:44, Timo Rothenpieler wrote:
On 16.06.2025 20:26, Andreas Rheinhardt wrote:
On 16.06.2025 14:55, James Almer wrote:
On 6/16/2025 9:38 AM, Timo Rothenpieler wrote:
On 13/06/2025 16:07, Timo Rothenpieler wrote: > On 10/06/2025 00:09, Andreas Rheinhardt wrote: >> James Almer: >>> On 6/9/2025 5:59 PM, Timo Rothenpieler wrote: >>>> On 08.06.2025 17:45, James Almer wrote: >>>>> On 6/8/2025 11:29 AM, Andreas Rheinhardt wrote: >>>>>> Timo Rothenpieler: >>>>>>> From: James Almer <jamrial@gmail.com> >>>>>>> >>>>>> I don't like that you add another allocator for this; instead >>>>>> we should >>>>>> add a generic allocator for the frame side-data types. >>>>> >>>>> Wont work for packet side data. And i purposely didn't add yet >>>>> another allocator that inserts the result into a frame, like >>>>> there's >>>>> in so many other modules, because eventually the generic one >>>>> would be >>>>> introduced. >>>>> >>>>> You said you wanted to take over my work on the generic >>>>> allocator, >>>>> but not sure if you did anything with it. The core issue was >>>>> handling >>>>> more complex types that didn't just have an extra nb_blocks >>>>> argument. >>>> >>>> So, what is the conclusion here? >>>> I'd like to push this set if you can come to an agreement. >>>> >>>> I haven't looked into it much, but implementing av_tdrdi_alloc() >>>> in a >>>> generic way does seem a bit hacky. And other types might need >>>> even >>>> more info for the allocation. >>> >>> The set LGTM. A custom frame side data allocator does not imply an >>> allocator is required for this struct. Frames are not the only >>> user. >>> >> >> I was thinking about a generic side data allocator (for the >> AVFrameSideDataType stuff, but it is not meant to be AVFrame >> specific), >> so that it can be used by more than just AVFrames. Will write one >> tomorrow. > > Would you be okay to for now pushing this with the API turned into > an internal one, so the public API can then be decided and > implemented later?
Will apply soon with the new public APIs turned into an ff_ symbol. No, the allocator is required, and an internal function like you suggest would need a separate internal header, and will make the struct virtually unusable outside lavu for non frame side data users.
It's only meant as a temporary measure until the final public API is figured out, so the rest can move on. Sorry for being an asshole and forgetting about this. Will really send a
Timo Rothenpieler: patch for what I intend tomorrow.
ping
Or did I simply miss the patch?
ping
Please push it. I shouldn't repeat myself in that the struct needs its own allocator regardless of what we do with frame side data.
On 01.07.2025 22:37, James Almer wrote:
On 7/1/2025 3:49 PM, Timo Rothenpieler wrote:
On 23.06.2025 17:44, Timo Rothenpieler wrote:
On 16.06.2025 20:26, Andreas Rheinhardt wrote:
On 16.06.2025 14:55, James Almer wrote:
On 6/16/2025 9:38 AM, Timo Rothenpieler wrote: > On 13/06/2025 16:07, Timo Rothenpieler wrote: >> On 10/06/2025 00:09, Andreas Rheinhardt wrote: >>> James Almer: >>>> On 6/9/2025 5:59 PM, Timo Rothenpieler wrote: >>>>> On 08.06.2025 17:45, James Almer wrote: >>>>>> On 6/8/2025 11:29 AM, Andreas Rheinhardt wrote: >>>>>>> Timo Rothenpieler: >>>>>>>> From: James Almer <jamrial@gmail.com> >>>>>>>> >>>>>>> I don't like that you add another allocator for this; instead >>>>>>> we should >>>>>>> add a generic allocator for the frame side-data types. >>>>>> >>>>>> Wont work for packet side data. And i purposely didn't add yet >>>>>> another allocator that inserts the result into a frame, like >>>>>> there's >>>>>> in so many other modules, because eventually the generic one >>>>>> would be >>>>>> introduced. >>>>>> >>>>>> You said you wanted to take over my work on the generic >>>>>> allocator, >>>>>> but not sure if you did anything with it. The core issue was >>>>>> handling >>>>>> more complex types that didn't just have an extra nb_blocks >>>>>> argument. >>>>> >>>>> So, what is the conclusion here? >>>>> I'd like to push this set if you can come to an agreement. >>>>> >>>>> I haven't looked into it much, but implementing av_tdrdi_alloc() >>>>> in a >>>>> generic way does seem a bit hacky. And other types might need >>>>> even >>>>> more info for the allocation. >>>> >>>> The set LGTM. A custom frame side data allocator does not >>>> imply an >>>> allocator is required for this struct. Frames are not the only >>>> user. >>>> >>> >>> I was thinking about a generic side data allocator (for the >>> AVFrameSideDataType stuff, but it is not meant to be AVFrame >>> specific), >>> so that it can be used by more than just AVFrames. Will write one >>> tomorrow. >> >> Would you be okay to for now pushing this with the API turned into >> an internal one, so the public API can then be decided and >> implemented later? > > Will apply soon with the new public APIs turned into an ff_ symbol. No, the allocator is required, and an internal function like you suggest would need a separate internal header, and will make the struct virtually unusable outside lavu for non frame side data users.
It's only meant as a temporary measure until the final public API is figured out, so the rest can move on. Sorry for being an asshole and forgetting about this. Will really send a
Timo Rothenpieler: patch for what I intend tomorrow.
ping
Or did I simply miss the patch?
ping
Please push it. I shouldn't repeat myself in that the struct needs its own allocator regardless of what we do with frame side data.
sure, applied
On 6/9/2025 7:09 PM, Andreas Rheinhardt wrote:
James Almer:
On 6/9/2025 5:59 PM, Timo Rothenpieler wrote:
On 08.06.2025 17:45, James Almer wrote:
On 6/8/2025 11:29 AM, Andreas Rheinhardt wrote:
Timo Rothenpieler:
From: James Almer <jamrial@gmail.com>
I don't like that you add another allocator for this; instead we should add a generic allocator for the frame side-data types.
Wont work for packet side data. And i purposely didn't add yet another allocator that inserts the result into a frame, like there's in so many other modules, because eventually the generic one would be introduced.
You said you wanted to take over my work on the generic allocator, but not sure if you did anything with it. The core issue was handling more complex types that didn't just have an extra nb_blocks argument.
So, what is the conclusion here? I'd like to push this set if you can come to an agreement.
I haven't looked into it much, but implementing av_tdrdi_alloc() in a generic way does seem a bit hacky. And other types might need even more info for the allocation.
The set LGTM. A custom frame side data allocator does not imply an allocator is required for this struct. Frames are not the only user.
Meant to say "isn't" here.
I was thinking about a generic side data allocator (for the AVFrameSideDataType stuff, but it is not meant to be AVFrame specific), so that it can be used by more than just AVFrames. Will write one tomorrow.
- Andreas
Frame side data, be it inside frames or not, is not the only user. An allocator for this struct is needed regardless of what we do with that.
participants (3)
-
Andreas Rheinhardt -
James Almer -
Timo Rothenpieler