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

h264: fix DTS extraction in case of long NALUs #98

Merged
merged 3 commits into from
Nov 17, 2023
Merged

Conversation

hsnks100
Copy link
Contributor

@hsnks100 hsnks100 commented Nov 14, 2023

The minimum bytes to estimate getPictureOrderCount is not 6.

A failure may occur when reading picOrderCntLsb due to bits truncated by EmulationPreventionRemove.

My idea is that you shouldn't cut it with [:6], you should pass the whole thing.

@hsnks100
Copy link
Contributor Author

hsnks100 commented Nov 14, 2023

image

In addition, I think that emulation prevention processing should be done after receiving the packet.

@hsnks100
Copy link
Contributor Author

@aler9 review my code.

Copy link

codecov bot commented Nov 16, 2023

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (1b36c5c) 84.21% compared to head (7cf968f) 84.16%.

Files Patch % Lines
pkg/codecs/h264/dts_extractor.go 50.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #98      +/-   ##
==========================================
- Coverage   84.21%   84.16%   -0.06%     
==========================================
  Files          71       71              
  Lines        5532     5533       +1     
==========================================
- Hits         4659     4657       -2     
- Misses        650      652       +2     
- Partials      223      224       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@aler9
Copy link
Member

aler9 commented Nov 16, 2023

Hello, the problem of this PR is that it strongly decreases performance, since NALUS can be really long, and if the truncation is removed, the entire NALU is scanned and copied in another buffer.

Just edit the truncation to the max possible extent needed to extract the POC, that should be

(max_size(first_mb_in_slice) + max_size(slice_type) + max_size(pic_parameter_set_id) +
max_size(frame_num) + max_size(pic_order_cnt_lsb)) * 4 / 3 =
(3 * max_size(golomb) + (max(Log2MaxFrameNumMinus4) + 4) / 8 + (max(Log2MaxPicOrderCntLsbMinus4) + 4) / 8) * 4 / 3 =
(3 * 4 + 2 + 2) * 4 / 3 = 22

@hsnks100
Copy link
Contributor Author

I applied review.

@aler9 aler9 changed the title add edgecase for DTSExtractor h264: fix DTS extraction in case of long NALUs Nov 17, 2023
@aler9 aler9 merged commit 151a3cb into bluenviron:main Nov 17, 2023
6 of 8 checks passed
@aler9
Copy link
Member

aler9 commented Nov 17, 2023

i improved performance a little and merged, thanks.

@github-actions github-actions bot locked and limited conversation to collaborators May 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants