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

use second sample in webm and mp4 formats #835

Closed
wants to merge 3 commits into from

Conversation

koln67
Copy link
Contributor

@koln67 koln67 commented Sep 2, 2020

Just as in #824 with mp2t, the second sample's duration should be stored instead of the first sample's duration for the webm and mp4 formats.

@koln67
Copy link
Contributor Author

koln67 commented Sep 8, 2020

I believe the webm timescale calculation was also supposed to be changed, but I'm not sure how to correctly calculate it.

Copy link
Contributor

@kqyang kqyang left a comment

Choose a reason for hiding this comment

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

Thanks. A few nits. Can you update the end to end test files?

@@ -144,7 +144,8 @@ class Segmenter {
ProgressListener* progress_listener_ = nullptr;
uint64_t progress_target_ = 0u;
uint64_t accumulated_progress_ = 0u;
uint32_t sample_duration_ = 0u;
uint32_t sample_durations_[2] = {0, 0};
uint32_t num_samples_ = 0u;
Copy link
Contributor

Choose a reason for hiding this comment

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

s/uint32_t/size_t/

@@ -139,7 +139,8 @@ class Segmenter {
uint64_t progress_target_ = 0;
uint64_t accumulated_progress_ = 0;
uint64_t first_timestamp_ = 0;
int64_t sample_duration_ = 0;
int64_t sample_durations_[2] = {0, 0};
int64_t num_samples_ = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

s/int64_t/size_t/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should num_samples be size_t? I'm assuming only sample_durations_[2] needs to be size_t

Copy link
Contributor

Choose a reason for hiding this comment

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

num_samples_ should be size type, so it should be size_t. sample_durations_ should still be int64_t.

@@ -98,7 +98,7 @@ class Segmenter {

/// @return The sample duration in the timescale of the media.
/// Returns 0 if no samples are added yet.
uint32_t sample_duration() const { return sample_duration_; }
uint32_t sample_duration() const { return sample_durations_[1]; }
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you change uint32_t to int64_t? Same below.

sample_durations_[num_samples_] = sample->duration();
if (num_samples_ == 0)
first_timestamp_ = sample->pts();
else
Copy link
Contributor

Choose a reason for hiding this comment

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

Check if (muxer_listener_)

@koln67
Copy link
Contributor Author

koln67 commented Sep 9, 2020

Replaced with #838

@koln67 koln67 closed this Sep 9, 2020
@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Jul 24, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: archived Archived and locked; will not be updated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants