[PATCH] gnu: gst-plugins-bad: Remove the svt-hevc input.

  • Done
  • quality assurance status badge
Details
2 participants
  • Liliana Marie Prikler
  • Maxim Cournoyer
Owner
unassigned
Submitted by
Maxim Cournoyer
Severity
normal
M
M
Maxim Cournoyer wrote on 1 Sep 2023 05:08
3e0b2b90b037404da7bf9be8932f9b0d69c1a39a.1693537692.git.maxim.cournoyer@gmail.com
* gnu/packages/gstreamer.scm (gst-plugins-bad)
[inputs]: Remove svt-hevc; add comment.
---
gnu/packages/gstreamer.scm | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

Toggle diff (23 lines)
diff --git a/gnu/packages/gstreamer.scm b/gnu/packages/gstreamer.scm
index 86886c025f..f38d2157bd 100644
--- a/gnu/packages/gstreamer.scm
+++ b/gnu/packages/gstreamer.scm
@@ -849,7 +849,9 @@ (define-public gst-plugins-bad
(inputs
(append
(if (target-x86?) (list mediasdk) '())
- (if (target-x86-64?) (list svt-hevc) '())
+ ;; Note: svt-hevc cannot be used, as it would break the package for
+ ;; older x86_64 CPUs such as Core 2 Duo that lack AVX2 (see:
+ ;; https://github.com/OpenVisualCloud/SVT-HEVC/issues/573#issuecomment-680678144).
(list bluez
bzip2
cairo

base-commit: d6966b8a5b4f2ddda2bc685b9642e7a1c2cbe17c
prerequisite-patch-id: c9372a1255e3bfb4f2e820f4e7c9706e3bf04203
prerequisite-patch-id: 5cbcf4b4ec5ee0db003b10898a2197c6b741973e
prerequisite-patch-id: 0a763f65a4b5042a187bd4aa039e80a3c173015d
prerequisite-patch-id: 71a9ea33b991aa2829aef26c51a3e892d34ab4e1
--
2.41.0
L
L
Liliana Marie Prikler wrote on 1 Sep 2023 06:21
(name . Raghav Gururajan)(address . rg@raghavgururajan.name)
4fa4754ecb68e0e9d7bc72c44229bcc404a5525a.camel@gmail.com
Am Donnerstag, dem 31.08.2023 um 23:08 -0400 schrieb Maxim Cournoyer:
Toggle quote (24 lines)
> * gnu/packages/gstreamer.scm (gst-plugins-bad)
> [inputs]: Remove svt-hevc; add comment.
> ---
>  gnu/packages/gstreamer.scm | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/gnu/packages/gstreamer.scm b/gnu/packages/gstreamer.scm
> index 86886c025f..f38d2157bd 100644
> --- a/gnu/packages/gstreamer.scm
> +++ b/gnu/packages/gstreamer.scm
> @@ -849,7 +849,9 @@ (define-public gst-plugins-bad
>      (inputs
>       (append
>        (if (target-x86?) (list mediasdk) '())
> -      (if (target-x86-64?) (list svt-hevc) '())
> +      ;; Note: svt-hevc cannot be used, as it would break the
> package for
> +      ;; older x86_64 CPUs such as Core 2 Duo that lack AVX2 (see:
> +      ;;
> https://github.com/OpenVisualCloud/SVT-HEVC/issues/573#issuecomment-680678144
> ).
>        (list bluez
>              bzip2
>              cairo
I think you should put that comment above the commented existing line
rather than deleting it outright. We should also look into building
svt-hevc without those CPU extensions; perhaps using tuning instead.

The comment could itself be shortened to 
svt-hevc is broken on older x86_64 CPUs [such as…]
see also <URL>
but you do you in terms of phrasing :)

Cheers
M
M
Maxim Cournoyer wrote on 2 Sep 2023 03:51
(name . Liliana Marie Prikler)(address . liliana.prikler@gmail.com)
878r9p1gxj.fsf@gmail.com
Hi Liliana,

Liliana Marie Prikler <liliana.prikler@gmail.com> writes:

Toggle quote (29 lines)
> Am Donnerstag, dem 31.08.2023 um 23:08 -0400 schrieb Maxim Cournoyer:
>> * gnu/packages/gstreamer.scm (gst-plugins-bad)
>> [inputs]: Remove svt-hevc; add comment.
>> ---
>>  gnu/packages/gstreamer.scm | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/gnu/packages/gstreamer.scm b/gnu/packages/gstreamer.scm
>> index 86886c025f..f38d2157bd 100644
>> --- a/gnu/packages/gstreamer.scm
>> +++ b/gnu/packages/gstreamer.scm
>> @@ -849,7 +849,9 @@ (define-public gst-plugins-bad
>>      (inputs
>>       (append
>>        (if (target-x86?) (list mediasdk) '())
>> -      (if (target-x86-64?) (list svt-hevc) '())
>> +      ;; Note: svt-hevc cannot be used, as it would break the
>> package for
>> +      ;; older x86_64 CPUs such as Core 2 Duo that lack AVX2 (see:
>> +      ;;
>> https://github.com/OpenVisualCloud/SVT-HEVC/issues/573#issuecomment-680678144
>> ).
>>        (list bluez
>>              bzip2
>>              cairo
> I think you should put that comment above the commented existing line
> rather than deleting it outright. We should also look into building
> svt-hevc without those CPU extensions; perhaps using tuning instead.

It's not possible to build svt-hevc without those extensions: they are
depended upon. Because it can't be used, I think it makes sense to not
leave dead code (since it's not going to be resolved in the future and
re-enabled).

Toggle quote (5 lines)
> The comment could itself be shortened to 
> svt-hevc is broken on older x86_64 CPUs [such as…]
> see also <URL>
> but you do you in terms of phrasing :)

:-) I've moved things a bit. Is it a 'LGTM' from you?

--
Thanks,
Maxim
L
L
Liliana Marie Prikler wrote on 2 Sep 2023 06:59
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
c4e8f02d7117d9ac3d315054a6aca049bb30fa37.camel@gmail.com
Am Freitag, dem 01.09.2023 um 21:51 -0400 schrieb Maxim Cournoyer:
Toggle quote (41 lines)
> Hi Liliana,
>
> Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
>
> > Am Donnerstag, dem 31.08.2023 um 23:08 -0400 schrieb Maxim
> > Cournoyer:
> > > * gnu/packages/gstreamer.scm (gst-plugins-bad)
> > > [inputs]: Remove svt-hevc; add comment.
> > > ---
> > >  gnu/packages/gstreamer.scm | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/gnu/packages/gstreamer.scm
> > > b/gnu/packages/gstreamer.scm
> > > index 86886c025f..f38d2157bd 100644
> > > --- a/gnu/packages/gstreamer.scm
> > > +++ b/gnu/packages/gstreamer.scm
> > > @@ -849,7 +849,9 @@ (define-public gst-plugins-bad
> > >      (inputs
> > >       (append
> > >        (if (target-x86?) (list mediasdk) '())
> > > -      (if (target-x86-64?) (list svt-hevc) '())
> > > +      ;; Note: svt-hevc cannot be used, as it would break the
> > > package for
> > > +      ;; older x86_64 CPUs such as Core 2 Duo that lack AVX2
> > > (see:
> > > +      ;;
> > > https://github.com/OpenVisualCloud/SVT-HEVC/issues/573#issuecomment-680678144
> > > ).
> > >        (list bluez
> > >              bzip2
> > >              cairo
> > I think you should put that comment above the commented existing
> > line rather than deleting it outright.  We should also look into
> > building svt-hevc without those CPU extensions; perhaps using
> > tuning instead.
>
> It's not possible to build svt-hevc without those extensions: they
> are depended upon.  Because it can't be used, I think it makes sense
> to not leave dead code (since it's not going to be resolved in the
> future and re-enabled).
Looking at the code of svt-hevc, these additional libraries appear to
be just adding versions of the methods already implemented in C.

You might want to look into commenting the following lines

add_subdirectory(ASM_SSE2)
add_subdirectory(ASM_SSSE3)
add_subdirectory(ASM_SSE4_1)
add_subdirectory(ASM_AVX2)

HTH

Toggle quote (6 lines)
> > The comment could itself be shortened to 
> >   svt-hevc is broken on older x86_64 CPUs [such as…]
> >   see also <URL>
> > but you do you in terms of phrasing :)
>
> :-)  I've moved things a bit.  Is it a 'LGTM' from you?
Well, I'd rather keep the svt-hevc issue above open than concluding it
cannot be done, but otherwise, yes, LGTM.

Cheers
M
M
Maxim Cournoyer wrote on 3 Sep 2023 16:49
(name . Liliana Marie Prikler)(address . liliana.prikler@gmail.com)
87o7ijuxaf.fsf@gmail.com
Hi Liliana,

Liliana Marie Prikler <liliana.prikler@gmail.com> writes:

[...]

Toggle quote (14 lines)
>> It's not possible to build svt-hevc without those extensions: they
>> are depended upon.  Because it can't be used, I think it makes sense
>> to not leave dead code (since it's not going to be resolved in the
>> future and re-enabled).
> Looking at the code of svt-hevc, these additional libraries appear to
> be just adding versions of the methods already implemented in C.
>
> You might want to look into commenting the following lines
>
> add_subdirectory(ASM_SSE2)
> add_subdirectory(ASM_SSSE3)
> add_subdirectory(ASM_SSE4_1)
> add_subdirectory(ASM_AVX2)

From my understanding and per upstream comments linked earlier, svt-hevc
is all about implementing some video codec using latest Intel
technologies. The project synopsis on Github reads:

Toggle snippet (4 lines)
HEVC encoder. Scalable Video Technology (SVT) is a software-based video
coding technology that is highly optimized for Intel® Xeon® processors

So I don't think it can be made to work (at least usefully) on older
processors, and it's explicitly an upstream non-goal.

[...]

Toggle quote (3 lines)
> Well, I'd rather keep the svt-hevc issue above open than concluding it
> cannot be done, but otherwise, yes, LGTM.

I think it's safe to conclude that it cannot be done, given what I've
written above. Does it make sense?

--
Thanks,
Maxim
M
M
Maxim Cournoyer wrote on 6 Sep 2023 05:47
Re: bug#65668: [PATCH] gnu: gst-plugins-bad: Remove the svt-hevc input.
(name . Liliana Marie Prikler)(address . liliana.prikler@gmail.com)
87msy09d3t.fsf_-_@gmail.com
Hi,

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

[...]

Toggle quote (6 lines)
>> Well, I'd rather keep the svt-hevc issue above open than concluding it
>> cannot be done, but otherwise, yes, LGTM.
>
> I think it's safe to conclude that it cannot be done, given what I've
> written above. Does it make sense?

I've now installed the change; closing.

--
Thanks,
Maxim
Closed
?
Your comment

This issue is archived.

To comment on this conversation send an email to 65668@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 65668
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch