[FFmpeg-devel] [PATCH] avformat/cafenc: fixed packet_size calculation
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
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
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(&bp, 0, FF_DVDCLUT_EXTRADATA_SIZE);
+
+av_bprintf(&bp, "palette: ");
+
+for (int i = 0; i < FF_DVDCLUT_CLUT_LEN; i++)
+av_bprintf(&bp, "%06"PRIx32"%s", clut[i],
+ i != (FF_DVDCLUT_CLUT_LEN - 1) ? ", " : "");
+
+av_bprintf(&bp, "\n");
+
+return ff_bprint_to_codecpar_extradata(par, &bp);
+}
+
+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
[FFmpeg-devel] [PATCH v14 1/2] libavformat/dvdvideo: add DVD-Video demuxer, powered by libdvdread and libdvdnav
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 b
[FFmpeg-devel] [PATCH] avformat/mpegts: add a ts_id exported option
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, 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
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
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
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(&new_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(&new_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(&gb, 21);
length_size = (bytestream2_get_byte(&gb) & 3) + 1;
-ret = hevc_extradata_to_annexb_internal(ctx, &gb, &new_extradata,
-&new_extradata_size);
-if (ret < 0)
-return ret;
+while (1) {
+GetByteContext gb_bak = gb;
+ret = hevc_extradata_to_annexb_internal(ctx, &gb, new_extradata,
+&new_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(&ctx->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 "unsubs
Re: [FFmpeg-devel] [PATCH 1/2] avcodec/s302m: enable non-PCM decoding
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
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()
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
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
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
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()
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
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
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
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
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
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
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
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
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
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
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; ext
Re: [FFmpeg-devel] [PATCH 1/2] avcodec/s302m: enable non-PCM decoding
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
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 (avctx->rc_initia
Re: [FFmpeg-devel] [PATCH] Fix rdiv always being set to 0 in vf_convolution.c
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
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 = &cbs_vp8_read_unit,
.write_unit= &cbs_vp8_write_unit,
-.flush = &cbs_vp8_flush,
-
.assemble_fragment = &cbs_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
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 wh
[FFmpeg-devel] [PATCH 4/4] avcodec: Remove superfluous '; ' outside of functions
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 b/libavcodec/x86/
[FFmpeg-devel] [PATCH 3/4] avcodec/vvc/vvcdsp: Remove pointless wrappers
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
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,
&fc->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,
&fc->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, &fc->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
--- a/libavc
[FFmpeg-devel] [PATCH 1/4] avcodec/vvc/vvc_ps: Check before access
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
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
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
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
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
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
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
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
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()
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
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()
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(&max_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(&ptr, 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()
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
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
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
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()
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(&max_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(&ptr, 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()
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(&max_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(&ptr, 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()
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(&max_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(&ptr, 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
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
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" -pi
[FFmpeg-devel] [PATCH 1/2] fftools/ffmpeg_mux_init: Fix attachment_filename use-after-free
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(&ost->logfile_prefix);
av_freep(&ost->apad);
+av_freep(&ost->attachment_filename);
+
#if FFMPEG_OPT_MAP_CHANNEL
av_freep(&ost->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, &ost);
if (err < 0) {
+av_free(attachment_filename);
av_freep(&attachment);
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
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
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
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
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(&gb) ||
>> - 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(&gb) ||
>> - 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
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
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年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
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
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
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
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
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(&constraints);
+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 w
[FFmpeg-devel] [PATCH v5 5/9] avcodec/vaapi_encode: extract gop configuration to base layer
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 P-frames
[FFmpeg-devel] [PATCH v5 4/9] avcodec/vaapi_encode: extract rc parameter configuration to base layer
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
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(&pic->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
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".
