PR #24643 opened by Daniel Campos Ramos (danielcamposramos) URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24643 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24643.patch
# Summary of changes `ff_h2645_sei_to_frame()` attached the frame packing arrangement as Stereo 3D side data unconditionally. When the container also signals a layout (a Matroska `StereoMode` element, for example), `ff_decode_frame_props()` has already attached that value, so the frame carries two Stereo 3D entries and the ffmpeg CLI aborts in the filtergraph setup: ``` [vf#0:0] Task finished with error: File exists ``` This adds `ff_decode_stereo3d_new()`, mirroring the mastering display and content light helpers, and uses it for the frame packing SEI. By default the coded SEI replaces the container value; with `-side_data_prefer_packet stereo3d` the container value is kept. H.264, HEVC and VVC share this path. The second commit only reindents. Fixes #24530. ## How it was found Our practice is to rerun the original reproducers after every patch. Doing that after #24628 (frame packing persistence) showed the persistence change makes this duplicate appear on every frame, instead of only on the frames that carry the SEI. The two PRs are independent, apply in either order, and were tested stacked. ## Verification 6 KB reproducer: the #24628 FATE sample remuxed with `mkvmerge --stereo-mode 0:top_bottom_left_first`, so the SEI says side by side and the container says top and bottom. | Build | Frame 0 Stereo 3D entries | `ffmpeg -i f -f null -` | |---|---|---| | master `cb5a637` | top and bottom, side by side | aborts, EEXIST | | this PR, default | side by side | ok | | this PR, `-side_data_prefer_packet stereo3d` | top and bottom | ok | | this PR stacked on #24628, default | side by side on every frame | ok | - `fate-h264-frame-packing-persistence` and `fate-hevc-frame-packing-persistence`: pass with both PRs stacked - `git diff --check`: pass; `git diff -w` on the reindent commit: empty - `tools/patcheck`: only the usual false positives (forward struct declaration, doc comment, existing if/else style moved by the reindent) - Built in a debian:trixie container, gcc 14.2.0 No FATE test is added here: the natural fixture is the #24628 sample with a container tag, and I am happy to add one once that sample is in fate-samples. ## Related - #24429: the same `EEXIST` failure class, for duplicate HDR side data in qsvdec. - #24531: inserting the SEI into a file that already carries a container tag produces exactly this combination. - [mpv #18490](https://github.com/mpv-player/mpv/pull/18490): the player side, where the in-band layout also takes precedence over the container tag. ## Collaboration provenance Daniel Campos Ramos directed the work, verified the results and is responsible for the submission. Claude Opus 5.5, running inside Claude CLI, reproduced the bug on master, wrote the helper and the call site, and ran the tests above. >From 74a81bae7e4d6246f9c2780208ae88f8f5485f0c Mon Sep 17 00:00:00 2001 From: Daniel Campos Ramos <[email protected]> Date: Wed, 23 Sep 2026 00:24:13 -0300 Subject: [PATCH 1/2] avcodec/h2645_sei: respect side data preference for frame packing ff_h2645_sei_to_frame() added the frame packing arrangement as Stereo 3D side data unconditionally. When the container also signals a layout, for example a Matroska StereoMode element, ff_decode_frame_props() has already attached that as Stereo 3D side data, so the frame ends up with two entries. The ffmpeg CLI then aborts in the filtergraph setup with AVERROR(EEXIST): [vf#0:0] Task finished with error: File exists Add ff_decode_stereo3d_new(), mirroring the mastering display and content light helpers, and use it here. By default the coded SEI now replaces the container value; with -side_data_prefer_packet stereo3d the container value is kept instead. This is the same class of failure as the duplicate HDR side data fixed in qsvdec (PR #24429). Fixes: https://code.ffmpeg.org/FFmpeg/FFmpeg/issues/24530 Signed-off-by: Daniel Campos Ramos <[email protected]> --- libavcodec/decode.c | 13 +++++++++++++ libavcodec/decode.h | 10 ++++++++++ libavcodec/h2645_sei.c | 9 ++++++--- 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/libavcodec/decode.c b/libavcodec/decode.c index 94dfc06a84..18b4cb8d12 100644 --- a/libavcodec/decode.c +++ b/libavcodec/decode.c @@ -2322,6 +2322,19 @@ int ff_decode_content_light_new(const AVCodecContext *avctx, AVFrame *frame, return *clm ? 0 : AVERROR(ENOMEM); } +int ff_decode_stereo3d_new(const AVCodecContext *avctx, AVFrame *frame, + AVStereo3D **stereo) +{ + if (side_data_pref(avctx, &frame->side_data, &frame->nb_side_data, + AV_FRAME_DATA_STEREO3D)) { + *stereo = NULL; + return 0; + } + + *stereo = av_stereo3d_create_side_data(frame); + return *stereo ? 0 : AVERROR(ENOMEM); +} + int ff_copy_palette(void *dst, const AVPacket *src, void *logctx) { size_t size; diff --git a/libavcodec/decode.h b/libavcodec/decode.h index 42f5e1e9df..05fef40f0c 100644 --- a/libavcodec/decode.h +++ b/libavcodec/decode.h @@ -206,6 +206,7 @@ int ff_frame_new_side_data_from_buf_ext(const AVCodecContext *avctx, struct AVMasteringDisplayMetadata; struct AVContentLightMetadata; +struct AVStereo3D; /** * Wrapper around av_mastering_display_metadata_create_side_data(), which @@ -241,4 +242,13 @@ int ff_decode_content_light_new_ext(const AVCodecContext *avctx, AVFrameSideData ***sd, int *nb_sd, struct AVContentLightMetadata **clm); +/** + * Wrapper around av_stereo3d_create_side_data(), which rejects side data + * overridden by the demuxer. Returns 0 on success, and a negative error code + * otherwise. If successful, *stereo may either be a pointer to the new side + * data, or NULL in case the side data was already present. + */ +int ff_decode_stereo3d_new(const AVCodecContext *avctx, AVFrame *frame, + struct AVStereo3D **stereo); + #endif /* AVCODEC_DECODE_H */ diff --git a/libavcodec/h2645_sei.c b/libavcodec/h2645_sei.c index 735199733c..868041f83b 100644 --- a/libavcodec/h2645_sei.c +++ b/libavcodec/h2645_sei.c @@ -529,11 +529,13 @@ int ff_h2645_sei_to_frame(AVFrame *frame, H2645SEI *sei, is_frame_packing_type_valid(fp->arrangement_type, codec_id) && fp->content_interpretation_type > 0 && fp->content_interpretation_type < 3) { - AVStereo3D *stereo = av_stereo3d_create_side_data(frame); + AVStereo3D *stereo; - if (!stereo) - return AVERROR(ENOMEM); + ret = ff_decode_stereo3d_new(avctx, frame, &stereo); + if (ret < 0) + return ret; + if (stereo) { switch (fp->arrangement_type) { #if CONFIG_H264_SEI case SEI_FPA_H264_TYPE_CHECKERBOARD: @@ -574,6 +576,7 @@ int ff_h2645_sei_to_frame(AVFrame *frame, H2645SEI *sei, else stereo->view = AV_STEREO3D_VIEW_RIGHT; } + } } if (sei->display_orientation.present && -- 2.52.0 >From 13c6a12a6a8127b02415adfc4935c683cbda8505 Mon Sep 17 00:00:00 2001 From: Daniel Campos Ramos <[email protected]> Date: Wed, 23 Sep 2026 00:24:19 -0300 Subject: [PATCH 2/2] avcodec/h2645_sei: reindent after the previous commit Signed-off-by: Daniel Campos Ramos <[email protected]> --- libavcodec/h2645_sei.c | 68 +++++++++++++++++++++--------------------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/libavcodec/h2645_sei.c b/libavcodec/h2645_sei.c index 868041f83b..f90cd53fcb 100644 --- a/libavcodec/h2645_sei.c +++ b/libavcodec/h2645_sei.c @@ -536,46 +536,46 @@ int ff_h2645_sei_to_frame(AVFrame *frame, H2645SEI *sei, return ret; if (stereo) { - switch (fp->arrangement_type) { + switch (fp->arrangement_type) { #if CONFIG_H264_SEI - case SEI_FPA_H264_TYPE_CHECKERBOARD: - stereo->type = AV_STEREO3D_CHECKERBOARD; - break; - case SEI_FPA_H264_TYPE_INTERLEAVE_COLUMN: - stereo->type = AV_STEREO3D_COLUMNS; - break; - case SEI_FPA_H264_TYPE_INTERLEAVE_ROW: - stereo->type = AV_STEREO3D_LINES; - break; + case SEI_FPA_H264_TYPE_CHECKERBOARD: + stereo->type = AV_STEREO3D_CHECKERBOARD; + break; + case SEI_FPA_H264_TYPE_INTERLEAVE_COLUMN: + stereo->type = AV_STEREO3D_COLUMNS; + break; + case SEI_FPA_H264_TYPE_INTERLEAVE_ROW: + stereo->type = AV_STEREO3D_LINES; + break; #endif - case SEI_FPA_TYPE_SIDE_BY_SIDE: - if (fp->quincunx_sampling_flag) - stereo->type = AV_STEREO3D_SIDEBYSIDE_QUINCUNX; - else - stereo->type = AV_STEREO3D_SIDEBYSIDE; - break; - case SEI_FPA_TYPE_TOP_BOTTOM: - stereo->type = AV_STEREO3D_TOPBOTTOM; - break; - case SEI_FPA_TYPE_INTERLEAVE_TEMPORAL: - stereo->type = AV_STEREO3D_FRAMESEQUENCE; - break; + case SEI_FPA_TYPE_SIDE_BY_SIDE: + if (fp->quincunx_sampling_flag) + stereo->type = AV_STEREO3D_SIDEBYSIDE_QUINCUNX; + else + stereo->type = AV_STEREO3D_SIDEBYSIDE; + break; + case SEI_FPA_TYPE_TOP_BOTTOM: + stereo->type = AV_STEREO3D_TOPBOTTOM; + break; + case SEI_FPA_TYPE_INTERLEAVE_TEMPORAL: + stereo->type = AV_STEREO3D_FRAMESEQUENCE; + break; #if CONFIG_H264_SEI - case SEI_FPA_H264_TYPE_2D: - stereo->type = AV_STEREO3D_2D; - break; + case SEI_FPA_H264_TYPE_2D: + stereo->type = AV_STEREO3D_2D; + break; #endif - } + } - if (fp->content_interpretation_type == 2) - stereo->flags = AV_STEREO3D_FLAG_INVERT; + if (fp->content_interpretation_type == 2) + stereo->flags = AV_STEREO3D_FLAG_INVERT; - if (fp->arrangement_type == SEI_FPA_TYPE_INTERLEAVE_TEMPORAL) { - if (fp->current_frame_is_frame0_flag) - stereo->view = AV_STEREO3D_VIEW_LEFT; - else - stereo->view = AV_STEREO3D_VIEW_RIGHT; - } + if (fp->arrangement_type == SEI_FPA_TYPE_INTERLEAVE_TEMPORAL) { + if (fp->current_frame_is_frame0_flag) + stereo->view = AV_STEREO3D_VIEW_LEFT; + else + stereo->view = AV_STEREO3D_VIEW_RIGHT; + } } } -- 2.52.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
