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.

Sign up for GitHub

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

Jump to bottom

Add AV1 Support #936

Merged
merged 1 commit into from
Aug 2, 2023
Merged

Add AV1 Support #936

merged 1 commit into from
Aug 2, 2023

Conversation

Sean-Der
Copy link
Contributor

I tried to keep this as simple/terse as possible.

It supports two inbound Packetization formats. Media that is supplied temporal unit at a time, or OBU at a time.

@Sean-Der Sean-Der force-pushed the master branch 4 times, most recently from b2a39eb to 6008032 Compare July 28, 2023 16:20
@Sean-Der
Copy link
Contributor Author

@paullouisageneau anything I can do to make PR easier to review?

If you want I can finish the media refactor for you, and then rebase this on top?

paullouisageneau
paullouisageneau reviewed Jul 31, 2023
View reviewed changes
Copy link
Owner

@paullouisageneau paullouisageneau left a comment

Choose a reason for hiding this comment

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

Thank you, this is a great addition!

Don't worry about the media refactoring, I'll merge this first and include it in the refactoring.

include/rtc/av1packetizationhandler.hpp
namespace rtc {

/// Handler for AV1 packetization
class RTC_CPP_EXPORT AV1PacketizationHandler final : public MediaChainableHandler {
Copy link
Owner

Choose a reason for hiding this comment

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

I'm currently refactoring media handlers to simplify the design in #929. One of the main changes will be that packetization handlers are not necessary anymore. Don't worry about it, I'll merge this and add it to the refactoring.

src/av1rtppacketizer.cpp Outdated Show resolved Hide resolved
src/av1rtppacketizer.cpp Outdated Show resolved Hide resolved
src/av1rtppacketizer.cpp Outdated Show resolved Hide resolved
src/av1rtppacketizer.cpp Outdated Show resolved Hide resolved
src/av1rtppacketizer.cpp Outdated Show resolved Hide resolved
@Sean-Der Sean-Der force-pushed the master branch 2 times, most recently from c14168d to cf13aac Compare August 1, 2023 03:40
@Sean-Der
Copy link
Contributor Author

Sean-Der commented Aug 1, 2023

All done! Can I get another review please @paullouisageneau

thanks

paullouisageneau
paullouisageneau reviewed Aug 2, 2023
View reviewed changes
Copy link
Owner

@paullouisageneau paullouisageneau left a comment

Choose a reason for hiding this comment

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

I'd prefer OBU to be spelled Obu or obu in lowercase for consistency, otherwise it looks good to me.

include/rtc/av1rtppacketizer.hpp Outdated Show resolved Hide resolved
include/rtc/av1rtppacketizer.hpp Outdated Show resolved Hide resolved
include/rtc/description.hpp Outdated Show resolved Hide resolved
@Sean-Der
Copy link
Contributor Author

Sean-Der commented Aug 2, 2023

Thanks again for the review @paullouisageneau :)

After merging could I have a tag please? This is going right into OBS, it is pretty cool seeing it in action

@paullouisageneau paullouisageneau merged commit 059d06e into paullouisageneau:master Aug 2, 2023
@paullouisageneau
Copy link
Owner

After merging could I have a tag please? This is going right into OBS, it is pretty cool seeing it in action

Great, it is tagged as v0.19.0-alpha.6

@DatCaptainHorse DatCaptainHorse mentioned this pull request Aug 4, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@paullouisageneau paullouisageneau paullouisageneau approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Sean-Der @paullouisageneau