| Message ID | CAHVN4mj9ha4iq2OPMdF5tyiWP2uToZ1TvRnZ1AJEypgDpgZNBA@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] libaom: Dont use aom_codec_av1_dx_algo. | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_ppc | success | Make finished |
| andriy/make_fate_ppc | success | Make fate finished |
On Wed, Oct 20, 2021 at 9:59 AM Matt Oliver <protogonoi@gmail.com> wrote: > > This fixes linking errors where variables cannot be correctly linked in > from an external shared library such as with msvc (requires dllimport which > is not used by libaom). Instead just call the function that returns the > same variable. > --- > libavcodec/libaomdec.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavcodec/libaomdec.c b/libavcodec/libaomdec.c > index 75ecc08970..d6b822fda2 100644 > --- a/libavcodec/libaomdec.c > +++ b/libavcodec/libaomdec.c > @@ -241,7 +241,7 @@ static av_cold int aom_free(AVCodecContext *avctx) > > static av_cold int av1_init(AVCodecContext *avctx) > { > - return aom_init(avctx, &aom_codec_av1_dx_algo); > + return aom_init(avctx, aom_codec_av1_dx()); > } > lgtm.
> > lgtm. > Thanks, I will push this assuming no other comments.
On 10/29/2021 11:43 PM, Matt Oliver wrote: >> >> lgtm. >> > > Thanks, > I will push this assuming no other comments. Does this compile and link with libaom 1.0.0? We unfortunately still support that version (despite being unusable speed wise) because distros like Debian still ship it.
> > Does this compile and link with libaom 1.0.0? We unfortunately still > support that version (despite being unusable speed wise) because distros > like Debian still ship it. > Just looked it up and yes version 1.0.0 includes the required aom_codec_av1_dx() function (line 37 of aomdx.h for those interested).
On Sat, 30 Oct 2021 at 14:13, Matt Oliver <protogonoi@gmail.com> wrote: > Does this compile and link with libaom 1.0.0? We unfortunately still >> support that version (despite being unusable speed wise) because distros >> like Debian still ship it. >> > > Just looked it up and yes version 1.0.0 includes the required > aom_codec_av1_dx() function (line 37 of aomdx.h for those interested). > I also noticed that libvpx does the same thing as libaom where it uses the direct variable instead of the function returning the variable. libvpx doesnt currently support static compilation so it's not an issue that has come up but was wondering whether it was worth submitting a patch preemptively fixing that issue as well.
On Fri, Oct 29, 2021 at 8:13 PM Matt Oliver <protogonoi@gmail.com> wrote: > > > > > Does this compile and link with libaom 1.0.0? We unfortunately still > > support that version (despite being unusable speed wise) because distros > > like Debian still ship it. > > > > Just looked it up and yes version 1.0.0 includes the required > aom_codec_av1_dx() function (line 37 of aomdx.h for those interested). > There's some coverage of supported versions of libvpx and libaom to help catch compatibility issues here: https://build.webmproject.org/jenkins/view/libvpx%20nightly/job/libvpx__ffmpeg/
On Fri, Oct 29, 2021 at 8:51 PM Matt Oliver <protogonoi@gmail.com> wrote: > > On Sat, 30 Oct 2021 at 14:13, Matt Oliver <protogonoi@gmail.com> wrote: > > > Does this compile and link with libaom 1.0.0? We unfortunately still > >> support that version (despite being unusable speed wise) because distros > >> like Debian still ship it. > >> > > > > Just looked it up and yes version 1.0.0 includes the required > > aom_codec_av1_dx() function (line 37 of aomdx.h for those interested). > > > > I also noticed that libvpx does the same thing as libaom where it uses the > direct variable instead of the function returning the variable. libvpx > doesnt currently support static compilation so it's not an issue that has > come up but was wondering whether it was worth submitting a patch > preemptively fixing that issue as well. > That would be fine. By static compilation do you mean generating an import lib for this scenario?
On Tue, 2 Nov 2021 at 04:41, James Zern <jzern-at-google.com@ffmpeg.org> wrote: > On Fri, Oct 29, 2021 at 8:51 PM Matt Oliver <protogonoi@gmail.com> wrote: > > > > On Sat, 30 Oct 2021 at 14:13, Matt Oliver <protogonoi@gmail.com> wrote: > > > > > Does this compile and link with libaom 1.0.0? We unfortunately still > > >> support that version (despite being unusable speed wise) because > distros > > >> like Debian still ship it. > > >> > > > > > > Just looked it up and yes version 1.0.0 includes the required > > > aom_codec_av1_dx() function (line 37 of aomdx.h for those interested). > > > > > > > I also noticed that libvpx does the same thing as libaom where it uses > the > > direct variable instead of the function returning the variable. libvpx > > doesnt currently support static compilation so it's not an issue that has > > come up but was wondering whether it was worth submitting a patch > > preemptively fixing that issue as well. > > > > That would be fine. By static compilation do you mean generating an > import lib for this scenario? > That was actually a typo, I meant libvpx doesnt support 'shared' compilation on windows (the configure script only allows it to be specified on ELF platforms). I'm not familiar with why libvpx doesnt currently allow it but if it did then ffmpeg would have the same issues as with libaom that was fixed with this patch. Which is why I was wondering if people were fine with me preemptively fixing it by patching ffmpeg now instead of waiting for it to be a potential issue down the line.
On Mon, Nov 1, 2021 at 6:11 PM Matt Oliver <protogonoi@gmail.com> wrote: > > On Tue, 2 Nov 2021 at 04:41, James Zern <jzern-at-google.com@ffmpeg.org> > wrote: > > > On Fri, Oct 29, 2021 at 8:51 PM Matt Oliver <protogonoi@gmail.com> wrote: > > > > > > On Sat, 30 Oct 2021 at 14:13, Matt Oliver <protogonoi@gmail.com> wrote: > > > > > > > Does this compile and link with libaom 1.0.0? We unfortunately still > > > >> support that version (despite being unusable speed wise) because > > distros > > > >> like Debian still ship it. > > > >> > > > > > > > > Just looked it up and yes version 1.0.0 includes the required > > > > aom_codec_av1_dx() function (line 37 of aomdx.h for those interested). > > > > > > > > > > I also noticed that libvpx does the same thing as libaom where it uses > > the > > > direct variable instead of the function returning the variable. libvpx > > > doesnt currently support static compilation so it's not an issue that has > > > come up but was wondering whether it was worth submitting a patch > > > preemptively fixing that issue as well. > > > > > > > That would be fine. By static compilation do you mean generating an > > import lib for this scenario? > > > > That was actually a typo, I meant libvpx doesnt support 'shared' > compilation on windows (the configure script only allows it to be specified > on ELF platforms). I'm not familiar with why libvpx doesnt currently allow > it but if it did then ffmpeg would have the same issues as with libaom that > was fixed with this patch. There's no reason someone couldn't build libvpx this way, but configure never had support added for it; libaom picked up support through its transition to cmake. > Which is why I was wondering if people were fine > with me preemptively fixing it by patching ffmpeg now instead of waiting > for it to be a potential issue down the line. > That's fine by me.
diff --git a/libavcodec/libaomdec.c b/libavcodec/libaomdec.c index 75ecc08970..d6b822fda2 100644 --- a/libavcodec/libaomdec.c +++ b/libavcodec/libaomdec.c @@ -241,7 +241,7 @@ static av_cold int aom_free(AVCodecContext *avctx) static av_cold int av1_init(AVCodecContext *avctx) { - return aom_init(avctx, &aom_codec_av1_dx_algo); + return aom_init(avctx, aom_codec_av1_dx()); } const AVCodec ff_libaom_av1_decoder = {