Add H.265 payloader fork #165 by y-kawawa · Pull Request #287 · pion/rtp

y-kawawa · 2025-01-04T07:29:37Z

Description

Add H265Payloader

fork from #165

H265Payloader function

Fragment Unit
Aggregation Unit
DONL
SkipAggregation

diff from #165

Operation check

Chrome
Safari

This change completes the H265 implementation.

y-kawawa · 2025-01-04T07:43:05Z

@kevmo314 @Sean-Der @lebedyncrs

Can you please confirm that you have made the corrections associated with the test?

y-kawawa · 2025-01-07T00:10:55Z

codecs/h265_packet.go

 		}

-		if len(nalu) <= int(mtu) {
+		naluLen := len(nalu) + 2


To prevent the MTU size from being exceeded during aggregation packet, the size is compared by adding 2 bytes for the aggregation header.

y-kawawa · 2025-01-07T00:12:14Z

codecs/h265_packet.go


 			if aggregationBufferSize+marginalAggregationSize > int(mtu) {
 				flushBufferedNals()
+				marginalAggregationSize = calcMarginalAggregationSize(nalu)


After flushing, recalculate because the margin conditions change.

y-kawawa · 2025-01-09T18:03:28Z

Base branch changed to master.

y-kawawa · 2025-01-09T18:06:53Z

@kevmo314 @Sean-Der @lebedyncrs

Changed to a form of forking PR.
Another implementation of H.265 Payloader has been added to #286 as well. Please adopt the preferred one.

lebedyncrs · 2025-01-09T20:11:48Z

@y-kawawa, @kevmo314 thank you very much for your work. I am happy to fund you a coffee, let me know how

kevmo314 · 2025-01-09T20:29:31Z

Thanks for adding the tests here. This change looks good to me. I know @Sean-Der had some thoughts on the tests so I'll let him approve but if he isn't able to get to this PR in a few days I'll also approve it so we can get it unblocked.

@lebedyncrs contributions to https://opencollective.com/pion are super appreciated :)

JoTurk · 2025-01-10T18:32:47Z

Hello! We’re working on updating the linters[1], Sorry I caused a conflict on your branch, Would you like me to fix it and fix the lint issues on your branch?

Thank you so much for your work! <3

Improve the RTP package #288

y-kawawa · 2025-01-10T19:13:17Z

@joeturki
Thanks for contacting us! Conflict fixed.
Approval is required to run the linter.
If you have authorization, could you please allow me to do so?

kevmo314 · 2025-01-10T19:15:18Z

@joeturki
Thanks for contacting us! Conflict fixed.
Approval is required to run the linter.
If you have authorization, could you please allow me to do so?

Approved the workflow run

codecov · 2025-01-10T19:16:51Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.69%. Comparing base (fee4338) to head (ab77ba5).
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #287      +/-   ##
==========================================
+ Coverage   83.70%   84.69%   +0.98%     
==========================================
  Files          24       24              
  Lines        2535     2699     +164     
==========================================
+ Hits         2122     2286     +164     
  Misses        354      354              
  Partials       59       59

Flag	Coverage Δ
go	`84.69% <100.00%> (+0.98%)`	⬆️
wasm	`84.06% <100.00%> (+1.03%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

codecs/h265_packet.go

y-kawawa · 2025-01-10T22:33:00Z

@kevmo314
Thanks for the multiple approvals.
I didn't want to do too much, but I made the following modifications

Revert H265Packet changes.
H265Packet is no longer comparable due to the inclusion of nalBuffer, etc. of slices in H265Packet.
Once reversed because it falls under the check for fixing incompatibilities.

JoTurk · 2025-01-10T22:52:19Z

@y-kawawa I approved it again, don't worry. as soon as I get a notification, I'll approve it :)

y-kawawa · 2025-01-10T23:18:23Z

@joeturki Thanks 🙏

DONL is increased by 2 the first time and 1 thereafter.

H265Packet is no longer comparable due to the inclusion of nalBuffer, etc. of slices in H265Packet. Once reversed because it falls under the check for fixing incompatibilities.

y-kawawa · 2025-01-11T01:25:04Z

I misunderstood Linter's point. (Fixed)

JoTurk · 2025-01-11T01:41:30Z

Btw you can install and run golangci-lint locally, we use v1.63.4, the config file is included with the repo, and it should just work, Only if you want :)

Thanks :)

y-kawawa · 2025-01-11T05:58:10Z

Thanks again and again .
I have zero linter errors with local golangci-lint v1.63.4 (couldn't get it down).

kevmo314 · 2025-01-11T18:03:55Z

This PR looks good. Thank you for the work adding the tests! I haven't heard from Sean on any other requests so I'll go ahead and approve so we can get this in.

Sean-Der · 2025-01-16T15:01:12Z

@kevmo314 thank you so much for helping with this :) Sorry I haven't responded more/been more involved. I am consistently underwater these days.

Your judgement has consistently been fantastic, merge away!

Add H265 payloader and depacketizer

dd05d22

This change completes the H265 implementation.

y-kawawa commented Jan 7, 2025

View reviewed changes

kawaway mentioned this pull request Jan 9, 2025

add h265 payloader #286

Closed

y-kawawa changed the base branch from h265 to master January 9, 2025 17:05

y-kawawa changed the title ~~Additional testing and associated bug fixes related to H.265 payloader~~ Add H.265 payloader fork #165 Jan 9, 2025

y-kawawa force-pushed the master branch 3 times, most recently from 8b9ba4e to e4d4bf1 Compare January 10, 2025 20:58

y-kawawa commented Jan 10, 2025

View reviewed changes

codecs/h265_packet.go Outdated Show resolved Hide resolved

y-kawawa force-pushed the master branch 2 times, most recently from cce2e93 to 4549554 Compare January 10, 2025 22:22

y-kawawa force-pushed the master branch from 4549554 to 4b294f4 Compare January 10, 2025 22:49

y-kawawa force-pushed the master branch from 4b294f4 to 4586c23 Compare January 10, 2025 23:17

y-kawawa added 5 commits January 11, 2025 09:38

Fix segv

ca36e35

Fix mut exceedance in aggregation packet

73c0f7e

Corrected donl margin calculations

4d72a25

DONL is increased by 2 the first time and 1 thereafter.

Add h.265 payloader test

71b84ad

Fix linter

c525417

y-kawawa added 4 commits January 11, 2025 09:38

Suppress linter nestif

7675943

Revert withdonl for compatibility

ff6206e

Temporary revert of H265Packet

41d84a9

H265Packet is no longer comparable due to the inclusion of nalBuffer, etc. of slices in H265Packet. Once reversed because it falls under the check for fixing incompatibilities.

Merge remote-tracking branch 'upstream/master'

99f5ac5

y-kawawa force-pushed the master branch from 4586c23 to 99f5ac5 Compare January 11, 2025 00:42

Suppress linter warning

ab77ba5

y-kawawa requested a review from kevmo314 January 11, 2025 17:21

kevmo314 approved these changes Jan 11, 2025

View reviewed changes

kevmo314 merged commit f1c7226 into pion:master Jan 11, 2025

Conversation

y-kawawa commented Jan 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

H265Payloader function

diff from #165

Operation check

Uh oh!

y-kawawa commented Jan 4, 2025

Uh oh!

y-kawawa Jan 7, 2025

Choose a reason for hiding this comment

Uh oh!

y-kawawa Jan 7, 2025

Choose a reason for hiding this comment

Uh oh!

y-kawawa commented Jan 9, 2025

Uh oh!

y-kawawa commented Jan 9, 2025

Uh oh!

lebedyncrs commented Jan 9, 2025

Uh oh!

kevmo314 commented Jan 9, 2025

Uh oh!

JoTurk commented Jan 10, 2025

Uh oh!

y-kawawa commented Jan 10, 2025

Uh oh!

kevmo314 commented Jan 10, 2025

Uh oh!

codecov bot commented Jan 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

y-kawawa commented Jan 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JoTurk commented Jan 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

y-kawawa commented Jan 10, 2025

Uh oh!

y-kawawa commented Jan 11, 2025

Uh oh!

JoTurk commented Jan 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

y-kawawa commented Jan 11, 2025

Uh oh!

kevmo314 commented Jan 11, 2025

Uh oh!

Sean-Der commented Jan 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

y-kawawa commented Jan 4, 2025 •

edited

Loading

codecov bot commented Jan 10, 2025 •

edited

Loading

y-kawawa commented Jan 10, 2025 •

edited

Loading

JoTurk commented Jan 10, 2025 •

edited

Loading

JoTurk commented Jan 11, 2025 •

edited

Loading