History

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).

wm4 committed 9 years ago

Commit ddef3d91 parent b12693f8 files · +64 −19Browse files

Unified · Split

8 files
  1. cmdutils.c +1 −9
  2. doc/APIchanges +8 −0
  3. ffmpeg.c +36 −5
  4. ffmpeg.h +1 −0
  5. ffmpeg_opt.c +8 −4
  6. libavformat/mov.c +2 −0
  7. libavformat/movenc.c +4 −0
  8. libavformat/version.h +4 −1
cmdutils.c+1−9
@@ -2097,18 +2097,10 @@ void *grow_array(void *array, int elem_size, int *size, int new_size) · 2096 unmodified lines
20972097
20982098 double get_rotation(AVStream *st)
20992099 {
2100− AVDictionaryEntry *rotate_tag = av_dict_get(st->metadata, "rotate", NULL, 0);
21012100 uint8_t* displaymatrix = av_stream_get_side_data(st,
21022101 AV_PKT_DATA_DISPLAYMATRIX, NULL);
21032102 double theta = 0;
2104−
2105− if (rotate_tag && *rotate_tag->value && strcmp(rotate_tag->value, "0")) {
2106− char *tail;
2107− theta = av_strtod(rotate_tag->value, &tail);
2108− if (*tail)
2109− theta = 0;
2110− }
2111− if (displaymatrix && !theta)
2103+ if (displaymatrix)
21122104 theta = -av_display_rotation_get((int32_t*) displaymatrix);
21132105
21142106 theta -= 360*floor(theta/360 + 0.9/360);
doc/APIchanges+8−0
@@ -15,6 +15,14 @@ libavutil: 2015-08-28 · 14 unmodified lines
1515
1616 API changes, most recent first:
1717
18+2017-03-xx - xxxxxxx - lavf 57.68.100 - avformat.h
19+ Deprecate that demuxers export the stream rotation angle in AVStream.metadata
20+ (via an entry named "rotate"). Use av_stream_get_side_data() with
21+ AV_PKT_DATA_DISPLAYMATRIX instead, and read the rotation angle with
22+ av_display_rotation_get(). The same is done for muxing. Instead of adding a
23+ "rotate" entry to AVStream.metadata, AV_PKT_DATA_DISPLAYMATRIX side data has
24+ to be added to the AVStream.
25+
1826 2017-03-xx - xxxxxxx - lavc 57.85.101 - avcodec.h
1927 vdpau hardware accelerated decoding now supports the new hwaccel API, which
2028 can create the decoder context and allocate hardware frame automatically.
ffmpeg.c+36−5
@@ -50,6 +50,7 @@ · 49 unmodified lines
5050 #include "libavutil/internal.h"
5151 #include "libavutil/intreadwrite.h"
5252 #include "libavutil/dict.h"
53+#include "libavutil/display.h"
5354 #include "libavutil/mathematics.h"
5455 #include "libavutil/pixdesc.h"
5556 #include "libavutil/avstring.h"
@@ -3067,9 +3068,6 @@ static int init_output_stream_streamcopy(OutputStream *ost) · 3011 unmodified lines
30673068 const AVPacketSideData *sd_src = &ist->st->side_data[i];
30683069 AVPacketSideData *sd_dst = &ost->st->side_data[ost->st->nb_side_data];
30693070
3070− if (ost->rotate_overridden && sd_src->type == AV_PKT_DATA_DISPLAYMATRIX)
3071− continue;
3072−
30733071 sd_dst->data = av_malloc(sd_src->size);
30743072 if (!sd_dst->data)
30753073 return AVERROR(ENOMEM);
@@ -3080,6 +3078,13 @@ static int init_output_stream_streamcopy(OutputStream *ost) · 4 unmodified lines
30803078 }
30813079 }
30823080
3081+ if (ost->rotate_overridden) {
3082+ uint8_t *sd = av_stream_new_side_data(ost->st, AV_PKT_DATA_DISPLAYMATRIX,
3083+ sizeof(int32_t) * 9);
3084+ if (sd)
3085+ av_display_rotation_set((int32_t *)sd, -ost->rotate_override_value);
3086+ }
3087+
30833088 ost->parser = av_parser_init(par_dst->codec_id);
30843089 ost->parser_avctx = avcodec_alloc_context3(NULL);
30853090 if (!ost->parser_avctx)
@@ -3233,6 +3238,11 @@ static int init_output_stream_encode(OutputStream *ost) · 147 unmodified lines
32333238
32343239 set_encoder_id(output_files[ost->file_index], ost);
32353240
3241+ // Muxers use AV_PKT_DATA_DISPLAYMATRIX to signal rotation. On the other
3242+ // hand, the legacy API makes demuxers set "rotate" metadata entries,
3243+ // which have to be filtered out to prevent leaking them to output files.
3244+ av_dict_set(&ost->st->metadata, "rotate", NULL, 0);
3245+
32363246 if (ist) {
32373247 ost->st->disposition = ist->st->disposition;
32383248
@@ -3470,6 +3480,26 @@ static int init_output_stream(OutputStream *ost, char *error, int error_len) · 231 unmodified lines
34703480 }
34713481 }
34723482
3483+ /*
3484+ * Add global input side data. For now this is naive, and copies it
3485+ * from the input stream's global side data. All side data should
3486+ * really be funneled over AVFrame and libavfilter, then added back to
3487+ * packet side data, and then potentially using the first packet for
3488+ * global side data.
3489+ */
3490+ if (ist) {
3491+ int i;
3492+ for (i = 0; i < ist->st->nb_side_data; i++) {
3493+ AVPacketSideData *sd = &ist->st->side_data[i];
3494+ uint8_t *dst = av_stream_new_side_data(ost->st, sd->type, sd->size);
3495+ if (!dst)
3496+ return AVERROR(ENOMEM);
3497+ memcpy(dst, sd->data, sd->size);
3498+ if (ist->autorotate && sd->type == AV_PKT_DATA_DISPLAYMATRIX)
3499+ av_display_rotation_set((uint32_t *)dst, 0);
3500+ }
3501+ }
3502+
34733503 // copy timebase while removing common factors
34743504 if (ost->st->time_base.num <= 0 || ost->st->time_base.den <= 0)
34753505 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) · 790 unmodified lines
42664296 AVPacketSideData *src_sd = &ist->st->side_data[i];
42674297 uint8_t *dst_data;
42684298
4269− if (av_packet_get_side_data(&pkt, src_sd->type, NULL))
4299+ if (src_sd->type == AV_PKT_DATA_DISPLAYMATRIX)
42704300 continue;
4271− if (ist->autorotate && src_sd->type == AV_PKT_DATA_DISPLAYMATRIX)
4301+
4302+ if (av_packet_get_side_data(&pkt, src_sd->type, NULL))
42724303 continue;
42734304
42744305 dst_data = av_packet_new_side_data(&pkt, src_sd->type, src_sd->size);
ffmpeg.h+1−0
@@ -475,6 +475,7 @@ typedef struct OutputStream { · 474 unmodified lines
475475 int force_fps;
476476 int top_field_first;
477477 int rotate_overridden;
478+ double rotate_override_value;
478479
479480 AVRational frame_aspect_ratio;
480481
ffmpeg_opt.c+8−4
@@ -2549,8 +2549,6 @@ loop_end: · 2548 unmodified lines
25492549 av_dict_copy(&output_streams[i]->st->metadata, ist->st->metadata, AV_DICT_DONT_OVERWRITE);
25502550 if (!output_streams[i]->stream_copy) {
25512551 av_dict_set(&output_streams[i]->st->metadata, "encoder", NULL, 0);
2552− if (ist->autorotate)
2553− av_dict_set(&output_streams[i]->st->metadata, "rotate", NULL, 0);
25542552 }
25552553 }
25562554
@@ -2640,9 +2638,15 @@ loop_end: · 83 unmodified lines
26402638 for (j = 0; j < oc->nb_streams; j++) {
26412639 ost = output_streams[nb_output_streams - oc->nb_streams + j];
26422640 if ((ret = check_stream_specifier(oc, oc->streams[j], stream_spec)) > 0) {
2643− av_dict_set(&oc->streams[j]->metadata, o->metadata[i].u.str, *val ? val : NULL, 0);
26442641 if (!strcmp(o->metadata[i].u.str, "rotate")) {
2645− ost->rotate_overridden = 1;
2642+ char *tail;
2643+ double theta = av_strtod(val, &tail);
2644+ if (!*tail) {
2645+ ost->rotate_overridden = 1;
2646+ ost->rotate_override_value = theta;
2647+ }
2648+ } else {
2649+ av_dict_set(&oc->streams[j]->metadata, o->metadata[i].u.str, *val ? val : NULL, 0);
26462650 }
26472651 } else if (ret < 0)
26482652 exit_program(1);
libavformat/mov.c+2−0
@@ -4027,6 +4027,7 @@ static int mov_read_tkhd(MOVContext *c, AVIOContext *pb, MOVAtom atom) · 4026 unmodified lines
40274027 for (j = 0; j < 3; j++)
40284028 sc->display_matrix[i * 3 + j] = res_display_matrix[i][j];
40294029
4030+#if FF_API_OLD_ROTATE_API
40304031 rotate = av_display_rotation_get(sc->display_matrix);
40314032 if (!isnan(rotate)) {
40324033 char rotate_buf[64];
@@ -4036,6 +4037,7 @@ static int mov_read_tkhd(MOVContext *c, AVIOContext *pb, MOVAtom atom) · 3 unmodified lines
40364037 snprintf(rotate_buf, sizeof(rotate_buf), "%g", rotate);
40374038 av_dict_set(&st->metadata, "rotate", rotate_buf, 0);
40384039 }
4040+#endif
40394041 }
40404042
40414043 // transform the display width/height according to the matrix
libavformat/movenc.c+4−0
@@ -2496,19 +2496,23 @@ static int mov_write_tkhd_tag(AVIOContext *pb, MOVMuxContext *mov, · 2495 unmodified lines
24962496 avio_wb16(pb, 0); /* reserved */
24972497
24982498 /* Matrix structure */
2499+#if FF_API_OLD_ROTATE_API
24992500 if (st && st->metadata) {
25002501 AVDictionaryEntry *rot = av_dict_get(st->metadata, "rotate", NULL, 0);
25012502 rotation = (rot && rot->value) ? atoi(rot->value) : 0;
25022503 }
2504+#endif
25032505 if (display_matrix) {
25042506 for (i = 0; i < 9; i++)
25052507 avio_wb32(pb, display_matrix[i]);
2508+#if FF_API_OLD_ROTATE_API
25062509 } else if (rotation == 90) {
25072510 write_matrix(pb, 0, 1, -1, 0, track->par->height, 0);
25082511 } else if (rotation == 180) {
25092512 write_matrix(pb, -1, 0, 0, -1, track->par->width, track->par->height);
25102513 } else if (rotation == 270) {
25112514 write_matrix(pb, 0, -1, 1, 0, 0, track->par->width);
2515+#endif
25122516 } else {
25132517 write_matrix(pb, 1, 0, 0, 1, 0, 0);
25142518 }
libavformat/version.h+4−1
@@ -32,7 +32,7 @@ · 31 unmodified lines
3232 // Major bumping may affect Ticket5467, 5421, 5451(compatibility with Chromium)
3333 // Also please add any ticket numbers that you believe might be affected here
3434 #define LIBAVFORMAT_VERSION_MAJOR 57
35−#define LIBAVFORMAT_VERSION_MINOR 67
35+#define LIBAVFORMAT_VERSION_MINOR 68
3636 #define LIBAVFORMAT_VERSION_MICRO 100
3737
3838 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
@@ -94,6 +94,9 @@ · 55 unmodified lines
9494 #ifndef FF_API_LAVF_KEEPSIDE_FLAG
9595 #define FF_API_LAVF_KEEPSIDE_FLAG (LIBAVFORMAT_VERSION_MAJOR < 58)
9696 #endif
97+#ifndef FF_API_OLD_ROTATE_API
98+#define FF_API_OLD_ROTATE_API (LIBAVFORMAT_VERSION_MAJOR < 58)
99+#endif
97100
98101
99102 #ifndef FF_API_R_FRAME_RATE

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t