| Message ID | 20230315144525.4140-1-anton@khirnov.net |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] lavc/libx265: flag as experimental | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
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
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.
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.
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
Quoting Anton Khirnov (2023-03-15 15:45:25) > 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. Ping. I still think this should be applied. The issues seem to have gotten worse, if anything, with recent ABI breakage.
On 17/10/2024 11:22, Anton Khirnov wrote: > Quoting Anton Khirnov (2023-03-15 15:45:25) >> 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. > > Ping. > > I still think this should be applied. The issues seem to have gotten > worse, if anything, with recent ABI breakage. I would second this, without something drastic like this, no attention will be brought to the quite serious issues of that encoder.
On 10/17/2024 10:49 AM, Timo Rothenpieler wrote: > On 17/10/2024 11:22, Anton Khirnov wrote: >> Quoting Anton Khirnov (2023-03-15 15:45:25) >>> 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. >> >> Ping. >> >> I still think this should be applied. The issues seem to have gotten >> worse, if anything, with recent ABI breakage. > > I would second this, without something drastic like this, no attention > will be brought to the quite serious issues of that encoder. Ok.
> 在 2024年10月17日，下午10:01，James Almer <jamrial@gmail.com> 写道： > > On 10/17/2024 10:49 AM, Timo Rothenpieler wrote: >>> On 17/10/2024 11:22, Anton Khirnov wrote: >>> Quoting Anton Khirnov (2023-03-15 15:45:25) >>>> 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. >>> >>> Ping. >>> >>> I still think this should be applied. The issues seem to have gotten >>> worse, if anything, with recent ABI breakage. >> I would second this, without something drastic like this, no attention will be brought to the quite serious issues of that encoder. > > Ok. +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". > > <OpenPGP_signature.asc>
Hi all , We understand your concern behind marking x265 as experimental but we wanted to let you know that our x265 development team is actively working on resolving multiple issues that have been brought to our notice so far through bitbucket issue tracker(including issue #482) and are planning to release v4.1 with all the fixes shortly. We are also continuously levelling up our tests to catch the issues beforehand and keep up code hygiene that we have maintained so far. Hence we request this forum to reconsider the decision for the benefit of several x265 users. *Thanks,* *Kirithika* On Thu, Oct 17, 2024 at 9:28 PM Zhao Zhili <quinkblack@foxmail.com> wrote: > > > 在 2024年10月17日，下午10:01，James Almer <jamrial@gmail.com> 写道： > > > > On 10/17/2024 10:49 AM, Timo Rothenpieler wrote: > >>> On 17/10/2024 11:22, Anton Khirnov wrote: > >>> Quoting Anton Khirnov (2023-03-15 15:45:25) > >>>> 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. > >>> > >>> Ping. > >>> > >>> I still think this should be applied. The issues seem to have gotten > >>> worse, if anything, with recent ABI breakage. > >> I would second this, without something drastic like this, no attention > will be brought to the quite serious issues of that encoder. > > > > Ok. > > +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". > > > > <OpenPGP_signature.asc> > > _______________________________________________ > 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". >
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,