Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

v10 no longer writes "rotate" metadata flag #1045

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
4 of 6 tasks
FirefoxMetzger opened this issue Oct 25, 2022 · 8 comments
Closed
4 of 6 tasks

v10 no longer writes "rotate" metadata flag #1045

FirefoxMetzger opened this issue Oct 25, 2022 · 8 comments

Comments

@FirefoxMetzger
Copy link
Contributor

FirefoxMetzger commented Oct 25, 2022

Overview

As of v10.0.0 the stream metadata flag rotate is no longer written to file. This may affect other metadata fields; however, for this one specifically, we have a unit-test downstream so I know for sure that it is affected.

Expected behavior

Setting a field in a video stream's metadata dict causes it to be written to the file.

Actual behavior

Stream-level metadata (at least the rotation flag) is no longer written.

Reproduction

import av
import numpy as np

print(f"PyAV version: {av.__version__}")

with av.open("test.mp4", "w") as container:
    container.metadata["comment"] = "This video has a rotation flag."

    stream = container.add_stream("libx264", 24)
    stream.metadata["rotate"] = "90"

    for _ in range(24):
        frame = av.VideoFrame.from_ndarray(np.ones((128, 128, 3), dtype=np.uint8))
        for packet in stream.encode(frame):
            container.mux(packet)

    for packet in stream.encode():
        container.mux(packet)

with av.open("test.mp4") as container:
    print(container.metadata)
    video_stream = container.streams.video[0]
    print(video_stream.metadata)

Using av v9.2 the above snippet produces:

PyAV version: 9.2.0
{'major_brand': 'isom', 'minor_version': '512', 'compatible_brands': 'isomiso2avc1mp41', 'encoder': 'Lavf58.76.100', 'comment': 'This video has a rotation flag.'}
{'rotate': '90', 'language': 'und', 'handler_name': 'VideoHandler', 'vendor_id': '[0][0][0][0]'}

However, when switching to v10 it produces:

PyAV version: 10.0.0
{'major_brand': 'isom', 'minor_version': '512', 'compatible_brands': 'isomiso2avc1mp41', 'encoder': 'Lavf59.27.100', 'comment': 'This video has a rotation flag.'}
{'language': 'und', 'handler_name': 'VideoHandler', 'vendor_id': '[0][0][0][0]'}

Versions

  • OS: Windows 11 and Ubuntu 20.04
  • PyAV runtime:
PyAV v10.0.0
library configuration: --disable-static --enable-shared --libdir=/c/cibw/vendor/lib --prefix=/c/cibw/vendor --disable-alsa --disable-doc --disable-mediafoundation --enable-fontconfig --enable-gmp --disable-gnutls --enable-gpl --enable-libaom --enable-libass --enable-libbluray --enable-libdav1d --enable-libfreetype --enable-libmp3lame --enable-libopencore-amrnb --enable-libopencore-amrwb --enable-libopenjpeg --enable-libopus --enable-libspeex --enable-libtheora --enable-libtwolame --enable-libvorbis --enable-libvpx --enable-libx264 --enable-libx265 --disable-libxcb --enable-libxml2 --enable-libxvid --enable-lzma --enable-version3 --enable-zlib
library license: GPL version 3 or later
libavcodec     59. 37.100
libavdevice    59.  7.100
libavfilter     8. 44.100
libavformat    59. 27.100
libavutil      57. 28.100
libswresample   4.  7.100
libswscale      6.  7.100
  • FFmpeg: (pypi shipped binary)

Research

I have done the following:

@hmaarrfk
Copy link
Contributor

Does this also affect conda-forge? it may be that something is wrong with the bundled ffmpeg. conda-forge I believe has allowed ffmpeg 5 with pyav 9.2

@hmaarrfk
Copy link
Contributor

hmaarrfk commented Jan 9, 2023

I tried it with conda-forge builds. i can indeed recreate.

It seems to be a bug in the pyav implementation, not the bundled ffmpeg libraries.

@hmaarrfk
Copy link
Contributor

It may be that ffmpeg 5 deprecated these options:
https://ffmpeg.org/pipermail/ffmpeg-devel/2022-May/296553.html

@hmaarrfk
Copy link
Contributor

Ok digging around in this, I think that the ffmpeg command, uses:

    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);
    }

but i'm not too sure...

@hmaarrfk
Copy link
Contributor

This isn't a bug so much as it is a "PyAV/we" need to update to the new API.

The old "rotate" flag (that they consider hack) needs to be updated.

FFmpeg/FFmpeg@ddef3d9

FFmpeg/FFmpeg@7b6012e

@HanzCEO
Copy link
Contributor

HanzCEO commented Jun 29, 2023

rotate metadata are indeed deprecated.

diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 0d1c84d6df..5a00886e05 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -2916,6 +2916,7 @@ loop_end:
             for (j = 0; j < oc->nb_streams; j++) {
                 ost = output_streams[nb_output_streams - oc->nb_streams + j];
                 if ((ret = check_stream_specifier(oc, oc->streams[j], stream_spec)) > 0) {
+#if FFMPEG_ROTATION_METADATA
                     if (!strcmp(o->metadata[i].u.str, "rotate")) {
                         char *tail;
                         double theta = av_strtod(val, &tail);
@@ -2923,9 +2924,18 @@ loop_end:
                             ost->rotate_overridden = 1;
                             ost->rotate_override_value = theta;
                         }
+
+                        av_log(NULL, AV_LOG_WARNING,
+                               "Conversion of a 'rotate' metadata key to a "
+                               "proper display matrix rotation is deprecated. "
+                               "See -display_rotation for setting rotation "
+                               "instead.");
                     } else {
+#endif
                         av_dict_set(&oc->streams[j]->metadata, o->metadata[i].u.str, *val ? val : NULL, 0);
+#if FFMPEG_ROTATION_METADATA
                     }
+#endif
                 } else if (ret < 0)
                     exit_program(1);
             }
--

It won't be set by av_dict_set() but instead get converted into display matrix rotation.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@WyattBlue
Copy link
Member

WyattBlue commented Dec 23, 2023

Although the api is different now, #1249 should address this. All major ffmpeg point releases I expect would have these breaking changes, including the upcoming ffmpeg 7.0.

@PyAV-Org PyAV-Org locked and limited conversation to collaborators Nov 13, 2024
@WyattBlue WyattBlue converted this issue into discussion #1630 Nov 13, 2024

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants