[FFmpeg-devel] [PATCH] avformat/cafenc: fixed packet_size calculation

2024-02-18 Thread sergey radionov
the problem is the very last packet
can be shorter than default packet_size
so it's required to exclude it from
packet_size calculations.
fixes #10465
---
 libavformat/cafenc.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/libavformat/cafenc.c b/libavformat/cafenc.c
index 67be59806c..fcc4838392 100644
--- a/libavformat/cafenc.c
+++ b/libavformat/cafenc.c
@@ -34,6 +34,8 @@ typedef struct {
 int size_buffer_size;
 int size_entries_used;
 int packets;
+int64_t duration;
+int64_t last_packet_duration;
 } CAFContext;
 
 static uint32_t codec_flags(enum AVCodecID codec_id) {
@@ -238,6 +240,8 @@ static int caf_write_packet(AVFormatContext *s, AVPacket 
*pkt)
 pkt_sizes[caf->size_entries_used++] = 128 | top;
 }
 pkt_sizes[caf->size_entries_used++] = pkt->size & 127;
+caf->duration += pkt->duration;
+caf->last_packet_duration = pkt->duration;
 caf->packets++;
 }
 avio_write(s->pb, pkt->data, pkt->size);
@@ -259,7 +263,11 @@ static int caf_write_trailer(AVFormatContext *s)
 if (!par->block_align) {
 int packet_size = samples_per_packet(par);
 if (!packet_size) {
-packet_size = st->duration / (caf->packets - 1);
+if (caf->duration) {
+packet_size = (caf->duration - caf->last_packet_duration) 
/ (caf->packets - 1);
+} else {
+packet_size = st->duration / (caf->packets - 1);
+}
 avio_seek(pb, FRAME_SIZE_OFFSET, SEEK_SET);
 avio_wb32(pb, packet_size);
 }
-- 
2.40.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-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH 1/2] avcodec/s302m: enable non-PCM decoding

2024-02-18 Thread Gyan Doshi



On 2024-02-19 03:16 am, Vittorio Giovara wrote:

On Sun, Feb 18, 2024 at 8:02 PM Gyan Doshi  wrote:



On 2024-02-18 11:33 pm, Anton Khirnov wrote:

Quoting Gyan Doshi (2024-02-18 05:06:30)

b) what "maximalist" interpretation?

A non-maximalist interpretation would be that a TC member is only
excluded from voting when they authored the patch that is being
disputed.

If the promulgators meant to only prevent proposers of the disputed
change to not take part, then
the verbiage would be different.

In looking up how this clause came to be present, I came across the
following messages:

https://ffmpeg.org/pipermail/ffmpeg-devel/2020-December/273443.html
(Nicolas George originally proposes this clause - wording is more
restrictive)

https://ffmpeg.org/pipermail/ffmpeg-devel/2021-January/274822.html
(this one is interesting, you objected to the clause but on the grounds
that it was all-encompassing i.e.  anyone commenting on the dispute was
potentially subjected to recusal and referred to some 'model'
discussion, so your describing my reading as maximalist is weird since
that is how you read it - you just happen to object to this rule)

https://ffmpeg.org/pipermail/ffmpeg-devel/2021-January/274826.html
(Ronald clarifies that "involved" should be constrained to just be one
of the two parties -- of which you happen to be one)

There's the matter of what the rule currently is, distinct from what it
should be. What it ideally should be is that the decision should be
taken by a fresh set of eyes consisting of those who haven't become or
are seen to be publicly invested in the outcome. So the TC should have a
set of alternates - those who can make up the quorum and constitute an
odd number of voters when some from the first 5 are recused.


I'd like to offer a lighter interpretation of the rule, the mailing list is
the common playing ground, where discussions and disagreements can be had.
In case of a technical "maximalist" disagreement, then either party can
invoke the TC to judge on the matter. If anyone in the TC is involved in
the patch, as if it's an author or significantly contributed to it, then
they should step away from voting. In other words the "level of
involvement" rule takes place at the TC level, not at the ffmpeg-devel
discussion.


The TC is invoked when there's an intractable dispute. So the dispute 
precedes the TC activity hence the parties to the dispute are the main 
opposing participants at the venue of the dispute wherever that is, and 
the rules applies to all main parties. Imagine there's a new feature to 
be added which doesn't exist in the codebase in any form so there's no 
status quo. Member A submits a patch using design pattern X. Member B 
objects and wants design pattern Y. Now let's say if only A was on the 
TC, then as per the arguments of some here, A should recuse themselves 
but if only B was on the TC, B gets to vote. That asymmetry is not 
supported in the wording nor would it be fair.




Also consider that even in a vote recusal, the member's
arguments will still be read and by all likelihood taken into consideration
by the TC, so yours seems to be a literal interpretation of the rule,
instead of the spirit of the rule, which in my opinion matters more.


Of course. There's no mind-reading or mind-control machine here. Humans 
aren't automatons either. The judges on any Supreme Court are older 
human beings with all the deep convictions that one acquires during a 
long lifetime but that's the best we can do. The rules are meant to be 
the most that is practically feasible within mutually observable 
reality, not ideally efficient within an omniscient universe.


Regards,
Gyan

___
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 v14 2/2] libavformat/dvdvideo: add CLUT utilities and enable subtitle palette support

2024-02-18 Thread Marth64
Signed-off-by: Marth64 
---
 doc/demuxers.texi |  5 +++
 libavformat/Makefile  |  2 +-
 libavformat/dvdclut.c | 76 +++
 libavformat/dvdclut.h | 37 +++
 libavformat/dvdvideodec.c | 14 
 5 files changed, 133 insertions(+), 1 deletion(-)
 create mode 100644 libavformat/dvdclut.c
 create mode 100644 libavformat/dvdclut.h

diff --git a/doc/demuxers.texi b/doc/demuxers.texi
index 062ea2ea42..d3c7675631 100644
--- a/doc/demuxers.texi
+++ b/doc/demuxers.texi
@@ -391,6 +391,11 @@ often with junk data intended for controlling a real DVD 
player's
 buffering speed and with no other material data value.
 Default is 1, true.
 
+@item clut_rgb @var{bool}
+Output subtitle palettes (CLUTs) as RGB, required for Matroska.
+Disable to output the palette in its original YUV colorspace.
+Default is 1, true.
+
 @end table
 
 @subsection Examples
diff --git a/libavformat/Makefile b/libavformat/Makefile
index eda1f6e177..501159d9ca 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -192,7 +192,7 @@ OBJS-$(CONFIG_DTS_MUXER) += rawenc.o
 OBJS-$(CONFIG_DV_MUXER)  += dvenc.o
 OBJS-$(CONFIG_DVBSUB_DEMUXER)+= dvbsub.o rawdec.o
 OBJS-$(CONFIG_DVBTXT_DEMUXER)+= dvbtxt.o rawdec.o
-OBJS-$(CONFIG_DVDVIDEO_DEMUXER)  += dvdvideodec.o
+OBJS-$(CONFIG_DVDVIDEO_DEMUXER)  += dvdvideodec.o dvdclut.o
 OBJS-$(CONFIG_DXA_DEMUXER)   += dxa.o
 OBJS-$(CONFIG_EA_CDATA_DEMUXER)  += eacdata.o
 OBJS-$(CONFIG_EA_DEMUXER)+= electronicarts.o
diff --git a/libavformat/dvdclut.c b/libavformat/dvdclut.c
new file mode 100644
index 00..2125e91541
--- /dev/null
+++ b/libavformat/dvdclut.c
@@ -0,0 +1,76 @@
+/*
+ * DVD-Video subpicture CLUT (Color Lookup Table) utilities
+ *
+ * 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 "libavutil/bprint.h"
+#include "libavutil/colorspace.h"
+#include "libavutil/common.h"
+
+#include "dvdclut.h"
+#include "internal.h"
+
+int ff_dvdclut_palette_extradata_cat(const uint32_t *clut,
+ const size_t clut_size,
+ AVCodecParameters *par)
+{
+int ret = 0;
+AVBPrint bp;
+
+if (clut_size != FF_DVDCLUT_CLUT_SIZE)
+return AVERROR(EINVAL);
+
+av_bprint_init(, 0, FF_DVDCLUT_EXTRADATA_SIZE);
+
+av_bprintf(, "palette: ");
+
+for (int i = 0; i < FF_DVDCLUT_CLUT_LEN; i++)
+av_bprintf(, "%06"PRIx32"%s", clut[i],
+   i != (FF_DVDCLUT_CLUT_LEN - 1) ? ", " : "");
+
+av_bprintf(, "\n");
+
+return ff_bprint_to_codecpar_extradata(par, );
+}
+
+int ff_dvdclut_yuv_to_rgb(uint32_t *clut, const size_t clut_size)
+{
+int y, cb, cr;
+uint8_t r, g, b;
+int r_add, g_add, b_add;
+
+if (clut_size != FF_DVDCLUT_CLUT_SIZE)
+return AVERROR(EINVAL);
+
+for (int i = 0; i < FF_DVDCLUT_CLUT_LEN; i++) {
+y  = (clut[i] >> 16) & 0xFF;
+cr = (clut[i] >> 8)  & 0xFF;
+cb = clut[i] & 0xFF;
+
+YUV_TO_RGB1_CCIR(cb, cr);
+
+y = (y - 16) * FIX(255.0 / 219.0);
+r = av_clip_uint8((y + r_add - 1024) >> SCALEBITS);
+g = av_clip_uint8((y + g_add - 1024) >> SCALEBITS);
+b = av_clip_uint8((y + b_add - 1024) >> SCALEBITS);
+
+clut[i] = (r << 16) | (g << 8) | b;
+}
+
+return 0;
+}
diff --git a/libavformat/dvdclut.h b/libavformat/dvdclut.h
new file mode 100644
index 00..41cea7e2c9
--- /dev/null
+++ b/libavformat/dvdclut.h
@@ -0,0 +1,37 @@
+/*
+ * DVD-Video subpicture CLUT (Color Lookup Table) utilities
+ *
+ * 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 

[FFmpeg-devel] [PATCH v14 1/2] libavformat/dvdvideo: add DVD-Video demuxer, powered by libdvdread and libdvdnav

2024-02-18 Thread Marth64
v14: Nothing major, but some good grooming and feedback corrections.
Enjoy, all! Hope this can make the release!

* Don't set codec level framerate or closed caption properties,
  also remove avcodec.h include as a result (CC flag detection remains,
  due to its low overhead and for future use in title listing)
* Cleanup timestamp calculation, don't drop B frames accidentally,
  and fix unnecessarily high timing delays for AC3 tracks
* Remove dead switch statement condition and context variable
* Typo and styling fixes, to prepare for menu demuxing future patch
* Fix potential leak and OOB in chapter functions
* Fix timestamp overflow bug introduced in v11

Signed-off-by: Marth64 
---
 Changelog |2 +
 configure |8 +
 doc/demuxers.texi |  130 
 libavformat/Makefile  |1 +
 libavformat/allformats.c  |1 +
 libavformat/dvdvideodec.c | 1411 +
 6 files changed, 1553 insertions(+)
 create mode 100644 libavformat/dvdvideodec.c

diff --git a/Changelog b/Changelog
index 610ee61dd6..d268075273 100644
--- a/Changelog
+++ b/Changelog
@@ -27,6 +27,8 @@ version :
 - a C11-compliant compiler is now required; note that this requirement
   will be bumped to C17 in the near future, so consider updating your
   build environment if it lacks C17 support
+- DVD-Video demuxer, powered by libdvdnav and libdvdread
+
 
 version 6.1:
 - libaribcaption decoder
diff --git a/configure b/configure
index 23066efa32..8b43c837de 100755
--- a/configure
+++ b/configure
@@ -227,6 +227,8 @@ External library support:
   --enable-libdavs2enable AVS2 decoding via libdavs2 [no]
   --enable-libdc1394   enable IIDC-1394 grabbing using libdc1394
and libraw1394 [no]
+  --enable-libdvdnav   enable libdvdnav, needed for DVD demuxing [no]
+  --enable-libdvdread  enable libdvdread, needed for DVD demuxing [no]
   --enable-libfdk-aac  enable AAC de/encoding via libfdk-aac [no]
   --enable-libfliteenable flite (voice synthesis) support via libflite 
[no]
   --enable-libfontconfig   enable libfontconfig, useful for drawtext filter 
[no]
@@ -1806,6 +1808,8 @@ EXTERNAL_LIBRARY_GPL_LIST="
 frei0r
 libcdio
 libdavs2
+libdvdnav
+libdvdread
 librubberband
 libvidstab
 libx264
@@ -3523,6 +3527,8 @@ dts_demuxer_select="dca_parser"
 dtshd_demuxer_select="dca_parser"
 dv_demuxer_select="dvprofile"
 dv_muxer_select="dvprofile"
+dvdvideo_demuxer_select="mpegps_demuxer"
+dvdvideo_demuxer_deps="libdvdnav libdvdread"
 dxa_demuxer_select="riffdec"
 eac3_demuxer_select="ac3_parser"
 evc_demuxer_select="evc_frame_merge_bsf evc_parser"
@@ -6770,6 +6776,8 @@ enabled libdav1d  && require_pkg_config libdav1d 
"dav1d >= 0.5.0" "dav1d
 enabled libdavs2  && require_pkg_config libdavs2 "davs2 >= 1.6.0" 
davs2.h davs2_decoder_open
 enabled libdc1394 && require_pkg_config libdc1394 libdc1394-2 
dc1394/dc1394.h dc1394_new
 enabled libdrm&& check_pkg_config libdrm libdrm xf86drm.h 
drmGetVersion
+enabled libdvdnav && require_pkg_config libdvdnav "dvdnav >= 6.1.1" 
dvdnav/dvdnav.h dvdnav_open2
+enabled libdvdread&& require_pkg_config libdvdread "dvdread >= 6.1.2" 
dvdread/dvd_reader.h DVDOpen2 -ldvdread
 enabled libfdk_aac&& { check_pkg_config libfdk_aac fdk-aac 
"fdk-aac/aacenc_lib.h" aacEncOpen ||
{ require libfdk_aac fdk-aac/aacenc_lib.h 
aacEncOpen -lfdk-aac &&
  warn "using libfdk without pkg-config"; } }
diff --git a/doc/demuxers.texi b/doc/demuxers.texi
index e4c5b560a6..062ea2ea42 100644
--- a/doc/demuxers.texi
+++ b/doc/demuxers.texi
@@ -285,6 +285,136 @@ This demuxer accepts the following option:
 
 @end table
 
+@section dvdvideo
+
+DVD-Video demuxer, powered by libdvdnav and libdvdread.
+
+Can directly ingest DVD titles, specifically sequential PGCs,
+into a conversion pipeline. Menus and seeking are not supported at this time.
+
+Block devices (DVD drives), ISO files, and directory structures are accepted.
+Activate with @code{-f dvdvideo} in front of one of these inputs.
+
+Underlying playback is handled by libdvdnav, and structure parsing by 
libdvdread.
+FFmpeg must be built with GPL library support available as well as the
+configure switches @code{--enable-libdvdnav} and @code{--enable-libdvdread}.
+
+You will need to provide either the desired "title number" or exact PGC/PG 
coordinates.
+Many open-source DVD players and tools can aid in providing this information.
+If not specified, the demuxer will default to title 1 which works for many 
discs.
+However, due to the flexibility of the format, it is recommended to check 
manually.
+There are many discs that are authored strangely or with invalid headers.
+
+If the input is a real DVD drive, please note that there are some drives which 
may
+silently fail on reading bad sectors from the disc, returning random 

[FFmpeg-devel] [PATCH] avformat/mpegts: add a ts_id exported option

2024-02-18 Thread James Almer
Replaces AVFormatContext.ts_id

Signed-off-by: James Almer 
---
To be pushed as part of the bump set.

 libavformat/avformat.h | 6 --
 libavformat/mpegts.c   | 9 +++--
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index 50bbd1949b..affc5fde07 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -1538,12 +1538,6 @@ typedef struct AVFormatContext {
 #define AVFMT_AVOID_NEG_TS_MAKE_NON_NEGATIVE 1 ///< Shift timestamps so they 
are non negative
 #define AVFMT_AVOID_NEG_TS_MAKE_ZERO 2 ///< Shift timestamps so that 
they start at 0
 
-/**
- * Transport stream id.
- * This will be moved into demuxer private options. Thus no API/ABI 
compatibility
- */
-int ts_id;
-
 /**
  * Audio preload in microseconds.
  * Note, not all formats support this and unpredictable things may happen 
if it is used when not supported.
diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
index 1cf390e98e..36fded8db8 100644
--- a/libavformat/mpegts.c
+++ b/libavformat/mpegts.c
@@ -167,6 +167,8 @@ struct MpegTSContext {
 int merge_pmt_versions;
 int max_packet_size;
 
+int id;
+
 /**/
 /* private mpegts data */
 /* scan context */
@@ -184,7 +186,10 @@ struct MpegTSContext {
 };
 
 #define MPEGTS_OPTIONS \
-{ "resync_size",   "set size limit for looking up a new synchronization", 
offsetof(MpegTSContext, resync_size), AV_OPT_TYPE_INT,  { .i64 =  
MAX_RESYNC_SIZE}, 0, INT_MAX,  AV_OPT_FLAG_DECODING_PARAM }
+{ "resync_size",   "set size limit for looking up a new synchronization", \
+offsetof(MpegTSContext, resync_size), AV_OPT_TYPE_INT, { .i64 =  
MAX_RESYNC_SIZE}, 0, INT_MAX, AV_OPT_FLAG_DECODING_PARAM }, \
+{ "ts_id", "transport stream id", \
+offsetof(MpegTSContext, id), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 
INT_MAX, AV_OPT_FLAG_EXPORT|AV_OPT_FLAG_READONLY }
 
 static const AVOption options[] = {
 MPEGTS_OPTIONS,
@@ -2554,7 +2559,7 @@ static void pat_cb(MpegTSFilter *filter, const uint8_t 
*section, int section_len
 
 if (skip_identical(h, tssf))
 return;
-ts->stream->ts_id = h->id;
+ts->id = h->id;
 
 for (;;) {
 sid = get16(, p_end);
-- 
2.43.2

___
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 1/2] avcodec/s302m: enable non-PCM decoding

2024-02-18 Thread Vittorio Giovara
On Mon, Feb 19, 2024 at 2:17 AM Michael Niedermayer 
wrote:

> On Sun, Feb 18, 2024 at 11:48:59PM +0100, Hendrik Leppkes wrote:
> > On Sun, Feb 18, 2024 at 11:34 PM Michael Niedermayer
> >  wrote:
> > >
> > > * A disagreement implies that there are 2 parties
> > > * And we assume here that what one party wants is better for FFmpeg
> than what the other wants.
> > > * The TC needs to find out which partys choice is better or suggest a
> 3rd choice.
> > > * If one but not the other party is a member of the TC then this
> decission becomes biased if that member votes
> > >
> > > Your interpretation suggests that the TC members are "above" everyone
> and should
> > > prevail in arguments they have with others.
> > >
> >
> > Noone is above the rules, but just because someone has an opinion and
> > shared it shouldn't disqualify them, because they were specifically
> > voted into the TC for their opinions on technical matters.
> > Would their opinion, and therefore their vote, change if someone else
> > was seen as the person "blocking"?
>
> I think you are mixing the concept of an oppinion and blocking a patch.
> following is how i see the concept
>
> If you state that you prefer a linked list but dont mind the patch as it is
> thats an oppinion
>
> If you state that you prefer a linked list and i tell you that i would
> prefer to keep an array and you say you are ok, again thats an oppinion
> the patch is not blocked
>
> If you state that you prefer a linked list and i tell you that i would
> prefer to keep an array and you now tell me that if i want an array i have
> to go to the TC then you are blocking the patch. You and me in this case
> are the cause of the TC being involved.
> Only at this point we would be parties to the disagreement IMHO
> and we cannot be the judge here
>
>
> >
> > What if multiple people had expressed disagreement with a patch, and
> > most of the TC was involved in the public discussion already? Do the
>
> The question would be who is actually blocking it and not just stating
> their oppinion.
>
>
> > remaining "uninvolved" people on the TC get all the decision power? Or
> > do we consider most of the TC already opposing it publicly as perhaps
> > an indicator that the patch might not be the way to go?
> > Thats what the TC was voted in for, to give their opinion on technical
> > matters and decide if needed, so why deprive them of their opinion,
> > just because they already stated it publicly? That makes no sense to
> > me.
>
> You certainly have a point but, again I think there are big differences
> between a TC oppinion and someone blocking a patch
>
> If a TC member states an oppinion and clearly explains the reasoning
> behind it
> that should have no impact on the TC members ability to vote. In fact it
> should
> lead to all parties discussing and resolving the conflict probably without
> the
> need to formally involve the TC
>
> IMHO, invoking the TC is already an exceptional situation and failure.
> and it shouldnt give the parties of that failure more influence in the
> decission.
> (which is another orthogonal reason why the parties of a conflict should
> not
>  be judges of the conflict)
>
> Its really strange.
>
> You know, if a judge files a lawsuit, that judge cannot be the judge in
> that lawsuit.
> This is a very simple concept.
> It seems some people here see "their friend" not being allowed to vote
> but thats not what this is about.
> If a TC member blocks a patch, that TC member cannot vote on how to resolve
> that blockage.
>
> If a TC member chooses not to block a patch so he retains the power in a
> potential future vote. Thats a game theoretic decission he makes to
> maximize
> his blocking power. But really if he doesnt block it it could be applied
> so this is not a logic decission. The logic decission is to block the patch
> if thats what he wants and if noone else is blocking it.
> game theoretically the example you provide above would never happen
> as there would never be more than 1 TC member blocking a patch.
>
> So IMO arguing that a person should be party to a disagreement and judge of
> it. But making this dependant on an argument where people have to act in an
> illogic way is really odd
>

i long for the day in which ffmpeg might actually seem like a functioning
community, where we would not constantly and needlessly bikeshed rules and
other politics,but that day is clearly not today
-- 
Vittorio
___
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 1/2] avcodec/s302m: enable non-PCM decoding

2024-02-18 Thread epirat07


On 17 Feb 2024, at 13:31, Rémi Denis-Courmont wrote:

> Le lauantaina 17. helmikuuta 2024, 13.46.27 EET Gyan Doshi a écrit :
>> As a TC member who is part of the disagreement, I believe your
>> participation is recused.
>
> Obviously not. We don't want to get into a situation whence TC members have an
> incentive not to participate in regular code reviews just so that they can
> participate in the hypothetical making of later related TC decisions. That
> would be both dumb and counter-productive.
>

I agree…

> Somebody disagreeing with you does not necessarily mean that they have a
> conflict of interest in the subject matter.

My understanding of this rule was too that it would be for conflict-of-interest
kind of cases, which I fail to see here…

>
> -- 
> 雷米‧德尼-库尔蒙
> 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".
___
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] avcodec/bsf/hevc_mp4toannexb: Don't realloc when creating new extradata

2024-02-18 Thread James Almer

On 2/17/2024 11:44 PM, Andreas Rheinhardt wrote:

AVCodecParameters.extradata is supposed to be allocated with
av_malloc(); av_realloc() and its wrappers do not guarantee
the proper alignment. Therefore parse the extradata twice:
Once to check its validity and to determine the eventual size
and a second time to actually write the new extradata.


Why would av_malloc alignment be needed for extradata?
I see the doxy says "Must be allocated with av_malloc()" but I'm fairly 
sure that was meant to be "Must be allocated with av_malloc() family of 
functions", like its AVCodecContext counterpart. The idea is that 
library users don't use normal malloc as extradata will be freed with 
av_free(), and not because it will be accessed by SIMD code.




(Of course, not reallocating the buffer is beneficial in itself.)

Signed-off-by: Andreas Rheinhardt 
---
  libavcodec/bsf/hevc_mp4toannexb.c | 44 +++
  1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/libavcodec/bsf/hevc_mp4toannexb.c 
b/libavcodec/bsf/hevc_mp4toannexb.c
index a695cba370..f5424e95b8 100644
--- a/libavcodec/bsf/hevc_mp4toannexb.c
+++ b/libavcodec/bsf/hevc_mp4toannexb.c
@@ -38,13 +38,11 @@ typedef struct HEVCBSFContext {
  } HEVCBSFContext;
  
  static int hevc_extradata_to_annexb_internal(void *logctx, GetByteContext *gb,

- uint8_t **new_extradatap,
+ uint8_t *new_extradata,
   size_t *new_extradata_sizep)
  {
  int num_arrays = bytestream2_get_byte(gb);
-uint8_t *new_extradata = NULL;
  size_t new_extradata_size = 0;
-int ret;
  
  for (int i = 0; i < num_arrays; i++) {

  int type = bytestream2_get_byte(gb) & 0x3f;
@@ -54,8 +52,7 @@ static int hevc_extradata_to_annexb_internal(void *logctx, 
GetByteContext *gb,
type == HEVC_NAL_SEI_PREFIX || type == HEVC_NAL_SEI_SUFFIX)) {
  av_log(logctx, AV_LOG_ERROR, "Invalid NAL unit type in extradata: 
%d\n",
 type);
-ret = AVERROR_INVALIDDATA;
-goto fail;
+return AVERROR_INVALIDDATA;
  }
  
  for (int j = 0; j < cnt; j++) {

@@ -64,26 +61,19 @@ static int hevc_extradata_to_annexb_internal(void *logctx, 
GetByteContext *gb,
  if (!nalu_len ||
  nalu_len > bytestream2_get_bytes_left(gb) ||
  4 + nalu_len > FFMIN(INT_MAX, SIZE_MAX) - 
AV_INPUT_BUFFER_PADDING_SIZE - new_extradata_size) {
-ret = AVERROR_INVALIDDATA;
-goto fail;
+return AVERROR_INVALIDDATA;
  }
-ret = av_reallocp(_extradata, new_extradata_size + nalu_len + 
4 + AV_INPUT_BUFFER_PADDING_SIZE);
-if (ret < 0)
-goto fail;
-
-AV_WB32(new_extradata + new_extradata_size, 1); // add the 
startcode
-bytestream2_get_buffer(gb, new_extradata + new_extradata_size + 4, 
nalu_len);
+if (new_extradata) {
+AV_WB32(new_extradata + new_extradata_size, 1); // add the 
startcode
+bytestream2_get_bufferu(gb, new_extradata + new_extradata_size 
+ 4, nalu_len);
+} else
+bytestream2_skipu(gb, nalu_len);
  new_extradata_size += 4 + nalu_len;
-memset(new_extradata + new_extradata_size, 0, 
AV_INPUT_BUFFER_PADDING_SIZE);
  }
  }
-*new_extradatap = new_extradata;
  *new_extradata_sizep = new_extradata_size;
  
  return 0;

-fail:
-av_freep(_extradata);
-return ret;
  }
  
  static int hevc_extradata_to_annexb(AVBSFContext *ctx)

@@ -100,10 +90,20 @@ static int hevc_extradata_to_annexb(AVBSFContext *ctx)
  bytestream2_skip(, 21);
  length_size = (bytestream2_get_byte() & 3) + 1;
  
-ret = hevc_extradata_to_annexb_internal(ctx, , _extradata,

-_extradata_size);
-if (ret < 0)
-return ret;
+while (1) {
+GetByteContext gb_bak = gb;
+ret = hevc_extradata_to_annexb_internal(ctx, , new_extradata,
+_extradata_size);
+if (ret < 0)
+return ret;
+if (new_extradata || !new_extradata_size)
+break;
+new_extradata = av_malloc(new_extradata_size + 
AV_INPUT_BUFFER_PADDING_SIZE);
+if (!new_extradata)
+return AVERROR(ENOMEM);
+memset(new_extradata + new_extradata_size, 0, 
AV_INPUT_BUFFER_PADDING_SIZE);
+gb = gb_bak;
+}
  
  av_freep(>par_out->extradata);

  ctx->par_out->extradata  = new_extradata;

___
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 1/2] avcodec/s302m: enable non-PCM decoding

2024-02-18 Thread Ronald S. Bultje
Hi,

On Sun, Feb 18, 2024 at 5:34 PM Michael Niedermayer 
wrote:

> More formally, you could define a "party to a disagreement" as
> all minimal sets of people whos non existence would resolve the
> disagreement
>

I think I agree with this interpretation of the rules.

Ronald
___
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] Fix rdiv always being set to 0 in vf_convolution.c

2024-02-18 Thread Stone Chen
Hi Marton,

Thanks for the feedback!

I'm not sure what dynamic reconfiguration is, from some searching I think
it might be related to commands?

On Sun, Feb 18, 2024 at 7:08 PM Marton Balint  wrote:

>
>
> On Sun, 18 Feb 2024, Stone Chen wrote:
>
> > In commit 6c45d34, a line was added that always sets rdiv to 0,
> overriding any user input. This removes that line allowing user set values
> for 0rdiv, 1rdiv, 2rdiv, 3rdiv to apply as expected. This fixes ticket
> #10294.
>
> This is likely not the correct fix, because resetting it to 0 was added to
> support dynamic reconfigurations.
>
> The way I see it, the user option rdiv-s and internally used rdivs
> should be separated, init_params should always recalculate the internal
> rdivs based on the user option rdivs...
>
> Regards,
> Marton
>
> >
> > Signed-off-by: Stone Chen 
> > ---
> > libavfilter/vf_convolution.c | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/libavfilter/vf_convolution.c b/libavfilter/vf_convolution.c
> > index bf67f392f6..a00bb2b3c4 100644
> > --- a/libavfilter/vf_convolution.c
> > +++ b/libavfilter/vf_convolution.c
> > @@ -674,7 +674,6 @@ static int param_init(AVFilterContext *ctx)
> > p = orig = av_strdup(s->matrix_str[i]);
> > if (p) {
> > s->matrix_length[i] = 0;
> > -s->rdiv[i] = 0.f;
> > sum = 0.f;
> >
> > while (s->matrix_length[i] < 49) {
> > --
> > 2.43.2
> >
> > ___
> > 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 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] avutil/mem: use C11 aligned_malloc()

2024-02-18 Thread James Almer

On 2/18/2024 9:08 PM, Michael Niedermayer wrote:

On Sun, Feb 18, 2024 at 01:16:36PM -0300, James Almer wrote:

Save for the Microsoft C Runtime library, where free() can't handle aligned
buffers, aligned_malloc() should be available and working on all supported
targets.
Also, malloc() alone may be sufficient if alignment requirement is low, so add
a check for it.

Signed-off-by: James Almer 
---
  configure   |  2 --
  libavutil/mem.c | 42 ++
  2 files changed, 6 insertions(+), 38 deletions(-)


This breaks build here

libavutil/mem.c: In function ‘av_malloc’:
libavutil/mem.c:108:15: error: implicit declaration of function 
‘aligned_malloc’; did you mean ‘aligned_alloc’? 
[-Werror=implicit-function-declaration]
  ptr = aligned_malloc(size, ALIGN);
^~
aligned_alloc
libavutil/mem.c:108:13: warning: assignment makes pointer from integer without 
a cast [-Wint-conversion]
  ptr = aligned_malloc(size, ALIGN);
  ^
cc1: some warnings being treated as errors
ffbuild/common.mak:81: recipe for target 'libavutil/mem.o' failed


Yes, i mistyped aligned_alloc as aligned_malloc.
___
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 1/2] avcodec/s302m: enable non-PCM decoding

2024-02-18 Thread Michael Niedermayer
On Sun, Feb 18, 2024 at 11:48:59PM +0100, Hendrik Leppkes wrote:
> On Sun, Feb 18, 2024 at 11:34 PM Michael Niedermayer
>  wrote:
> >
> > * A disagreement implies that there are 2 parties
> > * And we assume here that what one party wants is better for FFmpeg than 
> > what the other wants.
> > * The TC needs to find out which partys choice is better or suggest a 3rd 
> > choice.
> > * If one but not the other party is a member of the TC then this decission 
> > becomes biased if that member votes
> >
> > Your interpretation suggests that the TC members are "above" everyone and 
> > should
> > prevail in arguments they have with others.
> >
> 
> Noone is above the rules, but just because someone has an opinion and
> shared it shouldn't disqualify them, because they were specifically
> voted into the TC for their opinions on technical matters.
> Would their opinion, and therefore their vote, change if someone else
> was seen as the person "blocking"?

I think you are mixing the concept of an oppinion and blocking a patch.
following is how i see the concept

If you state that you prefer a linked list but dont mind the patch as it is
thats an oppinion

If you state that you prefer a linked list and i tell you that i would
prefer to keep an array and you say you are ok, again thats an oppinion
the patch is not blocked

If you state that you prefer a linked list and i tell you that i would
prefer to keep an array and you now tell me that if i want an array i have
to go to the TC then you are blocking the patch. You and me in this case
are the cause of the TC being involved.
Only at this point we would be parties to the disagreement IMHO
and we cannot be the judge here


> 
> What if multiple people had expressed disagreement with a patch, and
> most of the TC was involved in the public discussion already? Do the

The question would be who is actually blocking it and not just stating
their oppinion.


> remaining "uninvolved" people on the TC get all the decision power? Or
> do we consider most of the TC already opposing it publicly as perhaps
> an indicator that the patch might not be the way to go?
> Thats what the TC was voted in for, to give their opinion on technical
> matters and decide if needed, so why deprive them of their opinion,
> just because they already stated it publicly? That makes no sense to
> me.

You certainly have a point but, again I think there are big differences
between a TC oppinion and someone blocking a patch

If a TC member states an oppinion and clearly explains the reasoning behind it
that should have no impact on the TC members ability to vote. In fact it should
lead to all parties discussing and resolving the conflict probably without the
need to formally involve the TC

IMHO, invoking the TC is already an exceptional situation and failure.
and it shouldnt give the parties of that failure more influence in the 
decission.
(which is another orthogonal reason why the parties of a conflict should not
 be judges of the conflict)

Its really strange.

You know, if a judge files a lawsuit, that judge cannot be the judge in
that lawsuit.
This is a very simple concept.
It seems some people here see "their friend" not being allowed to vote
but thats not what this is about.
If a TC member blocks a patch, that TC member cannot vote on how to resolve
that blockage.

If a TC member chooses not to block a patch so he retains the power in a
potential future vote. Thats a game theoretic decission he makes to maximize
his blocking power. But really if he doesnt block it it could be applied
so this is not a logic decission. The logic decission is to block the patch
if thats what he wants and if noone else is blocking it.
game theoretically the example you provide above would never happen
as there would never be more than 1 TC member blocking a patch.

So IMO arguing that a person should be party to a disagreement and judge of
it. But making this dependant on an argument where people have to act in an
illogic way is really odd

thx

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When you are offended at any man's fault, turn to yourself and study your
own failings. Then you will forget your anger. -- Epictetus


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] Fix rdiv always being set to 0 in vf_convolution.c

2024-02-18 Thread Stone Chen
Hi Marton,

Thanks for the feedback!

I'm not sure what dynamic reconfiguration is, from some searching I think
it might be related to commands?

On Sun, Feb 18, 2024 at 7:08 PM Marton Balint  wrote:

>
>
> On Sun, 18 Feb 2024, Stone Chen wrote:
>
> > In commit 6c45d34, a line was added that always sets rdiv to 0,
> overriding any user input. This removes that line allowing user set values
> for 0rdiv, 1rdiv, 2rdiv, 3rdiv to apply as expected. This fixes ticket
> #10294.
>
> This is likely not the correct fix, because resetting it to 0 was added to
> support dynamic reconfigurations.
>
> The way I see it, the user option rdiv-s and internally used rdivs
> should be separated, init_params should always recalculate the internal
> rdivs based on the user option rdivs...
>
> Regards,
> Marton
>
> >
> > Signed-off-by: Stone Chen 
> > ---
> > libavfilter/vf_convolution.c | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/libavfilter/vf_convolution.c b/libavfilter/vf_convolution.c
> > index bf67f392f6..a00bb2b3c4 100644
> > --- a/libavfilter/vf_convolution.c
> > +++ b/libavfilter/vf_convolution.c
> > @@ -674,7 +674,6 @@ static int param_init(AVFilterContext *ctx)
> > p = orig = av_strdup(s->matrix_str[i]);
> > if (p) {
> > s->matrix_length[i] = 0;
> > -s->rdiv[i] = 0.f;
> > sum = 0.f;
> >
> > while (s->matrix_length[i] < 49) {
> > --
> > 2.43.2
> >
> > ___
> > 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 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] Fix rdiv always being set to 0 in vf_convolution.c

2024-02-18 Thread Marton Balint




On Sun, 18 Feb 2024, Stone Chen wrote:


In commit 6c45d34, a line was added that always sets rdiv to 0, overriding any 
user input. This removes that line allowing user set values for 0rdiv, 1rdiv, 
2rdiv, 3rdiv to apply as expected. This fixes ticket #10294.


This is likely not the correct fix, because resetting it to 0 was added to 
support dynamic reconfigurations.


The way I see it, the user option rdiv-s and internally used rdivs 
should be separated, init_params should always recalculate the internal 
rdivs based on the user option rdivs...


Regards,
Marton



Signed-off-by: Stone Chen 
---
libavfilter/vf_convolution.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/libavfilter/vf_convolution.c b/libavfilter/vf_convolution.c
index bf67f392f6..a00bb2b3c4 100644
--- a/libavfilter/vf_convolution.c
+++ b/libavfilter/vf_convolution.c
@@ -674,7 +674,6 @@ static int param_init(AVFilterContext *ctx)
p = orig = av_strdup(s->matrix_str[i]);
if (p) {
s->matrix_length[i] = 0;
-s->rdiv[i] = 0.f;
sum = 0.f;

while (s->matrix_length[i] < 49) {
--
2.43.2

___
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] avutil/mem: use C11 aligned_malloc()

2024-02-18 Thread Michael Niedermayer
On Sun, Feb 18, 2024 at 01:16:36PM -0300, James Almer wrote:
> Save for the Microsoft C Runtime library, where free() can't handle aligned
> buffers, aligned_malloc() should be available and working on all supported
> targets.
> Also, malloc() alone may be sufficient if alignment requirement is low, so add
> a check for it.
> 
> Signed-off-by: James Almer 
> ---
>  configure   |  2 --
>  libavutil/mem.c | 42 ++
>  2 files changed, 6 insertions(+), 38 deletions(-)

This breaks build here

libavutil/mem.c: In function ‘av_malloc’:
libavutil/mem.c:108:15: error: implicit declaration of function 
‘aligned_malloc’; did you mean ‘aligned_alloc’? 
[-Werror=implicit-function-declaration]
 ptr = aligned_malloc(size, ALIGN);
   ^~
   aligned_alloc
libavutil/mem.c:108:13: warning: assignment makes pointer from integer without 
a cast [-Wint-conversion]
 ptr = aligned_malloc(size, ALIGN);
 ^
cc1: some warnings being treated as errors
ffbuild/common.mak:81: recipe for target 'libavutil/mem.o' failed

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Dictatorship naturally arises out of democracy, and the most aggravated
form of tyranny and slavery out of the most extreme liberty. -- 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".


Re: [FFmpeg-devel] [PATCH] avcodec/jpeg2000dec: support of 2 fields in 1 AVPacket

2024-02-18 Thread Marton Balint



On Fri, 16 Feb 2024, Anton Khirnov wrote:


Quoting Tomas Härdin (2024-02-03 20:58:20)

I think people with knowledge how interlacing is handled in other
formats and codecs might want to chime in.


For MPEG codecs our decoders always output (properly signalled)
interlaced content as two interleaved fields in a single AVFrame flagged
with AV_FRAME_FLAG_INTERLACED. Its height is the height of the whole
frame, so double the number of lines in each field.

So IIUC this patch is taking the correct approach.


I believe interlaced HEVC has the same issue, the decoder generates field 
pictures and not frames. Would changing the HEVC decoder be acceptable to 
generate frames with interleaved fields?


Regards,
Marton
___
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 1/2] avcodec/x86/fpel: Remove declarations of inexistent functions

2024-02-18 Thread Andreas Rheinhardt
Andreas Rheinhardt:
> Forgotten in 50a8cbb23e9a982292bf7737004c97eba776c00e and
> a51279bbdea0d6db920d71980262bccd0ce78226.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/x86/fpel.h | 4 
>  1 file changed, 4 deletions(-)
> 
> diff --git a/libavcodec/x86/fpel.h b/libavcodec/x86/fpel.h
> index 4e83cf71c3..90f7051a48 100644
> --- a/libavcodec/x86/fpel.h
> +++ b/libavcodec/x86/fpel.h
> @@ -22,12 +22,8 @@
>  #include 
>  #include 
>  
> -void ff_avg_pixels4_mmx(uint8_t *block, const uint8_t *pixels,
> -ptrdiff_t line_size, int h);
>  void ff_avg_pixels4_mmxext(uint8_t *block, const uint8_t *pixels,
> ptrdiff_t line_size, int h);
> -void ff_avg_pixels8_mmx(uint8_t *block, const uint8_t *pixels,
> -ptrdiff_t line_size, int h);
>  void ff_avg_pixels8_mmxext(uint8_t *block, const uint8_t *pixels,
> ptrdiff_t line_size, int h);
>  void ff_avg_pixels16_mmx(uint8_t *block, const uint8_t *pixels,

Will apply this patchset tomorrow unless there are objections.

- Andreas

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

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


Re: [FFmpeg-devel] [PATCH] avcodec/jpeg2000dec: support of 2 fields in 1 AVPacket

2024-02-18 Thread Tomas Härdin
tor 2024-02-15 klockan 16:02 +0100 skrev Jerome Martinez:
> On 05/02/2024 01:19, Tomas Härdin wrote:
> > [...]
> > Which entry in the table would the provided file correspond to? To
> > me
> > it seems none of them fit. There's two fields, meaning two j2k
> > codestreams, in each corresponding essence element KLV packet (I
> > think,
> > unless CP packets get reassembled somewhere else). Entry I2 seems
> > closest but it specifies FULL_FRAME. I1 is otherwise tempting, but
> > there SampleRate should equal the field rate whereas the file has
> > SampleRate = 3/1001.
> 
> Other examples I have (not shareable) with 2 jp2k pictures per KLV
> have 
> identification from an old version of AmberFin iCR, I have no file
> with 
> the I2 correctly signaled, with my first example it isI2 (2 fields
> per 
> KLV) with I1 Header Metadata Property Values **but** with I2 essence 
> container label which has a content byte (byte 15 of the UL) of 0x04
> = I2.
> The AmberFin iCR files have the generic essence container label with 
> content byte of 0x01 = FU (Unspecified) so for my main use case we
> could 
> activate the search of the 2nd jp2k only if I2 is explicitly signaled
> by 
> the essence container label but it would prevent to catch the 2nd
> field 
> when this signaling is unspecified and buggy Frame layout + sample
> rate 
> + edit rate.

I'm not super stoked about implementing support for broken muxers.
Instead these companies should fix their code. But either way we at the
very least need a reliable way to detect these kinds of files if we are
to do this. There was no Software + Version information in the sample
provided, which is otherwise a reliable method to deal with shitty
muxers.

Always inserting a parser worsens performance, and dealing with j2k is
already very taxing. I've been working hard on ||izing j2kdec, and
adding more serial code would make that harder.

> On 03/02/2024 20:58, Tomas Härdin wrote:
> > The fastest way, in a player, is probably to do it with a shader.
> > That
> > should be the least amount of copies and the most cache coherent.
> 
> As far a I know the player is not aware that the AVFrame actually 
> contains a field so it can not apply a shader or something else,
> which 
> AVFrame field indicates that this is a a field to be interleaved with
> the next AVFrame before display?
> Currently for I1 files ffplay or VLC show double rate half height so
> it 
> seems that they don't catch that AVFrame contains a field.

There might be no way to do this at present, as Anton's reply
indicates.

Proper support for FrameLayout may require adding a bunch of fields to
AVFrame and/or AVCodecContext.

We could of course always convert to MIXED_FIELDS and just say that's
the way FFmpeg does things. Anyone who wants to do anything more fancy
is free to provide a patch or sufficient money. The present situation
where the other field is just discarded entirely is of course not
satisfactory either.

/Tomas
___
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 1/2] avcodec/s302m: enable non-PCM decoding

2024-02-18 Thread Hendrik Leppkes
On Sun, Feb 18, 2024 at 11:34 PM Michael Niedermayer
 wrote:
>
> * A disagreement implies that there are 2 parties
> * And we assume here that what one party wants is better for FFmpeg than what 
> the other wants.
> * The TC needs to find out which partys choice is better or suggest a 3rd 
> choice.
> * If one but not the other party is a member of the TC then this decission 
> becomes biased if that member votes
>
> Your interpretation suggests that the TC members are "above" everyone and 
> should
> prevail in arguments they have with others.
>

Noone is above the rules, but just because someone has an opinion and
shared it shouldn't disqualify them, because they were specifically
voted into the TC for their opinions on technical matters.
Would their opinion, and therefore their vote, change if someone else
was seen as the person "blocking"?

What if multiple people had expressed disagreement with a patch, and
most of the TC was involved in the public discussion already? Do the
remaining "uninvolved" people on the TC get all the decision power? Or
do we consider most of the TC already opposing it publicly as perhaps
an indicator that the patch might not be the way to go?
Thats what the TC was voted in for, to give their opinion on technical
matters and decide if needed, so why deprive them of their opinion,
just because they already stated it publicly? That makes no sense to
me.

- Hendrik
___
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 1/2] avcodec/s302m: enable non-PCM decoding

2024-02-18 Thread Vittorio Giovara
On Sun, Feb 18, 2024 at 11:34 PM Michael Niedermayer 
wrote:

> Hi
>
> On Sun, Feb 18, 2024 at 07:20:43PM +0100, Anton Khirnov wrote:
> > Quoting Michael Niedermayer (2024-02-18 01:43:14)
> > > "If the disagreement involves a member of the TC"
> > > does IMHO not preclude commenting on a patch.
> > >
> > > For a disagreement we need 2 parties. For example one party who
> > > wants a patch in and one who blocks the patch. or 2 parties where both
> > > block the other.
> > >
> > > Being a party of a disagreement would not make anyones opinon invalid.
> >
> > Anything that goes to TC is a disagreement.
>
> probably, thats true, yes
>
>
> > Anyone who expressed an
> > opinion on the patch then is 'a party to the disagreement'.
>
> no, i dont see it that way
> A developer blocking a patch is a party to the disagreement
> So is the developer who calls the TC because of that.
>

Disagree. If that basically means that no patches will be ever blocked by
the members of the TC!
They should express the best technical excellence of the whole community,
not be stifled from participating in the discussion

If it helps, I'll block the patch so that Anton can vote in the TC.
Do you see how slippery (and insane) this interpretation of the rule
becomes?



> Similarly if you look at real world court cases
> parties to the lawsuit are the one who is filling the lawsuit and
> the defendant.
> The thousand people expressing an oppinion in some random place are
> not parties to the disagreement
>

This is a false dichotomy, we're not a court case where we're interpreting
the law, we're trying to solve a technical disagreement.


> More formally, you could define a "party to a disagreement" as
> all minimal sets of people whos non existence would resolve the
> disagreement
>

By that reasoning you shouldn't vote either since you touched basically all
of ffmpeg codebase!


> * A disagreement implies that there are 2 parties
> * And we assume here that what one party wants is better for FFmpeg than
> what the other wants.
> * The TC needs to find out which partys choice is better or suggest a 3rd
> choice.
> * If one but not the other party is a member of the TC then this decission
> becomes biased if that member votes
>
> Your interpretation suggests that the TC members are "above" everyone and
> should
> prevail in arguments they have with others.
>

Nobody is saying that!!!


> I dont think the GA has given that power to the TC.
>

Are you suggesting to have a vote to rewrite/reinterpret the rule?
-- 
Vittorio
___
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 1/2] avcodec/s302m: enable non-PCM decoding

2024-02-18 Thread Michael Niedermayer
Hi

On Sun, Feb 18, 2024 at 07:20:43PM +0100, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2024-02-18 01:43:14)
> > "If the disagreement involves a member of the TC"
> > does IMHO not preclude commenting on a patch.
> > 
> > For a disagreement we need 2 parties. For example one party who
> > wants a patch in and one who blocks the patch. or 2 parties where both
> > block the other.
> > 
> > Being a party of a disagreement would not make anyones opinon invalid.
> 
> Anything that goes to TC is a disagreement.

probably, thats true, yes


> Anyone who expressed an
> opinion on the patch then is 'a party to the disagreement'.

no, i dont see it that way
A developer blocking a patch is a party to the disagreement
So is the developer who calls the TC because of that.


Similarly if you look at real world court cases
parties to the lawsuit are the one who is filling the lawsuit and
the defendant.
The thousand people expressing an oppinion in some random place are
not parties to the disagreement

More formally, you could define a "party to a disagreement" as
all minimal sets of people whos non existence would resolve the disagreement

That is if 3 people block a patch all of them are parties to a disagreement
but a person expressing an oppinion is not. Also the person(s) calling on the
TC are parties to a disagreement. And the main author(s) of the patch too are


> 
> > But I think it is reasonable that parties of a disagreement cannot be
> > the judge of the disagreement.
> 
> Why not? This is one of those truthy-sounding statements that does not
> actually hold up to scrutiny.

Imagine a judge kills someone and judges himself innocent afterwards in a panel 
of 5 judges

* A disagreement implies that there are 2 parties
* And we assume here that what one party wants is better for FFmpeg than what 
the other wants.
* The TC needs to find out which partys choice is better or suggest a 3rd 
choice.
* If one but not the other party is a member of the TC then this decission 
becomes biased if that member votes

Your interpretation suggests that the TC members are "above" everyone and should
prevail in arguments they have with others.

I dont think the GA has given that power to the TC.

thx

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have often repented speaking, but never of holding my tongue.
-- Xenocrates


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 1/2] avcodec/s302m: enable non-PCM decoding

2024-02-18 Thread Vittorio Giovara
On Sun, Feb 18, 2024 at 10:25 PM Nicolas George  wrote:

> Vittorio Giovara (12024-02-18):
> >  While I value your insights, I'd like to offer a different
> > viewpoint regarding the practice of recusing oneself from discussions.
>
> 
>
> That might be your viewpoint, but that is not what the rules we all
> agreed upon say.
>

Thank you for your prompt response, Nicolas. I appreciate the opportunity
to delve deeper into our governance rules and their interpretation within
the FFmpeg community.

While I understand that you're referencing the existing rules that we've
collectively agreed upon, I believe it's crucial for us to periodically
review and refine these rules to ensure they remain aligned with our
evolving community values and goals.

The rules we've agreed upon serve as a foundation for our governance
structure, providing guidelines for decision-making and ensuring fairness
and transparency in our processes. However, as our community grows and
evolves, it's natural for interpretations and perspectives to shift,
necessitating occasional reassessment of these rules.

Your reminder about the importance of adhering to the established rules is
valid, and I share your commitment to upholding them. However, I also
believe that our rules should be flexible enough to accommodate diverse
viewpoints and adapt to changing circumstances.

Therefore, I propose that we engage in a collaborative effort to revisit
and clarify our existing rules. By fostering open dialogue and soliciting
input from all members of the community, we can ensure that our governance
framework reflects the collective wisdom and values of our community.

Furthermore, this process of review and refinement will help address any
ambiguities or discrepancies in our rules, promoting greater clarity and
understanding among all stakeholders. It will also provide an opportunity
to incorporate new insights and best practices that may have emerged since
the rules were initially established.

In conclusion, I appreciate your reminder about the importance of adhering
to our agreed-upon rules. However, I believe that revisiting and clarifying
these rules is a proactive step toward strengthening our governance
processes and ensuring the continued success and sustainability of the
FFmpeg project.

Thank you for your dedication to the community, and I look forward to
engaging in productive discussions to enhance our governance framework.
-- 
Vittorai
___
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 1/2] avcodec/s302m: enable non-PCM decoding

2024-02-18 Thread Vittorio Giovara
On Sun, Feb 18, 2024 at 8:02 PM Gyan Doshi  wrote:

>
>
> On 2024-02-18 11:33 pm, Anton Khirnov wrote:
> > Quoting Gyan Doshi (2024-02-18 05:06:30)
> >> b) what "maximalist" interpretation?
> > A non-maximalist interpretation would be that a TC member is only
> > excluded from voting when they authored the patch that is being
> > disputed.
>
> If the promulgators meant to only prevent proposers of the disputed
> change to not take part, then
> the verbiage would be different.
>
> In looking up how this clause came to be present, I came across the
> following messages:
>
> https://ffmpeg.org/pipermail/ffmpeg-devel/2020-December/273443.html
> (Nicolas George originally proposes this clause - wording is more
> restrictive)
>
> https://ffmpeg.org/pipermail/ffmpeg-devel/2021-January/274822.html
> (this one is interesting, you objected to the clause but on the grounds
> that it was all-encompassing i.e.  anyone commenting on the dispute was
> potentially subjected to recusal and referred to some 'model'
> discussion, so your describing my reading as maximalist is weird since
> that is how you read it - you just happen to object to this rule)
>
> https://ffmpeg.org/pipermail/ffmpeg-devel/2021-January/274826.html
> (Ronald clarifies that "involved" should be constrained to just be one
> of the two parties -- of which you happen to be one)
>
> There's the matter of what the rule currently is, distinct from what it
> should be. What it ideally should be is that the decision should be
> taken by a fresh set of eyes consisting of those who haven't become or
> are seen to be publicly invested in the outcome. So the TC should have a
> set of alternates - those who can make up the quorum and constitute an
> odd number of voters when some from the first 5 are recused.
>

I'd like to offer a lighter interpretation of the rule, the mailing list is
the common playing ground, where discussions and disagreements can be had.
In case of a technical "maximalist" disagreement, then either party can
invoke the TC to judge on the matter. If anyone in the TC is involved in
the patch, as if it's an author or significantly contributed to it, then
they should step away from voting. In other words the "level of
involvement" rule takes place at the TC level, not at the ffmpeg-devel
discussion. Also consider that even in a vote recusal, the member's
arguments will still be read and by all likelihood taken into consideration
by the TC, so yours seems to be a literal interpretation of the rule,
instead of the spirit of the rule, which in my opinion matters more.
-- 
Vittorio
___
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 1/2] avcodec/s302m: enable non-PCM decoding

2024-02-18 Thread Nicolas George
Vittorio Giovara (12024-02-18):
>  While I value your insights, I'd like to offer a different
> viewpoint regarding the practice of recusing oneself from discussions.



That might be your viewpoint, but that is not what the rules we all
agreed upon say.

-- 
  Nicolas George
___
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 v5 8/9] avcodec: add D3D12VA hardware HEVC encoder

2024-02-18 Thread Mark Thompson

On 18/02/2024 08:45, tong1.wu-at-intel@ffmpeg.org wrote:

From: Tong Wu 

This implementation is based on D3D12 Video Encoding Spec:
https://microsoft.github.io/DirectX-Specs/d3d/D3D12VideoEncoding.html

Sample command line for transcoding:
ffmpeg.exe -hwaccel d3d12va -hwaccel_output_format d3d12 -i input.mp4
-c:v hevc_d3d12va output.mp4

Signed-off-by: Tong Wu 
---
  configure|6 +
  libavcodec/Makefile  |4 +-
  libavcodec/allcodecs.c   |1 +
  libavcodec/d3d12va_encode.c  | 1443 ++
  libavcodec/d3d12va_encode.h  |  275 ++
  libavcodec/d3d12va_encode_hevc.c | 1013 +
  libavcodec/hw_base_encode.h  |2 +-
  7 files changed, 2742 insertions(+), 2 deletions(-)


There are a load of references to H.264 below.  Do you have a working H.264 
implementation as well?


  create mode 100644 libavcodec/d3d12va_encode.c
  create mode 100644 libavcodec/d3d12va_encode.h
  create mode 100644 libavcodec/d3d12va_encode_hevc.c
diff --git a/configure b/configure
index f72533b7d2..682576aa91 100755
--- a/configure
+++ b/configure
@@ -2564,6 +2564,7 @@ CONFIG_EXTRA="
  tpeldsp
  vaapi_1
  vaapi_encode
+d3d12va_encode
  vc1dsp
  videodsp
  vp3dsp
@@ -3208,6 +3209,7 @@ wmv3_vaapi_hwaccel_select="vc1_vaapi_hwaccel"
  wmv3_vdpau_hwaccel_select="vc1_vdpau_hwaccel"
  
  # hardware-accelerated codecs

+d3d12va_encode_deps="d3d12va ID3D12VideoEncoder d3d12_encoder_feature"
  mediafoundation_deps="mftransform_h MFCreateAlignedMemoryBuffer"
  omx_deps="libdl pthreads"
  omx_rpi_select="omx"
@@ -3275,6 +3277,7 @@ h264_v4l2m2m_encoder_deps="v4l2_m2m h264_v4l2_m2m"
  hevc_amf_encoder_deps="amf"
  hevc_cuvid_decoder_deps="cuvid"
  hevc_cuvid_decoder_select="hevc_mp4toannexb_bsf"
+hevc_d3d12va_encoder_select="atsc_a53 cbs_h265 d3d12va_encode"


Spurious dependency on the non-CBS A53 stuff?  (If you want A53 we should add 
it to CBS properly.)


  hevc_mediacodec_decoder_deps="mediacodec"
  hevc_mediacodec_decoder_select="hevc_mp4toannexb_bsf hevc_parser"
  hevc_mediacodec_encoder_deps="mediacodec"
@@ -6617,6 +6620,9 @@ check_type "windows.h d3d11.h" "ID3D11VideoDecoder"
  check_type "windows.h d3d11.h" "ID3D11VideoContext"
  check_type "windows.h d3d12.h" "ID3D12Device"
  check_type "windows.h d3d12video.h" "ID3D12VideoDecoder"
+check_type "windows.h d3d12video.h" "ID3D12VideoEncoder"
+test_code cc "windows.h d3d12video.h" "D3D12_FEATURE_VIDEO feature = 
D3D12_FEATURE_VIDEO_ENCODER_CODEC" && \
+test_code cc "windows.h d3d12video.h" 
"D3D12_FEATURE_DATA_VIDEO_ENCODER_RESOURCE_REQUIREMENTS req" && enable 
d3d12_encoder_feature
  check_type "windows.h" "DPI_AWARENESS_CONTEXT" -D_WIN32_WINNT=0x0A00
  check_type "d3d9.h dxva2api.h" DXVA2_ConfigPictureDecode -D_WIN32_WINNT=0x0602
  check_func_headers mfapi.h MFCreateAlignedMemoryBuffer -lmfplat
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 23946f6ea3..50590b34f4 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -86,6 +86,7 @@ OBJS-$(CONFIG_CBS_MPEG2)   += cbs_mpeg2.o
  OBJS-$(CONFIG_CBS_VP8) += cbs_vp8.o vp8data.o
  OBJS-$(CONFIG_CBS_VP9) += cbs_vp9.o
  OBJS-$(CONFIG_CRYSTALHD)   += crystalhd.o
+OBJS-$(CONFIG_D3D12VA_ENCODE)  += d3d12va_encode.o hw_base_encode.o
  OBJS-$(CONFIG_DEFLATE_WRAPPER) += zlib_wrapper.o
  OBJS-$(CONFIG_DOVI_RPU)+= dovi_rpu.o
  OBJS-$(CONFIG_ERROR_RESILIENCE)+= error_resilience.o
@@ -437,6 +438,7 @@ OBJS-$(CONFIG_HEVC_DECODER)+= hevcdec.o 
hevc_mvs.o \
h274.o
  OBJS-$(CONFIG_HEVC_AMF_ENCODER)+= amfenc_hevc.o
  OBJS-$(CONFIG_HEVC_CUVID_DECODER)  += cuviddec.o
+OBJS-$(CONFIG_HEVC_D3D12VA_ENCODER)+= d3d12va_encode_hevc.o
  OBJS-$(CONFIG_HEVC_MEDIACODEC_DECODER) += mediacodecdec.o
  OBJS-$(CONFIG_HEVC_MEDIACODEC_ENCODER) += mediacodecenc.o
  OBJS-$(CONFIG_HEVC_MF_ENCODER) += mfenc.o mf_utils.o
@@ -1267,7 +1269,7 @@ SKIPHEADERS+= %_tablegen.h
  \
  
  SKIPHEADERS-$(CONFIG_AMF)  += amfenc.h

  SKIPHEADERS-$(CONFIG_D3D11VA)  += d3d11va.h dxva2_internal.h
-SKIPHEADERS-$(CONFIG_D3D12VA)  += d3d12va_decode.h
+SKIPHEADERS-$(CONFIG_D3D12VA)  += d3d12va_decode.h d3d12va_encode.h
  SKIPHEADERS-$(CONFIG_DXVA2)+= dxva2.h dxva2_internal.h
  SKIPHEADERS-$(CONFIG_JNI)  += ffjni.h
  SKIPHEADERS-$(CONFIG_LCMS2)+= fflcms2.h
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index ef8c3a6d7d..9a34974141 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -865,6 +865,7 @@ extern const FFCodec ff_h264_vaapi_encoder;
  extern const FFCodec ff_h264_videotoolbox_encoder;
  extern const FFCodec ff_hevc_amf_encoder;
  extern const FFCodec ff_hevc_cuvid_decoder;
+extern const FFCodec ff_hevc_d3d12va_encoder;
  

Re: [FFmpeg-devel] [PATCH 1/2] avcodec/s302m: enable non-PCM decoding

2024-02-18 Thread Vittorio Giovara
On Sun, Feb 18, 2024 at 7:40 PM Nicolas George  wrote:

> Anton Khirnov (12024-02-18):
> >   That is absurd and makes no sense.
>
> That makes absolute sense, unless you consider your opinion is worth
> more than the opinion of the other people in the project.
>
> A spot on the TC is not a license to consider one's opinion above the
> other's, and therefore comes with a trade-off, and it is exactly that.
>
> There are other members of the TC who did not take part in the
> discussion: recusing yourself is not an issue.
>
> Sitting on the TC is a duty, a responsibility to examine in details
> changes one do not care about to make an informed decision. Somebody who
> sees it as a means to power rather than a responsibility should be
> evicted as soon as possible.
>

Thank you for your perspective on TC responsibilities within the FFmpeg
project. While I value your insights, I'd like to offer a different
viewpoint regarding the practice of recusing oneself from discussions.

It's essential to recognize that each TC member holds a duty to actively
participate in discussions and decision-making processes. Recusing oneself
from discussions should be a rare occurrence reserved for situations where
a clear conflict of interest exists, rather than a default response to
certain topics.

Serving on the TC requires a commitment to thoroughly examine proposed
changes and contribute to informed decision-making. It's through active
engagement and thoughtful consideration of all viewpoints that we can
ensure the best outcomes for the project.

While I understand the importance of avoiding conflicts of interest, it's
equally vital for TC members to fulfill their responsibility to the project
by actively engaging in discussions and providing valuable insights.

In conclusion, let's uphold the principle of active participation and
responsibility in TC discussions to maintain the integrity and
effectiveness of our governance processes within the FFmpeg project.
-- 
Vittorai
___
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 v5 4/9] avcodec/vaapi_encode: extract rc parameter configuration to base layer

2024-02-18 Thread Mark Thompson

On 18/02/2024 08:45, tong1.wu-at-intel@ffmpeg.org wrote:

From: Tong Wu 

VAAPI and D3D12VA can share rate control configuration codes. Hence, it
can be moved to base layer for simplification.

Signed-off-by: Tong Wu 
---
  libavcodec/hw_base_encode.c| 151 
  libavcodec/hw_base_encode.h|  34 ++
  libavcodec/vaapi_encode.c  | 210 ++---
  libavcodec/vaapi_encode.h  |  14 +--
  libavcodec/vaapi_encode_av1.c  |   2 +-
  libavcodec/vaapi_encode_h264.c |   2 +-
  libavcodec/vaapi_encode_vp9.c  |   2 +-
  7 files changed, 227 insertions(+), 188 deletions(-)

diff --git a/libavcodec/hw_base_encode.c b/libavcodec/hw_base_encode.c
index f0e4ef9655..c20c47bf55 100644
--- a/libavcodec/hw_base_encode.c
+++ b/libavcodec/hw_base_encode.c
@@ -631,6 +631,157 @@ end:
  return 0;
  }
  
+int ff_hw_base_rc_mode_configure(AVCodecContext *avctx, const HWBaseEncodeRCMode *rc_mode,

+ int default_quality, HWBaseEncodeRCConfigure 
*rc_conf)
+{
+HWBaseEncodeContext *ctx = avctx->priv_data;
+
+if (!rc_mode || !rc_conf)
+return -1;
+
+if (rc_mode->bitrate) {
+if (avctx->bit_rate <= 0) {
+av_log(avctx, AV_LOG_ERROR, "Bitrate must be set for %s "
+   "RC mode.\n", rc_mode->name);
+return AVERROR(EINVAL);
+}
+
+if (rc_mode->mode == RC_MODE_AVBR) {
+// For maximum confusion AVBR is hacked into the existing API
+// by overloading some of the fields with completely different
+// meanings.


You definitely don't want this absurd wart from libva to leak into the common 
API parts.  Make new fields which have the desired meaning in the common parts 
and let the VAAPI code deal with this stupidity.


+
+// Target percentage does not apply in AVBR mode.
+rc_conf->rc_bits_per_second = avctx->bit_rate;
+
+// Accuracy tolerance range for meeting the specified target
+// bitrate.  It's very unclear how this is actually intended
+// to work - since we do want to get the specified bitrate,
+// set the accuracy to 100% for now.
+rc_conf->rc_target_percentage = 100;
+
+// Convergence period in frames.  The GOP size reflects the
+// user's intended block size for cutting, so reusing that
+// as the convergence period seems a reasonable default.
+rc_conf->rc_window_size = avctx->gop_size > 0 ? avctx->gop_size : 
60;
+
+} else if (rc_mode->maxrate) {
+if (avctx->rc_max_rate > 0) {
+if (avctx->rc_max_rate < avctx->bit_rate) {
+av_log(avctx, AV_LOG_ERROR, "Invalid bitrate settings: "
+   "bitrate (%"PRId64") must not be greater than "
+   "maxrate (%"PRId64").\n", avctx->bit_rate,
+   avctx->rc_max_rate);
+return AVERROR(EINVAL);
+}
+rc_conf->rc_bits_per_second   = avctx->rc_max_rate;
+rc_conf->rc_target_percentage = (avctx->bit_rate * 100) /
+ avctx->rc_max_rate;
+} else {
+// We only have a target bitrate, but this mode requires
+// that a maximum rate be supplied as well.  Since the
+// user does not want this to be a constraint, arbitrarily
+// pick a maximum rate of double the target rate.
+rc_conf->rc_bits_per_second   = 2 * avctx->bit_rate;
+rc_conf->rc_target_percentage = 50;
+}
+} else {
+if (avctx->rc_max_rate > avctx->bit_rate) {
+av_log(avctx, AV_LOG_WARNING, "Max bitrate is ignored "
+   "in %s RC mode.\n", rc_mode->name);
+}
+rc_conf->rc_bits_per_second   = avctx->bit_rate;
+rc_conf->rc_target_percentage = 100;
+}
+} else {
+rc_conf->rc_bits_per_second   = 0;
+rc_conf->rc_target_percentage = 100;
+}
+
+if (rc_mode->quality) {
+if (ctx->explicit_qp) {
+rc_conf->rc_quality = ctx->explicit_qp;
+} else if (avctx->global_quality > 0) {
+rc_conf->rc_quality = avctx->global_quality;
+} else {
+rc_conf->rc_quality = default_quality;
+av_log(avctx, AV_LOG_WARNING, "No quality level set; "
+   "using default (%d).\n", rc_conf->rc_quality);
+}
+} else {
+rc_conf->rc_quality = 0;
+}
+
+if (rc_mode->hrd) {
+if (avctx->rc_buffer_size)
+rc_conf->hrd_buffer_size = avctx->rc_buffer_size;
+else if (avctx->rc_max_rate > 0)
+rc_conf->hrd_buffer_size = avctx->rc_max_rate;
+else
+rc_conf->hrd_buffer_size = avctx->bit_rate;
+if 

Re: [FFmpeg-devel] [PATCH] Fix rdiv always being set to 0 in vf_convolution.c

2024-02-18 Thread Stone Chen
Sorry I think I didn't correctly attach the patch the first time.

On Sun, Feb 18, 2024 at 2:21 PM Stone Chen  wrote:

> In commit 6c45d34, a line was added that always sets rdiv to 0, overriding
> any user input. This removes that line allowing user set values for 0rdiv,
> 1rdiv, 2rdiv, 3rdiv to apply as expected. This fixes ticket #10294.
>
> Signed-off-by: Stone Chen 
> ---
>  libavfilter/vf_convolution.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/libavfilter/vf_convolution.c b/libavfilter/vf_convolution.c
> index bf67f392f6..a00bb2b3c4 100644
> --- a/libavfilter/vf_convolution.c
> +++ b/libavfilter/vf_convolution.c
> @@ -674,7 +674,6 @@ static int param_init(AVFilterContext *ctx)
>  p = orig = av_strdup(s->matrix_str[i]);
>  if (p) {
>  s->matrix_length[i] = 0;
> -s->rdiv[i] = 0.f;
>  sum = 0.f;
>
>  while (s->matrix_length[i] < 49) {
> --
> 2.43.2
>
>
From 38734506c1f651062e38c4e1281d8070b6e4bdaf Mon Sep 17 00:00:00 2001
From: Stone Chen 
Date: Sun, 18 Feb 2024 13:47:05 -0500
Subject: [PATCH] Fix rdiv always being set to 0 in vf_convolution.c

In commit 6c45d34, a line was added that always sets rdiv to 0, overriding any user input. This removes that line allowing user set values for 0rdiv, 1rdiv, 2rdiv, 3rdiv to apply as expected. This fixes ticket #10294.
---
 libavfilter/vf_convolution.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/libavfilter/vf_convolution.c b/libavfilter/vf_convolution.c
index bf67f392f6..a00bb2b3c4 100644
--- a/libavfilter/vf_convolution.c
+++ b/libavfilter/vf_convolution.c
@@ -674,7 +674,6 @@ static int param_init(AVFilterContext *ctx)
 p = orig = av_strdup(s->matrix_str[i]);
 if (p) {
 s->matrix_length[i] = 0;
-s->rdiv[i] = 0.f;
 sum = 0.f;
 
 while (s->matrix_length[i] < 49) {
-- 
2.43.2

___
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] avcodec/cbs_vp8: Remove empty flush callback

2024-02-18 Thread Andreas Rheinhardt
This callback is optional and should therefore only be set
if it actually does something.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/cbs_vp8.c | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/libavcodec/cbs_vp8.c b/libavcodec/cbs_vp8.c
index 065156c248..eabdef358f 100644
--- a/libavcodec/cbs_vp8.c
+++ b/libavcodec/cbs_vp8.c
@@ -353,11 +353,6 @@ static int cbs_vp8_assemble_fragment(CodedBitstreamContext 
*ctx,
 return AVERROR_PATCHWELCOME;
 }
 
-static void cbs_vp8_flush(CodedBitstreamContext *ctx)
-{
-// Do nothing.
-}
-
 static const CodedBitstreamUnitTypeDescriptor cbs_vp8_unit_types[] = {
 CBS_UNIT_TYPE_INTERNAL_REF(0, VP8RawFrame, data),
 CBS_UNIT_TYPE_END_OF_LIST,
@@ -374,7 +369,5 @@ const CodedBitstreamType ff_cbs_type_vp8 = {
 .read_unit = _vp8_read_unit,
 .write_unit= _vp8_write_unit,
 
-.flush = _vp8_flush,
-
 .assemble_fragment = _vp8_assemble_fragment,
 };
-- 
2.34.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-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH v5 2/9] avcodec/vaapi_encode: introduce a base layer for vaapi encode

2024-02-18 Thread Mark Thompson

On 18/02/2024 08:45, tong1.wu-at-intel@ffmpeg.org wrote:

From: Tong Wu 

Since VAAPI and future D3D12VA implementation may share the same dpb
logic and some other functions. A base layer encode context is introduced
as vaapi context's base and extract the related funtions to a common
file such as receive_packet, init, close, etc.

Signed-off-by: Tong Wu 
---
  libavcodec/Makefile |   2 +-
  libavcodec/hw_base_encode.c | 637 ++
  libavcodec/hw_base_encode.h | 277 ++
  libavcodec/vaapi_encode.c   | 924 +++-
  libavcodec/vaapi_encode.h   | 229 +---
  libavcodec/vaapi_encode_av1.c   |  73 +--
  libavcodec/vaapi_encode_h264.c  | 201 +++
  libavcodec/vaapi_encode_h265.c  | 163 +++---
  libavcodec/vaapi_encode_mjpeg.c |  22 +-
  libavcodec/vaapi_encode_mpeg2.c |  53 +-
  libavcodec/vaapi_encode_vp8.c   |  28 +-
  libavcodec/vaapi_encode_vp9.c   |  70 +--
  12 files changed, 1427 insertions(+), 1252 deletions(-)
  create mode 100644 libavcodec/hw_base_encode.c
  create mode 100644 libavcodec/hw_base_encode.h

diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 470d7cb9b1..23946f6ea3 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -164,7 +164,7 @@ OBJS-$(CONFIG_STARTCODE)   += startcode.o
  OBJS-$(CONFIG_TEXTUREDSP)  += texturedsp.o
  OBJS-$(CONFIG_TEXTUREDSPENC)   += texturedspenc.o
  OBJS-$(CONFIG_TPELDSP) += tpeldsp.o
-OBJS-$(CONFIG_VAAPI_ENCODE)+= vaapi_encode.o
+OBJS-$(CONFIG_VAAPI_ENCODE)+= vaapi_encode.o hw_base_encode.o
  OBJS-$(CONFIG_AV1_AMF_ENCODER) += amfenc_av1.o
  OBJS-$(CONFIG_VC1DSP)  += vc1dsp.o
  OBJS-$(CONFIG_VIDEODSP)+= videodsp.o
diff --git a/libavcodec/hw_base_encode.c b/libavcodec/hw_base_encode.c
...


I'm going to trust that the contents of this file are only moved around with no 
functional changes.  If that isn't the case please do say.


diff --git a/libavcodec/hw_base_encode.h b/libavcodec/hw_base_encode.h
new file mode 100644
index 00..70982c97b2
--- /dev/null
+++ b/libavcodec/hw_base_encode.h
@@ -0,0 +1,277 @@
+/*
+ * Copyright (c) 2024 Intel Corporation


I would avoid adding this.  Most of these files have content mostly copied from 
other files which are not exclusively copyright by Intel Corporation.


+ *
+ * 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
+ */
+
+#ifndef AVCODEC_HW_BASE_ENCODE_H
+#define AVCODEC_HW_BASE_ENCODE_H
+
+#include "libavutil/hwcontext.h"
+#include "libavutil/fifo.h"
+
+#include "avcodec.h"
+#include "hwconfig.h"


This file doesn't do anything with hwconfig stuff?


+
+enum {
+MAX_DPB_SIZE   = 16,
+MAX_PICTURE_REFERENCES = 2,
+MAX_REORDER_DELAY  = 16,
+MAX_ASYNC_DEPTH= 64,
+MAX_REFERENCE_LIST_NUM = 2,
+};
+
+enum {
+PICTURE_TYPE_IDR = 0,
+PICTURE_TYPE_I   = 1,
+PICTURE_TYPE_P   = 2,
+PICTURE_TYPE_B   = 3,
+};
+
+enum {
+// Codec supports controlling the subdivision of pictures into slices.
+FLAG_SLICE_CONTROL = 1 << 0,
+// Codec only supports constant quality (no rate control).
+FLAG_CONSTANT_QUALITY_ONLY = 1 << 1,
+// Codec is intra-only.
+FLAG_INTRA_ONLY= 1 << 2,
+// Codec supports B-pictures.
+FLAG_B_PICTURES= 1 << 3,
+// Codec supports referencing B-pictures.
+FLAG_B_PICTURE_REFERENCES  = 1 << 4,
+// Codec supports non-IDR key pictures (that is, key pictures do
+// not necessarily empty the DPB).
+FLAG_NON_IDR_KEY_PICTURES  = 1 << 5,
+// Codec output packet without timestamp delay, which means the
+// output packet has same PTS and DTS.
+FLAG_TIMESTAMP_NO_DELAY= 1 << 6,
+};
+
+enum {
+RC_MODE_AUTO,
+RC_MODE_CQP,
+RC_MODE_CBR,
+RC_MODE_VBR,
+RC_MODE_ICQ,


I thought ICQ was an Intel name which leaked into libva?  This probably 
shouldn't be in a generic API, maybe something about constant-quality.


+RC_MODE_QVBR,
+RC_MODE_AVBR,


More generally, I think you want to document what these modes are intended to be.  When 
they mapped directly to VAAPI then it was clear that the responsibility was 
"whatever libva gives you", but 

[FFmpeg-devel] [PATCH 4/4] avcodec: Remove superfluous '; ' outside of functions

2024-02-18 Thread Andreas Rheinhardt
Inside a function an extra ';' is a null statement;
but outside of it it simply must not happen.
So remove them.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/vvc/vvc_itx_1d.c  |  2 +-
 libavcodec/x86/h26x/h2656dsp.h   |  2 +-
 libavcodec/x86/hevcdsp_init.c| 78 
 libavcodec/x86/vvc/vvcdsp_init.c | 24 +-
 4 files changed, 53 insertions(+), 53 deletions(-)

diff --git a/libavcodec/vvc/vvc_itx_1d.c b/libavcodec/vvc/vvc_itx_1d.c
index 01a50aad25..cb076f680f 100644
--- a/libavcodec/vvc/vvc_itx_1d.c
+++ b/libavcodec/vvc/vvc_itx_1d.c
@@ -639,7 +639,7 @@ void ff_vvc_inv_dct2_64(int *coeffs, const ptrdiff_t 
stride, const size_t nz)
 coeffs[61 * stride] = E[2]  - O[2];
 coeffs[62 * stride] = E[1]  - O[1];
 coeffs[63 * stride] = E[0]  - O[0];
-};
+}
 
 static void matrix_mul(int *coeffs, const ptrdiff_t stride, const int8_t* 
matrix, const int size, const size_t nz)
 {
diff --git a/libavcodec/x86/h26x/h2656dsp.h b/libavcodec/x86/h26x/h2656dsp.h
index e31aae6b0d..0ea08b6a20 100644
--- a/libavcodec/x86/h26x/h2656dsp.h
+++ b/libavcodec/x86/h26x/h2656dsp.h
@@ -31,7 +31,7 @@
 
 #define H2656_PEL_PROTOTYPE(name, D, opt) \
 void ff_h2656_put_ ## name ## _ ## D ## _##opt(int16_t *dst, ptrdiff_t 
dststride, const uint8_t *_src, ptrdiff_t _srcstride, int height, const int8_t 
*hf, const int8_t *vf, int width);  \
-void ff_h2656_put_uni_ ## name ## _ ## D ## _##opt(uint8_t *_dst, ptrdiff_t 
_dststride, const uint8_t *_src, ptrdiff_t _srcstride, int height, const int8_t 
*hf, const int8_t *vf, int width);\
+void ff_h2656_put_uni_ ## name ## _ ## D ## _##opt(uint8_t *_dst, ptrdiff_t 
_dststride, const uint8_t *_src, ptrdiff_t _srcstride, int height, const int8_t 
*hf, const int8_t *vf, int width) \
 
 #define H2656_MC_8TAP_PROTOTYPES(fname, bitd, opt)\
 H2656_PEL_PROTOTYPE(fname##4,  bitd, opt);\
diff --git a/libavcodec/x86/hevcdsp_init.c b/libavcodec/x86/hevcdsp_init.c
index 3d4e41754d..f5bc342cd5 100644
--- a/libavcodec/x86/hevcdsp_init.c
+++ b/libavcodec/x86/hevcdsp_init.c
@@ -123,17 +123,17 @@ void ff_hevc_put_hevc_uni_ ## a ## _ ## depth ## 
_##opt(uint8_t *dst, ptrdiff_t
 #define FW_DIR_HV(npel, n, w, depth, opt) \
 FW_PUT_FUNCS(npel, npel ## _hv##w,  n ## tap_hv##w,  depth, opt)
 
-FW_PEL(4,   8, sse4);
-FW_PEL(6,   8, sse4);
-FW_PEL(8,   8, sse4);
-FW_PEL(12,  8, sse4);
-FW_PEL(16,  8, sse4);
-FW_PEL(4,  10, sse4);
-FW_PEL(6,  10, sse4);
-FW_PEL(8,  10, sse4);
-FW_PEL(4,  12, sse4);
-FW_PEL(6,  12, sse4);
-FW_PEL(8,  12, sse4);
+FW_PEL(4,   8, sse4)
+FW_PEL(6,   8, sse4)
+FW_PEL(8,   8, sse4)
+FW_PEL(12,  8, sse4)
+FW_PEL(16,  8, sse4)
+FW_PEL(4,  10, sse4)
+FW_PEL(6,  10, sse4)
+FW_PEL(8,  10, sse4)
+FW_PEL(4,  12, sse4)
+FW_PEL(6,  12, sse4)
+FW_PEL(8,  12, sse4)
 
 #define FW_EPEL(w, depth, opt) FW_DIR(epel, 4, w, depth, opt)
 #define FW_EPEL_HV(w, depth, opt) FW_DIR_HV(epel, 4, w, depth, opt)
@@ -141,18 +141,18 @@ FW_PEL(8,  12, sse4);
 FW_EPEL(w, depth, opt)   \
 FW_EPEL_HV(w, depth, opt)
 
-FW_EPEL(12,  8, sse4);
+FW_EPEL(12,  8, sse4)
 
-FW_EPEL_FUNCS(4,   8, sse4);
-FW_EPEL_FUNCS(6,   8, sse4);
-FW_EPEL_FUNCS(8,   8, sse4);
-FW_EPEL_FUNCS(16,  8, sse4);
-FW_EPEL_FUNCS(4,  10, sse4);
-FW_EPEL_FUNCS(6,  10, sse4);
-FW_EPEL_FUNCS(8,  10, sse4);
-FW_EPEL_FUNCS(4,  12, sse4);
-FW_EPEL_FUNCS(6,  12, sse4);
-FW_EPEL_FUNCS(8,  12, sse4);
+FW_EPEL_FUNCS(4,   8, sse4)
+FW_EPEL_FUNCS(6,   8, sse4)
+FW_EPEL_FUNCS(8,   8, sse4)
+FW_EPEL_FUNCS(16,  8, sse4)
+FW_EPEL_FUNCS(4,  10, sse4)
+FW_EPEL_FUNCS(6,  10, sse4)
+FW_EPEL_FUNCS(8,  10, sse4)
+FW_EPEL_FUNCS(4,  12, sse4)
+FW_EPEL_FUNCS(6,  12, sse4)
+FW_EPEL_FUNCS(8,  12, sse4)
 
 #define FW_QPEL(w, depth, opt) FW_DIR(qpel, 8, w, depth, opt)
 #define FW_QPEL_HV(w, depth, opt) FW_DIR_HV(qpel, 8, w, depth, opt)
@@ -160,31 +160,31 @@ FW_EPEL_FUNCS(8,  12, sse4);
 FW_QPEL(w, depth, opt)   \
 FW_QPEL_HV(w, depth, opt)
 
-FW_QPEL(12, 8, sse4);
-FW_QPEL(16, 8, sse4);
+FW_QPEL(12, 8, sse4)
+FW_QPEL(16, 8, sse4)
 
-FW_QPEL_FUNCS(4,   8, sse4);
-FW_QPEL_FUNCS(8,   8, sse4);
-FW_QPEL_FUNCS(4,  10, sse4);
-FW_QPEL_FUNCS(8,  10, sse4);
-FW_QPEL_FUNCS(4,  12, sse4);
-FW_QPEL_FUNCS(8,  12, sse4);
+FW_QPEL_FUNCS(4,   8, sse4)
+FW_QPEL_FUNCS(8,   8, sse4)
+FW_QPEL_FUNCS(4,  10, sse4)
+FW_QPEL_FUNCS(8,  10, sse4)
+FW_QPEL_FUNCS(4,  12, sse4)
+FW_QPEL_FUNCS(8,  12, sse4)
 
 #if HAVE_AVX2_EXTERNAL
 
-FW_PEL(32,  8, avx2);
-FW_PUT(pel, pel_pixels16, pixels16, 10, avx2);
+FW_PEL(32,  8, avx2)
+FW_PUT(pel, pel_pixels16, pixels16, 10, avx2)
 
-FW_EPEL(32,  8, avx2);
-FW_EPEL(16, 10, avx2);
+FW_EPEL(32,  8, avx2)
+FW_EPEL(16, 10, avx2)
 
-FW_EPEL_HV(32,  8, avx2);
-FW_EPEL_HV(16, 10, avx2);
+FW_EPEL_HV(32,  8, avx2)
+FW_EPEL_HV(16, 10, avx2)
 
-FW_QPEL(32,  8, avx2);
-FW_QPEL(16, 10, avx2);
+FW_QPEL(32,  8, avx2)
+FW_QPEL(16, 10, avx2)
 
-FW_QPEL_HV(16, 10, avx2);
+FW_QPEL_HV(16, 10, avx2)
 
 #endif
 #endif
diff --git a/libavcodec/x86/vvc/vvcdsp_init.c 

[FFmpeg-devel] [PATCH 3/4] avcodec/vvc/vvcdsp: Remove pointless wrappers

2024-02-18 Thread Andreas Rheinhardt
Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/vvc/vvcdsp.c  | 18 --
 libavcodec/vvc/vvcdsp_template.c |  2 +-
 2 files changed, 1 insertion(+), 19 deletions(-)

diff --git a/libavcodec/vvc/vvcdsp.c b/libavcodec/vvc/vvcdsp.c
index 214dc4e6c0..d63b9bc9b3 100644
--- a/libavcodec/vvc/vvcdsp.c
+++ b/libavcodec/vvc/vvcdsp.c
@@ -64,24 +64,6 @@ static int vvc_sad(const int16_t *src0, const int16_t *src1, 
int dx, int dy,
 return sad;
 }
 
-#define itx_fn(type, s)
 \
-static void itx_##type##_##s(int *coeffs, ptrdiff_t step, size_t nz)   
 \
-{  
 \
-  ff_vvc_inv_##type##_##s(coeffs, step, nz);   
 \
-}
-
-#define itx_fn_common(type) \
-itx_fn(type, 4);\
-itx_fn(type, 8);\
-itx_fn(type, 16);   \
-itx_fn(type, 32);   \
-
-itx_fn_common(dct2);
-itx_fn_common(dst7);
-itx_fn_common(dct8);
-itx_fn(dct2, 2);
-itx_fn(dct2, 64);
-
 typedef struct IntraEdgeParams {
 uint8_t* top;
 uint8_t* left;
diff --git a/libavcodec/vvc/vvcdsp_template.c b/libavcodec/vvc/vvcdsp_template.c
index f92c266478..33815d6765 100644
--- a/libavcodec/vvc/vvcdsp_template.c
+++ b/libavcodec/vvc/vvcdsp_template.c
@@ -97,7 +97,7 @@ static void FUNC(transform_bdpcm)(int *coeffs, const int 
width, const int height
 static void FUNC(ff_vvc_itx_dsp_init)(VVCItxDSPContext *const itx)
 {
 #define VVC_ITX(TYPE, type, s) 
 \
-itx->itx[TYPE][TX_SIZE_##s]  = itx_##type##_##s;   
 \
+itx->itx[TYPE][TX_SIZE_##s]  = ff_vvc_inv_##type##_##s;
 \
 
 #define VVC_ITX_COMMON(TYPE, type) 
 \
 VVC_ITX(TYPE, type, 4);
 \
-- 
2.34.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-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH 2/4] avcodec/vvc/vvc_ps: Use union for luts to avoid unaligned accesses

2024-02-18 Thread Andreas Rheinhardt
These arrays are currently accessed via uint16_t* pointers
although nothing guarantees their alignment. Furthermore,
this is problematic wrt the effective-type rules.
Fix this by using a union of arrays of uint8_t and uint16_t.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/vvc/vvc_filter.c  | 2 +-
 libavcodec/vvc/vvc_filter_template.c | 4 ++--
 libavcodec/vvc/vvc_inter.c   | 4 ++--
 libavcodec/vvc/vvc_ps.c  | 8 
 libavcodec/vvc/vvc_ps.h  | 7 ---
 libavcodec/vvc/vvcdsp.h  | 2 +-
 6 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/libavcodec/vvc/vvc_filter.c b/libavcodec/vvc/vvc_filter.c
index df77e443f6..5fa711c9e0 100644
--- a/libavcodec/vvc/vvc_filter.c
+++ b/libavcodec/vvc/vvc_filter.c
@@ -1328,5 +1328,5 @@ void ff_vvc_lmcs_filter(const VVCLocalContext *lc, const 
int x, const int y)
 const int height   = FFMIN(fc->ps.pps->height - y, ctb_size);
 uint8_t *data  = fc->frame->data[LUMA] + y * fc->frame->linesize[LUMA] 
+ (x << fc->ps.sps->pixel_shift);
 if (sc->sh.r->sh_lmcs_used_flag)
-fc->vvcdsp.lmcs.filter(data, fc->frame->linesize[LUMA], width, height, 
fc->ps.lmcs.inv_lut);
+fc->vvcdsp.lmcs.filter(data, fc->frame->linesize[LUMA], width, height, 
>ps.lmcs.inv_lut);
 }
diff --git a/libavcodec/vvc/vvc_filter_template.c 
b/libavcodec/vvc/vvc_filter_template.c
index b7eaef5125..9b3a0e46f7 100644
--- a/libavcodec/vvc/vvc_filter_template.c
+++ b/libavcodec/vvc/vvc_filter_template.c
@@ -22,9 +22,9 @@
 
 #include "libavcodec/h26x/h2656_sao_template.c"
 
-static void FUNC(lmcs_filter_luma)(uint8_t *_dst, ptrdiff_t dst_stride, const 
int width, const int height, const uint8_t *_lut)
+static void FUNC(lmcs_filter_luma)(uint8_t *_dst, ptrdiff_t dst_stride, const 
int width, const int height, const void *_lut)
 {
-const pixel *lut = (const pixel *)_lut;
+const pixel *lut = _lut;
 pixel *dst = (pixel*)_dst;
 dst_stride /= sizeof(pixel);
 
diff --git a/libavcodec/vvc/vvc_inter.c b/libavcodec/vvc/vvc_inter.c
index e05f3db93e..6c9c8a7165 100644
--- a/libavcodec/vvc/vvc_inter.c
+++ b/libavcodec/vvc/vvc_inter.c
@@ -571,7 +571,7 @@ static void pred_regular_luma(VVCLocalContext *lc, const 
int hf_idx, const int v
 const int intra_weight = ciip_derive_intra_weight(lc, x0, y0, sbw, 
sbh);
 fc->vvcdsp.intra.intra_pred(lc, x0, y0, sbw, sbh, 0);
 if (sc->sh.r->sh_lmcs_used_flag)
-fc->vvcdsp.lmcs.filter(inter, inter_stride, sbw, sbh, 
fc->ps.lmcs.fwd_lut);
+fc->vvcdsp.lmcs.filter(inter, inter_stride, sbw, sbh, 
>ps.lmcs.fwd_lut);
 fc->vvcdsp.inter.put_ciip(dst, dst_stride, sbw, sbh, inter, 
inter_stride, intra_weight);
 
 }
@@ -887,7 +887,7 @@ static void predict_inter(VVCLocalContext *lc)
 
 if (lc->sc->sh.r->sh_lmcs_used_flag && !cu->ciip_flag) {
 uint8_t* dst0 = POS(0, cu->x0, cu->y0);
-fc->vvcdsp.lmcs.filter(dst0, fc->frame->linesize[LUMA], cu->cb_width, 
cu->cb_height, fc->ps.lmcs.fwd_lut);
+fc->vvcdsp.lmcs.filter(dst0, fc->frame->linesize[LUMA], cu->cb_width, 
cu->cb_height, >ps.lmcs.fwd_lut);
 }
 }
 
diff --git a/libavcodec/vvc/vvc_ps.c b/libavcodec/vvc/vvc_ps.c
index 376027ed81..e6e46d2039 100644
--- a/libavcodec/vvc/vvc_ps.c
+++ b/libavcodec/vvc/vvc_ps.c
@@ -642,9 +642,9 @@ static int lmcs_derive_lut(VVCLMCS *lmcs, const H266RawAPS 
*rlmcs, const H266Raw
 const uint16_t fwd_sample = lmcs_derive_lut_sample(sample, lmcs->pivot,
 input_pivot, scale_coeff, idx_y, max);
 if (bit_depth > 8)
-((uint16_t *)lmcs->fwd_lut)[sample] = fwd_sample;
+lmcs->fwd_lut.u16[sample] = fwd_sample;
 else
-lmcs->fwd_lut[sample] = fwd_sample;
+lmcs->fwd_lut.u8 [sample] = fwd_sample;
 
 }
 
@@ -659,9 +659,9 @@ static int lmcs_derive_lut(VVCLMCS *lmcs, const H266RawAPS 
*rlmcs, const H266Raw
 inv_scale_coeff, i, max);
 
 if (bit_depth > 8)
-((uint16_t *)lmcs->inv_lut)[sample] = inv_sample;
+lmcs->inv_lut.u16[sample] = inv_sample;
 else
-lmcs->inv_lut[sample] = inv_sample;
+lmcs->inv_lut.u8 [sample] = inv_sample;
 }
 
 return 0;
diff --git a/libavcodec/vvc/vvc_ps.h b/libavcodec/vvc/vvc_ps.h
index 3d3aa061f5..5adf3f3453 100644
--- a/libavcodec/vvc/vvc_ps.h
+++ b/libavcodec/vvc/vvc_ps.h
@@ -191,9 +191,10 @@ typedef struct VVCLMCS {
 uint8_t  min_bin_idx;
 uint8_t  max_bin_idx;
 
-//*2 for high depth
-uint8_t  fwd_lut[LMCS_MAX_LUT_SIZE * 2];
-uint8_t  inv_lut[LMCS_MAX_LUT_SIZE * 2];
+union {
+uint8_t  u8[LMCS_MAX_LUT_SIZE];
+uint16_t u16[LMCS_MAX_LUT_SIZE]; ///< for high bit-depth
+} fwd_lut, inv_lut;
 
 uint16_t pivot[LMCS_MAX_BIN_SIZE + 1];
 uint16_t chroma_scale_coeff[LMCS_MAX_BIN_SIZE];
diff --git a/libavcodec/vvc/vvcdsp.h b/libavcodec/vvc/vvcdsp.h
index 6f59e73654..f4fb3cb7d7 100644
--- 

[FFmpeg-devel] [PATCH 1/4] avcodec/vvc/vvc_ps: Check before access

2024-02-18 Thread Andreas Rheinhardt
max_bin_idx can be at most LMCS_MAX_BIN_SIZE - 1 here,
so pivot[LCMS_MAX_BIN_SIZE + 1] may be accessed,
but pivot has only LCMS_MAX_BIN_SIZE + 1 elements
(unless the values of pivot were so that it is always
assured that pivot[LCMS_MAX_BIN_SIZE] is always < sample
(which it is iff it is always < 2^bit_depth - 1)).
So reorder the checks.

Signed-off-by: Andreas Rheinhardt 
---
I don't know whether this can be triggered at all.

 libavcodec/vvc/vvc_ps.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/vvc/vvc_ps.c b/libavcodec/vvc/vvc_ps.c
index 53a86321d6..376027ed81 100644
--- a/libavcodec/vvc/vvc_ps.c
+++ b/libavcodec/vvc/vvc_ps.c
@@ -652,7 +652,7 @@ static int lmcs_derive_lut(VVCLMCS *lmcs, const H266RawAPS 
*rlmcs, const H266Raw
 i = lmcs->min_bin_idx;
 for (uint16_t sample = 0; sample < max; sample++) {
 uint16_t inv_sample;
-while (sample >= lmcs->pivot[i + 1] && i <= lmcs->max_bin_idx)
+while (i <= lmcs->max_bin_idx && sample >= lmcs->pivot[i + 1])
 i++;
 
 inv_sample = lmcs_derive_lut_sample(sample, input_pivot, lmcs->pivot,
-- 
2.34.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-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH] Fix rdiv always being set to 0 in vf_convolution.c

2024-02-18 Thread Stone Chen
In commit 6c45d34, a line was added that always sets rdiv to 0, overriding any 
user input. This removes that line allowing user set values for 0rdiv, 1rdiv, 
2rdiv, 3rdiv to apply as expected. This fixes ticket #10294.

Signed-off-by: Stone Chen 
---
 libavfilter/vf_convolution.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/libavfilter/vf_convolution.c b/libavfilter/vf_convolution.c
index bf67f392f6..a00bb2b3c4 100644
--- a/libavfilter/vf_convolution.c
+++ b/libavfilter/vf_convolution.c
@@ -674,7 +674,6 @@ static int param_init(AVFilterContext *ctx)
 p = orig = av_strdup(s->matrix_str[i]);
 if (p) {
 s->matrix_length[i] = 0;
-s->rdiv[i] = 0.f;
 sum = 0.f;
 
 while (s->matrix_length[i] < 49) {
-- 
2.43.2

___
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 1/2] avcodec/s302m: enable non-PCM decoding

2024-02-18 Thread Nicolas George
Rémi Denis-Courmont (12024-02-18):
> I trust that you do know the meaning of the auxillary "should". That very 
> definitely and very obviously eliminates any "maximalist" interpretations.

Indeed. And I repeat what I already said in another mail:

If somebody dishonest wants to exploit that loophole, if somebody does
not do what the rules say they do, then the GA should do it for them.

> The maximalist interpretation is clearly nonsensical as per reductio ad 
> absurdum.

The maximalist interpretation was what was intended.

The maximalist interpretation also was what Anton understood when the
rule was put in place. It is a little hypocritical to pretend is means
something else now.

In fact, if you want to know everything, the current situation was
EXACTLY what I had in mind when I proposed the rule in the first place.
Including the identity of the TC with a conflict of interest.

>   This is a *guideline* or *recommendation*, and obviously it 
> _cannot_ 
> be applied to its logic extreme.

Human rules are never meant to be applied to their logic extreme,
arguments of this kind are bullshit.

> Instead Anton, and later rach other TC member will have to each determine for 
> themselves if they are so implicated in the disagreement as to justify 
> recusing themselves.
> 
> Gyan does not get to dictate it, and neither do you or I. There is no point 
> arguing this further, and I won't.

Gyan can call for a vote of no confidence in the TC or one of its
members.

-- 
  Nicolas George
___
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 1/2] avcodec/s302m: enable non-PCM decoding

2024-02-18 Thread Rémi Denis-Courmont
Le sunnuntaina 18. helmikuuta 2024, 20.40.14 EET Nicolas George a écrit :
> The world is “involves”, its meaning is inherently maximalist.

The wording is very clear (emphasis added): "If the disagreement involves a 
member of the TC, that member SHOULD recuse themselves from the decision."

I trust that you do know the meaning of the auxillary "should". That very 
definitely and very obviously eliminates any "maximalist" interpretations.

> Exactly: you were part of this disagreement, you should recuse yourself.

The maximalist interpretation is clearly nonsensical as per reductio ad 
absurdum. This is a *guideline* or *recommendation*, and obviously it _cannot_ 
be applied to its logic extreme.

Instead Anton, and later rach other TC member will have to each determine for 
themselves if they are so implicated in the disagreement as to justify 
recusing themselves.

Gyan does not get to dictate it, and neither do you or I. There is no point 
arguing this further, and I won't.

-- 
レミ・デニ-クールモン
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 1/2] avcodec/s302m: enable non-PCM decoding

2024-02-18 Thread Gyan Doshi



On 2024-02-18 11:33 pm, Anton Khirnov wrote:

Quoting Gyan Doshi (2024-02-18 05:06:30)

b) what "maximalist" interpretation?

A non-maximalist interpretation would be that a TC member is only
excluded from voting when they authored the patch that is being
disputed.


If the promulgators meant to only prevent proposers of the disputed 
change to not take part, then

the verbiage would be different.

In looking up how this clause came to be present, I came across the 
following messages:


https://ffmpeg.org/pipermail/ffmpeg-devel/2020-December/273443.html
(Nicolas George originally proposes this clause - wording is more 
restrictive)


https://ffmpeg.org/pipermail/ffmpeg-devel/2021-January/274822.html
(this one is interesting, you objected to the clause but on the grounds 
that it was all-encompassing i.e.  anyone commenting on the dispute was 
potentially subjected to recusal and referred to some 'model' 
discussion, so your describing my reading as maximalist is weird since 
that is how you read it - you just happen to object to this rule)


https://ffmpeg.org/pipermail/ffmpeg-devel/2021-January/274826.html
(Ronald clarifies that "involved" should be constrained to just be one 
of the two parties -- of which you happen to be one)


There's the matter of what the rule currently is, distinct from what it 
should be. What it ideally should be is that the decision should be 
taken by a fresh set of eyes consisting of those who haven't become or 
are seen to be publicly invested in the outcome. So the TC should have a 
set of alternates - those who can make up the quorum and constitute an 
odd number of voters when some from the first 5 are recused.


Regards,
Gyan

___
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 1/2] avcodec/s302m: enable non-PCM decoding

2024-02-18 Thread Nicolas George
Rémi Denis-Courmont (12024-02-18):
> This is an utterly absurd interpretation. By that logic, a TC member would 
> automatically become party to the disagreement by expressing an opinion 
> within 
> even the TC itself.

This is the most hypocritical argument put forward in this discussion
yet.

> In fact, if you would read it maximally that way, any who 
> has an opinion, even if they have not expressed it, would be a party.
> 
> So what then, the FFmpeg thought police?

And you break your own record in the very next sentence.

> You can argue that the rule is vague, and it is. But if anything, we can at 
> least eliminate absurd interpretations.

The rule is not vague at all.

>  (And in any case, it says "should", 
> not "must".)

Indeed. I wondered when somebody dishonest would try to exploit that
loophole.

The obvious answer is: if somebody in the TC does not do what the rules
say they SHOULD, then the general assembly SHOULD vote them out at the
next election. Or earlier, because a vote of no confidence can be
brought at any time.

-- 
  Nicolas George
___
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 1/2] avcodec/s302m: enable non-PCM decoding

2024-02-18 Thread Rémi Denis-Courmont
Le sunnuntaina 18. helmikuuta 2024, 2.43.14 EET Michael Niedermayer a écrit :
> > > You clearly are one of the parties to the disagreement, and "recuse
> > > themselves from the decision" is self-explanatory.
> > 
> > Such a maximalist interpretation makes no sense - why should my opinion
> > become invalid because I commented on a patch,
> 
> "If the disagreement involves a member of the TC"
> does IMHO not preclude commenting on a patch.
> 
> For a disagreement we need 2 parties.
> For example one party who wants a patch in and one who blocks the patch. or
> 2 parties where both block the other.

This is an utterly absurd interpretation. By that logic, a TC member would 
automatically become party to the disagreement by expressing an opinion within 
even the TC itself. In fact, if you would read it maximally that way, any who 
has an opinion, even if they have not expressed it, would be a party.

So what then, the FFmpeg thought police?

You can argue that the rule is vague, and it is. But if anything, we can at 
least eliminate absurd interpretations. (And in any case, it says "should", 
not "must".)

-- 
レミ・デニ-クールモン
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] fate: use an even more exotic channel layout mov-mp4-pcm-float test

2024-02-18 Thread James Almer

On 2/18/2024 3:38 PM, Marton Balint wrote:



On Sun, 18 Feb 2024, James Almer wrote:


On 2/18/2024 7:45 AM, Marton Balint wrote:

 The old layout happened to be a native layout and therefore missed some
 recently fixed layout parsing bugs.

 Signed-off-by: Marton Balint 
 ---
   tests/fate/mov.mak   | 2 +-
   tests/ref/fate/mov-mp4-pcm-float | 4 ++--
   2 files changed, 3 insertions(+), 3 deletions(-)

 diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak
 index 4850c8aa94..8d154c8b5b 100644
 --- a/tests/fate/mov.mak
 +++ b/tests/fate/mov.mak
 @@ -187,7 +187,7 @@ fate-mov-mp4-pcm: CMD = transcode wav
 $(TARGET_PATH)/tests/data/asynth-44100-1.w
   FATE_MOV_FFMPEG-$(call TRANSCODE, PCM_S16LE, MOV, WAV_DEMUXER
   PAN_FILTER) \
 += fate-mov-mp4-pcm-float
   fate-mov-mp4-pcm-float: tests/data/asynth-44100-1.wav
 -fate-mov-mp4-pcm-float: CMD = transcode wav
 $(TARGET_PATH)/tests/data/asynth-44100-1.wav mp4 "-af
 aresample,pan=FL+LFE+BR|c0=c0|c1=c0|c2=c0 -c:a pcm_f32le" "-map 0 -c 
copy

 -frames:a 0"
 +fate-mov-mp4-pcm-float: CMD = transcode wav
 $(TARGET_PATH)/tests/data/asynth-44100-1.wav mp4 "-af
 aresample,pan=FR+FL+FR|c0=c0|c1=c0|c2=c0 -c:a pcm_f32le" "-map 0 -c 
copy

 -frames:a 0"


Wouldn't FR+FL+LFE be enough to test this? While also generating a 
file that's realistic.


It depends on what you want to test. With the old code, for FR+FL+LFE,
only the channel order would have been wrong, with FR+FL+FR also the 
channel count.


Having the same channel position in the same track is not that 
theoretical, at least for MOV I have samples where an additional FR/FL 
track is used for Music/Effects. I admit, for MP4 that might be less 
common though, and I also admit that using a separate track for it would 
be better. But as we know, nothing is ideal in practice...


Nevertheless, I can change the test to FR+FL+LFE if that is preferred.


No, it's ok as is if it tests more things that can potentially regress.



Regards,
Marton
___
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 1/2] avcodec/s302m: enable non-PCM decoding

2024-02-18 Thread Nicolas George
Anton Khirnov (12024-02-18):
> A non-maximalist interpretation would be that a TC member is only
> excluded from voting when they authored the patch that is being
> disputed.

If the rules were meant to be interpreted that way, they would have been
written “if the patch was proposed by a member of the TC“, not “if the
disagreement involves a member of the TC”.

The world is “involves”, its meaning is inherently maximalist.

> Anything handled by the TC is a disagreement. Then according to your
> interpretation, any TC member who expressed an opinion on a patch
> (either positive or negative) becomes a party to the disagreement and
> cannot vote.

Exactly: you were part of this disagreement, you should recuse yourself.

>   That is absurd and makes no sense.

That makes absolute sense, unless you consider your opinion is worth
more than the opinion of the other people in the project.

A spot on the TC is not a license to consider one's opinion above the
other's, and therefore comes with a trade-off, and it is exactly that.

There are other members of the TC who did not take part in the
discussion: recusing yourself is not an issue.

Sitting on the TC is a duty, a responsibility to examine in details
changes one do not care about to make an informed decision. Somebody who
sees it as a means to power rather than a responsibility should be
evicted as soon as possible.

-- 
  Nicolas George
___
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] avutil/mem: use C11 aligned_malloc()

2024-02-18 Thread Rémi Denis-Courmont
Le sunnuntaina 18. helmikuuta 2024, 18.29.32 EET James Almer a écrit :
> Removing all the different target specific allocation functions in favor
> of the standard one. But your second point makes it moot, so patch
> withdrawn.

If you want to get code closer to standards for dealing with alignment, I 
would argue that using alignas() instead of nonstandard constructs comes first.

-- 
雷米‧德尼-库尔蒙
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] fate: use an even more exotic channel layout mov-mp4-pcm-float test

2024-02-18 Thread Marton Balint




On Sun, 18 Feb 2024, James Almer wrote:


On 2/18/2024 7:45 AM, Marton Balint wrote:

 The old layout happened to be a native layout and therefore missed some
 recently fixed layout parsing bugs.

 Signed-off-by: Marton Balint 
 ---
   tests/fate/mov.mak   | 2 +-
   tests/ref/fate/mov-mp4-pcm-float | 4 ++--
   2 files changed, 3 insertions(+), 3 deletions(-)

 diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak
 index 4850c8aa94..8d154c8b5b 100644
 --- a/tests/fate/mov.mak
 +++ b/tests/fate/mov.mak
 @@ -187,7 +187,7 @@ fate-mov-mp4-pcm: CMD = transcode wav
 $(TARGET_PATH)/tests/data/asynth-44100-1.w
   FATE_MOV_FFMPEG-$(call TRANSCODE, PCM_S16LE, MOV, WAV_DEMUXER
   PAN_FILTER) \
 += fate-mov-mp4-pcm-float
   fate-mov-mp4-pcm-float: tests/data/asynth-44100-1.wav
 -fate-mov-mp4-pcm-float: CMD = transcode wav
 $(TARGET_PATH)/tests/data/asynth-44100-1.wav mp4 "-af
 aresample,pan=FL+LFE+BR|c0=c0|c1=c0|c2=c0 -c:a pcm_f32le" "-map 0 -c copy
 -frames:a 0"
 +fate-mov-mp4-pcm-float: CMD = transcode wav
 $(TARGET_PATH)/tests/data/asynth-44100-1.wav mp4 "-af
 aresample,pan=FR+FL+FR|c0=c0|c1=c0|c2=c0 -c:a pcm_f32le" "-map 0 -c copy
 -frames:a 0"


Wouldn't FR+FL+LFE be enough to test this? While also generating a file 
that's realistic.


It depends on what you want to test. With the old code, for FR+FL+LFE,
only the channel order would have been wrong, with FR+FL+FR also the 
channel count.


Having the same channel position in the same track is not that 
theoretical, at least for MOV I have samples where an additional FR/FL 
track is used for Music/Effects. I admit, for MP4 that might be less 
common though, and I also admit that using a separate track for it would 
be better. But as we know, nothing is ideal in practice...


Nevertheless, I can change the test to FR+FL+LFE if that is preferred.

Regards,
Marton
___
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] avutil/mem: use C11 aligned_malloc()

2024-02-18 Thread Rémi Denis-Courmont
Le sunnuntaina 18. helmikuuta 2024, 18.16.36 EET James Almer a écrit :
> Save for the Microsoft C Runtime library, where free() can't handle aligned
> buffers, aligned_malloc() should be available and working on all supported
> targets.
> Also, malloc() alone may be sufficient if alignment requirement is low, so
> add a check for it.
> 
> Signed-off-by: James Almer 
> ---
>  configure   |  2 --
>  libavutil/mem.c | 42 ++
>  2 files changed, 6 insertions(+), 38 deletions(-)
> 
> diff --git a/configure b/configure
> index 7c45ac25c8..8fd2895ac2 100755
> --- a/configure
> +++ b/configure
> @@ -6450,8 +6450,6 @@ if test -n "$custom_allocator"; then
>  fi
> 
>  check_func_headers malloc.h _aligned_malloc && enable aligned_malloc
> -check_func  ${malloc_prefix}memalign&& enable memalign
> -check_func  ${malloc_prefix}posix_memalign  && enable posix_memalign
> 
>  check_func  access
>  check_func_headers stdlib.h arc4random_buf
> diff --git a/libavutil/mem.c b/libavutil/mem.c
> index 36b8940a0c..a72981d1ab 100644
> --- a/libavutil/mem.c
> +++ b/libavutil/mem.c
> @@ -100,44 +100,14 @@ void *av_malloc(size_t size)
>  if (size > atomic_load_explicit(_alloc_size, memory_order_relaxed))
> return NULL;
> 
> -#if HAVE_POSIX_MEMALIGN
> -if (size) //OS X on SDK 10.6 has a broken posix_memalign implementation
> -if (posix_memalign(, ALIGN, size))
> -ptr = NULL;
> -#elif HAVE_ALIGNED_MALLOC
> +#if HAVE_ALIGNED_MALLOC
>  ptr = _aligned_malloc(size, ALIGN);
> -#elif HAVE_MEMALIGN
> -#ifndef __DJGPP__
> -ptr = memalign(ALIGN, size);
> -#else
> -ptr = memalign(size, ALIGN);
> -#endif
> -/* Why 64?
> - * Indeed, we should align it:
> - *   on  4 for 386
> - *   on 16 for 486
> - *   on 32 for 586, PPro - K6-III
> - *   on 64 for K7 (maybe for P3 too).
> - * Because L1 and L2 caches are aligned on those values.
> - * But I don't want to code such logic here!
> - */
> -/* Why 32?
> - * For AVX ASM. SSE / NEON needs only 16.
> - * Why not larger? Because I did not see a difference in benchmarks ...
> - */
> -/* benchmarks with P3
> - * memalign(64) + 1  3071, 3051, 3032
> - * memalign(64) + 2  3051, 3032, 3041
> - * memalign(64) + 4  2911, 2896, 2915
> - * memalign(64) + 8  2545, 2554, 2550
> - * memalign(64) + 16 2543, 2572, 2563
> - * memalign(64) + 32 2546, 2545, 2571
> - * memalign(64) + 64 2570, 2533, 2558
> - *
> - * BTW, malloc seems to do 8-byte alignment by default here.
> - */
>  #else
> -ptr = malloc(size);
> +// malloc may already allocate sufficiently aligned buffers
> +if (ALIGN > _Alignof(max_align_t))

If you ever try to reintroduce something like this, you would need 
 here, and thus you should use alignof rather than _Alignof (which 
was already deprecated by C23 deprecated).

> +ptr = aligned_malloc(size, ALIGN);
> +else
> +ptr = malloc(size);
>  #endif
>  if(!ptr && !size) {
>  size = 1;


-- 
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] avutil/mem: use C11 aligned_malloc()

2024-02-18 Thread Rémi Denis-Courmont
Le sunnuntaina 18. helmikuuta 2024, 18.27.35 EET Andreas Rheinhardt a écrit :
> 1. The function is called aligned_alloc (how did you test this?).
> 2. C11: "The value of alignment shall be a valid alignment supported by
> the implementation and the value of size shall be an integral multiple
> of alignment."
> a) To use this, you would have to round size upwards; but this will make
> sanitiziers more lenient.
> b) If ALIGN is just not supported by the implementation, then everything
> is UB in C11.

The letter of the specification is that all alignments of types defined in the 
specification must be supported and other "may" be supported. The intent is 
clearly that all relevant alignments on the target platform are supported.

FFmpeg assumes that alignment 16, 32 and 64 are supported already anyhow, so 
this would not be introducing any *new* UB. In this respect, FFmpeg is doing 
UB on practically all platforms other than x86, which seems to be the only 
platform to need alignment of 32 and 64 bytes for anything.

IMO, FFmpeg should not use custom alignments beyond `alignof(max_align_t)` 
unless they are specifically needed on the given platform, but that's a 
potentially tedious clean-up task with zero practical gains.

> 3. What's the advantage of this patch anyway?

In theory, `aligned_alloc()` (not `aligned_malloc()`) supports alignment of 1 
and any legal value until `sizeof(void*)`, *unlike* `posix_memalign()`. But 
since you can just as well use `malloc()` for that purpose, that is not a real 
advantage.

-- 
レミ・デニ-クールモン
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 1/2] avcodec/s302m: enable non-PCM decoding

2024-02-18 Thread Anton Khirnov
Quoting Michael Niedermayer (2024-02-18 01:43:14)
> "If the disagreement involves a member of the TC"
> does IMHO not preclude commenting on a patch.
> 
> For a disagreement we need 2 parties. For example one party who
> wants a patch in and one who blocks the patch. or 2 parties where both
> block the other.
> 
> Being a party of a disagreement would not make anyones opinon invalid.

Anything that goes to TC is a disagreement. Anyone who expressed an
opinion on the patch then is 'a party to the disagreement'.

> But I think it is reasonable that parties of a disagreement cannot be
> the judge of the disagreement.

Why not? This is one of those truthy-sounding statements that does not
actually hold up to scrutiny.

TC members are not supposed to be impartial judges, because there is no
body of law for us to interpret. TC members are elected for their
opinions. And I see no good reason why those opinions should suddenly
become invalid just because they've been expressed before.

And again, interpreting this rule in this way means that TC members
are incentivized not to review patches. Given that TC members are also
often among the most active contributors, is that really what you want?

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

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


Re: [FFmpeg-devel] [PATCH 1/2] avcodec/s302m: enable non-PCM decoding

2024-02-18 Thread Anton Khirnov
Quoting Gyan Doshi (2024-02-18 05:06:30)
> b) what "maximalist" interpretation?

A non-maximalist interpretation would be that a TC member is only
excluded from voting when they authored the patch that is being
disputed.

> - I think the current patch is fine, you don't. That's a disagreement
> and you're involved in it, so the rule tells you to not be a judge.

Anything handled by the TC is a disagreement. Then according to your
interpretation, any TC member who expressed an opinion on a patch
(either positive or negative) becomes a party to the disagreement and
cannot vote. That is absurd and makes no sense.

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

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


Re: [FFmpeg-devel] [PATCH 1/2] fftools/ffmpeg_mux_init: Fix attachment_filename use-after-free

2024-02-18 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2024-02-18 16:10:06)
> The filename is freed with the OptionsContext and therefore
> there will be a use-after-free when reporting the filename
> in print_stream_maps(). So create a copy of the string.
> 
> This is a regression since 8aed3911fc454e79697e183660bf30d31334a64b.
> fate-lavf-mkv_attachment exhibits it (and reports a random nonsense
> filename here), but this does not make the test fail (not even with
> valgrind; only with ASAN, as it aborts on use-after-free).
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  fftools/ffmpeg.h  |  2 +-
>  fftools/ffmpeg_mux.c  |  2 ++
>  fftools/ffmpeg_mux_init.c | 10 +-
>  3 files changed, 12 insertions(+), 2 deletions(-)

Ok

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

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


Re: [FFmpeg-devel] [PATCH] avutil/mem: use C11 aligned_malloc()

2024-02-18 Thread James Almer

On 2/18/2024 1:27 PM, Andreas Rheinhardt wrote:

James Almer:

Save for the Microsoft C Runtime library, where free() can't handle aligned
buffers, aligned_malloc() should be available and working on all supported
targets.
Also, malloc() alone may be sufficient if alignment requirement is low, so add
a check for it.

Signed-off-by: James Almer 
---
  configure   |  2 --
  libavutil/mem.c | 42 ++
  2 files changed, 6 insertions(+), 38 deletions(-)

diff --git a/configure b/configure
index 7c45ac25c8..8fd2895ac2 100755
--- a/configure
+++ b/configure
@@ -6450,8 +6450,6 @@ if test -n "$custom_allocator"; then
  fi
  
  check_func_headers malloc.h _aligned_malloc && enable aligned_malloc

-check_func  ${malloc_prefix}memalign&& enable memalign
-check_func  ${malloc_prefix}posix_memalign  && enable posix_memalign
  
  check_func  access

  check_func_headers stdlib.h arc4random_buf
diff --git a/libavutil/mem.c b/libavutil/mem.c
index 36b8940a0c..a72981d1ab 100644
--- a/libavutil/mem.c
+++ b/libavutil/mem.c
@@ -100,44 +100,14 @@ void *av_malloc(size_t size)
  if (size > atomic_load_explicit(_alloc_size, memory_order_relaxed))
  return NULL;
  
-#if HAVE_POSIX_MEMALIGN

-if (size) //OS X on SDK 10.6 has a broken posix_memalign implementation
-if (posix_memalign(, ALIGN, size))
-ptr = NULL;
-#elif HAVE_ALIGNED_MALLOC
+#if HAVE_ALIGNED_MALLOC
  ptr = _aligned_malloc(size, ALIGN);
-#elif HAVE_MEMALIGN
-#ifndef __DJGPP__
-ptr = memalign(ALIGN, size);
-#else
-ptr = memalign(size, ALIGN);
-#endif
-/* Why 64?
- * Indeed, we should align it:
- *   on  4 for 386
- *   on 16 for 486
- *   on 32 for 586, PPro - K6-III
- *   on 64 for K7 (maybe for P3 too).
- * Because L1 and L2 caches are aligned on those values.
- * But I don't want to code such logic here!
- */
-/* Why 32?
- * For AVX ASM. SSE / NEON needs only 16.
- * Why not larger? Because I did not see a difference in benchmarks ...
- */
-/* benchmarks with P3
- * memalign(64) + 1  3071, 3051, 3032
- * memalign(64) + 2  3051, 3032, 3041
- * memalign(64) + 4  2911, 2896, 2915
- * memalign(64) + 8  2545, 2554, 2550
- * memalign(64) + 16 2543, 2572, 2563
- * memalign(64) + 32 2546, 2545, 2571
- * memalign(64) + 64 2570, 2533, 2558
- *
- * BTW, malloc seems to do 8-byte alignment by default here.
- */
  #else
-ptr = malloc(size);
+// malloc may already allocate sufficiently aligned buffers
+if (ALIGN > _Alignof(max_align_t))
+ptr = aligned_malloc(size, ALIGN);
+else
+ptr = malloc(size);
  #endif
  if(!ptr && !size) {
  size = 1;


1. The function is called aligned_alloc (how did you test this?).


By compiling on a target with _aligned_malloc(), which hid my mistake.


2. C11: "The value of alignment shall be a valid alignment supported by
the implementation and the value of size shall be an integral multiple
of alignment."


Well, that sure is not very useful...


a) To use this, you would have to round size upwards; but this will make
sanitiziers more lenient.
b) If ALIGN is just not supported by the implementation, then everything
is UB in C11.
3. What's the advantage of this patch anyway?


Removing all the different target specific allocation functions in favor 
of the standard one. But your second point makes it moot, so patch 
withdrawn.




- Andreas

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

To unsubscribe, visit link above, or email
ffmpeg-devel-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] avutil/mem: use C11 aligned_malloc()

2024-02-18 Thread Andreas Rheinhardt
James Almer:
> Save for the Microsoft C Runtime library, where free() can't handle aligned
> buffers, aligned_malloc() should be available and working on all supported
> targets.
> Also, malloc() alone may be sufficient if alignment requirement is low, so add
> a check for it.
> 
> Signed-off-by: James Almer 
> ---
>  configure   |  2 --
>  libavutil/mem.c | 42 ++
>  2 files changed, 6 insertions(+), 38 deletions(-)
> 
> diff --git a/configure b/configure
> index 7c45ac25c8..8fd2895ac2 100755
> --- a/configure
> +++ b/configure
> @@ -6450,8 +6450,6 @@ if test -n "$custom_allocator"; then
>  fi
>  
>  check_func_headers malloc.h _aligned_malloc && enable aligned_malloc
> -check_func  ${malloc_prefix}memalign&& enable memalign
> -check_func  ${malloc_prefix}posix_memalign  && enable posix_memalign
>  
>  check_func  access
>  check_func_headers stdlib.h arc4random_buf
> diff --git a/libavutil/mem.c b/libavutil/mem.c
> index 36b8940a0c..a72981d1ab 100644
> --- a/libavutil/mem.c
> +++ b/libavutil/mem.c
> @@ -100,44 +100,14 @@ void *av_malloc(size_t size)
>  if (size > atomic_load_explicit(_alloc_size, memory_order_relaxed))
>  return NULL;
>  
> -#if HAVE_POSIX_MEMALIGN
> -if (size) //OS X on SDK 10.6 has a broken posix_memalign implementation
> -if (posix_memalign(, ALIGN, size))
> -ptr = NULL;
> -#elif HAVE_ALIGNED_MALLOC
> +#if HAVE_ALIGNED_MALLOC
>  ptr = _aligned_malloc(size, ALIGN);
> -#elif HAVE_MEMALIGN
> -#ifndef __DJGPP__
> -ptr = memalign(ALIGN, size);
> -#else
> -ptr = memalign(size, ALIGN);
> -#endif
> -/* Why 64?
> - * Indeed, we should align it:
> - *   on  4 for 386
> - *   on 16 for 486
> - *   on 32 for 586, PPro - K6-III
> - *   on 64 for K7 (maybe for P3 too).
> - * Because L1 and L2 caches are aligned on those values.
> - * But I don't want to code such logic here!
> - */
> -/* Why 32?
> - * For AVX ASM. SSE / NEON needs only 16.
> - * Why not larger? Because I did not see a difference in benchmarks ...
> - */
> -/* benchmarks with P3
> - * memalign(64) + 1  3071, 3051, 3032
> - * memalign(64) + 2  3051, 3032, 3041
> - * memalign(64) + 4  2911, 2896, 2915
> - * memalign(64) + 8  2545, 2554, 2550
> - * memalign(64) + 16 2543, 2572, 2563
> - * memalign(64) + 32 2546, 2545, 2571
> - * memalign(64) + 64 2570, 2533, 2558
> - *
> - * BTW, malloc seems to do 8-byte alignment by default here.
> - */
>  #else
> -ptr = malloc(size);
> +// malloc may already allocate sufficiently aligned buffers
> +if (ALIGN > _Alignof(max_align_t))
> +ptr = aligned_malloc(size, ALIGN);
> +else
> +ptr = malloc(size);
>  #endif
>  if(!ptr && !size) {
>  size = 1;

1. The function is called aligned_alloc (how did you test this?).
2. C11: "The value of alignment shall be a valid alignment supported by
the implementation and the value of size shall be an integral multiple
of alignment."
a) To use this, you would have to round size upwards; but this will make
sanitiziers more lenient.
b) If ALIGN is just not supported by the implementation, then everything
is UB in C11.
3. What's the advantage of this patch anyway?

- Andreas

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

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


[FFmpeg-devel] [PATCH] avutil/mem: use C11 aligned_malloc()

2024-02-18 Thread James Almer
Save for the Microsoft C Runtime library, where free() can't handle aligned
buffers, aligned_malloc() should be available and working on all supported
targets.
Also, malloc() alone may be sufficient if alignment requirement is low, so add
a check for it.

Signed-off-by: James Almer 
---
 configure   |  2 --
 libavutil/mem.c | 42 ++
 2 files changed, 6 insertions(+), 38 deletions(-)

diff --git a/configure b/configure
index 7c45ac25c8..8fd2895ac2 100755
--- a/configure
+++ b/configure
@@ -6450,8 +6450,6 @@ if test -n "$custom_allocator"; then
 fi
 
 check_func_headers malloc.h _aligned_malloc && enable aligned_malloc
-check_func  ${malloc_prefix}memalign&& enable memalign
-check_func  ${malloc_prefix}posix_memalign  && enable posix_memalign
 
 check_func  access
 check_func_headers stdlib.h arc4random_buf
diff --git a/libavutil/mem.c b/libavutil/mem.c
index 36b8940a0c..a72981d1ab 100644
--- a/libavutil/mem.c
+++ b/libavutil/mem.c
@@ -100,44 +100,14 @@ void *av_malloc(size_t size)
 if (size > atomic_load_explicit(_alloc_size, memory_order_relaxed))
 return NULL;
 
-#if HAVE_POSIX_MEMALIGN
-if (size) //OS X on SDK 10.6 has a broken posix_memalign implementation
-if (posix_memalign(, ALIGN, size))
-ptr = NULL;
-#elif HAVE_ALIGNED_MALLOC
+#if HAVE_ALIGNED_MALLOC
 ptr = _aligned_malloc(size, ALIGN);
-#elif HAVE_MEMALIGN
-#ifndef __DJGPP__
-ptr = memalign(ALIGN, size);
-#else
-ptr = memalign(size, ALIGN);
-#endif
-/* Why 64?
- * Indeed, we should align it:
- *   on  4 for 386
- *   on 16 for 486
- *   on 32 for 586, PPro - K6-III
- *   on 64 for K7 (maybe for P3 too).
- * Because L1 and L2 caches are aligned on those values.
- * But I don't want to code such logic here!
- */
-/* Why 32?
- * For AVX ASM. SSE / NEON needs only 16.
- * Why not larger? Because I did not see a difference in benchmarks ...
- */
-/* benchmarks with P3
- * memalign(64) + 1  3071, 3051, 3032
- * memalign(64) + 2  3051, 3032, 3041
- * memalign(64) + 4  2911, 2896, 2915
- * memalign(64) + 8  2545, 2554, 2550
- * memalign(64) + 16 2543, 2572, 2563
- * memalign(64) + 32 2546, 2545, 2571
- * memalign(64) + 64 2570, 2533, 2558
- *
- * BTW, malloc seems to do 8-byte alignment by default here.
- */
 #else
-ptr = malloc(size);
+// malloc may already allocate sufficiently aligned buffers
+if (ALIGN > _Alignof(max_align_t))
+ptr = aligned_malloc(size, ALIGN);
+else
+ptr = malloc(size);
 #endif
 if(!ptr && !size) {
 size = 1;
-- 
2.43.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-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH] lavu/tx: correctly use a default scale parameter for all transform types

2024-02-18 Thread Lynne
Feb 17, 2024, 20:53 by d...@lynne.ee:

> This fixes the previous commit and adds more cases (DCT-I and DST-I).
>
> I am holding off on defining a scale parameter for FFTs as I'd like
> to use a complex value for them.
>
> Patch attached.
>

Pushed mine, which also added D(C/S)Ts.
___
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 2/2] tests/fate-run: Do not ignore errors from intermediate commands

2024-02-18 Thread Andreas Rheinhardt
Otherwise the test may pass while ignoring errors from sanitizers.

Signed-off-by: Andreas Rheinhardt 
---
 tests/fate-run.sh | 44 
 1 file changed, 28 insertions(+), 16 deletions(-)

diff --git a/tests/fate-run.sh b/tests/fate-run.sh
index 9257fb368b..4b85fbf464 100755
--- a/tests/fate-run.sh
+++ b/tests/fate-run.sh
@@ -334,7 +334,7 @@ do_avconv(){
 f="$1"
 shift
 set -- $* ${target_path}/$f
-run_avconv $*
+run_avconv $* || return
 do_md5sum $f
 echo $(wc -c $f)
 }
@@ -351,7 +351,8 @@ lavf_audio(){
 outdir="tests/data/lavf"
 file=${outdir}/lavf.$t
 test "$keep" -ge 1 || cleanfiles="$cleanfiles $file"
-do_avconv $file -auto_conversion_filters $DEC_OPTS $1 -ar 44100 -f s16le 
-i $pcm_src "$ENC_OPTS -metadata title=lavftest" -t 1 -qscale 10 $2
+do_avconv $file -auto_conversion_filters $DEC_OPTS $1 -ar 44100 -f s16le 
-i $pcm_src \
+  "$ENC_OPTS -metadata title=lavftest" -t 1 -qscale 10 $2 || return
 test "$4" = "disable_crc" ||
 do_avconv_crc $file -auto_conversion_filters $DEC_OPTS $3 -i 
$target_path/$file
 }
@@ -361,7 +362,8 @@ lavf_container(){
 outdir="tests/data/lavf"
 file=${outdir}/lavf.$t
 test "$keep" -ge 1 || cleanfiles="$cleanfiles $file"
-do_avconv $file -auto_conversion_filters $DEC_OPTS -f image2 -c:v pgmyuv 
-i $raw_src $DEC_OPTS -ar 44100 -f s16le $1 -i $pcm_src "$ENC_OPTS -metadata 
title=lavftest" -b:a 64k -t 1 -qscale:v 10 $2
+do_avconv $file -auto_conversion_filters $DEC_OPTS -f image2 -c:v pgmyuv 
-i $raw_src $DEC_OPTS \
+  -ar 44100 -f s16le $1 -i $pcm_src "$ENC_OPTS -metadata 
title=lavftest" -b:a 64k -t 1 -qscale:v 10 $2 || return
 test "$3" = "disable_crc" ||
 do_avconv_crc $file -auto_conversion_filters $DEC_OPTS -i 
$target_path/$file $3
 }
@@ -384,7 +386,8 @@ lavf_container_fate()
 file=${outdir}/lavf.$t
 cleanfiles="$cleanfiles $file"
 input="${target_samples}/$1"
-do_avconv $file -auto_conversion_filters $DEC_OPTS $2 -i "$input" 
"$ENC_OPTS -metadata title=lavftest" -vcodec copy -acodec copy
+do_avconv $file -auto_conversion_filters $DEC_OPTS $2 -i "$input" \
+  "$ENC_OPTS -metadata title=lavftest" -vcodec copy -acodec copy 
|| return
 do_avconv_crc $file -auto_conversion_filters $DEC_OPTS -i 
$target_path/$file $3
 }
 
@@ -401,7 +404,9 @@ lavf_image(){
 cleanfiles="$cleanfiles $filename"
 done
 fi
-run_avconv $DEC_OPTS -f image2 -c:v pgmyuv -i $raw_src $1 "$ENC_OPTS 
-metadata title=lavftest" -vf scale -frames $nb_frames -y -qscale 10 
$target_path/$file
+run_avconv $DEC_OPTS -f image2 -c:v pgmyuv -i $raw_src $1 \
+  "$ENC_OPTS -metadata title=lavftest" -vf scale -frames 
$nb_frames \
+  -y -qscale 10 $target_path/$file || return
 if [ -z "$no_file_checksums" ]; then
 do_md5sum ${outdir}/02.$t
 echo $(wc -c ${outdir}/02.$t)
@@ -414,7 +419,8 @@ lavf_image2pipe(){
 t="${t%pipe}"
 outdir="tests/data/lavf"
 file=${outdir}/${t}pipe.$t
-do_avconv $file -auto_conversion_filters $DEC_OPTS -f image2 -c:v pgmyuv 
-i $raw_src -f image2pipe "$ENC_OPTS -metadata title=lavftest" -t 1 -qscale 10
+do_avconv $file -auto_conversion_filters $DEC_OPTS -f image2 -c:v pgmyuv 
-i $raw_src \
+  -f image2pipe "$ENC_OPTS -metadata title=lavftest" -t 1 -qscale 
10 || return
 do_avconv_crc $file -auto_conversion_filters $DEC_OPTS -f image2pipe -i 
$target_path/$file
 }
 
@@ -423,7 +429,8 @@ lavf_video(){
 outdir="tests/data/lavf"
 file=${outdir}/lavf.$t
 test "$keep" -ge 1 || cleanfiles="$cleanfiles $file"
-do_avconv $file -auto_conversion_filters $DEC_OPTS -f image2 -c:v pgmyuv 
-i $raw_src "$ENC_OPTS -metadata title=lavftest" -t 1 -qscale 10 $1 $2
+do_avconv $file -auto_conversion_filters $DEC_OPTS -f image2 -c:v pgmyuv 
-i $raw_src \
+  "$ENC_OPTS -metadata title=lavftest" -t 1 -qscale 10 $1 $2 || 
return
 do_avconv_crc $file -auto_conversion_filters $DEC_OPTS -i 
$target_path/$file $1
 }
 
@@ -477,7 +484,7 @@ pixfmt_conversion(){
 file=${outdir}/${conversion}.yuv
 cleanfiles="$cleanfiles $raw_dst $file"
 run_avconv $DEC_OPTS -r 1 -f image2 -c:v pgmyuv -i $raw_src \
-   $ENC_OPTS -f rawvideo -t 1 -s 352x288 -pix_fmt $conversion 
$target_path/$raw_dst
+   $ENC_OPTS -f rawvideo -t 1 -s 352x288 -pix_fmt $conversion 
$target_path/$raw_dst || return
 do_avconv $file $DEC_OPTS -f rawvideo -s 352x288 -pix_fmt $conversion -i 
$target_path/$raw_dst \
   $ENC_OPTS -f rawvideo -s 352x288 -pix_fmt yuv444p
 }
@@ -516,7 +523,7 @@ pixfmts(){
 outertest=$test
 for pix_fmt in $pix_fmts; do
 test=$pix_fmt
-video_filter 
"${prefilter_chain}scale,format=$pix_fmt,$filter=$filter_args" -pix_fmt 
$pix_fmt -frames:v $nframes
+video_filter 
"${prefilter_chain}scale,format=$pix_fmt,$filter=$filter_args" 

[FFmpeg-devel] [PATCH 1/2] fftools/ffmpeg_mux_init: Fix attachment_filename use-after-free

2024-02-18 Thread Andreas Rheinhardt
The filename is freed with the OptionsContext and therefore
there will be a use-after-free when reporting the filename
in print_stream_maps(). So create a copy of the string.

This is a regression since 8aed3911fc454e79697e183660bf30d31334a64b.
fate-lavf-mkv_attachment exhibits it (and reports a random nonsense
filename here), but this does not make the test fail (not even with
valgrind; only with ASAN, as it aborts on use-after-free).

Signed-off-by: Andreas Rheinhardt 
---
 fftools/ffmpeg.h  |  2 +-
 fftools/ffmpeg_mux.c  |  2 ++
 fftools/ffmpeg_mux_init.c | 10 +-
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index 33750e0bb3..c394f60962 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -555,7 +555,7 @@ typedef struct OutputStream {
 AVDictionary *swr_opts;
 char *apad;
 
-const char *attachment_filename;
+char *attachment_filename;
 
 int keep_pix_fmt;
 
diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
index e65fe89992..5a648c0568 100644
--- a/fftools/ffmpeg_mux.c
+++ b/fftools/ffmpeg_mux.c
@@ -817,6 +817,8 @@ static void ost_free(OutputStream **post)
 av_freep(>logfile_prefix);
 av_freep(>apad);
 
+av_freep(>attachment_filename);
+
 #if FFMPEG_OPT_MAP_CHANNEL
 av_freep(>audio_channels_map);
 ost->audio_channels_mapped = 0;
diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
index 0718487c53..1abbb2d945 100644
--- a/fftools/ffmpeg_mux_init.c
+++ b/fftools/ffmpeg_mux_init.c
@@ -1741,6 +1741,7 @@ static int of_add_attachments(Muxer *mux, const 
OptionsContext *o)
 for (int i = 0; i < o->nb_attachments; i++) {
 AVIOContext *pb;
 uint8_t *attachment;
+char *attachment_filename;
 const char *p;
 int64_t len;
 
@@ -1788,13 +1789,20 @@ read_fail:
 av_log(mux, AV_LOG_VERBOSE, "Creating attachment stream from file 
%s\n",
o->attachments[i]);
 
+attachment_filename = av_strdup(o->attachments[i]);
+if (!attachment_filename) {
+av_free(attachment);
+return AVERROR(ENOMEM);
+}
+
 err = ost_add(mux, o, AVMEDIA_TYPE_ATTACHMENT, NULL, NULL, );
 if (err < 0) {
+av_free(attachment_filename);
 av_freep();
 return err;
 }
 
-ost->attachment_filename   = o->attachments[i];
+ost->attachment_filename   = attachment_filename;
 ost->par_in->extradata = attachment;
 ost->par_in->extradata_size= len;
 
-- 
2.34.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-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH] avutil/tests/pixelutils: Remove dead code

2024-02-18 Thread Andreas Rheinhardt
Forgotten in e6b125e3be19fb341fd9a759ad0af3b39ba35e0c.

Signed-off-by: Andreas Rheinhardt 
---
 libavutil/tests/pixelutils.c | 8 
 1 file changed, 8 deletions(-)

diff --git a/libavutil/tests/pixelutils.c b/libavutil/tests/pixelutils.c
index f5ddeb329d..828a7580d2 100644
--- a/libavutil/tests/pixelutils.c
+++ b/libavutil/tests/pixelutils.c
@@ -109,14 +109,6 @@ static int run_test(const char *test,
 int i, a, ret = 0;
 
 for (a = 0; a < 3; a++) {
-const uint8_t *block1 = b1;
-const uint8_t *block2 = b2;
-
-switch (a) {
-case 0: block1++; block2++; break;
-case 1:   block2++; break;
-case 2: break;
-}
 for (i = 1; i <= FF_ARRAY_ELEMS(sad_c); i++) {
 int r = run_single_test(test, b1, W1, b2, W2, a, i);
 if (r)
-- 
2.34.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-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH] avutil/channel_layout: print known layout names in custom layout

2024-02-18 Thread James Almer

On 2/13/2024 2:39 PM, James Almer wrote:

If a custom layout is equivalent to a native one, check if it matches one of the
known layout names and print that instead.

Signed-off-by: James Almer 
---
Should be applied after the patch this one is a reply to.

  libavutil/channel_layout.c| 68 +--
  tests/ref/fate/channel_layout |  4 +--
  2 files changed, 44 insertions(+), 28 deletions(-)


Will apply.
___
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: use an even more exotic channel layout mov-mp4-pcm-float test

2024-02-18 Thread James Almer

On 2/18/2024 7:45 AM, Marton Balint wrote:

The old layout happened to be a native layout and therefore missed some
recently fixed layout parsing bugs.

Signed-off-by: Marton Balint 
---
  tests/fate/mov.mak   | 2 +-
  tests/ref/fate/mov-mp4-pcm-float | 4 ++--
  2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak
index 4850c8aa94..8d154c8b5b 100644
--- a/tests/fate/mov.mak
+++ b/tests/fate/mov.mak
@@ -187,7 +187,7 @@ fate-mov-mp4-pcm: CMD = transcode wav 
$(TARGET_PATH)/tests/data/asynth-44100-1.w
  FATE_MOV_FFMPEG-$(call TRANSCODE, PCM_S16LE, MOV, WAV_DEMUXER PAN_FILTER) \
+= fate-mov-mp4-pcm-float
  fate-mov-mp4-pcm-float: tests/data/asynth-44100-1.wav
-fate-mov-mp4-pcm-float: CMD = transcode wav $(TARGET_PATH)/tests/data/asynth-44100-1.wav mp4 
"-af aresample,pan=FL+LFE+BR|c0=c0|c1=c0|c2=c0 -c:a pcm_f32le" "-map 0 -c copy 
-frames:a 0"
+fate-mov-mp4-pcm-float: CMD = transcode wav $(TARGET_PATH)/tests/data/asynth-44100-1.wav mp4 
"-af aresample,pan=FR+FL+FR|c0=c0|c1=c0|c2=c0 -c:a pcm_f32le" "-map 0 -c copy 
-frames:a 0"


Wouldn't FR+FL+LFE be enough to test this? While also generating a file 
that's realistic.


  
  fate-mov-pcm-remux: tests/data/asynth-44100-1.wav

  fate-mov-pcm-remux: CMD = md5 -i $(TARGET_PATH)/tests/data/asynth-44100-1.wav 
-map 0 -c copy -fflags +bitexact -f mp4
diff --git a/tests/ref/fate/mov-mp4-pcm-float b/tests/ref/fate/mov-mp4-pcm-float
index 851b79090c..7da8fd2aba 100644
--- a/tests/ref/fate/mov-mp4-pcm-float
+++ b/tests/ref/fate/mov-mp4-pcm-float
@@ -1,7 +1,7 @@
-691a76a847e0f3720c09cca341971f19 *tests/data/fate/mov-mp4-pcm-float.mp4
+7b998e652d5b7154e646a98bd2bf28a1 *tests/data/fate/mov-mp4-pcm-float.mp4
  3175929 tests/data/fate/mov-mp4-pcm-float.mp4
  #tb 0: 1/44100
  #media_type 0: audio
  #codec_id 0: pcm_f32le
  #sample_rate 0: 44100
-#channel_layout_name 0: 3 channels (FL+LFE+BR)
+#channel_layout_name 0: 3 channels (FR+FL+FR)

___
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 1/5] avcodec/bsf/(hevc|vvc)_mp4toannexb: Ensure extradata_size < INT_MAX

2024-02-18 Thread Andreas Rheinhardt
James Almer:
> On 2/17/2024 11:41 PM, Andreas Rheinhardt wrote:
>> AVCodecParameters.extradata_size is an int.
>>
>> Signed-off-by: Andreas Rheinhardt 
>> ---
>>   libavcodec/bsf/hevc_mp4toannexb.c | 2 +-
>>   libavcodec/bsf/vvc_mp4toannexb.c  | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavcodec/bsf/hevc_mp4toannexb.c
>> b/libavcodec/bsf/hevc_mp4toannexb.c
>> index 8eec18f31e..c0df2b79a6 100644
>> --- a/libavcodec/bsf/hevc_mp4toannexb.c
>> +++ b/libavcodec/bsf/hevc_mp4toannexb.c
>> @@ -69,7 +69,7 @@ static int hevc_extradata_to_annexb(AVBSFContext *ctx)
>>     if (!nalu_len ||
>>   nalu_len > bytestream2_get_bytes_left() ||
>> -    4 + AV_INPUT_BUFFER_PADDING_SIZE + nalu_len >
>> SIZE_MAX - new_extradata_size) {
>> +    4 + nalu_len > FFMIN(INT_MAX, SIZE_MAX) -
>> AV_INPUT_BUFFER_PADDING_SIZE - new_extradata_size) {
>>   ret = AVERROR_INVALIDDATA;
>>   goto fail;
>>   }
>> diff --git a/libavcodec/bsf/vvc_mp4toannexb.c
>> b/libavcodec/bsf/vvc_mp4toannexb.c
>> index 36bdae8f49..1b851f3223 100644
>> --- a/libavcodec/bsf/vvc_mp4toannexb.c
>> +++ b/libavcodec/bsf/vvc_mp4toannexb.c
>> @@ -159,7 +159,7 @@ static int vvc_extradata_to_annexb(AVBSFContext *ctx)
>>     if (!nalu_len ||
>>   nalu_len > bytestream2_get_bytes_left() ||
>> -    4 + AV_INPUT_BUFFER_PADDING_SIZE + nalu_len >
>> SIZE_MAX - new_extradata_size) {
>> +    4 + nalu_len > FFMIN(INT_MAX, SIZE_MAX) -
>> AV_INPUT_BUFFER_PADDING_SIZE - new_extradata_size) {
> 
> Just use INT_MAX, there's no point in this check. Do you expect a system
> where an int is smaller than the type meant to store size of buffers in
> memory?
> 

We typically try to avoid such assumptions (although we actually have
lots of INT_MAX <= SIZE_MAX assumptions in the codebase, namely lots of
instances where one uses an int and an allocation function).
In this case using FFMIN(INT_MAX, SIZE_MAX) will make it easier to find
these lines via git grep when extradata_size is switched to size_t.
Alternatively, one could add an AV_CODECPAR_MAX_EXTRADATA_SIZE define,
but this is even longer than FFMIN(INT_MAX, SIZE_MAX).

- Andreas

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

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


Re: [FFmpeg-devel] Subject: [PATCH 3/3] lavc/dnxhdenc: R-V V get_pixels_8x4_sym

2024-02-18 Thread flow gg
ping

flow gg  于2024年1月30日周二 00:22写道:

> > I expect that it would be faster to make one large load, and then 4 small
> > stores, but that might work only for exactly 128-bit vectors?
>
> This seems to require vle128, so I didn't modify it.
>
> > That's not needed. You can use immediate values.
> > You can reorder to avoid immediate data dependencies on the addresses.
> > In any case, you need to check the vector length in init.
>
> Okay, I've updated it in the reply.
>
> Rémi Denis-Courmont  于2024年1月29日周一 23:41写道:
>
>> Hi,
>>
>> +/*
>> + * Copyright (c) 2023 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 "libavutil/riscv/asm.S"
>> +
>> +func ff_get_pixels_8x4_sym_rvv, zve64x
>> +vsetivlizero, 8, e8, mf2, ta, ma
>> +vlse64.vv16, (a1), a2
>> +li  t0, 8 * 8
>> +vsetvli zero, t0, e16, m4, ta, ma
>> +vzext.vf2   v8, v16
>> +vse16.v v8, (a0)
>> +li  a2, 8*2
>>
>> That's not needed. You can use immediate values.
>>
>> +vsetivlizero, 2, e8, mf8, ta, ma
>> +addia1, a0, 48
>> +addia0, a0, 32*2
>> +vle64.v v0, (a1)
>> +vse64.v v0, (a0)
>> +sub a1, a1, a2
>> +vle64.v v0, (a1)
>> +add a0, a0, a2
>> +vse64.v v0, (a0)
>> +sub a1, a1, a2
>> +vle64.v v0, (a1)
>> +add a0, a0, a2
>> +vse64.v v0, (a0)
>> +sub a1, a1, a2
>> +vle64.v v0, (a1)
>> +add a0, a0, a2
>> +vse64.v v0, (a0)
>>
>> You can reorder to avoid immediate data dependencies on the addresses.
>>
>> I expect that it would be faster to make one large load, and then 4 small
>> stores, but that might work only for exactly 128-bit vectors?
>>
>> In any case, you need to check the vector length in init.
>>
>> +
>> +ret
>> +endfunc
>>
>> --
>> 雷米‧德尼-库尔蒙
>> 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".
>>
>
___
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 v2] {avcodec, tests}: rename the bundled Mesa AV1 vulkan video headers

2024-02-18 Thread Jan Ekström
On Thu, Feb 15, 2024 at 9:21 PM Lynne  wrote:
>
> Feb 15, 2024, 20:18 by jee...@gmail.com:
>
> > This together with adjusting the inclusion define allows for the
> > build to not fail with latest Vulkan-Headers that contain the
> > stabilized Vulkan AV1 decoding definitions.
> >
> > Compilation fails currently as the AV1 header is getting included
> > via hwcontext_vulkan.h ->  -> vulkan_core.h, which
> > finally includes vk_video/vulkan_video_codec_av1std.h and the decode
> > header, leading to the bundled header to never defining anything
> > due to the inclusion define being the same.
> >
> > This fix is imperfect, as it leads to additional re-definition
> > warnings for things such as
> > VK_STD_VULKAN_VIDEO_CODEC_AV1_DECODE_SPEC_VERSION. , but it is
> > not clear how to otherwise have the bundled version trump the
> > actually standardized one for a short-term compilation fix.
> > ---
> >
>
> LGTM

Thanks, applied to master as e06ce6d2b45edac4a2df04f304e18d4727417d24
, as this takes care of the current build failure with latest vulkan
headers.

If there are no objections or issues to this way of handling the
issue, I will later cherry-pick this to release/6.1 , which also
contains these files.

Jan
___
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: use an even more exotic channel layout mov-mp4-pcm-float test

2024-02-18 Thread Zhao Zhili

> 在 2024年2月18日,下午6:45,Marton Balint  写道:
> 
> The old layout happened to be a native layout and therefore missed some
> recently fixed layout parsing bugs.
> 
> Signed-off-by: Marton Balint 
> ---
> tests/fate/mov.mak   | 2 +-
> tests/ref/fate/mov-mp4-pcm-float | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak
> index 4850c8aa94..8d154c8b5b 100644
> --- a/tests/fate/mov.mak
> +++ b/tests/fate/mov.mak
> @@ -187,7 +187,7 @@ fate-mov-mp4-pcm: CMD = transcode wav 
> $(TARGET_PATH)/tests/data/asynth-44100-1.w
> FATE_MOV_FFMPEG-$(call TRANSCODE, PCM_S16LE, MOV, WAV_DEMUXER PAN_FILTER) \
>   += fate-mov-mp4-pcm-float
> fate-mov-mp4-pcm-float: tests/data/asynth-44100-1.wav
> -fate-mov-mp4-pcm-float: CMD = transcode wav 
> $(TARGET_PATH)/tests/data/asynth-44100-1.wav mp4 "-af 
> aresample,pan=FL+LFE+BR|c0=c0|c1=c0|c2=c0 -c:a pcm_f32le" "-map 0 -c copy 
> -frames:a 0"
> +fate-mov-mp4-pcm-float: CMD = transcode wav 
> $(TARGET_PATH)/tests/data/asynth-44100-1.wav mp4 "-af 
> aresample,pan=FR+FL+FR|c0=c0|c1=c0|c2=c0 -c:a pcm_f32le" "-map 0 -c copy 
> -frames:a 0"
> 
> fate-mov-pcm-remux: tests/data/asynth-44100-1.wav
> fate-mov-pcm-remux: CMD = md5 -i $(TARGET_PATH)/tests/data/asynth-44100-1.wav 
> -map 0 -c copy -fflags +bitexact -f mp4
> diff --git a/tests/ref/fate/mov-mp4-pcm-float 
> b/tests/ref/fate/mov-mp4-pcm-float
> index 851b79090c..7da8fd2aba 100644
> --- a/tests/ref/fate/mov-mp4-pcm-float
> +++ b/tests/ref/fate/mov-mp4-pcm-float
> @@ -1,7 +1,7 @@
> -691a76a847e0f3720c09cca341971f19 *tests/data/fate/mov-mp4-pcm-float.mp4
> +7b998e652d5b7154e646a98bd2bf28a1 *tests/data/fate/mov-mp4-pcm-float.mp4
> 3175929 tests/data/fate/mov-mp4-pcm-float.mp4
> #tb 0: 1/44100
> #media_type 0: audio
> #codec_id 0: pcm_f32le
> #sample_rate 0: 44100
> -#channel_layout_name 0: 3 channels (FL+LFE+BR)
> +#channel_layout_name 0: 3 channels (FR+FL+FR)

It’s possible to create such file manually, however, I’m wondering how it works 
physically with two speakers at the same position (FR)?

> --
> 2.35.3
> 
> ___
> 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] fate: use an even more exotic channel layout mov-mp4-pcm-float test

2024-02-18 Thread Marton Balint
The old layout happened to be a native layout and therefore missed some
recently fixed layout parsing bugs.

Signed-off-by: Marton Balint 
---
 tests/fate/mov.mak   | 2 +-
 tests/ref/fate/mov-mp4-pcm-float | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak
index 4850c8aa94..8d154c8b5b 100644
--- a/tests/fate/mov.mak
+++ b/tests/fate/mov.mak
@@ -187,7 +187,7 @@ fate-mov-mp4-pcm: CMD = transcode wav 
$(TARGET_PATH)/tests/data/asynth-44100-1.w
 FATE_MOV_FFMPEG-$(call TRANSCODE, PCM_S16LE, MOV, WAV_DEMUXER PAN_FILTER) \
   += fate-mov-mp4-pcm-float
 fate-mov-mp4-pcm-float: tests/data/asynth-44100-1.wav
-fate-mov-mp4-pcm-float: CMD = transcode wav 
$(TARGET_PATH)/tests/data/asynth-44100-1.wav mp4 "-af 
aresample,pan=FL+LFE+BR|c0=c0|c1=c0|c2=c0 -c:a pcm_f32le" "-map 0 -c copy 
-frames:a 0"
+fate-mov-mp4-pcm-float: CMD = transcode wav 
$(TARGET_PATH)/tests/data/asynth-44100-1.wav mp4 "-af 
aresample,pan=FR+FL+FR|c0=c0|c1=c0|c2=c0 -c:a pcm_f32le" "-map 0 -c copy 
-frames:a 0"
 
 fate-mov-pcm-remux: tests/data/asynth-44100-1.wav
 fate-mov-pcm-remux: CMD = md5 -i $(TARGET_PATH)/tests/data/asynth-44100-1.wav 
-map 0 -c copy -fflags +bitexact -f mp4
diff --git a/tests/ref/fate/mov-mp4-pcm-float b/tests/ref/fate/mov-mp4-pcm-float
index 851b79090c..7da8fd2aba 100644
--- a/tests/ref/fate/mov-mp4-pcm-float
+++ b/tests/ref/fate/mov-mp4-pcm-float
@@ -1,7 +1,7 @@
-691a76a847e0f3720c09cca341971f19 *tests/data/fate/mov-mp4-pcm-float.mp4
+7b998e652d5b7154e646a98bd2bf28a1 *tests/data/fate/mov-mp4-pcm-float.mp4
 3175929 tests/data/fate/mov-mp4-pcm-float.mp4
 #tb 0: 1/44100
 #media_type 0: audio
 #codec_id 0: pcm_f32le
 #sample_rate 0: 44100
-#channel_layout_name 0: 3 channels (FL+LFE+BR)
+#channel_layout_name 0: 3 channels (FR+FL+FR)
-- 
2.35.3

___
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 v5 8/9] avcodec: add D3D12VA hardware HEVC encoder

2024-02-18 Thread tong1 . wu-at-intel . com
From: Tong Wu 

This implementation is based on D3D12 Video Encoding Spec:
https://microsoft.github.io/DirectX-Specs/d3d/D3D12VideoEncoding.html

Sample command line for transcoding:
ffmpeg.exe -hwaccel d3d12va -hwaccel_output_format d3d12 -i input.mp4
-c:v hevc_d3d12va output.mp4

Signed-off-by: Tong Wu 
---
 configure|6 +
 libavcodec/Makefile  |4 +-
 libavcodec/allcodecs.c   |1 +
 libavcodec/d3d12va_encode.c  | 1443 ++
 libavcodec/d3d12va_encode.h  |  275 ++
 libavcodec/d3d12va_encode_hevc.c | 1013 +
 libavcodec/hw_base_encode.h  |2 +-
 7 files changed, 2742 insertions(+), 2 deletions(-)
 create mode 100644 libavcodec/d3d12va_encode.c
 create mode 100644 libavcodec/d3d12va_encode.h
 create mode 100644 libavcodec/d3d12va_encode_hevc.c

diff --git a/configure b/configure
index f72533b7d2..682576aa91 100755
--- a/configure
+++ b/configure
@@ -2564,6 +2564,7 @@ CONFIG_EXTRA="
 tpeldsp
 vaapi_1
 vaapi_encode
+d3d12va_encode
 vc1dsp
 videodsp
 vp3dsp
@@ -3208,6 +3209,7 @@ wmv3_vaapi_hwaccel_select="vc1_vaapi_hwaccel"
 wmv3_vdpau_hwaccel_select="vc1_vdpau_hwaccel"
 
 # hardware-accelerated codecs
+d3d12va_encode_deps="d3d12va ID3D12VideoEncoder d3d12_encoder_feature"
 mediafoundation_deps="mftransform_h MFCreateAlignedMemoryBuffer"
 omx_deps="libdl pthreads"
 omx_rpi_select="omx"
@@ -3275,6 +3277,7 @@ h264_v4l2m2m_encoder_deps="v4l2_m2m h264_v4l2_m2m"
 hevc_amf_encoder_deps="amf"
 hevc_cuvid_decoder_deps="cuvid"
 hevc_cuvid_decoder_select="hevc_mp4toannexb_bsf"
+hevc_d3d12va_encoder_select="atsc_a53 cbs_h265 d3d12va_encode"
 hevc_mediacodec_decoder_deps="mediacodec"
 hevc_mediacodec_decoder_select="hevc_mp4toannexb_bsf hevc_parser"
 hevc_mediacodec_encoder_deps="mediacodec"
@@ -6617,6 +6620,9 @@ check_type "windows.h d3d11.h" "ID3D11VideoDecoder"
 check_type "windows.h d3d11.h" "ID3D11VideoContext"
 check_type "windows.h d3d12.h" "ID3D12Device"
 check_type "windows.h d3d12video.h" "ID3D12VideoDecoder"
+check_type "windows.h d3d12video.h" "ID3D12VideoEncoder"
+test_code cc "windows.h d3d12video.h" "D3D12_FEATURE_VIDEO feature = 
D3D12_FEATURE_VIDEO_ENCODER_CODEC" && \
+test_code cc "windows.h d3d12video.h" 
"D3D12_FEATURE_DATA_VIDEO_ENCODER_RESOURCE_REQUIREMENTS req" && enable 
d3d12_encoder_feature
 check_type "windows.h" "DPI_AWARENESS_CONTEXT" -D_WIN32_WINNT=0x0A00
 check_type "d3d9.h dxva2api.h" DXVA2_ConfigPictureDecode -D_WIN32_WINNT=0x0602
 check_func_headers mfapi.h MFCreateAlignedMemoryBuffer -lmfplat
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 23946f6ea3..50590b34f4 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -86,6 +86,7 @@ OBJS-$(CONFIG_CBS_MPEG2)   += cbs_mpeg2.o
 OBJS-$(CONFIG_CBS_VP8) += cbs_vp8.o vp8data.o
 OBJS-$(CONFIG_CBS_VP9) += cbs_vp9.o
 OBJS-$(CONFIG_CRYSTALHD)   += crystalhd.o
+OBJS-$(CONFIG_D3D12VA_ENCODE)  += d3d12va_encode.o hw_base_encode.o
 OBJS-$(CONFIG_DEFLATE_WRAPPER) += zlib_wrapper.o
 OBJS-$(CONFIG_DOVI_RPU)+= dovi_rpu.o
 OBJS-$(CONFIG_ERROR_RESILIENCE)+= error_resilience.o
@@ -437,6 +438,7 @@ OBJS-$(CONFIG_HEVC_DECODER)+= hevcdec.o 
hevc_mvs.o \
   h274.o
 OBJS-$(CONFIG_HEVC_AMF_ENCODER)+= amfenc_hevc.o
 OBJS-$(CONFIG_HEVC_CUVID_DECODER)  += cuviddec.o
+OBJS-$(CONFIG_HEVC_D3D12VA_ENCODER)+= d3d12va_encode_hevc.o
 OBJS-$(CONFIG_HEVC_MEDIACODEC_DECODER) += mediacodecdec.o
 OBJS-$(CONFIG_HEVC_MEDIACODEC_ENCODER) += mediacodecenc.o
 OBJS-$(CONFIG_HEVC_MF_ENCODER) += mfenc.o mf_utils.o
@@ -1267,7 +1269,7 @@ SKIPHEADERS+= %_tablegen.h
  \
 
 SKIPHEADERS-$(CONFIG_AMF)  += amfenc.h
 SKIPHEADERS-$(CONFIG_D3D11VA)  += d3d11va.h dxva2_internal.h
-SKIPHEADERS-$(CONFIG_D3D12VA)  += d3d12va_decode.h
+SKIPHEADERS-$(CONFIG_D3D12VA)  += d3d12va_decode.h d3d12va_encode.h
 SKIPHEADERS-$(CONFIG_DXVA2)+= dxva2.h dxva2_internal.h
 SKIPHEADERS-$(CONFIG_JNI)  += ffjni.h
 SKIPHEADERS-$(CONFIG_LCMS2)+= fflcms2.h
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index ef8c3a6d7d..9a34974141 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -865,6 +865,7 @@ extern const FFCodec ff_h264_vaapi_encoder;
 extern const FFCodec ff_h264_videotoolbox_encoder;
 extern const FFCodec ff_hevc_amf_encoder;
 extern const FFCodec ff_hevc_cuvid_decoder;
+extern const FFCodec ff_hevc_d3d12va_encoder;
 extern const FFCodec ff_hevc_mediacodec_decoder;
 extern const FFCodec ff_hevc_mediacodec_encoder;
 extern const FFCodec ff_hevc_mf_encoder;
diff --git a/libavcodec/d3d12va_encode.c b/libavcodec/d3d12va_encode.c
new file mode 100644
index 00..24898dbcb1
--- /dev/null
+++ b/libavcodec/d3d12va_encode.c
@@ -0,0 +1,1443 @@
+/*

[FFmpeg-devel] [PATCH v5 7/9] avutil/hwcontext_d3d12va: add Flags for resource creation

2024-02-18 Thread tong1 . wu-at-intel . com
From: Tong Wu 

Flags field is added to support diffferent resource creation.

Signed-off-by: Tong Wu 
---
 doc/APIchanges| 3 +++
 libavutil/hwcontext_d3d12va.c | 2 +-
 libavutil/hwcontext_d3d12va.h | 8 
 libavutil/version.h   | 2 +-
 4 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/doc/APIchanges b/doc/APIchanges
index 4e94132cf0..e1ed7334db 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -2,6 +2,9 @@ The last version increases of all libraries were on 2023-02-09
 
 API changes, most recent first:
 
+2024-01-xx - xx - lavu 58.40.100 - hwcontext_d3d12va.h
+ Add AVD3D12VAFramesContext.flags
+
 2024-02-16 - xx - lavu 58.39.100 - pixfmt.h
   Add AV_VIDEO_MAX_PLANES
 
diff --git a/libavutil/hwcontext_d3d12va.c b/libavutil/hwcontext_d3d12va.c
index 3acd5ac43a..b9984a4151 100644
--- a/libavutil/hwcontext_d3d12va.c
+++ b/libavutil/hwcontext_d3d12va.c
@@ -237,7 +237,7 @@ static AVBufferRef *d3d12va_pool_alloc(void *opaque, size_t 
size)
 .Format   = hwctx->format,
 .SampleDesc   = {.Count = 1, .Quality = 0 },
 .Layout   = D3D12_TEXTURE_LAYOUT_UNKNOWN,
-.Flags= D3D12_RESOURCE_FLAG_NONE,
+.Flags= hwctx->flags,
 };
 
 frame = av_mallocz(sizeof(AVD3D12VAFrame));
diff --git a/libavutil/hwcontext_d3d12va.h b/libavutil/hwcontext_d3d12va.h
index ff06e6f2ef..608dbac97f 100644
--- a/libavutil/hwcontext_d3d12va.h
+++ b/libavutil/hwcontext_d3d12va.h
@@ -129,6 +129,14 @@ typedef struct AVD3D12VAFramesContext {
  * If unset, will be automatically set.
  */
 DXGI_FORMAT format;
+
+/**
+ * This field is used to specify options for working with resources.
+ * If unset, this will be D3D12_RESOURCE_FLAG_NONE.
+ *
+ * @see: 
https://learn.microsoft.com/en-us/windows/win32/api/d3d12/ne-d3d12-d3d12_resource_flags.
+ */
+D3D12_RESOURCE_FLAGS flags;
 } AVD3D12VAFramesContext;
 
 #endif /* AVUTIL_HWCONTEXT_D3D12VA_H */
diff --git a/libavutil/version.h b/libavutil/version.h
index 9f45af93df..3fbc292a68 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,7 +79,7 @@
  */
 
 #define LIBAVUTIL_VERSION_MAJOR  58
-#define LIBAVUTIL_VERSION_MINOR  39
+#define LIBAVUTIL_VERSION_MINOR  40
 #define LIBAVUTIL_VERSION_MICRO 100
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
-- 
2.41.0.windows.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-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH v5 9/9] Changelog: add D3D12VA HEVC encoder changelog

2024-02-18 Thread tong1 . wu-at-intel . com
From: Tong Wu 

Signed-off-by: Tong Wu 
---
 Changelog | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Changelog b/Changelog
index c5fb21d198..6923c451c5 100644
--- a/Changelog
+++ b/Changelog
@@ -24,6 +24,7 @@ version :
 - ffmpeg CLI options may now be used as -/opt , which is equivalent
   to -opt >
 - showinfo bitstream filter
+- D3D12VA HEVC encoder
 
 version 6.1:
 - libaribcaption decoder
-- 
2.41.0.windows.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-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH v5 6/9] avcodec/vaapi_encode: extract a get_recon_format function to base layer

2024-02-18 Thread tong1 . wu-at-intel . com
From: Tong Wu 

Get constraints and set recon frame format can be shared with other HW
encoder such as D3D12. Extract this part as a new function to base
layer.

Signed-off-by: Tong Wu 
---
 libavcodec/hw_base_encode.c | 58 +
 libavcodec/hw_base_encode.h |  2 ++
 libavcodec/vaapi_encode.c   | 51 ++--
 3 files changed, 63 insertions(+), 48 deletions(-)

diff --git a/libavcodec/hw_base_encode.c b/libavcodec/hw_base_encode.c
index bb9fe70239..7497e0397e 100644
--- a/libavcodec/hw_base_encode.c
+++ b/libavcodec/hw_base_encode.c
@@ -836,6 +836,64 @@ int ff_hw_base_init_gop_structure(AVCodecContext *avctx, 
uint32_t ref_l0, uint32
 return 0;
 }
 
+int ff_hw_base_get_recon_format(AVCodecContext *avctx, const void *hwconfig, 
enum AVPixelFormat *fmt)
+{
+HWBaseEncodeContext *ctx = avctx->priv_data;
+AVHWFramesConstraints *constraints = NULL;
+enum AVPixelFormat recon_format;
+int err, i;
+
+constraints = av_hwdevice_get_hwframe_constraints(ctx->device_ref,
+  hwconfig);
+if (!constraints) {
+err = AVERROR(ENOMEM);
+goto fail;
+}
+
+// Probably we can use the input surface format as the surface format
+// of the reconstructed frames.  If not, we just pick the first (only?)
+// format in the valid list and hope that it all works.
+recon_format = AV_PIX_FMT_NONE;
+if (constraints->valid_sw_formats) {
+for (i = 0; constraints->valid_sw_formats[i] != AV_PIX_FMT_NONE; i++) {
+if (ctx->input_frames->sw_format ==
+constraints->valid_sw_formats[i]) {
+recon_format = ctx->input_frames->sw_format;
+break;
+}
+}
+if (recon_format == AV_PIX_FMT_NONE) {
+// No match.  Just use the first in the supported list and
+// hope for the best.
+recon_format = constraints->valid_sw_formats[0];
+}
+} else {
+// No idea what to use; copy input format.
+recon_format = ctx->input_frames->sw_format;
+}
+av_log(avctx, AV_LOG_DEBUG, "Using %s as format of "
+   "reconstructed frames.\n", av_get_pix_fmt_name(recon_format));
+
+if (ctx->surface_width  < constraints->min_width  ||
+ctx->surface_height < constraints->min_height ||
+ctx->surface_width  > constraints->max_width ||
+ctx->surface_height > constraints->max_height) {
+av_log(avctx, AV_LOG_ERROR, "Hardware does not support encoding at "
+   "size %dx%d (constraints: width %d-%d height %d-%d).\n",
+   ctx->surface_width, ctx->surface_height,
+   constraints->min_width,  constraints->max_width,
+   constraints->min_height, constraints->max_height);
+err = AVERROR(EINVAL);
+goto fail;
+}
+
+*fmt = recon_format;
+err = 0;
+fail:
+av_hwframe_constraints_free();
+return err;
+}
+
 int ff_hw_base_encode_init(AVCodecContext *avctx)
 {
 HWBaseEncodeContext *ctx = avctx->priv_data;
diff --git a/libavcodec/hw_base_encode.h b/libavcodec/hw_base_encode.h
index ca72b7e118..e0133d65f0 100644
--- a/libavcodec/hw_base_encode.h
+++ b/libavcodec/hw_base_encode.h
@@ -279,6 +279,8 @@ int ff_hw_base_rc_mode_configure(AVCodecContext *avctx, 
const HWBaseEncodeRCMode
 int ff_hw_base_init_gop_structure(AVCodecContext *avctx, uint32_t ref_l0, 
uint32_t ref_l1,
   int flags, int prediction_pre_only);
 
+int ff_hw_base_get_recon_format(AVCodecContext *avctx, const void *hwconfig, 
enum AVPixelFormat *fmt);
+
 int ff_hw_base_encode_init(AVCodecContext *avctx);
 
 int ff_hw_base_encode_close(AVCodecContext *avctx);
diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index 0bce3ce105..84a81559e1 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -1898,9 +1898,8 @@ static av_cold int 
vaapi_encode_create_recon_frames(AVCodecContext *avctx)
 HWBaseEncodeContext *base_ctx = avctx->priv_data;
 VAAPIEncodeContext   *ctx = avctx->priv_data;
 AVVAAPIHWConfig *hwconfig = NULL;
-AVHWFramesConstraints *constraints = NULL;
 enum AVPixelFormat recon_format;
-int err, i;
+int err;
 
 hwconfig = av_hwdevice_hwconfig_alloc(base_ctx->device_ref);
 if (!hwconfig) {
@@ -1909,52 +1908,9 @@ static av_cold int 
vaapi_encode_create_recon_frames(AVCodecContext *avctx)
 }
 hwconfig->config_id = ctx->va_config;
 
-constraints = av_hwdevice_get_hwframe_constraints(base_ctx->device_ref,
-  hwconfig);
-if (!constraints) {
-err = AVERROR(ENOMEM);
-goto fail;
-}
-
-// Probably we can use the input surface format as the surface format
-// of the reconstructed frames.  If not, we just pick the first (only?)
-// format in the valid list and hope that it all works.
-

[FFmpeg-devel] [PATCH v5 5/9] avcodec/vaapi_encode: extract gop configuration to base layer

2024-02-18 Thread tong1 . wu-at-intel . com
From: Tong Wu 

Signed-off-by: Tong Wu 
---
 libavcodec/hw_base_encode.c | 54 +
 libavcodec/hw_base_encode.h |  3 +++
 libavcodec/vaapi_encode.c   | 52 +++
 3 files changed, 61 insertions(+), 48 deletions(-)

diff --git a/libavcodec/hw_base_encode.c b/libavcodec/hw_base_encode.c
index c20c47bf55..bb9fe70239 100644
--- a/libavcodec/hw_base_encode.c
+++ b/libavcodec/hw_base_encode.c
@@ -782,6 +782,60 @@ int ff_hw_base_rc_mode_configure(AVCodecContext *avctx, 
const HWBaseEncodeRCMode
 return 0;
 }
 
+int ff_hw_base_init_gop_structure(AVCodecContext *avctx, uint32_t ref_l0, 
uint32_t ref_l1,
+  int flags, int prediction_pre_only)
+{
+HWBaseEncodeContext *ctx = avctx->priv_data;
+
+if (flags & FLAG_INTRA_ONLY || avctx->gop_size <= 1) {
+av_log(avctx, AV_LOG_VERBOSE, "Using intra frames only.\n");
+ctx->gop_size = 1;
+} else if (ref_l0 < 1) {
+av_log(avctx, AV_LOG_ERROR, "Driver does not support any "
+   "reference frames.\n");
+return AVERROR(EINVAL);
+} else if (!(flags & FLAG_B_PICTURES) || ref_l1 < 1 ||
+   avctx->max_b_frames < 1 || prediction_pre_only) {
+if (ctx->p_to_gpb)
+   av_log(avctx, AV_LOG_VERBOSE, "Using intra and B-frames "
+  "(supported references: %d / %d).\n",
+  ref_l0, ref_l1);
+else
+av_log(avctx, AV_LOG_VERBOSE, "Using intra and P-frames "
+   "(supported references: %d / %d).\n", ref_l0, ref_l1);
+ctx->gop_size = avctx->gop_size;
+ctx->p_per_i  = INT_MAX;
+ctx->b_per_p  = 0;
+} else {
+   if (ctx->p_to_gpb)
+   av_log(avctx, AV_LOG_VERBOSE, "Using intra and B-frames "
+  "(supported references: %d / %d).\n",
+  ref_l0, ref_l1);
+   else
+   av_log(avctx, AV_LOG_VERBOSE, "Using intra, P- and B-frames "
+  "(supported references: %d / %d).\n", ref_l0, ref_l1);
+ctx->gop_size = avctx->gop_size;
+ctx->p_per_i  = INT_MAX;
+ctx->b_per_p  = avctx->max_b_frames;
+if (flags & FLAG_B_PICTURE_REFERENCES) {
+ctx->max_b_depth = FFMIN(ctx->desired_b_depth,
+ av_log2(ctx->b_per_p) + 1);
+} else {
+ctx->max_b_depth = 1;
+}
+}
+
+if (flags & FLAG_NON_IDR_KEY_PICTURES) {
+ctx->closed_gop  = !!(avctx->flags & AV_CODEC_FLAG_CLOSED_GOP);
+ctx->gop_per_idr = ctx->idr_interval + 1;
+} else {
+ctx->closed_gop  = 1;
+ctx->gop_per_idr = 1;
+}
+
+return 0;
+}
+
 int ff_hw_base_encode_init(AVCodecContext *avctx)
 {
 HWBaseEncodeContext *ctx = avctx->priv_data;
diff --git a/libavcodec/hw_base_encode.h b/libavcodec/hw_base_encode.h
index 4072b514d3..ca72b7e118 100644
--- a/libavcodec/hw_base_encode.h
+++ b/libavcodec/hw_base_encode.h
@@ -276,6 +276,9 @@ int ff_hw_base_encode_receive_packet(AVCodecContext *avctx, 
AVPacket *pkt);
 int ff_hw_base_rc_mode_configure(AVCodecContext *avctx, const 
HWBaseEncodeRCMode *rc_mode,
  int default_quality, HWBaseEncodeRCConfigure 
*rc_conf);
 
+int ff_hw_base_init_gop_structure(AVCodecContext *avctx, uint32_t ref_l0, 
uint32_t ref_l1,
+  int flags, int prediction_pre_only);
+
 int ff_hw_base_encode_init(AVCodecContext *avctx);
 
 int ff_hw_base_encode_close(AVCodecContext *avctx);
diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index 30e5deac08..0bce3ce105 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -1443,7 +1443,7 @@ static av_cold int 
vaapi_encode_init_gop_structure(AVCodecContext *avctx)
 VAStatus vas;
 VAConfigAttrib attr = { VAConfigAttribEncMaxRefFrames };
 uint32_t ref_l0, ref_l1;
-int prediction_pre_only;
+int prediction_pre_only, err;
 
 vas = vaGetConfigAttributes(ctx->hwctx->display,
 ctx->va_profile,
@@ -1507,53 +1507,9 @@ static av_cold int 
vaapi_encode_init_gop_structure(AVCodecContext *avctx)
 }
 #endif
 
-if (ctx->codec->flags & FLAG_INTRA_ONLY ||
-avctx->gop_size <= 1) {
-av_log(avctx, AV_LOG_VERBOSE, "Using intra frames only.\n");
-base_ctx->gop_size = 1;
-} else if (ref_l0 < 1) {
-av_log(avctx, AV_LOG_ERROR, "Driver does not support any "
-   "reference frames.\n");
-return AVERROR(EINVAL);
-} else if (!(ctx->codec->flags & FLAG_B_PICTURES) ||
-   ref_l1 < 1 || avctx->max_b_frames < 1 ||
-   prediction_pre_only) {
-if (base_ctx->p_to_gpb)
-   av_log(avctx, AV_LOG_VERBOSE, "Using intra and B-frames "
-  "(supported references: %d / %d).\n",
-  ref_l0, ref_l1);
-else
-av_log(avctx, AV_LOG_VERBOSE, "Using intra and 

[FFmpeg-devel] [PATCH v5 4/9] avcodec/vaapi_encode: extract rc parameter configuration to base layer

2024-02-18 Thread tong1 . wu-at-intel . com
From: Tong Wu 

VAAPI and D3D12VA can share rate control configuration codes. Hence, it
can be moved to base layer for simplification.

Signed-off-by: Tong Wu 
---
 libavcodec/hw_base_encode.c| 151 
 libavcodec/hw_base_encode.h|  34 ++
 libavcodec/vaapi_encode.c  | 210 ++---
 libavcodec/vaapi_encode.h  |  14 +--
 libavcodec/vaapi_encode_av1.c  |   2 +-
 libavcodec/vaapi_encode_h264.c |   2 +-
 libavcodec/vaapi_encode_vp9.c  |   2 +-
 7 files changed, 227 insertions(+), 188 deletions(-)

diff --git a/libavcodec/hw_base_encode.c b/libavcodec/hw_base_encode.c
index f0e4ef9655..c20c47bf55 100644
--- a/libavcodec/hw_base_encode.c
+++ b/libavcodec/hw_base_encode.c
@@ -631,6 +631,157 @@ end:
 return 0;
 }
 
+int ff_hw_base_rc_mode_configure(AVCodecContext *avctx, const 
HWBaseEncodeRCMode *rc_mode,
+ int default_quality, HWBaseEncodeRCConfigure 
*rc_conf)
+{
+HWBaseEncodeContext *ctx = avctx->priv_data;
+
+if (!rc_mode || !rc_conf)
+return -1;
+
+if (rc_mode->bitrate) {
+if (avctx->bit_rate <= 0) {
+av_log(avctx, AV_LOG_ERROR, "Bitrate must be set for %s "
+   "RC mode.\n", rc_mode->name);
+return AVERROR(EINVAL);
+}
+
+if (rc_mode->mode == RC_MODE_AVBR) {
+// For maximum confusion AVBR is hacked into the existing API
+// by overloading some of the fields with completely different
+// meanings.
+
+// Target percentage does not apply in AVBR mode.
+rc_conf->rc_bits_per_second = avctx->bit_rate;
+
+// Accuracy tolerance range for meeting the specified target
+// bitrate.  It's very unclear how this is actually intended
+// to work - since we do want to get the specified bitrate,
+// set the accuracy to 100% for now.
+rc_conf->rc_target_percentage = 100;
+
+// Convergence period in frames.  The GOP size reflects the
+// user's intended block size for cutting, so reusing that
+// as the convergence period seems a reasonable default.
+rc_conf->rc_window_size = avctx->gop_size > 0 ? avctx->gop_size : 
60;
+
+} else if (rc_mode->maxrate) {
+if (avctx->rc_max_rate > 0) {
+if (avctx->rc_max_rate < avctx->bit_rate) {
+av_log(avctx, AV_LOG_ERROR, "Invalid bitrate settings: "
+   "bitrate (%"PRId64") must not be greater than "
+   "maxrate (%"PRId64").\n", avctx->bit_rate,
+   avctx->rc_max_rate);
+return AVERROR(EINVAL);
+}
+rc_conf->rc_bits_per_second   = avctx->rc_max_rate;
+rc_conf->rc_target_percentage = (avctx->bit_rate * 100) /
+ avctx->rc_max_rate;
+} else {
+// We only have a target bitrate, but this mode requires
+// that a maximum rate be supplied as well.  Since the
+// user does not want this to be a constraint, arbitrarily
+// pick a maximum rate of double the target rate.
+rc_conf->rc_bits_per_second   = 2 * avctx->bit_rate;
+rc_conf->rc_target_percentage = 50;
+}
+} else {
+if (avctx->rc_max_rate > avctx->bit_rate) {
+av_log(avctx, AV_LOG_WARNING, "Max bitrate is ignored "
+   "in %s RC mode.\n", rc_mode->name);
+}
+rc_conf->rc_bits_per_second   = avctx->bit_rate;
+rc_conf->rc_target_percentage = 100;
+}
+} else {
+rc_conf->rc_bits_per_second   = 0;
+rc_conf->rc_target_percentage = 100;
+}
+
+if (rc_mode->quality) {
+if (ctx->explicit_qp) {
+rc_conf->rc_quality = ctx->explicit_qp;
+} else if (avctx->global_quality > 0) {
+rc_conf->rc_quality = avctx->global_quality;
+} else {
+rc_conf->rc_quality = default_quality;
+av_log(avctx, AV_LOG_WARNING, "No quality level set; "
+   "using default (%d).\n", rc_conf->rc_quality);
+}
+} else {
+rc_conf->rc_quality = 0;
+}
+
+if (rc_mode->hrd) {
+if (avctx->rc_buffer_size)
+rc_conf->hrd_buffer_size = avctx->rc_buffer_size;
+else if (avctx->rc_max_rate > 0)
+rc_conf->hrd_buffer_size = avctx->rc_max_rate;
+else
+rc_conf->hrd_buffer_size = avctx->bit_rate;
+if (avctx->rc_initial_buffer_occupancy) {
+if (avctx->rc_initial_buffer_occupancy > rc_conf->hrd_buffer_size) 
{
+av_log(avctx, AV_LOG_ERROR, "Invalid RC buffer settings: "
+   "must have initial buffer size (%d) <= "
+   "buffer size 

[FFmpeg-devel] [PATCH v5 3/9] avcodec/vaapi_encode: extract set_output_property to base layer

2024-02-18 Thread tong1 . wu-at-intel . com
From: Tong Wu 

Signed-off-by: Tong Wu 
---
 libavcodec/hw_base_encode.c | 40 +
 libavcodec/hw_base_encode.h |  3 +++
 libavcodec/vaapi_encode.c   | 44 ++---
 3 files changed, 45 insertions(+), 42 deletions(-)

diff --git a/libavcodec/hw_base_encode.c b/libavcodec/hw_base_encode.c
index 62adda2fc3..f0e4ef9655 100644
--- a/libavcodec/hw_base_encode.c
+++ b/libavcodec/hw_base_encode.c
@@ -385,6 +385,46 @@ static int hw_base_encode_clear_old(AVCodecContext *avctx)
 return 0;
 }
 
+int ff_hw_base_encode_set_output_property(AVCodecContext *avctx,
+  HWBaseEncodePicture *pic,
+  AVPacket *pkt, int flag_no_delay)
+{
+HWBaseEncodeContext *ctx = avctx->priv_data;
+
+if (pic->type == PICTURE_TYPE_IDR)
+pkt->flags |= AV_PKT_FLAG_KEY;
+
+pkt->pts = pic->pts;
+pkt->duration = pic->duration;
+
+// for no-delay encoders this is handled in generic codec
+if (avctx->codec->capabilities & AV_CODEC_CAP_DELAY &&
+avctx->flags & AV_CODEC_FLAG_COPY_OPAQUE) {
+pkt->opaque  = pic->opaque;
+pkt->opaque_ref  = pic->opaque_ref;
+pic->opaque_ref = NULL;
+}
+
+if (flag_no_delay) {
+pkt->dts = pkt->pts;
+return 0;
+}
+
+if (ctx->output_delay == 0) {
+pkt->dts = pkt->pts;
+} else if (pic->encode_order < ctx->decode_delay) {
+if (ctx->ts_ring[pic->encode_order] < INT64_MIN + ctx->dts_pts_diff)
+pkt->dts = INT64_MIN;
+else
+pkt->dts = ctx->ts_ring[pic->encode_order] - ctx->dts_pts_diff;
+} else {
+pkt->dts = ctx->ts_ring[(pic->encode_order - ctx->decode_delay) %
+(3 * ctx->output_delay + ctx->async_depth)];
+}
+
+return 0;
+}
+
 static int hw_base_encode_check_frame(AVCodecContext *avctx,
   const AVFrame *frame)
 {
diff --git a/libavcodec/hw_base_encode.h b/libavcodec/hw_base_encode.h
index 70982c97b2..b836b22e6b 100644
--- a/libavcodec/hw_base_encode.h
+++ b/libavcodec/hw_base_encode.h
@@ -237,6 +237,9 @@ typedef struct HWBaseEncodeContext {
 AVPacket*tail_pkt;
 } HWBaseEncodeContext;
 
+int ff_hw_base_encode_set_output_property(AVCodecContext *avctx, 
HWBaseEncodePicture *pic,
+  AVPacket *pkt, int flag_no_delay);
+
 int ff_hw_base_encode_receive_packet(AVCodecContext *avctx, AVPacket *pkt);
 
 int ff_hw_base_encode_init(AVCodecContext *avctx);
diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index e2f968c36d..2d839a1202 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -668,47 +668,6 @@ fail_at_end:
 return err;
 }
 
-static int vaapi_encode_set_output_property(AVCodecContext *avctx,
-HWBaseEncodePicture *pic,
-AVPacket *pkt)
-{
-HWBaseEncodeContext *base_ctx = avctx->priv_data;
-VAAPIEncodeContext *ctx = avctx->priv_data;
-
-if (pic->type == PICTURE_TYPE_IDR)
-pkt->flags |= AV_PKT_FLAG_KEY;
-
-pkt->pts = pic->pts;
-pkt->duration = pic->duration;
-
-// for no-delay encoders this is handled in generic codec
-if (avctx->codec->capabilities & AV_CODEC_CAP_DELAY &&
-avctx->flags & AV_CODEC_FLAG_COPY_OPAQUE) {
-pkt->opaque = pic->opaque;
-pkt->opaque_ref = pic->opaque_ref;
-pic->opaque_ref = NULL;
-}
-
-if (ctx->codec->flags & FLAG_TIMESTAMP_NO_DELAY) {
-pkt->dts = pkt->pts;
-return 0;
-}
-
-if (base_ctx->output_delay == 0) {
-pkt->dts = pkt->pts;
-} else if (pic->encode_order < base_ctx->decode_delay) {
-if (base_ctx->ts_ring[pic->encode_order] < INT64_MIN + 
base_ctx->dts_pts_diff)
-pkt->dts = INT64_MIN;
-else
-pkt->dts = base_ctx->ts_ring[pic->encode_order] - 
base_ctx->dts_pts_diff;
-} else {
-pkt->dts = base_ctx->ts_ring[(pic->encode_order - 
base_ctx->decode_delay) %
- (3 * base_ctx->output_delay + 
base_ctx->async_depth)];
-}
-
-return 0;
-}
-
 static int vaapi_encode_get_coded_buffer_size(AVCodecContext *avctx, 
VABufferID buf_id)
 {
 VAAPIEncodeContext *ctx = avctx->priv_data;
@@ -860,7 +819,8 @@ static int vaapi_encode_output(AVCodecContext *avctx,
 av_log(avctx, AV_LOG_DEBUG, "Output read for pic %"PRId64"/%"PRId64".\n",
base_pic->display_order, base_pic->encode_order);
 
-vaapi_encode_set_output_property(avctx, base_pic, pkt_ptr);
+ff_hw_base_encode_set_output_property(avctx, base_pic, pkt_ptr,
+  ctx->codec->flags & 
FLAG_TIMESTAMP_NO_DELAY);
 
 end:
 ff_refstruct_unref(>output_buffer_ref);
-- 
2.41.0.windows.1

___
ffmpeg-devel 

[FFmpeg-devel] [PATCH v5 1/9] avcodec/vaapi_encode: move pic->input_surface initialization to encode_alloc

2024-02-18 Thread tong1 . wu-at-intel . com
From: Tong Wu 

When allocating the VAAPIEncodePicture, pic->input_surface can be
initialized right in the place. This movement simplifies the send_frame
logic and is the preparation for moving vaapi_encode_send_frame to the base 
layer.

Signed-off-by: Tong Wu 
---
 libavcodec/vaapi_encode.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index 86f4110cd2..38d855fbd4 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -878,7 +878,8 @@ static int vaapi_encode_discard(AVCodecContext *avctx,
 return 0;
 }
 
-static VAAPIEncodePicture *vaapi_encode_alloc(AVCodecContext *avctx)
+static VAAPIEncodePicture *vaapi_encode_alloc(AVCodecContext *avctx,
+  const AVFrame *frame)
 {
 VAAPIEncodeContext *ctx = avctx->priv_data;
 VAAPIEncodePicture *pic;
@@ -895,7 +896,7 @@ static VAAPIEncodePicture 
*vaapi_encode_alloc(AVCodecContext *avctx)
 }
 }
 
-pic->input_surface = VA_INVALID_ID;
+pic->input_surface = (VASurfaceID)(uintptr_t)frame->data[3];
 pic->recon_surface = VA_INVALID_ID;
 pic->output_buffer = VA_INVALID_ID;
 
@@ -1332,7 +1333,7 @@ static int vaapi_encode_send_frame(AVCodecContext *avctx, 
AVFrame *frame)
 if (err < 0)
 return err;
 
-pic = vaapi_encode_alloc(avctx);
+pic = vaapi_encode_alloc(avctx, frame);
 if (!pic)
 return AVERROR(ENOMEM);
 
@@ -1345,7 +1346,6 @@ static int vaapi_encode_send_frame(AVCodecContext *avctx, 
AVFrame *frame)
 if (ctx->input_order == 0 || frame->pict_type == AV_PICTURE_TYPE_I)
 pic->force_idr = 1;
 
-pic->input_surface = (VASurfaceID)(uintptr_t)frame->data[3];
 pic->pts = frame->pts;
 pic->duration = frame->duration;
 
-- 
2.41.0.windows.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-requ...@ffmpeg.org with subject "unsubscribe".