OpenVisualCloud / SVT-VP9

SVT VP9 encoder. Scalable Video Technology (SVT) is a software-based video coding technology that is highly optimized for Intel® Xeon® processors. Using the open source SVT-VP9 encoder, it is possible to spread video encoding processing across multiple Intel® Xeon® processors to achieve a real advantage of processing efficiency.
Other
222 stars 48 forks source link

FFmpeg: make master patch compatible with master #128

Closed Geo25rey closed 3 years ago

tianjunwork commented 3 years ago

Thank you @Geo25rey for the patch. Is the change of libwavpack in ffmpeg master causing the issue? If yes, maybe putting libsvtvp9 in front of libvpx? If it works, we can avoid adding another patch file.

1480c1 commented 3 years ago

@tianjunwork @guojiansheng0925 I am wondering if it would be better to split the patch files into two separate patches, one being a patch file to add the configure, makefile, and other file changes that usually changes based on ffmpeg version, and another second patch that only contains libsvt_vp9.c. That way, we can make sure that any change to libsvt_vp9.c will be applied to all ffmpeg version without needing to regenerate the other patches, and the individual ffmpeg version-specific changes can be made quickly while also making sure we can customize the patches to whichever version we need to?

1480c1 commented 3 years ago

That way instead of

diff --git a/libavcodec/libsvt_vp9.c b/libavcodec/libsvt_vp9.c
new file mode 100644
index 0000000000..a557019c9b
--- /dev/null
+++ b/libavcodec/libsvt_vp9.c

being repeated in each patch file, it's moved to it's own commit and patch?

Geo25rey commented 3 years ago

It seems that in addition to the removal of libwavpack, the most line numbers have changed. Otherwise, I would have just added the new lines to the patch.

Also, it's probably a good idea to keep a specific patch for the master branch for packages that rely on ffmpeg/master like ffmpeg-amd-full. My guess is this patch will become version n4.4 of ffmpeg.

Edit: I meant ffmpeg-amd-full-git not ffmpeg-amd-full.

1480c1 commented 3 years ago

I think it may be fine for us to deviate a bit in the patchset since it seems they use their own patch version as well (https://aur.archlinux.org/cgit/aur.git/tree/040-ffmpeg-add-svt-vp9.patch?h=ffmpeg-amd-full-git), but also I think the benefit in terms of maintenance will be much larger than what we currently have to do with multiple patches containing the same file, and thus whenever we need to update one, we have to update them all.

tianjunwork commented 3 years ago

@1480c1 Chris, https://github.com/OpenVisualCloud/SVT-VP9/issues/108, libsvt_vp9.c may also have conflict but with less chance. Split the patch files may not reduce much maintenance effort. Also it may confuse the user.

Geo25rey commented 3 years ago

@tianjunwork So what's the status of this PR? What are my recommendations?

tianjunwork commented 3 years ago

Thanks Chris @1480c1 reviewing. @Geo25rey Thank you for the contribution. I will test it one more time. Then go ahead with merge if no issue.

Geo25rey commented 3 years ago

Thanks @1480c1 and @tianjunwork. Glad to contribute.