avformat, ffmpeg: deprecate old rotation API

The old "API" that signaled rotation as a metadata value has been
replaced by DISPLAYMATRIX side data quite a while ago.

There is no reason to make muxers/demuxers/API users support both. In
addition, the metadata API is dangerous, as user tags could "leak" into
it, creating unintended features or bugs.

ffmpeg CLI has to be updated to use the new API. In particular, we must
not allow to leak the "rotate" tag into the muxer. Some muxers will
catch this properly (like mov), but others (like mkv) can add it as
generic tag. Note applications, which use libavformat and assume the
old rotate API, will interpret such "rotate" user tags as rotate
metadata (which it is not), and incorrectly rotate the video.

The ffmpeg/ffplay tools drop the use of the old API for muxing and
demuxing, as all muxers/demuxers support the new API. This will mean
that the tools will not mistakenly interpret per-track "rotate" user
tags as rotate metadata. It will _not_ be treated as regression.

Unfortunately, hacks have been added, that allow the user to override
rotation by setting metadata explicitly, e.g. via

  -metadata:s:v:0 rotate=0

See references to trac #4560. fate-filter-meta-4560-rotate0 tests this.
It's easier to adjust the hack for supporting it than arguing for its
removal, so ffmpeg CLI now explicitly catches this case, and essentially
replaces the "rotate" value with a display matrix side data. (It would
be easier for both user and implementation to create an explicit option
for rotation.)

When the code under FF_API_OLD_ROTATE_API is disabled, one FATE
reference file has to be updated (because "rotate" is not exported
anymore).

Tested-by: Michael Niedermayer <michael@niedermayer.cc>
Reviewed-by: Michael Niedermayer <michael@niedermayer.cc>

1 files changed, 36 insertions, 5 deletions

diff --git a/ffmpeg.c b/ffmpeg.c
index 62e7d8222d..3aa1e7854d 100644
--- a/ffmpeg.c
+++ b/ffmpeg.c
@@ -50,6 +50,7 @@
 #include "libavutil/internal.h"
 #include "libavutil/intreadwrite.h"
 #include "libavutil/dict.h"
+#include "libavutil/display.h"
 #include "libavutil/mathematics.h"
 #include "libavutil/pixdesc.h"
 #include "libavutil/avstring.h"
@@ -3067,9 +3068,6 @@ static int init_output_stream_streamcopy(OutputStream *ost)
             const AVPacketSideData *sd_src = &ist->st->side_data[i];
             AVPacketSideData *sd_dst = &ost->st->side_data[ost->st->nb_side_data];
 
-            if (ost->rotate_overridden && sd_src->type == AV_PKT_DATA_DISPLAYMATRIX)
-                continue;
-
             sd_dst->data = av_malloc(sd_src->size);
             if (!sd_dst->data)
                 return AVERROR(ENOMEM);
@@ -3080,6 +3078,13 @@ static int init_output_stream_streamcopy(OutputStream *ost)
         }
     }
 
+    if (ost->rotate_overridden) {
+        uint8_t *sd = av_stream_new_side_data(ost->st, AV_PKT_DATA_DISPLAYMATRIX,
+                                              sizeof(int32_t) * 9);
+        if (sd)
+            av_display_rotation_set((int32_t *)sd, -ost->rotate_override_value);
+    }
+
     ost->parser = av_parser_init(par_dst->codec_id);
     ost->parser_avctx = avcodec_alloc_context3(NULL);
     if (!ost->parser_avctx)
@@ -3233,6 +3238,11 @@ static int init_output_stream_encode(OutputStream *ost)
 
     set_encoder_id(output_files[ost->file_index], ost);
 
+    // Muxers use AV_PKT_DATA_DISPLAYMATRIX to signal rotation. On the other
+    // hand, the legacy API makes demuxers set "rotate" metadata entries,
+    // which have to be filtered out to prevent leaking them to output files.
+    av_dict_set(&ost->st->metadata, "rotate", NULL, 0);
+
     if (ist) {
         ost->st->disposition          = ist->st->disposition;
 
@@ -3470,6 +3480,26 @@ static int init_output_stream(OutputStream *ost, char *error, int error_len)
             }
         }
 
+        /*
+         * Add global input side data. For now this is naive, and copies it
+         * from the input stream's global side data. All side data should
+         * really be funneled over AVFrame and libavfilter, then added back to
+         * packet side data, and then potentially using the first packet for
+         * global side data.
+         */
+        if (ist) {
+            int i;
+            for (i = 0; i < ist->st->nb_side_data; i++) {
+                AVPacketSideData *sd = &ist->st->side_data[i];
+                uint8_t *dst = av_stream_new_side_data(ost->st, sd->type, sd->size);
+                if (!dst)
+                    return AVERROR(ENOMEM);
+                memcpy(dst, sd->data, sd->size);
+                if (ist->autorotate && sd->type == AV_PKT_DATA_DISPLAYMATRIX)
+                    av_display_rotation_set((uint32_t *)dst, 0);
+            }
+        }
+
         // copy timebase while removing common factors
         if (ost->st->time_base.num <= 0 || ost->st->time_base.den <= 0)
             ost->st->time_base = av_add_q(ost->enc_ctx->time_base, (AVRational){0, 1});
@@ -4266,9 +4296,10 @@ static int process_input(int file_index)
             AVPacketSideData *src_sd = &ist->st->side_data[i];
             uint8_t *dst_data;
 
-            if (av_packet_get_side_data(&pkt, src_sd->type, NULL))
+            if (src_sd->type == AV_PKT_DATA_DISPLAYMATRIX)
                 continue;
-            if (ist->autorotate && src_sd->type == AV_PKT_DATA_DISPLAYMATRIX)
+
+            if (av_packet_get_side_data(&pkt, src_sd->type, NULL))
                 continue;
 
             dst_data = av_packet_new_side_data(&pkt, src_sd->type, src_sd->size);