H265NalUnitFragment: Mask nuhTempIdPlus1 correctly
#1073
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, `H265NalUnitFragment::fragmentsFrom()` was masking the value of nuhTempIdPlus1 against the value 0xE (0b1110) which was corrupting the generated NALU fragments. This commit updates the `fragmentsFrom()` method to mask against the value 0x7 (0b111) instead. However, it looks like the masking in this method is redundant with the equivalent masking that is done by `H265NalUnitHeader`'s setters. According to [RFC 7798](https://datatracker.ietf.org/doc/html/rfc7798#section-1.1.4): 1.1.4. NAL Unit Header HEVC maintains the NAL unit concept of H.264 with modifications. HEVC uses a two-byte NAL unit header, as shown in Figure 1. The payload of a NAL unit refers to the NAL unit excluding the NAL unit header. +---------------+---------------+ |0|1|2|3|4|5|6|7|0|1|2|3|4|5|6|7| +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ |F| Type | LayerId | TID | +-------------+-----------------+ Figure 1: The Structure of the HEVC NAL Unit Header [...] TID: 3 bits nuh_temporal_id_plus1. This field specifies the temporal identifier of the NAL unit plus 1. The value of TemporalId is equal to TID minus 1. A TID value of 0 is illegal to ensure that there is at least one bit in the NAL unit header equal to 1, so to enable independent considerations of start code emulations in the NAL unit header and in the NAL unit payload data. The minimum value of the "TID" field is 1, because it will have 1 subtracted from it to recover the original HEVC "TemporalId" value. Without this commit, the RTP fragments being generated by libdatachannel's `H265RtpPacketizer` were being rejected by this consistency check in libavformat: https://github.com/FFmpeg/FFmpeg/blob/release/6.1/libavformat/rtpdec_hevc.c#L219-L223 With this commit, I can successfully relay HEVC video from ffmpeg through a libdatachannel program (based on the media-sender and media-receiver examples) and play it with ffplay.
paullouisageneau
approved these changes
Dec 21, 2023
paullouisageneau
added a commit
that referenced
this pull request
Jan 7, 2024
H265NalUnitFragment: Mask `nuhTempIdPlus1` correctly
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously,
H265NalUnitFragment::fragmentsFrom()was masking the value of nuhTempIdPlus1 against the value 0xE (0b1110) which was corrupting the generated NALU fragments. This commit updates thefragmentsFrom()method to mask against the value 0x7 (0b111) instead. However, it looks like the masking in this method is redundant with the equivalent masking that is done byH265NalUnitHeader's setters.According to RFC 7798:
The minimum value of the "TID" field is 1, because it will have 1 subtracted from it to recover the original HEVC "TemporalId" value.
Without this commit, the RTP fragments being generated by libdatachannel's
H265RtpPacketizerwere being rejected by this consistency check in libavformat:https://github.com/FFmpeg/FFmpeg/blob/release/6.1/libavformat/rtpdec_hevc.c#L219-L223
With this commit, I can successfully relay HEVC video from ffmpeg through a libdatachannel program (based on the media-sender and media-receiver examples) and play it with ffplay.