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

Add QVBR Portrait Support #29

Closed
wants to merge 11 commits into from

Conversation

arivera-skycred
Copy link

@arivera-skycred arivera-skycred commented Jun 6, 2019

Issue #, if available:
#28

Description of changes:

  • Updated README
  • Added portrait templates for QVBR enabled workflow
  • Added new layer to use the latest version of the aws-sdk in the encode lambda function
  • Updated encode lambda function to use the Rotate parameter
  • Updated profiler lambda function to detect the presence of rotate metadata and swap dimensions for profile selection
  • Update profiler lambda function to pick the correct profile based on the appropriate dimension based on portrait vs. landscape

Testing

  • Test each orientation from iOS - PASS
  • Test each orientation from Android - IN PROGRESS

NOTE: this has not been fully tested yet. I have had successful tests using portrait videos that were actually landscape but had the rotation meta data. Looking for help to fully test.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@tomnight
Copy link
Contributor

tomnight commented Jun 7, 2019

Thanks for the feedback and the detail. We will review the changes and get the update support for portrait videos on to our features road map.

@@ -38,16 +38,26 @@ exports.handler = async (event) => {
event[key] = data.Item[key];
});
let mediaInfo = JSON.parse(event.srcMediainfo);
event.srcHeight = mediaInfo.video[0].height;
event.srcWidth = mediaInfo.video[0].width;
event.rotation = mediaInfo.video[0].rotation;

Choose a reason for hiding this comment

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

Hi @arivera-skycred,
Can you confirm this pull request is up-to-date?
This change you introduced requires the rotation property returned by mediainfo to be included in the event, but that file is not included in this PR.

@dscpinheiro
Copy link

Hi @arivera-skycred,

Thanks for your contribution, but for the new version of the solution we just released (v5.0.0 - https://aws.amazon.com/solutions/video-on-demand-on-aws/), we ended up using the approach described on #27

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants