From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> To: ffmpeg-devel@ffmpeg.org Cc: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> Subject: [FFmpeg-devel] [PATCH 4/9] avformat/matroskaenc: Don't add side-data to input stream Date: Fri, 11 Aug 2023 12:43:23 +0200 Message-ID: <AS8P250MB07449DF6EF5776C7FE6C373F8F10A@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM> (raw) In-Reply-To: <AS8P250MB07441DD2024A768DE773EEFA8F10A@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM> When muxing, the AVStreams' side-data is typically set by the caller before avformat_write_header(); it is not documented to be else. Yet the Matroska muxer added an AVStereo3D side data if certain metadata was present: Since commit 4d686fb721b485ebbc4c7779d927d876c1e630f7 (adding support for AVStereo3D stream side-data), the Matroska muxer checked certain stream tags that contain Matroska's StereoMode and (if they are present) converted this value into an AVStereo3D struct that gets attached to the AVStream (reusing a function from the demuxer). Afterwards the AVStereo3D side data struct (whether it has just been added by the muxer or not) gets parsed and converted back into a Matroska StereoMode. Besides being an API violation this change broke StereoMode values without a corresponding AVStereo3D (namely the anaglyph ones). This commit fixes this: A StereoMode given via tags is now used-as-is; if no such tag exists and an AVStereo3D side data exists, it is converted into the corresponding StereoMode (if possible). The new STEREOMODE_STEREO3D_MAPPING has been put to good use for this. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- mkv_write_tracks() now no longer modifies the public AVStream at all any more. libavformat/matroskaenc.c | 125 +++++++++++++++++++------------------- 1 file changed, 63 insertions(+), 62 deletions(-) diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index 9bdf087c67..20d3af3b42 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -1580,95 +1580,96 @@ static void mkv_write_field_order(EbmlWriter *writer, int is_webm, #define MAX_STEREO_MODE_ELEMS 1 static int mkv_write_stereo_mode(AVFormatContext *s, EbmlWriter *writer, - AVStream *st, int is_webm, + const AVStream *st, int is_webm, int *h_width, int *h_height) { const AVDictionaryEntry *tag; MatroskaVideoStereoModeType format = MATROSKA_VIDEO_STEREOMODE_TYPE_NB; - const AVStereo3D *stereo; + + /* The following macros create bitfields where the ith bit + * indicates whether the MatroskaVideoStereoModeType with that value + * uses double width/height or is WebM compatible. */ +#define FLAG(STEREOMODETYPE, BOOL) | (BOOL) << (STEREOMODETYPE) +#define WDIV1(STEREOMODETYPE, STEREO3DTYPE, FLAGS, WDIV, HDIV, WEBM) \ + FLAG(STEREOMODETYPE, WDIV) +#define WDIV2(STEREOMODETYPE, WDIV, HDIV, WEBM) \ + FLAG(STEREOMODETYPE, WDIV) + // The zero in the following line consumes the first '|'. + const unsigned width_bitfield = 0 STEREOMODE_STEREO3D_MAPPING(WDIV1, WDIV2); + +#define HDIV1(STEREOMODETYPE, STEREO3DTYPE, FLAGS, WDIV, HDIV, WEBM) \ + FLAG(STEREOMODETYPE, HDIV) +#define HDIV2(STEREOMODETYPE, WDIV, HDIV, WEBM) \ + FLAG(STEREOMODETYPE, HDIV) + const unsigned height_bitfield = 0 STEREOMODE_STEREO3D_MAPPING(HDIV1, HDIV2); + +#define WEBM1(STEREOMODETYPE, STEREO3DTYPE, FLAGS, WDIV, HDIV, WEBM) \ + FLAG(STEREOMODETYPE, WEBM) +#define WEBM2(STEREOMODETYPE, WDIV, HDIV, WEBM) \ + FLAG(STEREOMODETYPE, WEBM) + const unsigned webm_bitfield = 0 STEREOMODE_STEREO3D_MAPPING(WEBM1, WEBM2); *h_width = 1; *h_height = 1; - // convert metadata into proper side data and add it to the stream + if ((tag = av_dict_get(st->metadata, "stereo_mode", NULL, 0)) || (tag = av_dict_get( s->metadata, "stereo_mode", NULL, 0))) { - long stereo_mode = strtol(tag->value, NULL, 0); for (int i = 0; i < MATROSKA_VIDEO_STEREOMODE_TYPE_NB; i++) if (!strcmp(tag->value, ff_matroska_video_stereo_mode[i])){ - stereo_mode = i; + format = i; break; } - - if ((unsigned long)stereo_mode < MATROSKA_VIDEO_STEREOMODE_TYPE_NB && - stereo_mode != 10 && stereo_mode != 12) { - int ret = ff_mkv_stereo3d_conv(st, stereo_mode); - if (ret < 0) - return ret; + if (format == MATROSKA_VIDEO_STEREOMODE_TYPE_NB) { + long stereo_mode = strtol(tag->value, NULL, 0); + if ((unsigned long)stereo_mode >= MATROSKA_VIDEO_STEREOMODE_TYPE_NB) + goto fail; + format = stereo_mode; } - } + } else { + const AVStereo3D *stereo; + /* The following macro presumes all MATROSKA_VIDEO_STEREOMODE_TYPE_* + * values to be in the range 0..254. */ +#define STEREOMODE(STEREOMODETYPE, STEREO3DTYPE, FLAGS, WDIV, HDIV, WEBM) \ + [(STEREO3DTYPE)][!!((FLAGS) & AV_STEREO3D_FLAG_INVERT)] = (STEREOMODETYPE) + 1, +#define IGNORE(STEREOMODETYPE, WDIV, HDIV, WEBM) + static const unsigned char conversion_table[][2] = { + STEREOMODE_STEREO3D_MAPPING(STEREOMODE, IGNORE) + }; + int fmt; stereo = (const AVStereo3D*)av_stream_get_side_data(st, AV_PKT_DATA_STEREO3D, NULL); - if (stereo) { - switch (stereo->type) { - case AV_STEREO3D_2D: - format = MATROSKA_VIDEO_STEREOMODE_TYPE_MONO; - break; - case AV_STEREO3D_SIDEBYSIDE: - format = (stereo->flags & AV_STEREO3D_FLAG_INVERT) - ? MATROSKA_VIDEO_STEREOMODE_TYPE_RIGHT_LEFT - : MATROSKA_VIDEO_STEREOMODE_TYPE_LEFT_RIGHT; - *h_width = 2; - break; - case AV_STEREO3D_TOPBOTTOM: - format = MATROSKA_VIDEO_STEREOMODE_TYPE_TOP_BOTTOM; - if (stereo->flags & AV_STEREO3D_FLAG_INVERT) - format--; - *h_height = 2; - break; - case AV_STEREO3D_CHECKERBOARD: - format = MATROSKA_VIDEO_STEREOMODE_TYPE_CHECKERBOARD_LR; - if (stereo->flags & AV_STEREO3D_FLAG_INVERT) - format--; - break; - case AV_STEREO3D_LINES: - format = MATROSKA_VIDEO_STEREOMODE_TYPE_ROW_INTERLEAVED_LR; - if (stereo->flags & AV_STEREO3D_FLAG_INVERT) - format--; - *h_height = 2; - break; - case AV_STEREO3D_COLUMNS: - format = MATROSKA_VIDEO_STEREOMODE_TYPE_COL_INTERLEAVED_LR; - if (stereo->flags & AV_STEREO3D_FLAG_INVERT) - format--; - *h_width = 2; - break; - case AV_STEREO3D_FRAMESEQUENCE: - format = MATROSKA_VIDEO_STEREOMODE_TYPE_BOTH_EYES_BLOCK_LR; - if (stereo->flags & AV_STEREO3D_FLAG_INVERT) - format++; - break; - } - } + if (!stereo) + return 0; - if (format == MATROSKA_VIDEO_STEREOMODE_TYPE_NB) - return 0; + /* A garbage AVStereo3D or something with no Matroska analogon. */ + if ((unsigned)stereo->type >= FF_ARRAY_ELEMS(conversion_table)) + return 0; + + fmt = conversion_table[stereo->type][!!(stereo->flags & AV_STEREO3D_FLAG_INVERT)]; + /* Format with no Matroska analogon; ignore it */ + if (!fmt) + return 0; + format = fmt - 1; + } // if webm, do not write unsupported modes - if ((is_webm && - format > MATROSKA_VIDEO_STEREOMODE_TYPE_TOP_BOTTOM && - format != MATROSKA_VIDEO_STEREOMODE_TYPE_RIGHT_LEFT) - || format >= MATROSKA_VIDEO_STEREOMODE_TYPE_NB) { - av_log(s, AV_LOG_ERROR, - "The specified stereo mode is not valid.\n"); - return AVERROR(EINVAL); + if (is_webm && !(webm_bitfield >> format)) { + goto fail; } + *h_width = 1 << ((width_bitfield >> format) & 1); + *h_height = 1 << ((height_bitfield >> format) & 1); + // write StereoMode if format is valid ebml_writer_add_uint(writer, MATROSKA_ID_VIDEOSTEREOMODE, format); return 0; +fail: + av_log(s, AV_LOG_ERROR, + "The specified stereo mode is not valid.\n"); + return AVERROR(EINVAL); } static void mkv_write_blockadditionmapping(AVFormatContext *s, const MatroskaMuxContext *mkv, -- 2.34.1 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
next prev parent reply other threads:[~2023-08-11 10:42 UTC|newest] Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-08-11 10:34 [FFmpeg-devel] [PATCH 1/9] avformat/matroskaenc: Avoid atoi() Andreas Rheinhardt 2023-08-11 10:43 ` [FFmpeg-devel] [PATCH 2/9] avformat/matroska: Move ff_matroska_video_stereo_plane to demuxer Andreas Rheinhardt 2023-08-11 10:43 ` [FFmpeg-devel] [PATCH 3/9] avformat/matroska: Add macro for stereomode<->AVStereo3D correspondence Andreas Rheinhardt 2023-08-11 10:43 ` Andreas Rheinhardt [this message] 2023-08-11 10:43 ` [FFmpeg-devel] [PATCH 5/9] avformat/matroskaenc: Improve message for WebM-incompatible StereoModes Andreas Rheinhardt 2023-08-11 10:43 ` [FFmpeg-devel] [PATCH 6/9] avformat/matroska: Move ff_mkv_stereo3d_conv() to demuxer Andreas Rheinhardt 2023-08-11 10:43 ` [FFmpeg-devel] [PATCH 7/9] avformat/matroskadec: Replace switch with array Andreas Rheinhardt 2023-08-11 10:43 ` [FFmpeg-devel] [PATCH 8/9] avformat/matroskadec: Use named constants instead of their value Andreas Rheinhardt 2023-08-11 13:20 ` Paul B Mahol 2023-08-11 10:43 ` [FFmpeg-devel] [PATCH 9/9] avformat/riffdec: Pass logctx as void* instead of AVFormatContext* Andreas Rheinhardt 2023-08-11 22:58 ` [FFmpeg-devel] [PATCH 10/11] fate/matroska: Add test for stereo 3D Andreas Rheinhardt 2023-08-12 16:27 ` Andreas Rheinhardt 2023-08-11 22:58 ` [FFmpeg-devel] [PATCH 11/11] fate/matroska: Fix requirements of fate-matroska-alac-remux test Andreas Rheinhardt 2023-08-13 23:00 ` [FFmpeg-devel] [PATCH 1/9] avformat/matroskaenc: Avoid atoi() Andreas Rheinhardt
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=AS8P250MB07449DF6EF5776C7FE6C373F8F10A@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM \ --to=andreas.rheinhardt@outlook.com \ --cc=ffmpeg-devel@ffmpeg.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel This inbox may be cloned and mirrored by anyone: git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \ ffmpegdev@gitmailbox.com public-inbox-index ffmpegdev Example config snippet for mirrors. AGPL code for this site: git clone https://public-inbox.org/public-inbox.git