[FFmpeg-devel] [PATCH] lavc/libx265: flag as experimental

[FFmpeg-devel] [PATCH] lavc/libx265: flag as experimental

[Anton Khirnov]

This encoder leaks and overreads, as can be seen e.g. by running an
encode under valgrind with default encoder parameters. This was known
upstream since at least 2019 (e.g. bitbucket issue #482) but never fixed
until now.

Since upstream does not seem to practice basic code hygiene, make sure
people do not use this encoder without knowing what they are getting
into.
---
 libavcodec/libx265.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c
index 420d0953af..d4511251a5 100644
--- a/libavcodec/libx265.c
+++ b/libavcodec/libx265.c
@@ -895,7 +895,8 @@ FFCodec ff_libx265_encoder = {
     .p.id             = AV_CODEC_ID_HEVC,
     .p.capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY |
                         AV_CODEC_CAP_OTHER_THREADS |
-                        AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE,
+                        AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE |
+                        AV_CODEC_CAP_EXPERIMENTAL,
     .p.priv_class     = &class,
     .p.wrapper_name   = "libx265",
     .init             = libx265_encode_init,
-- 
2.39.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] lavc/libx265: flag as experimental

[James Almer]

On 3/15/2023 11:45 AM, Anton Khirnov wrote:
> This encoder leaks and overreads, as can be seen e.g. by running an
> encode under valgrind with default encoder parameters. This was known
> upstream since at least 2019 (e.g. bitbucket issue #482) but never fixed
> until now.
> 
> Since upstream does not seem to practice basic code hygiene, make sure
> people do not use this encoder without knowing what they are getting
> into.

This is a really bad idea. It will break several scripts and command 
lines that were working perfectly fine and giving the desired results.

> ---
>   libavcodec/libx265.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c
> index 420d0953af..d4511251a5 100644
> --- a/libavcodec/libx265.c
> +++ b/libavcodec/libx265.c
> @@ -895,7 +895,8 @@ FFCodec ff_libx265_encoder = {
>       .p.id             = AV_CODEC_ID_HEVC,
>       .p.capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY |
>                           AV_CODEC_CAP_OTHER_THREADS |
> -                        AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE,
> +                        AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE |
> +                        AV_CODEC_CAP_EXPERIMENTAL,
>       .p.priv_class     = &class,
>       .p.wrapper_name   = "libx265",
>       .init             = libx265_encode_init,
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] lavc/libx265: flag as experimental

[Tobias Rapp]

On 15/03/2023 15:45, Anton Khirnov wrote:

> This encoder leaks and overreads, as can be seen e.g. by running an
> encode under valgrind with default encoder parameters. This was known
> upstream since at least 2019 (e.g. bitbucket issue #482) but never fixed
> until now.
>
> Since upstream does not seem to practice basic code hygiene, make sure
> people do not use this encoder without knowing what they are getting
> into.
> ---
>   libavcodec/libx265.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c
> index 420d0953af..d4511251a5 100644
> --- a/libavcodec/libx265.c
> +++ b/libavcodec/libx265.c
> @@ -895,7 +895,8 @@ FFCodec ff_libx265_encoder = {
>       .p.id             = AV_CODEC_ID_HEVC,
>       .p.capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY |
>                           AV_CODEC_CAP_OTHER_THREADS |
> -                        AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE,
> +                        AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE |
> +                        AV_CODEC_CAP_EXPERIMENTAL,
>       .p.priv_class     = &class,
>       .p.wrapper_name   = "libx265",
>       .init             = libx265_encode_init,

My understanding of this flag for encoders is that it indicates some 
possible experimental/non-standard bitstream features contained in 
output data, not that the implementation itself is considered sub-par.

Regards, Tobias

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] lavc/libx265: flag as experimental

[Anton Khirnov]

Quoting James Almer (2023-03-15 15:46:41)
> On 3/15/2023 11:45 AM, Anton Khirnov wrote:
> > This encoder leaks and overreads, as can be seen e.g. by running an
> > encode under valgrind with default encoder parameters. This was known
> > upstream since at least 2019 (e.g. bitbucket issue #482) but never fixed
> > until now.
> > 
> > Since upstream does not seem to practice basic code hygiene, make sure
> > people do not use this encoder without knowing what they are getting
> > into.
> 
> This is a really bad idea. It will break several scripts and command 
> lines that were working perfectly fine and giving the desired results.

I thought we prioritize security and correctness over keeping
commandlines working at all cost. Besides, this might motivate someone
into actually fixing x265.

We have a long-standing reputation for being an endless source of
security issues. I think we should take stands like this more often if
we want to get rid of it.

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] lavc/libx265: flag as experimental

[Anton Khirnov]

Quoting Tobias Rapp (2023-03-15 16:00:04)
> On 15/03/2023 15:45, Anton Khirnov wrote:
> 
> > This encoder leaks and overreads, as can be seen e.g. by running an
> > encode under valgrind with default encoder parameters. This was known
> > upstream since at least 2019 (e.g. bitbucket issue #482) but never fixed
> > until now.
> >
> > Since upstream does not seem to practice basic code hygiene, make sure
> > people do not use this encoder without knowing what they are getting
> > into.
> > ---
> >   libavcodec/libx265.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c
> > index 420d0953af..d4511251a5 100644
> > --- a/libavcodec/libx265.c
> > +++ b/libavcodec/libx265.c
> > @@ -895,7 +895,8 @@ FFCodec ff_libx265_encoder = {
> >       .p.id             = AV_CODEC_ID_HEVC,
> >       .p.capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY |
> >                           AV_CODEC_CAP_OTHER_THREADS |
> > -                        AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE,
> > +                        AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE |
> > +                        AV_CODEC_CAP_EXPERIMENTAL,
> >       .p.priv_class     = &class,
> >       .p.wrapper_name   = "libx265",
> >       .init             = libx265_encode_init,
> 
> My understanding of this flag for encoders is that it indicates some 
> possible experimental/non-standard bitstream features contained in 
> output data, not that the implementation itself is considered sub-par.

Our internal aac encoder used to be marked as experimental. vorbis and
opus still are.
So it's clearly about the implementation quality rather than standard
compliance.

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] lavc/libx265: flag as experimental

[Andreas Rheinhardt]

Tobias Rapp:
> On 15/03/2023 15:45, Anton Khirnov wrote:
> 
>> This encoder leaks and overreads, as can be seen e.g. by running an
>> encode under valgrind with default encoder parameters. This was known
>> upstream since at least 2019 (e.g. bitbucket issue #482) but never fixed
>> until now.
>>
>> Since upstream does not seem to practice basic code hygiene, make sure
>> people do not use this encoder without knowing what they are getting
>> into.
>> ---
>>   libavcodec/libx265.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c
>> index 420d0953af..d4511251a5 100644
>> --- a/libavcodec/libx265.c
>> +++ b/libavcodec/libx265.c
>> @@ -895,7 +895,8 @@ FFCodec ff_libx265_encoder = {
>>       .p.id             = AV_CODEC_ID_HEVC,
>>       .p.capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY |
>>                           AV_CODEC_CAP_OTHER_THREADS |
>> -                        AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE,
>> +                        AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE |
>> +                        AV_CODEC_CAP_EXPERIMENTAL,
>>       .p.priv_class     = &class,
>>       .p.wrapper_name   = "libx265",
>>       .init             = libx265_encode_init,
> 
> My understanding of this flag for encoders is that it indicates some
> possible experimental/non-standard bitstream features contained in
> output data, not that the implementation itself is considered sub-par.
> 

You are confusing the AV_CODEC_CAP_EXPERIMENTAL with
FF_COMPLIANCE_EXPERIMENTAL.

- Andreas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".