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

Feature/new example encoding hlsv3 with aes #50

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

rautility
Copy link

No description provided.

@rautility rautility requested a review from jamescyeh July 27, 2018 17:17
Copy link
Contributor

@jamescyeh jamescyeh left a comment

Choose a reason for hiding this comment

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

just two minor things, but lgtm


// AES - Configuration
AesEncryptionDrm aesEncryptionDrm = new AesEncryptionDrm();
aesEncryptionDrm.setKey("<SET_YOUR_STRING_KEY>"); // 128 bit (16 bytes) ex.: cab5b529ae28d5cc5e3e7bc3fd4a544d
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe let's have this info as a const (err I mean static) at the top of the file, but it's a minor quibble.

// AES - Configuration
AesEncryptionDrm aesEncryptionDrm = new AesEncryptionDrm();
aesEncryptionDrm.setKey("<SET_YOUR_STRING_KEY>"); // 128 bit (16 bytes) ex.: cab5b529ae28d5cc5e3e7bc3fd4a544d
aesEncryptionDrm.setIv("<SET_YOUR_STRING_IV>"); // 128 bit (16 bytes) ex.: 08eecef4b026deec395234d94218273d
Copy link
Contributor

Choose a reason for hiding this comment

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

same

@rautility rautility requested a review from gfronza July 31, 2018 17:23
Copy link
Contributor

@gfronza gfronza left a comment

Choose a reason for hiding this comment

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

only two minor code styling remarks.


// Encoding Configuration - Adding h264 Representations
List<H264Representation> h264Representations = new ArrayList<>();
h264Representations.add ( new H264Representation(null, 240, null, 400L, ProfileH264.HIGH));
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove the whitespace between ( and new).

Copy link
Author

Choose a reason for hiding this comment

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

done

aacConfiguration.setRate(48000f);
aacConfiguration = bitmovinApi.configuration.audioAAC.create(aacConfiguration);

for (H264Representation representation : h264Representations) {
Copy link
Contributor

Choose a reason for hiding this comment

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

the { goes to the next line.

Copy link
Author

Choose a reason for hiding this comment

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

done

@rautility rautility requested a review from rkersche July 31, 2018 19:54
inputStreamVideo.setSelectionMode(StreamSelectionMode.AUTO);
inputStreamVideo.setPosition(0);

Stream audioStream = new Stream();
Copy link

Choose a reason for hiding this comment

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

Audio stream doesn't need to be added for every representation. It's sufficient to do it once.

Copy link
Author

Choose a reason for hiding this comment

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

Done

@rautility rautility requested a review from rkersche August 6, 2018 17:04

// Encoding Configuration - Adding h264 Representations
List<H264Representation> h264Representations = new ArrayList<>();
h264Representations.add (new H264Representation(null, 240, null, 400L, ProfileH264.HIGH));
Copy link

Choose a reason for hiding this comment

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

No space after add

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.

4 participants