Skip to content
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

Added ability to correct upload video with a rotation record in the metadata #2218

Merged
merged 12 commits into from
Nov 5, 2020

Conversation

Marishka17
Copy link
Contributor

@Marishka17 Marishka17 commented Sep 24, 2020

Motivation and context

Ability to correct upload video with a rotation record in the metadata

How has this been tested?

Manual, tests

Checklist

License

  • I submit my code changes under the same MIT License that covers the project.
    Feel free to contact the maintainers if that's a concern.
  • I have updated the license header for each file (see an example below)
# Copyright (C) 2020 Intel Corporation
#
# SPDX-License-Identifier: MIT

@Marishka17
Copy link
Contributor Author

If a video was loaded with a rotation record in the metadata and the 'Use cache' checkbox was enabled, incorrect sizes will be saved.
This will be fixed after the merge with the PR.

@Marishka17 Marishka17 requested a review from azhavoro September 24, 2020 11:47
@Marishka17 Marishka17 linked an issue Sep 24, 2020 that may be closed by this pull request
@coveralls
Copy link

coveralls commented Sep 24, 2020

Coverage Status

Coverage increased (+0.05%) to 60.66% when pulling 188df45 on mk/video_frame_rotate into 70e89d6 on develop.

@azhavoro azhavoro linked an issue Sep 30, 2020 that may be closed by this pull request
2 tasks
@nmanovic
Copy link
Contributor

@Marishka17 , what is the status of the PR?

@Marishka17
Copy link
Contributor Author

I left out one moment, that we don't have installed ffmpeg inside container.
This was the only way I have found so far how to generate a video with rotation record in the metadata of video stream.
I wrote to PyAV gitter. Maybe someone can suggest another way.

@Marishka17 Marishka17 changed the title [WIP] Added ability to correct upload video with a rotation record in the metadata Added ability to correct upload video with a rotation record in the metadata Oct 16, 2020
@Marishka17 Marishka17 changed the title Added ability to correct upload video with a rotation record in the metadata [WIP] Added ability to correct upload video with a rotation record in the metadata Oct 16, 2020
@Marishka17 Marishka17 changed the title [WIP] Added ability to correct upload video with a rotation record in the metadata Added ability to correct upload video with a rotation record in the metadata Oct 19, 2020
azhavoro
azhavoro previously approved these changes Oct 20, 2020
Copy link
Contributor

@azhavoro azhavoro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

frame = av.VideoFrame().from_ndarray(
rotate_image(
frame.to_ndarray(format='bgr24'),
360 - int(container.streams.video[0].metadata.get('rotate'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a couple of questions here:

  • what the method returns: video_stream.metadata.get('rotate')?
  • what is the difference between previous expression and container.streams.video[0].metadata.get('rotate')
  • can container.streams.video[0].metadata.get('rotate') be always converted to int?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what the method returns: video_stream.metadata.get('rotate')?

string of rotation angle

what is the difference between previous expression and container.streams.video[0].metadata.get('rotate')

this is analogue

can container.streams.video[0].metadata.get('rotate') be always converted to int?

I checked for some values like 60, 90, 180, 270.. and in such cases - yes

@@ -177,6 +195,14 @@ def frame_sizes(self):
container.seek(offset=next(iter(self.key_frames.values())), stream=video_stream)
for packet in container.demux(video_stream):
for frame in packet.decode():
if video_stream.metadata.get('rotate'):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code duplication?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No

@@ -74,3 +75,16 @@ def av_scan_paths(*paths):
res = subprocess.run(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
if res.returncode:
raise ValidationError(res.stdout)

def rotate_image(image, angle):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we support rotation which isn't multiply pi/2 by an integer? Is it a real case to rotate a frame by 30 degrees? Is it possible at all?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Artificially (write the desired value in the metadata) you can create such a video.

@azhavoro azhavoro linked an issue Nov 5, 2020 that may be closed by this pull request
@nmanovic nmanovic merged commit bbfa880 into develop Nov 5, 2020
@nmanovic nmanovic deleted the mk/video_frame_rotate branch November 5, 2020 19:49
@Marishka17 Marishka17 mentioned this pull request Mar 17, 2021
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants