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

Libhb vce h265 10bit #4538

Merged
merged 4 commits into from Oct 12, 2022
Merged

Libhb vce h265 10bit #4538

merged 4 commits into from Oct 12, 2022

Conversation

OvchinnikovDmitrii
Copy link
Contributor

Description of Change:

Added H.265 10 bit(AMD VCE) encoder. issue - #4537

Test on:

  • Windows 10+ (via MinGW)
  • macOS 10.13+
  • Ubuntu Linux

@sr55
Copy link
Contributor

sr55 commented Sep 9, 2022

Hi, Thanks for the patch.

Have the ffmpeg patches been submitted upstream yet?

I'll review over & test over the weekend if I get a moment.

@@ -563,6 +563,7 @@ struct hb_job_s
#define HB_VCODEC_FFMPEG_VP9 0x0000080
#define HB_VCODEC_FFMPEG_VCE_H264 0x00040000
#define HB_VCODEC_FFMPEG_VCE_H265 0x00080000
#define HB_VCODEC_FFMPEG_VCE_H265_10BIT 0x00080001
Copy link
Contributor

Choose a reason for hiding this comment

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

HB_VCODEC_ is kind of a bit field, and each codec should be a single bit. But I'll have to reassign them anyway for the VP9 10 bit pull request, so let's wait a bit to fix this.

libhb/common.c Outdated Show resolved Hide resolved
@OvchinnikovDmitrii
Copy link
Contributor Author

OvchinnikovDmitrii commented Sep 21, 2022

Have the ffmpeg patches been submitted upstream yet?

I have send these patches to ffmpeg-devel(https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=7411)
but there was no reaction at the moment.

@sr55
Copy link
Contributor

sr55 commented Sep 24, 2022

Validated on Windows. Seems to work quite nicely. If the cosmetic issue mention above is fixed, I'm happy for this to go in.

@sr55
Copy link
Contributor

sr55 commented Sep 24, 2022

Looks like we may need a quick resolve on this PR as well please.

@OvchinnikovDmitrii
Copy link
Contributor Author

Validated on Windows. Seems to work quite nicely. If the cosmetic issue mention above is fixed, I'm happy for this to go in.

Thank you for the positive feedback! I corrected the cosmetic issues in the encoder registration. What to do with "HB_VCODEC_FFMPEG_VCE_H265_10 BIT 0x00080001" ? have the values already been reassigned?( for the VP9 10 beat pull request)

@galad87
Copy link
Contributor

galad87 commented Sep 28, 2022

They will in #4578 , which should be merged soon after a bit of testing.

@galad87
Copy link
Contributor

galad87 commented Sep 30, 2022

#4578 has been merged, if you can rebase it and move the pixel format entry to encavcodec.c it should be good to go.

@OvchinnikovDmitrii
Copy link
Contributor Author

#4578 has been merged, if you can rebase it and move the pixel format entry to encavcodec.c it should be good to go.

Thanks! I rebased and moved the definition vce_pix_formats_10bit.

@galad87 galad87 merged commit 17f0722 into HandBrake:master Oct 12, 2022
@galad87
Copy link
Contributor

galad87 commented Oct 12, 2022

Tested on Windows and merged, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

3 participants