Re: [FFmpeg-devel] Develop FFmpeg through your browser?
Hello, On Sat, 9 Nov 2024, at 11:16, Rémi Denis-Courmont wrote: > The community will always have the option to move to another host thanks to > Git's decentralised design. The only thing to worry about would be losing the > requests and issues history. But even that could probably be copied and > archived elsewhere regularly if you are afraid that VideoLAN would turn evil. This is important to emphasis: moving from one gitlab instance to another is really not difficult. -- Jean-Baptiste Kempf - President +33 672 704 734 https://jbkempf.com/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] libavfilter: integral images and minkowski pooling for SSIM - ping
On Tue, 22 Oct 2024, 07:25 Michael Niedermayer, wrote: > On Mon, Oct 21, 2024 at 07:32:20PM +0200, AndreaMastroberti wrote: > > --- > > doc/filters.texi | 14 +++ > > libavfilter/ssim.h| 6 + > > libavfilter/version.h | 4 +- > > libavfilter/vf_ssim.c | 277 -- > > 4 files changed, 291 insertions(+), 10 deletions(-) > > > > diff --git a/doc/filters.texi b/doc/filters.texi > > index 2eb4a380fb..df5c3a9305 100644 > > --- a/doc/filters.texi > > +++ b/doc/filters.texi > > @@ -22848,6 +22848,20 @@ The description of the accepted parameters > follows. > > If specified the filter will use the named file to save the SSIM of > > each individual frame. When filename equals "-" the data is sent to > > standard output. > > + > > first column whitespace is corrupted > > Applying: libavfilter: integral images and minkowski pooling for SSIM - > ping > error: corrupt patch at line 13 > Patch failed at 0001 libavfilter: integral images and minkowski pooling > for SSIM - ping > > [...] > -- > Another reason we should move to Gitlab. Almost every other day I seem patches corrupted by whitespace because it's hard to send patches to the ML. Kieran > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Develop FFmpeg through your browser?
Le sunnuntaina 3. marraskuuta 2024, 1.56.26 EET Michael Niedermayer a écrit : > Should we move to a different development system ? Simply no. The issue is the review and merge process, not the software development as such. E-mail was not designed to do code reviews. It does a pretty poor job at in- depth code review, and even worse job at tracking patchsets. Maybe some day Git itself will be extended to provide some form of code review support, but I doubt this. Or maybe some other VCS with such support will outshine Git, but I doubt that even more. In the mean time, the only options for the review process are email and web interfaces. Web interfaces are nowhere near ideal, but having done considerable review work with both alternatives, it is clear to me that they are a hell of a lot better than email. > I dont know, but I belive we should carefully consider if we want to move to > a gitlab like system that is a commercal / corporate and not community > driven system that we can get stuck in. My gut feeling is that, from a purely pragmatic standpoint, communities are better off with an open-source yet commercial offering supported by a viable business, than with an also open-source project precariously maintained by a small community of hobbyists. With that said, I think most people (including myself) are not familiar with the community-based options such as Gitea or its fork Forgejo. If they address merge request and code reviews as Gitlab, I think most of the Gitlab proponents here would be content with moving to that instead. > Companies get sold change owner/leader, and in no time gitlab can be owned > by microsoft or another corporation Nobody seriously suggested switching to proprietary forges such as Github or Gitee (or at least not this time around). > and be merged with github or similar for example. > Also i have a dislike for these browser based systems. That's beside the question really. I don't exactly love Gitlab either, or web apps in general. I just dislike the bugged email-based workflow even worse. And there are currently no alternative: I wouldn't mind a native desktop app, but I don't think that such thing exists, nor that it would be viable without support at the Git level (i.e. storing the review data in Git rather than in a relational database). > Should we move to videolan? This is a seperate question and has nothing to > do with changing the "development system" ? > We can install gitlab on our infrastructure, if the community decides that > it wants gitlab. We can also install anything else the community wants. If > people do move to videolan, i will not come along. I see not the slightest > reason to give up our independance. TBH moving to VideoLAN is a purely practical question. That means Thresh and other VideoLAN admins would do the maintenance, and the VideoLAN foundation would pay for the hosting. In other words, nobody here has to be burdened with additional work and cost. The community will always have the option to move to another host thanks to Git's decentralised design. The only thing to worry about would be losing the requests and issues history. But even that could probably be copied and archived elsewhere regularly if you are afraid that VideoLAN would turn evil. All other things being equal, I aree with you that it would seem more sensible for FFmpeg to host its own web forge - but it is questionable that all relevant things are equal as of yet in terms of how VideoLAN and FFmpeg infrastructures are managed. -- Rémi Denis-Courmont http://www.remlab.net/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/jpegxl_parser: clear window
On Thu, Nov 07, 2024 at 11:04:50AM -0500, Leo Izen wrote: > On 11/7/24 12:28 AM, Kacper Michajlow wrote: > > On Tue, 5 Nov 2024 at 11:05, Leo Izen wrote: > > > > > > This says reported by kacper, could you send me the fuzz testcase? > > > > I've sent you testcases privately. > > > > - Kacper > > Thanks, I received them. I'll take a look today. i see you posted a better fix, so consider my patch here withdrown thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB It is dangerous to be right in matters on which the established authorities are wrong. -- Voltaire signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 6/7] ffv1: use standard syntax for indexing state
On 11/10/24 02:36, Michael Niedermayer wrote:
On Sat, Nov 09, 2024 at 08:22:25AM +0100, Lynne via ffmpeg-devel wrote:
It isn't immediately obvious what indexing this array does.
Use standard syntax instead.
---
libavcodec/ffv1.h | 2 +-
libavcodec/ffv1dec_template.c | 2 +-
libavcodec/ffv1enc_template.c | 4 ++--
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/libavcodec/ffv1.h b/libavcodec/ffv1.h
index 2af457be27..ed71e238e0 100644
--- a/libavcodec/ffv1.h
+++ b/libavcodec/ffv1.h
@@ -63,7 +63,7 @@ typedef struct VlcState {
typedef struct PlaneContext {
int quant_table_index;
int context_count;
-uint8_t (*state)[CONTEXT_SIZE];
+uint8_t *state;
VlcState *vlc_state;
} PlaneContext;
diff --git a/libavcodec/ffv1dec_template.c b/libavcodec/ffv1dec_template.c
index 2da6bd935d..3c95741b32 100644
--- a/libavcodec/ffv1dec_template.c
+++ b/libavcodec/ffv1dec_template.c
@@ -71,7 +71,7 @@ RENAME(decode_line)(FFV1Context *f, FFV1SliceContext *sc,
av_assert2(context < p->context_count);
if (ac != AC_GOLOMB_RICE) {
-diff = get_symbol_inline(c, p->state[context], 1);
+diff = get_symbol_inline(c, &p->state[32*context], 1);
} else {
if (context == 0 && run_mode == 0)
run_mode = 1;
diff --git a/libavcodec/ffv1enc_template.c b/libavcodec/ffv1enc_template.c
index bc14926ab9..e17e40a327 100644
--- a/libavcodec/ffv1enc_template.c
+++ b/libavcodec/ffv1enc_template.c
@@ -75,10 +75,10 @@ RENAME(encode_line)(FFV1Context *f, FFV1SliceContext *sc,
if (ac != AC_GOLOMB_RICE) {
if (pass1) {
-put_symbol_inline(c, p->state[context], diff, 1, sc->rc_stat,
+put_symbol_inline(c, &p->state[32*context], diff, 1,
sc->rc_stat,
sc->rc_stat2[p->quant_table_index][context]);
} else {
-put_symbol_inline(c, p->state[context], diff, 1, NULL, NULL);
+put_symbol_inline(c, &p->state[32*context], diff, 1, NULL,
NULL);
I would prefer that a comment is added to "uint8_t (*state)[CONTEXT_SIZE];"
if its unclear what it represents.
Also the "32" is hardcoded and duplicates in the patch which is bad if it
needs to be changed
thx
[...]
Yeah, it was just not syntax I had encountered before.
Consider this patch dropped.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 6/7] ffv1: use standard syntax for indexing state
On Sat, Nov 09, 2024 at 08:22:25AM +0100, Lynne via ffmpeg-devel wrote:
> It isn't immediately obvious what indexing this array does.
> Use standard syntax instead.
> ---
> libavcodec/ffv1.h | 2 +-
> libavcodec/ffv1dec_template.c | 2 +-
> libavcodec/ffv1enc_template.c | 4 ++--
> 3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/libavcodec/ffv1.h b/libavcodec/ffv1.h
> index 2af457be27..ed71e238e0 100644
> --- a/libavcodec/ffv1.h
> +++ b/libavcodec/ffv1.h
> @@ -63,7 +63,7 @@ typedef struct VlcState {
> typedef struct PlaneContext {
> int quant_table_index;
> int context_count;
> -uint8_t (*state)[CONTEXT_SIZE];
> +uint8_t *state;
> VlcState *vlc_state;
> } PlaneContext;
>
> diff --git a/libavcodec/ffv1dec_template.c b/libavcodec/ffv1dec_template.c
> index 2da6bd935d..3c95741b32 100644
> --- a/libavcodec/ffv1dec_template.c
> +++ b/libavcodec/ffv1dec_template.c
> @@ -71,7 +71,7 @@ RENAME(decode_line)(FFV1Context *f, FFV1SliceContext *sc,
> av_assert2(context < p->context_count);
>
> if (ac != AC_GOLOMB_RICE) {
> -diff = get_symbol_inline(c, p->state[context], 1);
> +diff = get_symbol_inline(c, &p->state[32*context], 1);
> } else {
> if (context == 0 && run_mode == 0)
> run_mode = 1;
> diff --git a/libavcodec/ffv1enc_template.c b/libavcodec/ffv1enc_template.c
> index bc14926ab9..e17e40a327 100644
> --- a/libavcodec/ffv1enc_template.c
> +++ b/libavcodec/ffv1enc_template.c
> @@ -75,10 +75,10 @@ RENAME(encode_line)(FFV1Context *f, FFV1SliceContext *sc,
>
> if (ac != AC_GOLOMB_RICE) {
> if (pass1) {
> -put_symbol_inline(c, p->state[context], diff, 1, sc->rc_stat,
> +put_symbol_inline(c, &p->state[32*context], diff, 1,
> sc->rc_stat,
>
> sc->rc_stat2[p->quant_table_index][context]);
> } else {
> -put_symbol_inline(c, p->state[context], diff, 1, NULL, NULL);
> +put_symbol_inline(c, &p->state[32*context], diff, 1, NULL,
> NULL);
I would prefer that a comment is added to "uint8_t (*state)[CONTEXT_SIZE];"
if its unclear what it represents.
Also the "32" is hardcoded and duplicates in the patch which is bad if it
needs to be changed
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The bravest are surely those who have the clearest vision
of what is before them, glory and danger alike, and yet
notwithstanding go out to meet it. -- Thucydides
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] fate: Add a target for listing failed tests
On Thu, Nov 07, 2024 at 11:36:24AM +0200, Martin Storsjö wrote: > If running tests with "make -j fate", the execution will stop > after the first failing test. To get an overview of the whole > test suite, one rather would run "make -k -j fate", which then > again buries the results about what tests actually failed further > up in the console log. > > Add a target so one can run "make fate-list-failing", to see a list > of all tests that failed the last time they were executed. > > Also add a companion target "fate-clear-results" which removes all > the old test results. (When executing a subset of tests, the result > files of all tests that aren't executed stay untouched. This also > allows getting rid of results for tests that no longer are present > in the testsuite.) > --- > So far, I've always just manually run > "cat tests/data/fate/*.rep | cut -f 1-2 -d : | grep -v :0", but > perhaps we should at least wrap it up in something more convenient > for the other developers. > --- > doc/build_system.txt | 6 ++ > doc/fate.texi| 7 +++ > tests/Makefile | 6 ++ > 3 files changed, 19 insertions(+) > should make -j32 fate-list-failing run the tests ? (if they did never run) It seems it does not thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB In fact, the RIAA has been known to suggest that students drop out of college or go to community college in order to be able to afford settlements. -- The RIAA signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 0/2] fix for seeking in HLS with FMP4 media
vectronic 于2024年11月8日周五 05:31写道：
>
> I am again updating and resubmitting a patch which fixes the following ticket:
>
> https://trac.ffmpeg.org/ticket/7359
>
> This fix has been tested and recommended by several people now and I have
> been using it in production successfully since I submitted it 6 years ago...
>
> The main patch in the series fixes HLS seeking when the HLS package consists
> of fmp4 files. It mirrors behaviour from mpegts.c in mov.c.
>
> The issue can be demonstrated as follows:
>
> ffmpeg -ss 9 -i http://vectronic.io/hls_seek_issue/fmp4/in.m3u8 -t 3 out.mp4
>
> This produces a zero duration output file.
>
> With the patch applied, the example produces a 3 second output file as
> expected.
>
> --
>
> Within hls.c when hls_read_seek() is called it resets the stream position as
> follows:
>
> /* Reset the pos, to let the mpegts demuxer know we've seeked. */
> pls->pb.pos = 0;
>
> There is corresponding support for this in the mpegts handle_packets() code
> to check if the position has been reset and clear out any PES packets:
>
> if (avio_tell(s->pb) != ts->last_pos) {
> int i;
> av_log(ts->stream, AV_LOG_TRACE, "Skipping after seek\n");
> /* seek detected, flush pes buffer */
>
> This behaviour needed to be mirrored in the mov demuxer. In mov.c it now
> detects if the pos has been reset in mov_read_packet(). It clears fragments
> and indexes and searches for the next root i.e. the next fragment via
> mov_switch_root().
>
> The second patch in the series simply improves a comment about this behaviour
> in hls.c
>
>
> vectronic (2):
> avformat/mov: if pos has been reset, clear fragments and indexes and
> search for next root
> avformat/hls: improve comment
>
> libavformat/hls.c | 2 +-
> libavformat/mov.c | 39 ---
> 2 files changed, 37 insertions(+), 4 deletions(-)
>
> --
> 2.39.5 (Apple Git-154)
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Patchset applied.
Thanks
Steven
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] fate/jpeg2000dec: add missing ISO/IEC 15444-4 conformance tests
On Fri, Nov 08, 2024 at 01:09:34PM -0800, p...@sandflow.com wrote: > From: Pierre-Anthony Lemieux > > --- > tests/fate/jpeg2000.mak | 122 ++- > tests/ref/fate/jpeg2000dec-ds0_hm_15_b8 | 6 ++ > tests/ref/fate/jpeg2000dec-ds0_ht_02_b11 | 6 ++ > tests/ref/fate/jpeg2000dec-ds0_ht_02_b12 | 6 ++ > tests/ref/fate/jpeg2000dec-ds0_ht_03_b11 | 6 ++ > tests/ref/fate/jpeg2000dec-ds0_ht_03_b14 | 6 ++ > tests/ref/fate/jpeg2000dec-ds0_ht_04_b11 | 6 ++ > tests/ref/fate/jpeg2000dec-ds0_ht_04_b12 | 6 ++ > tests/ref/fate/jpeg2000dec-ds0_ht_05_b11 | 6 ++ > tests/ref/fate/jpeg2000dec-ds0_ht_05_b12 | 6 ++ > tests/ref/fate/jpeg2000dec-ds0_ht_07_b11 | 6 ++ > tests/ref/fate/jpeg2000dec-ds0_ht_07_b15 | 6 ++ > tests/ref/fate/jpeg2000dec-ds0_ht_07_b16 | 6 ++ > tests/ref/fate/jpeg2000dec-ds0_ht_08_b11 | 6 ++ > tests/ref/fate/jpeg2000dec-ds0_ht_08_b15 | 6 ++ > tests/ref/fate/jpeg2000dec-ds0_ht_08_b16 | 6 ++ > tests/ref/fate/jpeg2000dec-ds0_ht_09_b11 | 6 ++ > tests/ref/fate/jpeg2000dec-ds0_ht_10_b11 | 6 ++ > tests/ref/fate/jpeg2000dec-ds0_ht_11_b10 | 6 ++ > tests/ref/fate/jpeg2000dec-ds0_ht_12_b11 | 6 ++ > tests/ref/fate/jpeg2000dec-ds0_ht_14_b11 | 6 ++ > tests/ref/fate/jpeg2000dec-ds0_ht_15_b11 | 6 ++ > tests/ref/fate/jpeg2000dec-ds0_ht_15_b14 | 6 ++ > tests/ref/fate/jpeg2000dec-ds0_ht_16_b11 | 6 ++ > tests/ref/fate/jpeg2000dec-ds1_ht_01_b11 | 6 ++ > tests/ref/fate/jpeg2000dec-ds1_ht_01_b12 | 6 ++ > tests/ref/fate/jpeg2000dec-ds1_ht_02_b11 | 6 ++ > tests/ref/fate/jpeg2000dec-ds1_ht_02_b12 | 6 ++ > tests/ref/fate/jpeg2000dec-ds1_ht_03_b11 | 6 ++ > tests/ref/fate/jpeg2000dec-ds1_ht_03_b12 | 6 ++ > tests/ref/fate/jpeg2000dec-ds1_ht_04_b9 | 6 ++ > tests/ref/fate/jpeg2000dec-ds1_ht_05_b11 | 6 ++ > tests/ref/fate/jpeg2000dec-ds1_ht_06_b11 | 6 ++ > tests/ref/fate/jpeg2000dec-hifi_ht1_02 | 6 ++ > tests/ref/fate/jpeg2000dec-hifi_p1_02| 6 ++ > tests/ref/fate/jpeg2000dec-p1_01 | 6 ++ > tests/ref/fate/jpeg2000dec-p1_02 | 6 ++ > tests/ref/fate/jpeg2000dec-p1_03 | 6 ++ > tests/ref/fate/jpeg2000dec-p1_04 | 6 ++ > tests/ref/fate/jpeg2000dec-p1_05 | 6 ++ > tests/ref/fate/jpeg2000dec-p1_06 | 6 ++ > 41 files changed, 361 insertions(+), 1 deletion(-) > create mode 100644 tests/ref/fate/jpeg2000dec-ds0_hm_15_b8 > create mode 100644 tests/ref/fate/jpeg2000dec-ds0_ht_02_b11 > create mode 100644 tests/ref/fate/jpeg2000dec-ds0_ht_02_b12 > create mode 100644 tests/ref/fate/jpeg2000dec-ds0_ht_03_b11 > create mode 100644 tests/ref/fate/jpeg2000dec-ds0_ht_03_b14 > create mode 100644 tests/ref/fate/jpeg2000dec-ds0_ht_04_b11 > create mode 100644 tests/ref/fate/jpeg2000dec-ds0_ht_04_b12 > create mode 100644 tests/ref/fate/jpeg2000dec-ds0_ht_05_b11 > create mode 100644 tests/ref/fate/jpeg2000dec-ds0_ht_05_b12 > create mode 100644 tests/ref/fate/jpeg2000dec-ds0_ht_07_b11 > create mode 100644 tests/ref/fate/jpeg2000dec-ds0_ht_07_b15 > create mode 100644 tests/ref/fate/jpeg2000dec-ds0_ht_07_b16 > create mode 100644 tests/ref/fate/jpeg2000dec-ds0_ht_08_b11 > create mode 100644 tests/ref/fate/jpeg2000dec-ds0_ht_08_b15 > create mode 100644 tests/ref/fate/jpeg2000dec-ds0_ht_08_b16 > create mode 100644 tests/ref/fate/jpeg2000dec-ds0_ht_09_b11 > create mode 100644 tests/ref/fate/jpeg2000dec-ds0_ht_10_b11 > create mode 100644 tests/ref/fate/jpeg2000dec-ds0_ht_11_b10 > create mode 100644 tests/ref/fate/jpeg2000dec-ds0_ht_12_b11 > create mode 100644 tests/ref/fate/jpeg2000dec-ds0_ht_14_b11 > create mode 100644 tests/ref/fate/jpeg2000dec-ds0_ht_15_b11 > create mode 100644 tests/ref/fate/jpeg2000dec-ds0_ht_15_b14 > create mode 100644 tests/ref/fate/jpeg2000dec-ds0_ht_16_b11 > create mode 100644 tests/ref/fate/jpeg2000dec-ds1_ht_01_b11 > create mode 100644 tests/ref/fate/jpeg2000dec-ds1_ht_01_b12 > create mode 100644 tests/ref/fate/jpeg2000dec-ds1_ht_02_b11 > create mode 100644 tests/ref/fate/jpeg2000dec-ds1_ht_02_b12 > create mode 100644 tests/ref/fate/jpeg2000dec-ds1_ht_03_b11 > create mode 100644 tests/ref/fate/jpeg2000dec-ds1_ht_03_b12 > create mode 100644 tests/ref/fate/jpeg2000dec-ds1_ht_04_b9 > create mode 100644 tests/ref/fate/jpeg2000dec-ds1_ht_05_b11 > create mode 100644 tests/ref/fate/jpeg2000dec-ds1_ht_06_b11 > create mode 100644 tests/ref/fate/jpeg2000dec-hifi_ht1_02 > create mode 100644 tests/ref/fate/jpeg2000dec-hifi_p1_02 > create mode 100644 tests/ref/fate/jpeg2000dec-p1_01 > create mode 100644 tests/ref/fate/jpeg2000dec-p1_02 > create mode 100644 tests/ref/fate/jpeg2000dec-p1_03 > create mode 100644 tests/ref/fate/jpeg2000dec-p1_04 > create mode 100644 tests/ref/fate/jpeg2000dec-p1_05 > create mode 100644 tests/ref/fate/jpeg2000dec-p1_06 on linux x86-32 --- tests/ref/fate/jpeg2000dec-p1_04 2024-11-10 01:23:28.599497603 +0100 +++ tests/data/fate/jpeg2000dec-p1_04 2024-11-10 02:38:03.497270241 +0100 @@ -3,4
Re: [FFmpeg-devel] [PATCH 3/7] ffv1enc: split off encoder initialization into a separate function
On Sat, Nov 09, 2024 at 08:22:22AM +0100, Lynne via ffmpeg-devel wrote: > --- > libavcodec/ffv1enc.c | 352 +++ > libavcodec/ffv1enc.h | 30 > 2 files changed, 216 insertions(+), 166 deletions(-) > create mode 100644 libavcodec/ffv1enc.h This breaks: ./ffmpeg -i lena.pnm -vf scale=255:255,format=yuv420p -slices 12 -level 4 -strict -2 -vcodec ffv1 -y /tmp/ffv1-255.nut thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The educated differ from the uneducated as much as the living from the dead. -- Aristotle signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC] dormant git accounts
On Sun, Nov 10, 2024 at 12:18 AM Michael Niedermayer wrote: > > Hi all > > Should we disable git accounts for developers who have not been active since > a long time (like 10 years) ? > > (if these developers come back, the account would then be enabled again) > but disabling such accounts may improve security (lots of "if" here but > assuming they loose their key, assuming whoever gets hold of the key > has interrest and ability to attack ffmpeg and and and, the risk here > is likely low but not 0) > > thx > I agree with this operation ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] avfilter/framepool: align the frame buffers
And not just the linesizes. Use the extra align bytes allocated for this
purpose.
Signed-off-by: James Almer
---
libavfilter/framepool.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/libavfilter/framepool.c b/libavfilter/framepool.c
index e8621e07ac..1a1fc0de1e 100644
--- a/libavfilter/framepool.c
+++ b/libavfilter/framepool.c
@@ -139,7 +139,9 @@ FFFramePool *ff_frame_pool_audio_init(AVBufferRef*
(*alloc)(size_t size),
if (ret < 0)
goto fail;
-pool->pools[0] = av_buffer_pool_init(pool->linesize[0], NULL);
+if (pool->linesize[0] > SIZE_MAX - align)
+goto fail;
+pool->pools[0] = av_buffer_pool_init(pool->linesize[0] + align, NULL);
if (!pool->pools[0])
goto fail;
@@ -219,7 +221,7 @@ AVFrame *ff_frame_pool_get(FFFramePool *pool)
if (!frame->buf[i])
goto fail;
-frame->data[i] = frame->buf[i]->data;
+frame->data[i] = (uint8_t
*)FFALIGN((uintptr_t)frame->buf[i]->data, pool->align);
}
if (desc->flags & AV_PIX_FMT_FLAG_PAL) {
@@ -256,13 +258,15 @@ AVFrame *ff_frame_pool_get(FFFramePool *pool)
frame->buf[i] = av_buffer_pool_get(pool->pools[0]);
if (!frame->buf[i])
goto fail;
-frame->extended_data[i] = frame->data[i] = frame->buf[i]->data;
+frame->extended_data[i] = frame->data[i] =
+(uint8_t *)FFALIGN((uintptr_t)frame->buf[i]->data,
pool->align);
}
for (i = 0; i < frame->nb_extended_buf; i++) {
frame->extended_buf[i] = av_buffer_pool_get(pool->pools[0]);
if (!frame->extended_buf[i])
goto fail;
-frame->extended_data[i + AV_NUM_DATA_POINTERS] =
frame->extended_buf[i]->data;
+frame->extended_data[i + AV_NUM_DATA_POINTERS] =
+(uint8_t *)FFALIGN((uintptr_t)frame->extended_buf[i]->data,
pool->align);
}
break;
--
2.47.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/sapdec: check return value of avcodec_parameters_copy()
Will push in 2 days Thanks ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] fate/pixfmt: disable dithering in the scale filter
On 11/9/2024 1:19 PM, Michael Niedermayer wrote: On Sat, Nov 09, 2024 at 11:05:32AM -0300, James Almer wrote: On 11/8/2024 9:03 PM, Michael Niedermayer wrote: On Fri, Nov 08, 2024 at 04:56:38PM -0300, James Almer wrote: On 11/8/2024 5:15 AM, Martin Storsjö wrote: On Thu, 7 Nov 2024, James Almer wrote: Should fix fate failures across different systems. Signed-off-by: James Almer --- This only hides the bug in the dither path for unscaled planar yuv 8bit to hbd and vice-versa. Someone more familiar with the code should check what exactly is going on. LGTM, thanks! Good job in hunting this one down - it seems odd as the exact output value seems stable on each machine (and valgrind doesn't find any nondeterminism), but the output is different on most machines. The only tests that fail are those using the DITHER_COPY method in planarCopyWrapper() when dither is used, and the only difference compared to the no dither path is accessing the "dithers" static const array defined in the same file. Maybe compilers are doing something weird when aligning it, or accessing it? Not sure i fully understand but the brute force way to debug this is maybe just add a printf() to dump every value then diff between to see where differences start. I can't even reproduce the issue locally, so... is this a threads >1 issue ? Doesn't look like, it happens on fate clients running the default THREADS=1. OpenPGP_signature.asc Description: OpenPGP digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] fate/pixfmt: disable dithering in the scale filter
On Sat, Nov 09, 2024 at 11:05:32AM -0300, James Almer wrote: > On 11/8/2024 9:03 PM, Michael Niedermayer wrote: > > On Fri, Nov 08, 2024 at 04:56:38PM -0300, James Almer wrote: > > > On 11/8/2024 5:15 AM, Martin Storsjö wrote: > > > > On Thu, 7 Nov 2024, James Almer wrote: > > > > > > > > > Should fix fate failures across different systems. > > > > > > > > > > Signed-off-by: James Almer > > > > > --- > > > > > This only hides the bug in the dither path for unscaled planar yuv > > > > > 8bit to > > > > > hbd and vice-versa. > > > > > Someone more familiar with the code should check what exactly is > > > > > going on. > > > > > > > > LGTM, thanks! Good job in hunting this one down - it seems odd as the > > > > exact output value seems stable on each machine (and valgrind doesn't > > > > find any nondeterminism), but the output is different on most machines. > > > > > > The only tests that fail are those using the DITHER_COPY method in > > > planarCopyWrapper() when dither is used, and the only difference compared > > > to > > > the no dither path is accessing the "dithers" static const array defined > > > in > > > the same file. > > > Maybe compilers are doing something weird when aligning it, or accessing > > > it? > > > > Not sure i fully understand but the brute force way to debug this is maybe > > just add a printf() to dump every value then diff between to see where > > differences > > start. > > I can't even reproduce the issue locally, so... is this a threads >1 issue ? thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Good people do not need laws to tell them to act responsibly, while bad people will find a way around the laws. -- Plato signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [RFC] dormant git accounts
Hi all Should we disable git accounts for developers who have not been active since a long time (like 10 years) ? (if these developers come back, the account would then be enabled again) but disabling such accounts may improve security (lots of "if" here but assuming they loose their key, assuming whoever gets hold of the key has interrest and ability to attack ffmpeg and and and, the risk here is likely low but not 0) thx -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Its not that you shouldnt use gotos but rather that you should write readable code and code with gotos often but not always is less readable signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC] dormant git accounts
On 11/9/2024 1:18 PM, Michael Niedermayer wrote: Hi all Should we disable git accounts for developers who have not been active since a long time (like 10 years) ? (if these developers come back, the account would then be enabled again) but disabling such accounts may improve security (lots of "if" here but assuming they loose their key, assuming whoever gets hold of the key has interrest and ability to attack ffmpeg and and and, the risk here is likely low but not 0) Yes. OpenPGP_signature.asc Description: OpenPGP digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] lavfi/zscale: fix tmp ptr alignment for zimg_filter_graph_process
--- libavfilter/vf_zscale.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/libavfilter/vf_zscale.c b/libavfilter/vf_zscale.c index 4ba059064b..87fe691cfd 100644 --- a/libavfilter/vf_zscale.c +++ b/libavfilter/vf_zscale.c @@ -750,7 +750,9 @@ static int filter_slice(AVFilterContext *ctx, void *data, int job_nr, int n_jobs } if (!s->graph[job_nr]) return AVERROR(EINVAL); -ret = zimg_filter_graph_process(s->graph[job_nr], &src_buf, &dst_buf, s->tmp[job_nr], 0, 0, 0, 0); +ret = zimg_filter_graph_process(s->graph[job_nr], &src_buf, &dst_buf, +(uint8_t *)FFALIGN((uintptr_t)s->tmp[job_nr], ZIMG_ALIGNMENT), +0, 0, 0, 0); if (ret) return print_zimg_error(ctx); @@ -765,7 +767,9 @@ static int filter_slice(AVFilterContext *ctx, void *data, int job_nr, int n_jobs if (!s->alpha_graph[job_nr]) return AVERROR(EINVAL); -ret = zimg_filter_graph_process(s->alpha_graph[job_nr], &src_buf, &dst_buf, s->tmp[job_nr], 0, 0, 0, 0); +ret = zimg_filter_graph_process(s->alpha_graph[job_nr], &src_buf, &dst_buf, +(uint8_t *)FFALIGN((uintptr_t)s->tmp[job_nr], ZIMG_ALIGNMENT), +0, 0, 0, 0); if (ret) return print_zimg_error(ctx); } -- 2.43.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavu/get_video_buffer: also align data pointers
On 11/9/2024 1:57 PM, Pavel Koshevoy wrote:
This avoids unpleasant surprises to av_frame_get_buffer callers
that explicitly specified 64-byte alignment and didn't get
AVFrame.data pointers that are 64-byte aligned.
Again, the doxy is clear that only the buffer sizes are aligned, not the
pointers. I'd rather not make this change here. Please see
https://ffmpeg.org//pipermail/ffmpeg-devel/2024-November/335811.html
In particular, this fixes an issue in vf_zscale.
https://github.com/sekrit-twc/zimg/issues/212
---
libavutil/frame.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/libavutil/frame.c b/libavutil/frame.c
index f0a0dba018..7faf7aeae8 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -216,6 +216,7 @@ static int get_video_buffer(AVFrame *frame, int align)
total_size += sizes[i];
}
+total_size += align - 1;
frame->buf[0] = av_buffer_alloc(total_size);
if (!frame->buf[0]) {
ret = AVERROR(ENOMEM);
@@ -223,7 +224,8 @@ static int get_video_buffer(AVFrame *frame, int align)
}
if ((ret = av_image_fill_pointers(frame->data, frame->format, padded_height,
- frame->buf[0]->data, frame->linesize)) <
0)
+ (uint8_t
*)FFALIGN((uintptr_t)frame->buf[0]->data, align),
+ frame->linesize)) < 0)
goto fail;
for (int i = 1; i < 4; i++) {
OpenPGP_signature.asc
Description: OpenPGP digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavfi/zscale: Fix unaligned data ptr issues in realign_frame
On Sat, Nov 9, 2024 at 9:47 AM James Almer wrote: > On 11/9/2024 12:55 PM, Pavel Koshevoy wrote: > > On Sat, Nov 9, 2024 at 6:22 AM James Almer wrote: > > > >> On 11/9/2024 1:40 AM, Pavel Koshevoy wrote: > >>> On Fri, Nov 8, 2024 at 7:29 PM James Almer wrote: > >>> > On 11/8/2024 10:51 PM, Pavel Koshevoy wrote: > > I ran into segfaults in zimg when I attempted to use zscale > > to convert a 512x538@yuv444p16le(tv) image from HLG to Bt.709 > > with this filter chain: > > > > > > >> > buffer=width=512:height=538:pix_fmt=yuv444p16le:time_base=1/1:sar=1/1,zscale=min=2020_ncl:rin=limited:pin=2020:tin=arib-std-b67:cin=left:t=linear,format=gbrpf32le,tonemap=gamma:desat=0,zscale=tin=linear:npl=100:p=709:m=709:r=limited:t=709,format=pix_fmts=yuv444p16le,buffersink > > > > I found several issues: > > - realign_frame called av_pix_fmt_count_planes with incorrect > >> parameter. > > - av_frame_get_buffer did not align data pointers to specified > >> alignment. > > Because it's not supposed to. The align parameter doxy states "buffer > size alignment", which is the result of aligning the stride/linesizes, > not the data pointers. > > You may want to use ff_default_get_video_buffer2(), which will add > align > amount of bytes to the allocated buffers (On top of aligning the > linesizes to it), and then align the pointers with FFALIGN(). > > >>> > >>> I am not the maintainer of vf_zscale, and I am not intimately familiar > >> with > >>> private ffmpeg APIs such as ff_default_get_video_buffer2, and at first > >>> glance that function > >>> doesn't appear to be a drop-in replacement for av_frame_get_buffer. > >>> > >>> ff_default_get_video_buffer2 requires AVFilterLink parameter?! -- > >>> realign_frame doesn't > >>> have that, it has an AVFrame which it needs to re-align to make it > >>> compatible with libzimg API. > >> > >> It's trivial to make realign_frame take the necessary AVFilterLink as > >> input argument. > >> > >>> > >>> ... and why isn't av_frame_get_buffer populating AVFrame.data pointers > >>> aligned > >>> as specified by the align parameter? It costs at most (align - 1) more > >>> padding bytes > >>> to make it align the pointers, so I don't understand why it doesn't do > >> that. > >> > >> Buffer alignment is set at configure time. It will be set to the highest > >> needed alignment for the enabled simd (64 if avx512, else 32 if avx, > >> else 16 if neon/sse, else 8). This is handled by av_malloc(), which is > >> ultimately used by all alloc functions. > >> > > > > Yes, I have noticed this while stepping through ffmpeg and zimg code. > > The surprising thing I've observed is that ff_get_cpu_max_align_x86() > > (called from av_cpu_max_align()) returned 8 ... it's surprising for an > > ffmpeg > > built and running on a Ryzen R9 5950x -- I would have expected 32. > > > > As a side note ... this ffmpeg build (and zimg build) were produced by > > conan, > > so perhaps the conan recipe for ffmpeg needs some changes to make > > av_cpu_max_align() work as expected on 5950x. > > (https://conan.io/center/recipes/ffmpeg) > > > > > > > >> As an specific alignment is not guaranteed, workarounds are needed if a > >> module requires one. > >> > > > > That is true of av_malloc, but av_frame_get_buffer is given an explicit > > align parameter, > > and it could trivially align the pointers accordingly making any external > > workarounds > > unnecessary. > > > > I still think my change to av_frame_get_buffer is the better approach: > > - it doesn't break the ABI > > - it doesn't break the API > > - it improves the API behavior at little cost in allocated buffer padding > > - it likely fixes other (unknown) instances where av_frame_get_buffer > >was expected to provide aligned data pointers and didn't, and the > caller > > is unaware of this. > > > > > > > >> > >> I took the time to do it for zscale, as follows: > >> > >> > > Thank you for providing this patch. However, I would argue that this > > functionality > > (allocating a sufficient buffer and filling an AVFrame with properly > > aligned data pointers > > according to an explicitly specified alignment parameter) should be > > available > > via a public AVFrame API like av_frame_get_buffer, > > and not require calls to a private libavfilter API. > > > > It feels a little bit like a Banana - Gorilla - Jungle problem when an > > AVFilterLink > > is required to produce an AVFrame with properly aligned data pointers. > > I sent a separate patch to have ff_default_get_video_buffer2() align the > buffers using the provided align value. See "avfilter/framepool: align > the frame buffers". > ff_default_get_video_buffer2() also uses a buffer pool, so it's better > than av_frame_get_buffer() in this case. > > > I still think my change to get_video_buffer buys us more than it costs (including fixing the issue in vf_zscale), so I've submitted a separate patch for it. If that chang
Re: [FFmpeg-devel] [PATCH] lavfi/zscale: Fix unaligned data ptr issues in realign_frame
On Sat, Nov 9, 2024 at 9:53 AM James Almer wrote: > On 11/9/2024 12:55 PM, Pavel Koshevoy wrote: > > On Sat, Nov 9, 2024 at 6:22 AM James Almer wrote: > > > >> On 11/9/2024 1:40 AM, Pavel Koshevoy wrote: > >>> On Fri, Nov 8, 2024 at 7:29 PM James Almer wrote: > >>> > On 11/8/2024 10:51 PM, Pavel Koshevoy wrote: > > I ran into segfaults in zimg when I attempted to use zscale > > to convert a 512x538@yuv444p16le(tv) image from HLG to Bt.709 > > with this filter chain: > > > > > > >> > buffer=width=512:height=538:pix_fmt=yuv444p16le:time_base=1/1:sar=1/1,zscale=min=2020_ncl:rin=limited:pin=2020:tin=arib-std-b67:cin=left:t=linear,format=gbrpf32le,tonemap=gamma:desat=0,zscale=tin=linear:npl=100:p=709:m=709:r=limited:t=709,format=pix_fmts=yuv444p16le,buffersink > > > > I found several issues: > > - realign_frame called av_pix_fmt_count_planes with incorrect > >> parameter. > > - av_frame_get_buffer did not align data pointers to specified > >> alignment. > > Because it's not supposed to. The align parameter doxy states "buffer > size alignment", which is the result of aligning the stride/linesizes, > not the data pointers. > > You may want to use ff_default_get_video_buffer2(), which will add > align > amount of bytes to the allocated buffers (On top of aligning the > linesizes to it), and then align the pointers with FFALIGN(). > > >>> > >>> I am not the maintainer of vf_zscale, and I am not intimately familiar > >> with > >>> private ffmpeg APIs such as ff_default_get_video_buffer2, and at first > >>> glance that function > >>> doesn't appear to be a drop-in replacement for av_frame_get_buffer. > >>> > >>> ff_default_get_video_buffer2 requires AVFilterLink parameter?! -- > >>> realign_frame doesn't > >>> have that, it has an AVFrame which it needs to re-align to make it > >>> compatible with libzimg API. > >> > >> It's trivial to make realign_frame take the necessary AVFilterLink as > >> input argument. > >> > >>> > >>> ... and why isn't av_frame_get_buffer populating AVFrame.data pointers > >>> aligned > >>> as specified by the align parameter? It costs at most (align - 1) more > >>> padding bytes > >>> to make it align the pointers, so I don't understand why it doesn't do > >> that. > >> > >> Buffer alignment is set at configure time. It will be set to the highest > >> needed alignment for the enabled simd (64 if avx512, else 32 if avx, > >> else 16 if neon/sse, else 8). This is handled by av_malloc(), which is > >> ultimately used by all alloc functions. > >> > > > > Yes, I have noticed this while stepping through ffmpeg and zimg code. > > The surprising thing I've observed is that ff_get_cpu_max_align_x86() > > (called from av_cpu_max_align()) returned 8 ... it's surprising for an > > ffmpeg > > built and running on a Ryzen R9 5950x -- I would have expected 32. > > Yeah, av_cpu_max_align() returns a value depending on runtime > parameters, so unless you force cpuflags using av_force_cpu_flags() to > disable everything SSE and above (or it being disabled during > configure), it should not return 8. > I certainly did not call av_force_cpu_flags myself, and as far as I can see conan ffmpeg recipe does not pass any --disable-sse or --disable-avx options to ffmpegs configure script ... I will have to investigate this separately when I am back at work. > > > > As a side note ... this ffmpeg build (and zimg build) were produced by > > conan, > > so perhaps the conan recipe for ffmpeg needs some changes to make > > av_cpu_max_align() work as expected on 5950x. > > (https://conan.io/center/recipes/ffmpeg) > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavfi/zscale: fix tmp buffer ptr alignment for zimg_filter_graph_process (v2)
On 11/9/2024 2:05 PM, Pavel Koshevoy wrote: --- libavfilter/vf_zscale.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/libavfilter/vf_zscale.c b/libavfilter/vf_zscale.c index 4ba059064b..07e295652d 100644 --- a/libavfilter/vf_zscale.c +++ b/libavfilter/vf_zscale.c @@ -630,7 +630,7 @@ static int graphs_build(AVFrame *in, AVFrame *out, const AVPixFmtDescriptor *des if (s->tmp[job_nr]) av_freep(&s->tmp[job_nr]); -s->tmp[job_nr] = av_calloc(size, 1); +s->tmp[job_nr] = av_calloc(size + ZIMG_ALIGNMENT - 1, 1); if (!s->tmp[job_nr]) return AVERROR(ENOMEM); @@ -750,7 +750,9 @@ static int filter_slice(AVFilterContext *ctx, void *data, int job_nr, int n_jobs } if (!s->graph[job_nr]) return AVERROR(EINVAL); -ret = zimg_filter_graph_process(s->graph[job_nr], &src_buf, &dst_buf, s->tmp[job_nr], 0, 0, 0, 0); +ret = zimg_filter_graph_process(s->graph[job_nr], &src_buf, &dst_buf, +(uint8_t *)FFALIGN((uintptr_t)s->tmp[job_nr], ZIMG_ALIGNMENT), +0, 0, 0, 0); if (ret) return print_zimg_error(ctx); @@ -765,7 +767,9 @@ static int filter_slice(AVFilterContext *ctx, void *data, int job_nr, int n_jobs if (!s->alpha_graph[job_nr]) return AVERROR(EINVAL); -ret = zimg_filter_graph_process(s->alpha_graph[job_nr], &src_buf, &dst_buf, s->tmp[job_nr], 0, 0, 0, 0); +ret = zimg_filter_graph_process(s->alpha_graph[job_nr], &src_buf, &dst_buf, +(uint8_t *)FFALIGN((uintptr_t)s->tmp[job_nr], ZIMG_ALIGNMENT), +0, 0, 0, 0); if (ret) return print_zimg_error(ctx); } Should be ok. Will push soon unless there are objections. OpenPGP_signature.asc Description: OpenPGP digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavu/get_video_buffer: also align data pointers
On Sat, Nov 9, 2024 at 10:27 AM James Almer wrote:
> On 11/9/2024 1:57 PM, Pavel Koshevoy wrote:
> > This avoids unpleasant surprises to av_frame_get_buffer callers
> > that explicitly specified 64-byte alignment and didn't get
> > AVFrame.data pointers that are 64-byte aligned.
>
> Again, the doxy is clear that only the buffer sizes are aligned, not the
> pointers. I'd rather not make this change here. Please see
> https://ffmpeg.org//pipermail/ffmpeg-devel/2024-November/335811.html
>
>
So if we update the doxy then the change will be accepted?
I don't understand the reluctance to improve the behavior of this API.
> >
> > In particular, this fixes an issue in vf_zscale.
> > https://github.com/sekrit-twc/zimg/issues/212
> > ---
> > libavutil/frame.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavutil/frame.c b/libavutil/frame.c
> > index f0a0dba018..7faf7aeae8 100644
> > --- a/libavutil/frame.c
> > +++ b/libavutil/frame.c
> > @@ -216,6 +216,7 @@ static int get_video_buffer(AVFrame *frame, int
> align)
> > total_size += sizes[i];
> > }
> >
> > +total_size += align - 1;
> > frame->buf[0] = av_buffer_alloc(total_size);
> > if (!frame->buf[0]) {
> > ret = AVERROR(ENOMEM);
> > @@ -223,7 +224,8 @@ static int get_video_buffer(AVFrame *frame, int
> align)
> > }
> >
> > if ((ret = av_image_fill_pointers(frame->data, frame->format,
> padded_height,
> > - frame->buf[0]->data,
> frame->linesize)) < 0)
> > + (uint8_t
> *)FFALIGN((uintptr_t)frame->buf[0]->data, align),
> > + frame->linesize)) < 0)
> > goto fail;
> >
> > for (int i = 1; i < 4; i++) {
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/2] avcodec/x86/diracdsp: Migrate last remaining MMX function to SSE2
On Thu, Nov 07, 2024 at 06:25:14PM +0900, Kyosuke Kawakami wrote: > The add_dirac_obmc8_mmx function was the only MMX function left. This > patch migrates it to SSE2. > > Signed-off-by: Kyosuke Kawakami > --- > libavcodec/x86/diracdsp.asm| 4 +--- > libavcodec/x86/diracdsp_init.c | 10 +++--- > 2 files changed, 4 insertions(+), 10 deletions(-) this seems to break fate: --- ./tests/ref/fate/dirac 2024-11-08 01:56:45.380642896 +0100 +++ tests/data/fate/dirac 2024-11-09 12:54:45.409451735 +0100 @@ -4,32 +4,32 @@ #dimensions 0: 320x240 #sar 0: 1/1 0, 0, 0,1, 115200, 0xf73819e8 -0, 1, 1,1, 115200, 0x082e3788 -0, 2, 2,1, 115200, 0x9fe73790 -0, 3, 3,1, 115200, 0x58f63cc4 -0, 4, 4,1, 115200, 0xd91c3767 -0, 5, 5,1, 115200, 0xac39658c -0, 6, 6,1, 115200, 0xd6d57406 -0, 7, 7,1, 115200, 0x791b707f -0, 8, 8,1, 115200, 0x02e05e31 -0, 9, 9,1, 115200, 0x7737ca43 -0, 10, 10,1, 115200, 0xa9b5b019 -0, 11, 11,1, 115200, 0x8b2685be -0, 12, 12,1, 115200, 0x2f547334 -0, 13, 13,1, 115200, 0x9c2ba0ad -0, 14, 14,1, 115200, 0x17069da3 -0, 15, 15,1, 115200, 0xbc7fadd1 -0, 16, 16,1, 115200, 0xbf651cce -0, 17, 17,1, 115200, 0x2e1abc0d -0, 18, 18,1, 115200, 0xc6c9a945 -0, 19, 19,1, 115200, 0x5234c510 -0, 20, 20,1, 115200, 0x84b5ab26 -0, 21, 21,1, 115200, 0xf01da61e -0, 22, 22,1, 115200, 0xfb339d74 -0, 23, 23,1, 115200, 0x0973bf98 -0, 24, 24,1, 115200, 0x7467023a -0, 25, 25,1, 115200, 0x3c8ba9a1 -0, 26, 26,1, 115200, 0xdc699e3e -0, 27, 27,1, 115200, 0xe57f9d2e -0, 28, 28,1, 115200, 0x79b18bc4 -0, 29, 29,1, 115200, 0x4c4c98a0 +0, 1, 1,1, 115200, 0x79d478bd +0, 2, 2,1, 115200, 0x2cad71ae +0, 3, 3,1, 115200, 0xde5e70f0 +0, 4, 4,1, 115200, 0x051eda0a +0, 5, 5,1, 115200, 0x8ddb0bc5 +0, 6, 6,1, 115200, 0x2ef5b7f3 +0, 7, 7,1, 115200, 0xac002aa2 +0, 8, 8,1, 115200, 0xb535e712 +0, 9, 9,1, 115200, 0x1e7f66bb +0, 10, 10,1, 115200, 0x61255fdb +0, 11, 11,1, 115200, 0x8ef6ba7a +0, 12, 12,1, 115200, 0x71e4330e +0, 13, 13,1, 115200, 0x277e78d8 +0, 14, 14,1, 115200, 0xb4a01606 +0, 15, 15,1, 115200, 0x2589c385 +0, 16, 16,1, 115200, 0x698464a1 +0, 17, 17,1, 115200, 0xa49ca790 +0, 18, 18,1, 115200, 0xc993165c +0, 19, 19,1, 115200, 0x03650ebc +0, 20, 20,1, 115200, 0xa686e472 +0, 21, 21,1, 115200, 0x5e265cd5 +0, 22, 22,1, 115200, 0xcb6b5084 +0, 23, 23,1, 115200, 0xc186b9e0 +0, 24, 24,1, 115200, 0xa767fc74 +0, 25, 25,1, 115200, 0x118894d1 +0, 26, 26,1, 115200, 0x753246a0 +0, 27, 27,1, 115200, 0x3e8268fc +0, 28, 28,1, 115200, 0x95107de0 +0, 29, 29,1, 115200, 0x437035db Test dirac failed. Look at tests/data/fate/dirac.err for details. make: *** [tests/Makefile:311: fate-dirac] Error 1 thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I am the wisest man alive, for I know one thing, and that is that I know nothing. -- Socrates signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavfi/zscale: Fix unaligned data ptr issues in realign_frame
On 11/9/2024 1:40 AM, Pavel Koshevoy wrote:
On Fri, Nov 8, 2024 at 7:29 PM James Almer wrote:
On 11/8/2024 10:51 PM, Pavel Koshevoy wrote:
I ran into segfaults in zimg when I attempted to use zscale
to convert a 512x538@yuv444p16le(tv) image from HLG to Bt.709
with this filter chain:
buffer=width=512:height=538:pix_fmt=yuv444p16le:time_base=1/1:sar=1/1,zscale=min=2020_ncl:rin=limited:pin=2020:tin=arib-std-b67:cin=left:t=linear,format=gbrpf32le,tonemap=gamma:desat=0,zscale=tin=linear:npl=100:p=709:m=709:r=limited:t=709,format=pix_fmts=yuv444p16le,buffersink
I found several issues:
- realign_frame called av_pix_fmt_count_planes with incorrect parameter.
- av_frame_get_buffer did not align data pointers to specified alignment.
Because it's not supposed to. The align parameter doxy states "buffer
size alignment", which is the result of aligning the stride/linesizes,
not the data pointers.
You may want to use ff_default_get_video_buffer2(), which will add align
amount of bytes to the allocated buffers (On top of aligning the
linesizes to it), and then align the pointers with FFALIGN().
I am not the maintainer of vf_zscale, and I am not intimately familiar with
private ffmpeg APIs such as ff_default_get_video_buffer2, and at first
glance that function
doesn't appear to be a drop-in replacement for av_frame_get_buffer.
ff_default_get_video_buffer2 requires AVFilterLink parameter?! --
realign_frame doesn't
have that, it has an AVFrame which it needs to re-align to make it
compatible with libzimg API.
It's trivial to make realign_frame take the necessary AVFilterLink as
input argument.
... and why isn't av_frame_get_buffer populating AVFrame.data pointers
aligned
as specified by the align parameter? It costs at most (align - 1) more
padding bytes
to make it align the pointers, so I don't understand why it doesn't do that.
Buffer alignment is set at configure time. It will be set to the highest
needed alignment for the enabled simd (64 if avx512, else 32 if avx,
else 16 if neon/sse, else 8). This is handled by av_malloc(), which is
ultimately used by all alloc functions.
As an specific alignment is not guaranteed, workarounds are needed if a
module requires one.
I took the time to do it for zscale, as follows:
diff --git a/libavfilter/vf_zscale.c b/libavfilter/vf_zscale.c
index 4ba059064b..c6518b0f9f 100644
--- a/libavfilter/vf_zscale.c
+++ b/libavfilter/vf_zscale.c
@@ -34,6 +34,7 @@
#include "filters.h"
#include "formats.h"
#include "video.h"
+#include "libavutil/cpu.h"
#include "libavutil/eval.h"
#include "libavutil/internal.h"
#include "libavutil/intreadwrite.h"
@@ -657,27 +658,23 @@ static int graphs_build(AVFrame *in, AVFrame *out, const
AVPixFmtDescriptor *des
return 0;
}
-static int realign_frame(const AVPixFmtDescriptor *desc, AVFrame **frame, int
needs_copy)
+static int realign_frame(AVFilterLink *link, const AVPixFmtDescriptor *desc,
AVFrame **frame, int needs_copy)
{
AVFrame *aligned = NULL;
int ret = 0, plane, planes;
/* Realign any unaligned input frame. */
-planes = av_pix_fmt_count_planes(desc->nb_components);
+planes = av_pix_fmt_count_planes((*frame)->format);
for (plane = 0; plane < planes; plane++) {
int p = desc->comp[plane].plane;
if ((uintptr_t)(*frame)->data[p] % ZIMG_ALIGNMENT ||
(*frame)->linesize[p] % ZIMG_ALIGNMENT) {
-if (!(aligned = av_frame_alloc())) {
-ret = AVERROR(ENOMEM);
-goto fail;
-}
+aligned = ff_default_get_video_buffer2(link, (*frame)->width,
(*frame)->height,
+ FFMAX(av_cpu_max_align(),
ZIMG_ALIGNMENT));
+if (!aligned)
+return AVERROR(ENOMEM);
-aligned->format = (*frame)->format;
-aligned->width = (*frame)->width;
-aligned->height = (*frame)->height;
-
-if ((ret = av_frame_get_buffer(aligned, ZIMG_ALIGNMENT)) < 0)
-goto fail;
+for (int i = 0; i < 4 && aligned->data[i]; i++)
+aligned->data[i] = (uint8_t
*)FFALIGN((uintptr_t)aligned->data[i], FFMAX(av_cpu_max_align(), ZIMG_ALIGNMENT));
if (needs_copy && (ret = av_frame_copy(aligned, *frame)) < 0)
goto fail;
@@ -802,20 +799,22 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
(s->src_format.pixel_type !=s->dst_format.pixel_type) ||
(s->src_format.transfer_characteristics
!=s->dst_format.transfer_characteristics)
){
-out = ff_get_video_buffer(outlink, outlink->w, outlink->h);
+out = ff_default_get_video_buffer2(outlink, outlink->w, outlink->h,
+ FFMAX(av_cpu_max_align(),
ZIMG_ALIGNMENT));
if (!out) {
ret = AVERROR(ENOMEM);
goto fail;
}
-if ((ret = realign_frame(odesc, &out, 0)) < 0)
-goto fail;
+for
Re: [FFmpeg-devel] [PATCH] lavfi/zscale: Fix unaligned data ptr issues in realign_frame
On 11/9/2024 12:55 PM, Pavel Koshevoy wrote:
On Sat, Nov 9, 2024 at 6:22 AM James Almer wrote:
On 11/9/2024 1:40 AM, Pavel Koshevoy wrote:
On Fri, Nov 8, 2024 at 7:29 PM James Almer wrote:
On 11/8/2024 10:51 PM, Pavel Koshevoy wrote:
I ran into segfaults in zimg when I attempted to use zscale
to convert a 512x538@yuv444p16le(tv) image from HLG to Bt.709
with this filter chain:
buffer=width=512:height=538:pix_fmt=yuv444p16le:time_base=1/1:sar=1/1,zscale=min=2020_ncl:rin=limited:pin=2020:tin=arib-std-b67:cin=left:t=linear,format=gbrpf32le,tonemap=gamma:desat=0,zscale=tin=linear:npl=100:p=709:m=709:r=limited:t=709,format=pix_fmts=yuv444p16le,buffersink
I found several issues:
- realign_frame called av_pix_fmt_count_planes with incorrect
parameter.
- av_frame_get_buffer did not align data pointers to specified
alignment.
Because it's not supposed to. The align parameter doxy states "buffer
size alignment", which is the result of aligning the stride/linesizes,
not the data pointers.
You may want to use ff_default_get_video_buffer2(), which will add align
amount of bytes to the allocated buffers (On top of aligning the
linesizes to it), and then align the pointers with FFALIGN().
I am not the maintainer of vf_zscale, and I am not intimately familiar
with
private ffmpeg APIs such as ff_default_get_video_buffer2, and at first
glance that function
doesn't appear to be a drop-in replacement for av_frame_get_buffer.
ff_default_get_video_buffer2 requires AVFilterLink parameter?! --
realign_frame doesn't
have that, it has an AVFrame which it needs to re-align to make it
compatible with libzimg API.
It's trivial to make realign_frame take the necessary AVFilterLink as
input argument.
... and why isn't av_frame_get_buffer populating AVFrame.data pointers
aligned
as specified by the align parameter? It costs at most (align - 1) more
padding bytes
to make it align the pointers, so I don't understand why it doesn't do
that.
Buffer alignment is set at configure time. It will be set to the highest
needed alignment for the enabled simd (64 if avx512, else 32 if avx,
else 16 if neon/sse, else 8). This is handled by av_malloc(), which is
ultimately used by all alloc functions.
Yes, I have noticed this while stepping through ffmpeg and zimg code.
The surprising thing I've observed is that ff_get_cpu_max_align_x86()
(called from av_cpu_max_align()) returned 8 ... it's surprising for an
ffmpeg
built and running on a Ryzen R9 5950x -- I would have expected 32.
As a side note ... this ffmpeg build (and zimg build) were produced by
conan,
so perhaps the conan recipe for ffmpeg needs some changes to make
av_cpu_max_align() work as expected on 5950x.
(https://conan.io/center/recipes/ffmpeg)
As an specific alignment is not guaranteed, workarounds are needed if a
module requires one.
That is true of av_malloc, but av_frame_get_buffer is given an explicit
align parameter,
and it could trivially align the pointers accordingly making any external
workarounds
unnecessary.
I still think my change to av_frame_get_buffer is the better approach:
- it doesn't break the ABI
- it doesn't break the API
- it improves the API behavior at little cost in allocated buffer padding
- it likely fixes other (unknown) instances where av_frame_get_buffer
was expected to provide aligned data pointers and didn't, and the caller
is unaware of this.
I took the time to do it for zscale, as follows:
Thank you for providing this patch. However, I would argue that this
functionality
(allocating a sufficient buffer and filling an AVFrame with properly
aligned data pointers
according to an explicitly specified alignment parameter) should be
available
via a public AVFrame API like av_frame_get_buffer,
and not require calls to a private libavfilter API.
It feels a little bit like a Banana - Gorilla - Jungle problem when an
AVFilterLink
is required to produce an AVFrame with properly aligned data pointers.
I sent a separate patch to have ff_default_get_video_buffer2() align the
buffers using the provided align value. See "avfilter/framepool: align
the frame buffers".
ff_default_get_video_buffer2() also uses a buffer pool, so it's better
than av_frame_get_buffer() in this case.
diff --git a/libavfilter/vf_zscale.c b/libavfilter/vf_zscale.c
index 4ba059064b..c6518b0f9f 100644
--- a/libavfilter/vf_zscale.c
+++ b/libavfilter/vf_zscale.c
@@ -34,6 +34,7 @@
#include "filters.h"
#include "formats.h"
#include "video.h"
+#include "libavutil/cpu.h"
#include "libavutil/eval.h"
#include "libavutil/internal.h"
#include "libavutil/intreadwrite.h"
@@ -657,27 +658,23 @@ static int graphs_build(AVFrame *in, AVFrame *out,
const AVPixFmtDescriptor *des
return 0;
}
-static int realign_frame(const AVPixFmtDescriptor *desc, AVFrame
**frame, int needs_copy)
+static int realign_frame(AVFilterLink *link, const AVPixFmtDescriptor
*desc, AVFrame **frame, int needs_copy)
{
AVFrame *aligned = NULL
Re: [FFmpeg-devel] [PATCH] lavfi/zscale: fix tmp ptr alignment for zimg_filter_graph_process
On 11/9/2024 1:37 PM, Pavel Koshevoy wrote: --- libavfilter/vf_zscale.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/libavfilter/vf_zscale.c b/libavfilter/vf_zscale.c index 4ba059064b..87fe691cfd 100644 --- a/libavfilter/vf_zscale.c +++ b/libavfilter/vf_zscale.c @@ -750,7 +750,9 @@ static int filter_slice(AVFilterContext *ctx, void *data, int job_nr, int n_jobs } if (!s->graph[job_nr]) return AVERROR(EINVAL); -ret = zimg_filter_graph_process(s->graph[job_nr], &src_buf, &dst_buf, s->tmp[job_nr], 0, 0, 0, 0); +ret = zimg_filter_graph_process(s->graph[job_nr], &src_buf, &dst_buf, +(uint8_t *)FFALIGN((uintptr_t)s->tmp[job_nr], ZIMG_ALIGNMENT), Does s->tmp[job_nr] have enough extra bytes for this? +0, 0, 0, 0); if (ret) return print_zimg_error(ctx); @@ -765,7 +767,9 @@ static int filter_slice(AVFilterContext *ctx, void *data, int job_nr, int n_jobs if (!s->alpha_graph[job_nr]) return AVERROR(EINVAL); -ret = zimg_filter_graph_process(s->alpha_graph[job_nr], &src_buf, &dst_buf, s->tmp[job_nr], 0, 0, 0, 0); +ret = zimg_filter_graph_process(s->alpha_graph[job_nr], &src_buf, &dst_buf, +(uint8_t *)FFALIGN((uintptr_t)s->tmp[job_nr], ZIMG_ALIGNMENT), +0, 0, 0, 0); if (ret) return print_zimg_error(ctx); } OpenPGP_signature.asc Description: OpenPGP digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavfi/zscale: fix tmp ptr alignment for zimg_filter_graph_process
On Sat, Nov 9, 2024 at 9:43 AM James Almer wrote: > On 11/9/2024 1:37 PM, Pavel Koshevoy wrote: > > --- > > libavfilter/vf_zscale.c | 8 ++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/libavfilter/vf_zscale.c b/libavfilter/vf_zscale.c > > index 4ba059064b..87fe691cfd 100644 > > --- a/libavfilter/vf_zscale.c > > +++ b/libavfilter/vf_zscale.c > > @@ -750,7 +750,9 @@ static int filter_slice(AVFilterContext *ctx, void > *data, int job_nr, int n_jobs > > } > > if (!s->graph[job_nr]) > > return AVERROR(EINVAL); > > -ret = zimg_filter_graph_process(s->graph[job_nr], &src_buf, > &dst_buf, s->tmp[job_nr], 0, 0, 0, 0); > > +ret = zimg_filter_graph_process(s->graph[job_nr], &src_buf, > &dst_buf, > > +(uint8_t > *)FFALIGN((uintptr_t)s->tmp[job_nr], ZIMG_ALIGNMENT), > > Does s->tmp[job_nr] have enough extra bytes for this? > probably no, I'll send another patch. > > > +0, 0, 0, 0); > > if (ret) > > return print_zimg_error(ctx); > > > > @@ -765,7 +767,9 @@ static int filter_slice(AVFilterContext *ctx, void > *data, int job_nr, int n_jobs > > > > if (!s->alpha_graph[job_nr]) > > return AVERROR(EINVAL); > > -ret = zimg_filter_graph_process(s->alpha_graph[job_nr], > &src_buf, &dst_buf, s->tmp[job_nr], 0, 0, 0, 0); > > +ret = zimg_filter_graph_process(s->alpha_graph[job_nr], > &src_buf, &dst_buf, > > +(uint8_t > *)FFALIGN((uintptr_t)s->tmp[job_nr], ZIMG_ALIGNMENT), > > +0, 0, 0, 0); > > if (ret) > > return print_zimg_error(ctx); > > } > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] lavu/get_video_buffer: also align data pointers
This avoids unpleasant surprises to av_frame_get_buffer callers
that explicitly specified 64-byte alignment and didn't get
AVFrame.data pointers that are 64-byte aligned.
In particular, this fixes an issue in vf_zscale.
https://github.com/sekrit-twc/zimg/issues/212
---
libavutil/frame.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/libavutil/frame.c b/libavutil/frame.c
index f0a0dba018..7faf7aeae8 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -216,6 +216,7 @@ static int get_video_buffer(AVFrame *frame, int align)
total_size += sizes[i];
}
+total_size += align - 1;
frame->buf[0] = av_buffer_alloc(total_size);
if (!frame->buf[0]) {
ret = AVERROR(ENOMEM);
@@ -223,7 +224,8 @@ static int get_video_buffer(AVFrame *frame, int align)
}
if ((ret = av_image_fill_pointers(frame->data, frame->format,
padded_height,
- frame->buf[0]->data, frame->linesize)) <
0)
+ (uint8_t
*)FFALIGN((uintptr_t)frame->buf[0]->data, align),
+ frame->linesize)) < 0)
goto fail;
for (int i = 1; i < 4; i++) {
--
2.43.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavfi/zscale: Fix unaligned data ptr issues in realign_frame
On 11/9/2024 12:55 PM, Pavel Koshevoy wrote: On Sat, Nov 9, 2024 at 6:22 AM James Almer wrote: On 11/9/2024 1:40 AM, Pavel Koshevoy wrote: On Fri, Nov 8, 2024 at 7:29 PM James Almer wrote: On 11/8/2024 10:51 PM, Pavel Koshevoy wrote: I ran into segfaults in zimg when I attempted to use zscale to convert a 512x538@yuv444p16le(tv) image from HLG to Bt.709 with this filter chain: buffer=width=512:height=538:pix_fmt=yuv444p16le:time_base=1/1:sar=1/1,zscale=min=2020_ncl:rin=limited:pin=2020:tin=arib-std-b67:cin=left:t=linear,format=gbrpf32le,tonemap=gamma:desat=0,zscale=tin=linear:npl=100:p=709:m=709:r=limited:t=709,format=pix_fmts=yuv444p16le,buffersink I found several issues: - realign_frame called av_pix_fmt_count_planes with incorrect parameter. - av_frame_get_buffer did not align data pointers to specified alignment. Because it's not supposed to. The align parameter doxy states "buffer size alignment", which is the result of aligning the stride/linesizes, not the data pointers. You may want to use ff_default_get_video_buffer2(), which will add align amount of bytes to the allocated buffers (On top of aligning the linesizes to it), and then align the pointers with FFALIGN(). I am not the maintainer of vf_zscale, and I am not intimately familiar with private ffmpeg APIs such as ff_default_get_video_buffer2, and at first glance that function doesn't appear to be a drop-in replacement for av_frame_get_buffer. ff_default_get_video_buffer2 requires AVFilterLink parameter?! -- realign_frame doesn't have that, it has an AVFrame which it needs to re-align to make it compatible with libzimg API. It's trivial to make realign_frame take the necessary AVFilterLink as input argument. ... and why isn't av_frame_get_buffer populating AVFrame.data pointers aligned as specified by the align parameter? It costs at most (align - 1) more padding bytes to make it align the pointers, so I don't understand why it doesn't do that. Buffer alignment is set at configure time. It will be set to the highest needed alignment for the enabled simd (64 if avx512, else 32 if avx, else 16 if neon/sse, else 8). This is handled by av_malloc(), which is ultimately used by all alloc functions. Yes, I have noticed this while stepping through ffmpeg and zimg code. The surprising thing I've observed is that ff_get_cpu_max_align_x86() (called from av_cpu_max_align()) returned 8 ... it's surprising for an ffmpeg built and running on a Ryzen R9 5950x -- I would have expected 32. Yeah, av_cpu_max_align() returns a value depending on runtime parameters, so unless you force cpuflags using av_force_cpu_flags() to disable everything SSE and above (or it being disabled during configure), it should not return 8. As a side note ... this ffmpeg build (and zimg build) were produced by conan, so perhaps the conan recipe for ffmpeg needs some changes to make av_cpu_max_align() work as expected on 5950x. (https://conan.io/center/recipes/ffmpeg) OpenPGP_signature.asc Description: OpenPGP digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] lavfi/zscale: fix tmp buffer ptr alignment for zimg_filter_graph_process (v2)
--- libavfilter/vf_zscale.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/libavfilter/vf_zscale.c b/libavfilter/vf_zscale.c index 4ba059064b..07e295652d 100644 --- a/libavfilter/vf_zscale.c +++ b/libavfilter/vf_zscale.c @@ -630,7 +630,7 @@ static int graphs_build(AVFrame *in, AVFrame *out, const AVPixFmtDescriptor *des if (s->tmp[job_nr]) av_freep(&s->tmp[job_nr]); -s->tmp[job_nr] = av_calloc(size, 1); +s->tmp[job_nr] = av_calloc(size + ZIMG_ALIGNMENT - 1, 1); if (!s->tmp[job_nr]) return AVERROR(ENOMEM); @@ -750,7 +750,9 @@ static int filter_slice(AVFilterContext *ctx, void *data, int job_nr, int n_jobs } if (!s->graph[job_nr]) return AVERROR(EINVAL); -ret = zimg_filter_graph_process(s->graph[job_nr], &src_buf, &dst_buf, s->tmp[job_nr], 0, 0, 0, 0); +ret = zimg_filter_graph_process(s->graph[job_nr], &src_buf, &dst_buf, +(uint8_t *)FFALIGN((uintptr_t)s->tmp[job_nr], ZIMG_ALIGNMENT), +0, 0, 0, 0); if (ret) return print_zimg_error(ctx); @@ -765,7 +767,9 @@ static int filter_slice(AVFilterContext *ctx, void *data, int job_nr, int n_jobs if (!s->alpha_graph[job_nr]) return AVERROR(EINVAL); -ret = zimg_filter_graph_process(s->alpha_graph[job_nr], &src_buf, &dst_buf, s->tmp[job_nr], 0, 0, 0, 0); +ret = zimg_filter_graph_process(s->alpha_graph[job_nr], &src_buf, &dst_buf, +(uint8_t *)FFALIGN((uintptr_t)s->tmp[job_nr], ZIMG_ALIGNMENT), +0, 0, 0, 0); if (ret) return print_zimg_error(ctx); } -- 2.43.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC] dormant git accounts
Le lauantaina 9. marraskuuta 2024, 18.18.08 EET Michael Niedermayer a écrit : > Hi all > > Should we disable git accounts for developers who have not been active since > a long time (like 10 years) ? Yes but git is probably the least dangerous of credentials to keep stale. A backdoor getting pushed with a stale and stolen SSH private key would be noticed and rectified in no time. What most people are concerned about right now is the incomplete documentation of any and all credentials - not just git write access - and more generally the lack of transparency. Once that is sorted out, we can start arguing about what should be revoked. -- 雷米‧德尼-库尔蒙 http://www.remlab.net/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC] libpostproc splitout
Le lauantaina 9. marraskuuta 2024, 18.11.07 EET Michael Niedermayer a écrit : > > And sorry but while I am all for splitting postproc to a separate > > repository, it is at best a few hundreds euros worth of consulting time. > > The difficult part, if there is one, is to reach the agreement to do it, > > not to actually do it. > Iam sorry but spliting out libpostproc into a maintainable seperate source > tree with self tests and everything is not just "a few hundread euros work". Your question was literally whether "should libpostproc be split out into a seperate (sic) source repository". I maintain that that would be worth a couple hundred eurors - if it had not already been done. The question of maintaining libpostproc is mostly orthogonal. The value of that work could be essentially anything from zero to hundred of thousands, since it is completely dependent on how much additions and renovations are taken up. However if we assume that the library is to be split out, then this is no longer a matter for FFmpeg-devel, making everybody here incompetent (in the judicial sense) to answer the question. On the other hand, if we assume that it does not get separated, I think the lack of activity tells about the impopularity of the library. I don't see the point in spending time and/or money if nobody cares anymore, unless this is a research project but that does not seem to be what you are proposing. -- 雷米‧德尼-库尔蒙 http://www.remlab.net/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavu/get_video_buffer: also align data pointers
On 11/9/2024 2:34 PM, Pavel Koshevoy wrote: On Sat, Nov 9, 2024 at 10:27 AM James Almer wrote: On 11/9/2024 1:57 PM, Pavel Koshevoy wrote: This avoids unpleasant surprises to av_frame_get_buffer callers that explicitly specified 64-byte alignment and didn't get AVFrame.data pointers that are 64-byte aligned. Again, the doxy is clear that only the buffer sizes are aligned, not the pointers. I'd rather not make this change here. Please see https://ffmpeg.org//pipermail/ffmpeg-devel/2024-November/335811.html So if we update the doxy then the change will be accepted? I don't understand the reluctance to improve the behavior of this API. I simply prefer to not change how public API behaves if the problem can be fixed with an internal change. That said, if other developers agree with you, then I'm not going to block such a change. OpenPGP_signature.asc Description: OpenPGP digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavu/get_video_buffer: also align data pointers
On Sat, Nov 9, 2024 at 10:39 AM James Almer wrote: > On 11/9/2024 2:34 PM, Pavel Koshevoy wrote: > > On Sat, Nov 9, 2024 at 10:27 AM James Almer wrote: > > > >> On 11/9/2024 1:57 PM, Pavel Koshevoy wrote: > >>> This avoids unpleasant surprises to av_frame_get_buffer callers > >>> that explicitly specified 64-byte alignment and didn't get > >>> AVFrame.data pointers that are 64-byte aligned. > >> > >> Again, the doxy is clear that only the buffer sizes are aligned, not the > >> pointers. I'd rather not make this change here. Please see > >> https://ffmpeg.org//pipermail/ffmpeg-devel/2024-November/335811.html > >> > >> > > So if we update the doxy then the change will be accepted? > > I don't understand the reluctance to improve the behavior of this API. > > I simply prefer to not change how public API behaves if the problem can > be fixed with an internal change. > That said, if other developers agree with you, then I'm not going to > block such a change. > > AVFrame.data pointers being aligned according to the explicitly specified align parameter is not going to break any caller. It is a safe change, and I would think a welcome change for any users of av_frame_get_buffer. I too would like to hear additional comments from other developers. Thank you, Pavel. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/speexdec: fix decoding regressions
Sorry, I didn't notice the license change. On Sat, Nov 9, 2024 at 10:33 AM Michael Niedermayer wrote: > Hi > > I have received the mail below from paul, I belive it refers to these > patches here > > On Sat, Nov 09, 2024 at 08:09:15AM +0100, Paul B Mahol wrote: > > I see you reviewing one of stolen (The author of mail stole multiple > > patches and changed original authorship) code patches from librempeg > > project, specifically for speex decoder. > > > > As librempeg is GPL2 only you are not allowed to merge it to FFmpeg > project. > > > > Not complying with license in this specific case and any case in future > > will take more actions in future against such practices. > > > > [...] > > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Everything should be made as simple as possible, but not simpler. > -- Albert Einstein > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] fate/pixfmt: disable dithering in the scale filter
On 11/8/2024 9:03 PM, Michael Niedermayer wrote: On Fri, Nov 08, 2024 at 04:56:38PM -0300, James Almer wrote: On 11/8/2024 5:15 AM, Martin Storsjö wrote: On Thu, 7 Nov 2024, James Almer wrote: Should fix fate failures across different systems. Signed-off-by: James Almer --- This only hides the bug in the dither path for unscaled planar yuv 8bit to hbd and vice-versa. Someone more familiar with the code should check what exactly is going on. LGTM, thanks! Good job in hunting this one down - it seems odd as the exact output value seems stable on each machine (and valgrind doesn't find any nondeterminism), but the output is different on most machines. The only tests that fail are those using the DITHER_COPY method in planarCopyWrapper() when dither is used, and the only difference compared to the no dither path is accessing the "dithers" static const array defined in the same file. Maybe compilers are doing something weird when aligning it, or accessing it? Not sure i fully understand but the brute force way to debug this is maybe just add a printf() to dump every value then diff between to see where differences start. I can't even reproduce the issue locally, so... OpenPGP_signature.asc Description: OpenPGP digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/speexdec: fix decoding regressions
Hi I have received the mail below from paul, I belive it refers to these patches here On Sat, Nov 09, 2024 at 08:09:15AM +0100, Paul B Mahol wrote: > I see you reviewing one of stolen (The author of mail stole multiple > patches and changed original authorship) code patches from librempeg > project, specifically for speex decoder. > > As librempeg is GPL2 only you are not allowed to merge it to FFmpeg project. > > Not complying with license in this specific case and any case in future > will take more actions in future against such practices. > [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Everything should be made as simple as possible, but not simpler. -- Albert Einstein signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 3/5] lavc/vvc_mc: R-V V put_uni_pixels
Le maanantaina 28. lokakuuta 2024, 19.08.24 EET u...@foxmail.com a écrit : > From: sunyuechi > > k230 > banana_f3 put_uni_pixels_chroma_8_4x4_c: 128.3 ( > 1.00x)90.5 ( 1.00x) put_uni_pixels_chroma_8_4x4_rvv_i32: > 17.6 ( 7.30x)17.4 ( 5.18x) put_uni_pixels_chroma_8_8x8_c: > 295.1 ( 1.00x)163.2 ( 1.00x) > put_uni_pixels_chroma_8_8x8_rvv_i32:35.8 ( 8.24x) > 27.9 ( 5.84x) put_uni_pixels_chroma_8_16x16_c: 619.3 > ( 1.00x)267.4 ( 1.00x) put_uni_pixels_chroma_8_16x16_rvv_i32: >72.8 ( 8.50x)48.7 ( 5.49x) put_uni_pixels_chroma_8_32x32_c: > 1433.8 ( 1.00x)538.2 ( 1.00x) > put_uni_pixels_chroma_8_32x32_rvv_i32: 230.3 ( 6.23x) > 236.2 ( 2.28x) put_uni_pixels_chroma_8_64x64_c: 3517.3 > ( 1.00x)1455.0 ( 1.00x) put_uni_pixels_chroma_8_64x64_rvv_i32: >813.6 ( 4.32x)590.2 ( 2.47x) put_uni_pixels_chroma_8_128x128_c: > 10174.6 ( 1.00x)5798.7 ( 1.00x) > put_uni_pixels_chroma_8_128x128_rvv_i32: 2989.3 ( 3.40x) > 2371.4 ( 2.45x) put_uni_pixels_luma_8_4x4_c: > 128.6 ( 1.00x)90.5 ( 1.00x) put_uni_pixels_luma_8_4x4_rvv_i32: > 17.3 ( 7.42x)17.4 ( 5.18x) put_uni_pixels_luma_8_8x8_c: > 295.1 ( 1.00x)142.4 ( 1.00x) > put_uni_pixels_luma_8_8x8_rvv_i32: 26.6 (11.10x) > 27.9 ( 5.10x) put_uni_pixels_luma_8_16x16_c: 600.6 > ( 1.00x)277.7 ( 1.00x) put_uni_pixels_luma_8_16x16_rvv_i32: >82.1 ( 7.32x)48.7 ( 5.70x) put_uni_pixels_luma_8_32x32_c: > 1406.1 ( 1.00x)528.0 ( 1.00x) > put_uni_pixels_luma_8_32x32_rvv_i32: 230.3 ( 6.10x) > 131.9 ( 4.00x) put_uni_pixels_luma_8_64x64_c:4600.6 > ( 1.00x)1309.2 ( 1.00x) put_uni_pixels_luma_8_64x64_rvv_i32: > 1073.1 ( 4.29x)382.2 ( 3.43x) put_uni_pixels_luma_8_128x128_c: > 11350.3 ( 1.00x)3506.9 ( 1.00x) > put_uni_pixels_luma_8_128x128_rvv_i32:3119.1 ( 3.64x) > 2017.5 ( 1.74x) --- > libavcodec/riscv/h26x/h2656_inter_rvv.S | 53 + > libavcodec/riscv/h26x/h2656dsp.h| 33 +++ > libavcodec/riscv/vvc/Makefile | 3 +- > libavcodec/riscv/vvc/vvcdsp_init.c | 5 +++ > 4 files changed, 93 insertions(+), 1 deletion(-) > create mode 100644 libavcodec/riscv/h26x/h2656_inter_rvv.S > create mode 100644 libavcodec/riscv/h26x/h2656dsp.h > > diff --git a/libavcodec/riscv/h26x/h2656_inter_rvv.S > b/libavcodec/riscv/h26x/h2656_inter_rvv.S new file mode 100644 > index 00..6692e33acf > --- /dev/null > +++ b/libavcodec/riscv/h26x/h2656_inter_rvv.S > @@ -0,0 +1,53 @@ > +/* > + * Copyright (c) 2024 Institue of Software Chinese Academy of Sciences > (ISCAS). + * > + * This file is part of FFmpeg. > + * > + * FFmpeg is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * FFmpeg is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with FFmpeg; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 > USA + */ > + > +#include "libavcodec/riscv/h26x/asm.S" > + > +.macro put_uni_pixels w, vlen, id > +\id\w\vlen: > +.if \w == 128 && \vlen == 128 > +lit0, \w > +vsetvli zero, t0, e8, m8, ta, ma > +.else > +vsetvlstatic8 \w, \vlen > +.endif > +1: > +vle8.vv0, (a2) > +addi a4, a4, -1 > +vse8.vv0, (a0) > +add a2, a2, a3 > +add a0, a0, a1 > +bnez a4, 1b > +ret > +.endm Is this going to be reused anywhere? it seems the macro is only used once atm. Also is there a reason to use RVV here instead of just unaligned RVI? > + > +.macro func_put_uni_pixels vlen > +func ff_h2656_put_uni_pixels_8_rvv_\vlen\(), zve32x, zbb, zba > +lpad0 > +POW2_JMP_TABLE4, \vlen > +POW2_J\vlen, 4, a7 > +.irp w,2,4,8,16,32,64,128 > +put_uni_pixels\w, \vlen, 4 > +.endr > +endfunc > +.endm > + > +func_put_uni_pixels 256 > +func_put_uni_pixels 128 > diff --git a/
Re: [FFmpeg-devel] [PATCH v7] avcodec/jpeg2000: Fix FF_DWT97_INT to pass the conformance testing defined in ISO/IEC 15444-4
On 11/8/2024 2:23 AM, Pierre-Anthony Lemieux wrote: Ok will merge the patch below. src/libavcodec/jpeg2000dec.c:1972:117: runtime error: left shift of 1 by 31 places cannot be represented in type 'int' https://fate.ffmpeg.org/report.cgi?time=20241109021127&slot=x86_64-archlinux-gcc-ubsan OpenPGP_signature.asc Description: OpenPGP digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavfi/zscale: Fix unaligned data ptr issues in realign_frame
On Sat, Nov 9, 2024 at 6:22 AM James Almer wrote:
> On 11/9/2024 1:40 AM, Pavel Koshevoy wrote:
> > On Fri, Nov 8, 2024 at 7:29 PM James Almer wrote:
> >
> >> On 11/8/2024 10:51 PM, Pavel Koshevoy wrote:
> >>> I ran into segfaults in zimg when I attempted to use zscale
> >>> to convert a 512x538@yuv444p16le(tv) image from HLG to Bt.709
> >>> with this filter chain:
> >>>
> >>>
> >>
> buffer=width=512:height=538:pix_fmt=yuv444p16le:time_base=1/1:sar=1/1,zscale=min=2020_ncl:rin=limited:pin=2020:tin=arib-std-b67:cin=left:t=linear,format=gbrpf32le,tonemap=gamma:desat=0,zscale=tin=linear:npl=100:p=709:m=709:r=limited:t=709,format=pix_fmts=yuv444p16le,buffersink
> >>>
> >>> I found several issues:
> >>> - realign_frame called av_pix_fmt_count_planes with incorrect
> parameter.
> >>> - av_frame_get_buffer did not align data pointers to specified
> alignment.
> >>
> >> Because it's not supposed to. The align parameter doxy states "buffer
> >> size alignment", which is the result of aligning the stride/linesizes,
> >> not the data pointers.
> >>
> >> You may want to use ff_default_get_video_buffer2(), which will add align
> >> amount of bytes to the allocated buffers (On top of aligning the
> >> linesizes to it), and then align the pointers with FFALIGN().
> >>
> >
> > I am not the maintainer of vf_zscale, and I am not intimately familiar
> with
> > private ffmpeg APIs such as ff_default_get_video_buffer2, and at first
> > glance that function
> > doesn't appear to be a drop-in replacement for av_frame_get_buffer.
> >
> > ff_default_get_video_buffer2 requires AVFilterLink parameter?! --
> > realign_frame doesn't
> > have that, it has an AVFrame which it needs to re-align to make it
> > compatible with libzimg API.
>
> It's trivial to make realign_frame take the necessary AVFilterLink as
> input argument.
>
> >
> > ... and why isn't av_frame_get_buffer populating AVFrame.data pointers
> > aligned
> > as specified by the align parameter? It costs at most (align - 1) more
> > padding bytes
> > to make it align the pointers, so I don't understand why it doesn't do
> that.
>
> Buffer alignment is set at configure time. It will be set to the highest
> needed alignment for the enabled simd (64 if avx512, else 32 if avx,
> else 16 if neon/sse, else 8). This is handled by av_malloc(), which is
> ultimately used by all alloc functions.
>
Yes, I have noticed this while stepping through ffmpeg and zimg code.
The surprising thing I've observed is that ff_get_cpu_max_align_x86()
(called from av_cpu_max_align()) returned 8 ... it's surprising for an
ffmpeg
built and running on a Ryzen R9 5950x -- I would have expected 32.
As a side note ... this ffmpeg build (and zimg build) were produced by
conan,
so perhaps the conan recipe for ffmpeg needs some changes to make
av_cpu_max_align() work as expected on 5950x.
(https://conan.io/center/recipes/ffmpeg)
> As an specific alignment is not guaranteed, workarounds are needed if a
> module requires one.
>
That is true of av_malloc, but av_frame_get_buffer is given an explicit
align parameter,
and it could trivially align the pointers accordingly making any external
workarounds
unnecessary.
I still think my change to av_frame_get_buffer is the better approach:
- it doesn't break the ABI
- it doesn't break the API
- it improves the API behavior at little cost in allocated buffer padding
- it likely fixes other (unknown) instances where av_frame_get_buffer
was expected to provide aligned data pointers and didn't, and the caller
is unaware of this.
>
> I took the time to do it for zscale, as follows:
>
>
Thank you for providing this patch. However, I would argue that this
functionality
(allocating a sufficient buffer and filling an AVFrame with properly
aligned data pointers
according to an explicitly specified alignment parameter) should be
available
via a public AVFrame API like av_frame_get_buffer,
and not require calls to a private libavfilter API.
It feels a little bit like a Banana - Gorilla - Jungle problem when an
AVFilterLink
is required to produce an AVFrame with properly aligned data pointers.
> > diff --git a/libavfilter/vf_zscale.c b/libavfilter/vf_zscale.c
> > index 4ba059064b..c6518b0f9f 100644
> > --- a/libavfilter/vf_zscale.c
> > +++ b/libavfilter/vf_zscale.c
> > @@ -34,6 +34,7 @@
> > #include "filters.h"
> > #include "formats.h"
> > #include "video.h"
> > +#include "libavutil/cpu.h"
> > #include "libavutil/eval.h"
> > #include "libavutil/internal.h"
> > #include "libavutil/intreadwrite.h"
> > @@ -657,27 +658,23 @@ static int graphs_build(AVFrame *in, AVFrame *out,
> const AVPixFmtDescriptor *des
> > return 0;
> > }
> >
> > -static int realign_frame(const AVPixFmtDescriptor *desc, AVFrame
> **frame, int needs_copy)
> > +static int realign_frame(AVFilterLink *link, const AVPixFmtDescriptor
> *desc, AVFrame **frame, int needs_copy)
> > {
> > AVFrame *aligned = NULL;
> > int ret = 0, plane, planes;
> >
> > /* Realign any unaligned i
Re: [FFmpeg-devel] [PATCH] doc/bitstream_filters: elaborate on h264_redundant_pps
Pushed ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] lavfi/zscale: fix call to av_pix_fmt_count_planes
realign_frame called av_pix_fmt_count_planes with incorrect parameter.
---
libavfilter/vf_zscale.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavfilter/vf_zscale.c b/libavfilter/vf_zscale.c
index 4ba059064b..c29c599000 100644
--- a/libavfilter/vf_zscale.c
+++ b/libavfilter/vf_zscale.c
@@ -663,7 +663,7 @@ static int realign_frame(const AVPixFmtDescriptor *desc,
AVFrame **frame, int ne
int ret = 0, plane, planes;
/* Realign any unaligned input frame. */
-planes = av_pix_fmt_count_planes(desc->nb_components);
+planes = av_pix_fmt_count_planes((*frame)->format);
for (plane = 0; plane < planes; plane++) {
int p = desc->comp[plane].plane;
if ((uintptr_t)(*frame)->data[p] % ZIMG_ALIGNMENT ||
(*frame)->linesize[p] % ZIMG_ALIGNMENT) {
--
2.43.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC] libpostproc splitout
On Fri, Nov 08, 2024 at 08:00:50PM +0200, Rémi Denis-Courmont wrote: > Le torstaina 7. marraskuuta 2024, 22.04.04 EET Michael Niedermayer a écrit : > > On Thu, Nov 07, 2024 at 02:48:13PM +, Derek Buitenhuis wrote: > > > On 11/6/2024 11:11 PM, Michael Niedermayer wrote: > > > > 3. actually remove libpostproc from master repository (2025 future) > > > > (send to SPI/STF/Invoice in future)> > > > I also did this exact work for Libav in 2012. It was very little work. Not > > > 5k. > > This is slander > > Yes, you are slandering Derek by accusing him of slander out of nowhere. > > Even if Derek were lying, that would not be slander, since the work in > question is not a person and cannot be "slandered". The original statement, as well as yours imply that i would request significantly more money for work worth significantly less That is damaging my reputation and is against a person which is me. It also may damage FFmpegs ability to fund future work as other developers might not want to be caught in similar claims. Thus arguably you are also causing damage to teh project with these claims. I also like to refer again to teh fact this is a government grant that was publically advertised, others could have taken it if it was thousands of euros for work worth hundreads. Also the whole proposal was reviewed by external experts. On the 13th march 2024 we received this "We are very happy to announce that your project was positively reviewed by our external experts." > > And sorry but while I am all for splitting postproc to a separate repository, > it is at best a few hundreds euros worth of consulting time. The difficult > part, > if there is one, is to reach the agreement to do it, not to actually do it. Iam sorry but spliting out libpostproc into a maintainable seperate source tree with self tests and everything is not just "a few hundread euros work". And also, i have already said that i intend to donate the 15k thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Rewriting code that is poorly written but fully understood is good. Rewriting code that one doesnt understand is a sign that one is less smart than the original author, trying to rewrite it will not make it better. signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
