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

Fix hevc dcr generation #64

Merged
merged 2 commits into from
Jul 1, 2024
Merged

Conversation

gBillal
Copy link
Contributor

@gBillal gBillal commented Jun 30, 2024

No description provided.

@gBillal gBillal requested a review from varsill as a code owner June 30, 2024 23:50
Copy link
Contributor

@varsill varsill left a comment

Choose a reason for hiding this comment

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

BTW Could you update the module comment so that it shows in which document HEVCDecoderConfigurationRecord is defined?

@@ -80,8 +80,8 @@ defmodule Membrane.H265.DecoderConfigurationRecord do
progressive_source_flag::1, interlaced_source_flag::1, non_packed_constraint_flag::1,
frame_only_constraint_flag::1, reserved_zero_44bits::44, level_idc, 0b1111::4, 0::12,
0b111111::6, 0::2, 0b111111::6, chroma_format_idc::2, 0b11111::5,
bit_depth_luma_minus8::3, 0b11111::5, bit_depth_chroma_minus8::3, 0::19,
num_temporal_layers + 1::2, temporal_id_nested::1, nalu_length_size - 1::2-integer>>
bit_depth_luma_minus8::3, 0b11111::5, bit_depth_chroma_minus8::3, 0::18,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we could add some meaningful names to this:

Suggested change
bit_depth_luma_minus8::3, 0b11111::5, bit_depth_chroma_minus8::3, 0::18,
avarage_framerate = 0 # not set
constant_framerate = 0 # not set
bit_depth_luma_minus8::3, 0b11111::5, bit_depth_chroma_minus8::3, avarage_framerate::16, constant_framerate::2,

@gBillal
Copy link
Contributor Author

gBillal commented Jul 1, 2024

BTW Could you update the module comment so that it shows in which document HEVCDecoderConfigurationRecord is defined?

It's written here

@gBillal gBillal requested a review from varsill July 1, 2024 09:42
@varsill
Copy link
Contributor

varsill commented Jul 1, 2024

It's written here

Oh, my bad, indeed it's there, I've looked at the wrong version of the spec

@varsill varsill merged commit b214b1d into membraneframework:master Jul 1, 2024
@varsill
Copy link
Contributor

varsill commented Jul 1, 2024

@gBillal Thank you for the contribution!

@gBillal gBillal deleted the fix-hevc-dcr branch July 1, 2024 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants