Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
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".

  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