Re: [FFmpeg-devel] [PATCH 1/2] avformat/matroskadec: assert non NULL buf
Michael Niedermayer: > The code is only called if size is > 0 so buf should not be NULL > > Helps: CID610554 > > Signed-off-by: Michael Niedermayer > --- > libavformat/matroskadec.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c > index 4715f1b7d4..de73f97aca 100644 > --- a/libavformat/matroskadec.c > +++ b/libavformat/matroskadec.c > @@ -3701,6 +3701,8 @@ static int matroska_parse_block(MatroskaDemuxContext > *matroska, AVBufferRef *buf > uint64_t num; > int trust_default_duration; > > +av_assert1(buf); > + > ffio_init_context(&pb, data, size, 0, NULL, NULL, NULL, NULL); > > if ((n = ebml_read_num(matroska, &pb.pub, 8, &num, 1)) < 0) Ok. - 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/libx264: don't define X264_API_IMPORTS when compiling static
On 5/19/2022 11:52 PM, softworkz wrote: > This commit adds a check for the definition of _LIB which indicates > static linking. Doesn't this indiate that FFmpeg is being compiled statically, and not necessarily that x264 is? Googling also seems to indicate that this definition is no longer available on newer MSVC versions. - Derek ___ 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/libx264: don't define X264_API_IMPORTS when compiling static
> -Original Message- > From: ffmpeg-devel On Behalf Of > Derek Buitenhuis > Sent: Friday, May 20, 2022 10:50 AM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH] avcodec/libx264: don't define > X264_API_IMPORTS when compiling static > > On 5/19/2022 11:52 PM, softworkz wrote: > > This commit adds a check for the definition of _LIB which indicates > > static linking. > > Googling also seems to indicate that this > definition is no longer available on newer MSVC versions. Probably we have read the same article on SO ;-) But it's not true. When creating a "Static Library" project in Visual Studio 2019, the _LIB macro is defined. When creating a dll project, then it is not defined (instead there is _USRDLL and _WINDLL) Though, building ffmpeg with the VS project system is not the officially supported way for compiling ffmpeg with MSVC, which is performing the build on MSYS2/MinGW from which it is calling the cl.exe and link.exe binaries directly. In that case, no _LIB macro is defined which means that this commit doesn't affect the official MSVC build method. > Doesn't this indicate that FFmpeg is being compiled statically, and not > necessarily that x264 is? Correct. Or to be precise, it indicates that libavcodec is compiled statically. But the thing is: 1. At this point we are in the world of the VS project system (Only) 2. When - in this case - you would want to link the ffmpeg libs statically but x264 as dll, then you'll need to configure this specifically for that. Part of this would be to define "X264_API_IMPORTS" - but in the project properties, not in this code file. In other words: this is a narrow-scoped fix that doesn't affect default ffmpeg build behavior with MSVC. The underlying issue most likely applies to the default MSVC build method as well, but that's out of my scope because I don't use it that way and I can't test that. Thanks, softworkz ___ 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 1/4] tests/fate/vcodec: drop unnecessary options
jpeg2000 will be chosen by default, there is no reason to prescribe it explicitly. No other test does so. --- tests/fate/vcodec.mak | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/fate/vcodec.mak b/tests/fate/vcodec.mak index 3c9b7f1ff7..c19fb8633a 100644 --- a/tests/fate/vcodec.mak +++ b/tests/fate/vcodec.mak @@ -6,7 +6,7 @@ fate-vsynth%: CODEC = $(word 3, $(subst -, ,$(@))) fate-vsynth%: FMT = avi fate-vsynth%: DEFAULT_SIZE = -s 352x288 fate-vsynth3-%: DEFAULT_SIZE = -s $(FATEW)x$(FATEH) -fate-vsynth%: CMD = enc_dec "rawvideo $(DEFAULT_SIZE) -pix_fmt yuv420p $(RAWDECOPTS)" $(SRC) $(FMT) "-c $(CODEC) $(ENCOPTS)" rawvideo "-pix_fmt yuv420p -vsync passthrough $(DECOPTS)" "$(DECINOPTS)" +fate-vsynth%: CMD = enc_dec "rawvideo $(DEFAULT_SIZE) -pix_fmt yuv420p $(RAWDECOPTS)" $(SRC) $(FMT) "-c $(CODEC) $(ENCOPTS)" rawvideo "-pix_fmt yuv420p -vsync passthrough $(DECOPTS)" fate-vsynth%: CMP_UNIT = 1 fate-vsynth%: REF = $(SRC_PATH)/tests/ref/vsynth/$(@:fate-%=%) @@ -218,9 +218,7 @@ fate-vsynth%-jpegls: DECOPTS = -sws_flags area FATE_VCODEC_SCALE-$(call ENCDEC, JPEG2000, AVI) += jpeg2000 jpeg2000-97 fate-vsynth%-jpeg2000:ENCOPTS = -qscale 7 -strict experimental -pred 1 -pix_fmt rgb24 -fate-vsynth%-jpeg2000:DECINOPTS = -c:v jpeg2000 fate-vsynth%-jpeg2000-97: ENCOPTS = -qscale 7 -strict experimental -pix_fmt rgb24 -fate-vsynth%-jpeg2000-97: DECINOPTS = -c:v jpeg2000 FATE_VCODEC-$(call ENCDEC, LJPEG MJPEG, AVI) += ljpeg fate-vsynth%-ljpeg: ENCOPTS = -strict -1 -- 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] tests/fate-run: give consistent names to enc_dec() arguments
enc_dec() performs two ffmpeg runs - the first one encoding a source
file into a specified output format, the second one decoding previously
encoded file.
The arguments to this function currently have confusing names - e.g.
dec_opt contains _output_ (i.e. encoding) options for the second
(decoding) ffmpeg invocation. It is also possible to supply _input_
(i.e. decoding) options for the second ffmpeg run, but the argument
is currently unnamed and referred to by number.
Add an _in/_out suffix to argument names to make it clear what they are
used for. Give a name to input options for the decoding ffmpeg run.
---
tests/fate-run.sh | 23 ---
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/tests/fate-run.sh b/tests/fate-run.sh
index 7e3457b3ba..6501dea8f8 100755
--- a/tests/fate-run.sh
+++ b/tests/fate-run.sh
@@ -199,26 +199,27 @@ DEC_OPTS="-threads $threads -thread_type $thread_type
-idct simple $FLAGS"
ENC_OPTS="-threads 1-idct simple -dct fastint"
enc_dec(){
-src_fmt=$1
+enc_fmt_in=$1
srcfile=$2
-enc_fmt=$3
-enc_opt=$4
-dec_fmt=$5
-dec_opt=$6
+enc_fmt_out=$3
+enc_opt_out=$4
+dec_fmt_out=$5
+dec_opt_out=$6
+dec_opt_in=$7
ffprobe_opts=$8
-encfile="${outdir}/${test}.${enc_fmt}"
-decfile="${outdir}/${test}.out.${dec_fmt}"
+encfile="${outdir}/${test}.${enc_fmt_out}"
+decfile="${outdir}/${test}.out.${dec_fmt_out}"
cleanfiles="$cleanfiles $decfile"
test "$keep" -ge 1 || cleanfiles="$cleanfiles $encfile"
tsrcfile=$(target_path $srcfile)
tencfile=$(target_path $encfile)
tdecfile=$(target_path $decfile)
-ffmpeg -auto_conversion_filters -f $src_fmt $DEC_OPTS -i $tsrcfile
$ENC_OPTS $enc_opt $FLAGS \
--f $enc_fmt -y $tencfile || return
+ffmpeg -auto_conversion_filters -f $enc_fmt_in $DEC_OPTS -i $tsrcfile
$ENC_OPTS $enc_opt_out $FLAGS \
+-f $enc_fmt_out -y $tencfile || return
do_md5sum $encfile
echo $(wc -c $encfile)
-ffmpeg -auto_conversion_filters $7 $DEC_OPTS -i $tencfile $ENC_OPTS
$dec_opt $FLAGS \
--f $dec_fmt -y $tdecfile || return
+ffmpeg -auto_conversion_filters $dec_opt_in $DEC_OPTS -i $tencfile
$ENC_OPTS $dec_opt_out $FLAGS \
+-f $dec_fmt_out -y $tdecfile || return
do_md5sum $decfile
tests/tiny_psnr${HOSTEXECSUF} $srcfile $decfile $cmp_unit $cmp_shift
test -z $ffprobe_opts || \
--
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 3/4] fftools/ffmpeg: fix 2pass log file names
Use the global stream index rather than an unrelated variable in the
filename.
Broken in 6d5d9246042.
---
fftools/ffmpeg_opt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 47e8b9b7bd..c719bc2d0a 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -1867,7 +1867,7 @@ static OutputStream *new_video_stream(OptionsContext *o,
AVFormatContext *oc, in
snprintf(logfilename, sizeof(logfilename), "%s-%d.log",
ost->logfile_prefix ? ost->logfile_prefix :
DEFAULT_PASS_LOGFILENAME_PREFIX,
- i);
+ nb_output_streams - 1);
if (!strcmp(ost->enc->name, "libx264")) {
av_dict_set(&ost->encoder_opts, "stats", logfilename,
AV_DICT_DONT_OVERWRITE);
} else {
--
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 4/4] tests/fate/vcodec: add tests for ffv1 2pass mode
---
tests/fate-run.sh | 12
tests/fate/vcodec.mak | 7 +--
tests/ref/vsynth/vsynth1-ffv1-2pass | 4
tests/ref/vsynth/vsynth2-ffv1-2pass | 4
tests/ref/vsynth/vsynth3-ffv1-2pass | 4
tests/ref/vsynth/vsynth_lena-ffv1-2pass | 4
6 files changed, 33 insertions(+), 2 deletions(-)
create mode 100644 tests/ref/vsynth/vsynth1-ffv1-2pass
create mode 100644 tests/ref/vsynth/vsynth2-ffv1-2pass
create mode 100644 tests/ref/vsynth/vsynth3-ffv1-2pass
create mode 100644 tests/ref/vsynth/vsynth_lena-ffv1-2pass
diff --git a/tests/fate-run.sh b/tests/fate-run.sh
index 6501dea8f8..525e8e5499 100755
--- a/tests/fate-run.sh
+++ b/tests/fate-run.sh
@@ -207,6 +207,7 @@ enc_dec(){
dec_opt_out=$6
dec_opt_in=$7
ffprobe_opts=$8
+twopass=$9
encfile="${outdir}/${test}.${enc_fmt_out}"
decfile="${outdir}/${test}.out.${dec_fmt_out}"
cleanfiles="$cleanfiles $decfile"
@@ -214,6 +215,17 @@ enc_dec(){
tsrcfile=$(target_path $srcfile)
tencfile=$(target_path $encfile)
tdecfile=$(target_path $decfile)
+
+if [ -n "$twopass" ]; then
+logfile_prefix="${outdir}/${test}.pass1"
+cleanfiles="$cleanfiles ${logfile_prefix}-0.log"
+tlogfile_prefix=$(target_path $logfile_prefix)
+ffmpeg -auto_conversion_filters -f $enc_fmt_in $DEC_OPTS -i $tsrcfile
\
+$ENC_OPTS $enc_opt_out $FLAGS -pass 1 -passlogfile
$tlogfile_prefix \
+-f $enc_fmt_out -y $tencfile || return
+enc_opt_out="$enc_opt_out -pass 2 -passlogfile $tlogfile_prefix"
+fi
+
ffmpeg -auto_conversion_filters -f $enc_fmt_in $DEC_OPTS -i $tsrcfile
$ENC_OPTS $enc_opt_out $FLAGS \
-f $enc_fmt_out -y $tencfile || return
do_md5sum $encfile
diff --git a/tests/fate/vcodec.mak b/tests/fate/vcodec.mak
index c19fb8633a..8ca17950ea 100644
--- a/tests/fate/vcodec.mak
+++ b/tests/fate/vcodec.mak
@@ -6,7 +6,7 @@ fate-vsynth%: CODEC = $(word 3, $(subst -, ,$(@)))
fate-vsynth%: FMT = avi
fate-vsynth%: DEFAULT_SIZE = -s 352x288
fate-vsynth3-%: DEFAULT_SIZE = -s $(FATEW)x$(FATEH)
-fate-vsynth%: CMD = enc_dec "rawvideo $(DEFAULT_SIZE) -pix_fmt yuv420p
$(RAWDECOPTS)" $(SRC) $(FMT) "-c $(CODEC) $(ENCOPTS)" rawvideo "-pix_fmt
yuv420p -vsync passthrough $(DECOPTS)"
+fate-vsynth%: CMD = enc_dec "rawvideo $(DEFAULT_SIZE) -pix_fmt yuv420p
$(RAWDECOPTS)" $(SRC) $(FMT) "-c $(CODEC) $(ENCOPTS)" rawvideo "-pix_fmt
yuv420p -vsync passthrough $(DECOPTS)" "" "" ${TWOPASS}
fate-vsynth%: CMP_UNIT = 1
fate-vsynth%: REF = $(SRC_PATH)/tests/ref/vsynth/$(@:fate-%=%)
@@ -155,7 +155,8 @@ $(FATE_VCODEC_DV:%=fate-vsynth\%-%): FMT = dv
$(FATE_VCODEC_DV:%=fate-vsynth\%-%): DECOPTS += $(DEFAULT_SIZE)
FATE_VCODEC-$(call ENCDEC, FFV1, AVI) += ffv1 ffv1-v0 \
- ffv1-v3-yuv420p
+ ffv1-v3-yuv420p \
+ ffv1-2pass
FATE_VCODEC_SCALE-$(call ENCDEC, FFV1, AVI) += ffv1-v3-yuv422p10
ffv1-v3-yuv444p16 \
ffv1-v3-bgr0 ffv1-v3-rgb48
fate-vsynth%-ffv1: ENCOPTS = -slices 4
@@ -173,6 +174,8 @@ fate-vsynth%-ffv1-v3-bgr0: DECOPTS = -sws_flags
neighbor+bitexact
fate-vsynth%-ffv1-v3-rgb48: ENCOPTS = -level 3 -pix_fmt rgb48 -strict -2 \
-sws_flags neighbor+bitexact
fate-vsynth%-ffv1-v3-rgb48: DECOPTS = -sws_flags neighbor+bitexact
+fate-vsynth%-ffv1-2pass: TWOPASS = 1
+fate-vsynth%-ffv1-2pass: ENCOPTS = -coder range_tab -context 1
FATE_VCODEC-$(call ENCDEC, FFVHUFF, AVI) += ffvhuff
FATE_VCODEC_SCALE-$(call ENCDEC, FFVHUFF, AVI) += ffvhuff444 ffvhuff420p12
ffvhuff422p10left ffvhuff444p16
diff --git a/tests/ref/vsynth/vsynth1-ffv1-2pass
b/tests/ref/vsynth/vsynth1-ffv1-2pass
new file mode 100644
index 00..c27c9691d2
--- /dev/null
+++ b/tests/ref/vsynth/vsynth1-ffv1-2pass
@@ -0,0 +1,4 @@
+7332cfda96233acc7178b09868c07ad7 *tests/data/fate/vsynth1-ffv1-2pass.avi
+2382244 tests/data/fate/vsynth1-ffv1-2pass.avi
+c5ccac874dbf808e9088bc3107860042
*tests/data/fate/vsynth1-ffv1-2pass.out.rawvideo
+stddev:0.00 PSNR:999.99 MAXDIFF:0 bytes: 7603200/ 7603200
diff --git a/tests/ref/vsynth/vsynth2-ffv1-2pass
b/tests/ref/vsynth/vsynth2-ffv1-2pass
new file mode 100644
index 00..26c20db24d
--- /dev/null
+++ b/tests/ref/vsynth/vsynth2-ffv1-2pass
@@ -0,0 +1,4 @@
+2f5af924c6f7de1d4c34ec2dc9fca4ac *tests/data/fate/vsynth2-ffv1-2pass.avi
+3530664 tests/data/fate/vsynth2-ffv1-2pass.avi
+36d7ca943916e1743cefa609eba0205c
*tests/data/fate/vsynth2-ffv1-2pass.out.rawvideo
+stddev:0.00 PSNR:999.99 MAXDIFF:0 bytes: 7603200/ 7603200
diff --git a/tests/ref/vsynth/vsynth3-ffv1-2pass
b/tests/ref/vsynth/vsynth3-ffv1-2pass
new file mode 100644
index 00..dd0fd615f4
--- /dev/null
+++ b/tests/ref/vsynth/vsynth3-ffv1-2pass
@@ -0,0 +1,4 @
Re: [FFmpeg-devel] [PATCH] avcodec/libx264: don't define X264_API_IMPORTS when compiling static
On 20/05/2022 00:52, softworkz wrote: From: softworkz The definition of X264_API_IMPORTS is required for shared linking (when MSVC is used) but it must not be defined in case of static builds as is stated in x264.h: This doesn't seem right. It's about shared or static linking of libx264 itself, not ffmpeg. The correct flag should come via pkg-config at configure time. ___ 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/4] tests/fate/vcodec: drop unnecessary options
Anton Khirnov: > jpeg2000 will be chosen by default, there is no reason to prescribe it > explicitly. No other test does so. > --- > tests/fate/vcodec.mak | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/tests/fate/vcodec.mak b/tests/fate/vcodec.mak > index 3c9b7f1ff7..c19fb8633a 100644 > --- a/tests/fate/vcodec.mak > +++ b/tests/fate/vcodec.mak > @@ -6,7 +6,7 @@ fate-vsynth%: CODEC = $(word 3, $(subst -, ,$(@))) > fate-vsynth%: FMT = avi > fate-vsynth%: DEFAULT_SIZE = -s 352x288 > fate-vsynth3-%: DEFAULT_SIZE = -s $(FATEW)x$(FATEH) > -fate-vsynth%: CMD = enc_dec "rawvideo $(DEFAULT_SIZE) -pix_fmt yuv420p > $(RAWDECOPTS)" $(SRC) $(FMT) "-c $(CODEC) $(ENCOPTS)" rawvideo "-pix_fmt > yuv420p -vsync passthrough $(DECOPTS)" "$(DECINOPTS)" > +fate-vsynth%: CMD = enc_dec "rawvideo $(DEFAULT_SIZE) -pix_fmt yuv420p > $(RAWDECOPTS)" $(SRC) $(FMT) "-c $(CODEC) $(ENCOPTS)" rawvideo "-pix_fmt > yuv420p -vsync passthrough $(DECOPTS)" > fate-vsynth%: CMP_UNIT = 1 > fate-vsynth%: REF = $(SRC_PATH)/tests/ref/vsynth/$(@:fate-%=%) > > @@ -218,9 +218,7 @@ fate-vsynth%-jpegls: DECOPTS = -sws_flags area > > FATE_VCODEC_SCALE-$(call ENCDEC, JPEG2000, AVI) += jpeg2000 jpeg2000-97 > fate-vsynth%-jpeg2000:ENCOPTS = -qscale 7 -strict > experimental -pred 1 -pix_fmt rgb24 > -fate-vsynth%-jpeg2000:DECINOPTS = -c:v jpeg2000 > fate-vsynth%-jpeg2000-97: ENCOPTS = -qscale 7 -strict > experimental -pix_fmt rgb24 > -fate-vsynth%-jpeg2000-97: DECINOPTS = -c:v jpeg2000 > > FATE_VCODEC-$(call ENCDEC, LJPEG MJPEG, AVI) += ljpeg > fate-vsynth%-ljpeg: ENCOPTS = -strict -1 LGTM. - 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 v4] avutil/csp: create avpriv API for colorspace structs
On Wed, May 18, 2022 at 08:27:48PM +0200, Michael Niedermayer wrote:
> On Wed, May 18, 2022 at 08:23:38PM +0200, Michael Niedermayer wrote:
> > On Wed, May 18, 2022 at 11:18:17AM -0400, Leo Izen wrote:
> > > This commit moves some of the functionality from avfilter/colorspace
> > > into avutil/csp and exposes it as an avpriv API so it can be used by
> > > libavcodec and/or libavformat.
> > [...]
> > > +#ifndef AVUTIL_CSP_H
> > > +#define AVUTIL_CSP_H
> > > +
> > > +#include "libavutil/pixfmt.h"
> > > +
> > > +typedef struct AVLumaCoefficients {
> > > +double cr, cg, cb;
> > > +} AVLumaCoefficients;
> > > +
> > > +typedef struct AVPrimaryCoefficients {
> > > +double xr, yr, xg, yg, xb, yb;
> > > +} AVPrimaryCoefficients;
> > > +
> > > +typedef struct AVWhitepointCoefficients {
> > > +double xw, yw;
> > > +} AVWhitepointCoefficients;
> >
> > As said, these should not be floating point.
> > Adding a new public API and changing it later is messy, this
> > should be changed before its made public
>
> i now see you replaced some public by avpriv in the latest patch
> but still i think this should be changed to fixed point or AVRational
> first. Even as API between the libs its messy to change it later
> it would require us to keep the double API when its changed until
> the next major bump
I see some discussion related to this on the IRC log from when i was
sleeping. Maybe it would be better to keep this on the mailing list
Also iam not sure my concern was clearly worded so ill sort my argument
and concerns so its clearer below:
1. exactly representing values
if you have a 0.1 you can represent that exactly as AVRational 1/10 but
maybe shockingly a double cannot.
Try a printf %f of 0.1 and it will do 0.10 looks good but thats
deception
try that with more precission %100.99f shows this:
0.155511151231257827021181583404541015625
so if we want to use exactly the values from the spec, doubles with a
unit/base of 1 do not work.
int or even doubles with a base/unit of 3 might work exactly if
AVRational is unpopular. 3 instead of 1 is for that one pesky 1/3
1b. the exact value that 0.1 has in float/double depends on the precission
IEEE float
0.10001490116119384765625
IEEE double
0.155511151231257827021181583404541015625
long double
0.100013552527156068805425093160010874271392822265625
So there will be slight differences if (intermediate) types anywhere arent
exactly the same
2. someone said, you need to pick a denominator when doing float -> rational
av_d2q() will pick the best denominator for you.
3. avpriv_ vs av_
avpriv is evil, it combines the pain of ABI/API compatibility while the
public cant use it
4. rounding, regressions and inexactness
doubles/floats have in the past broken regression tests. They do not
always but i suggest we avoid them when theres no clear advantage
like higher speed in speed relevant code or much simpler code
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The day soldiers stop bringing you their problems is the day you have stopped
leading them. They have either lost confidence that you can help or concluded
you do not care. Either case is a failure of leadership. - Colin Powell
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/libx264: don't define X264_API_IMPORTS when compiling static
> -Original Message- > From: ffmpeg-devel On Behalf Of Timo > Rothenpieler > Sent: Friday, May 20, 2022 12:18 PM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH] avcodec/libx264: don't define > X264_API_IMPORTS when compiling static > > On 20/05/2022 00:52, softworkz wrote: > > From: softworkz > > > > The definition of X264_API_IMPORTS is required for shared linking > > (when MSVC is used) but it must not be defined in case of static > > builds as is stated in x264.h: > > This doesn't seem right. It's about shared or static linking of > libx264 > itself, not ffmpeg. How about some custom macro like DISABLE_X264_API_IMPORTS that one can set when desired? In that case there wouldn't be any logical irritation. > The correct flag should come via pkg-config at configure time. There has been a patch which does that, but it didn't go anywhere: https://ffmpeg.org/pipermail/ffmpeg-devel/2021-October/287426.html That's why I wanted something straight and simple which doesn't hurt anybody. Thanks, softworkz ___ 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 v4] avutil/csp: create avpriv API for colorspace structs
Hi Michael,
On Fri, May 20, 2022 at 6:28 AM Michael Niedermayer
wrote:
> 1. exactly representing values
>
This isn't actually what I meant when I made the argument. If the spec says
"0.137", I'd expect to be able to git grep the source code for "0.137" and
find where it's defined. This is lost with AVRational, where it becomes {
137, 1000 }. This may sound silly, but I find this helpful.
One way to address this is to add the exact value in a comment, like
"(AVRational) { 137, 1000 }, // 0.137". This isn't pretty but retains
grep-discoverability.
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] avcodec/libx264: don't define X264_API_IMPORTS when compiling static
On 20/05/2022 12:39, Soft Works wrote: -Original Message- From: ffmpeg-devel On Behalf Of Timo Rothenpieler Sent: Friday, May 20, 2022 12:18 PM To: ffmpeg-devel@ffmpeg.org Subject: Re: [FFmpeg-devel] [PATCH] avcodec/libx264: don't define X264_API_IMPORTS when compiling static On 20/05/2022 00:52, softworkz wrote: From: softworkz The definition of X264_API_IMPORTS is required for shared linking (when MSVC is used) but it must not be defined in case of static builds as is stated in x264.h: This doesn't seem right. It's about shared or static linking of libx264 itself, not ffmpeg. How about some custom macro like DISABLE_X264_API_IMPORTS that one can set when desired? In that case there wouldn't be any logical irritation. I'm still quite confused what the actual issue here is. Countless libraries ffmpeg depends on need those kind of macros to set the correct function import preamble. Why does x264 need special treatment? It correctly sets the desired flag via its pkg-config file. Is this some "pkg-config does not exist with msvc" thing? ___ 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 v4] avutil/csp: create avpriv API for colorspace structs
On 5/20/2022 7:28 AM, Michael Niedermayer wrote:
On Wed, May 18, 2022 at 08:27:48PM +0200, Michael Niedermayer wrote:
On Wed, May 18, 2022 at 08:23:38PM +0200, Michael Niedermayer wrote:
On Wed, May 18, 2022 at 11:18:17AM -0400, Leo Izen wrote:
This commit moves some of the functionality from avfilter/colorspace
into avutil/csp and exposes it as an avpriv API so it can be used by
libavcodec and/or libavformat.
[...]
+#ifndef AVUTIL_CSP_H
+#define AVUTIL_CSP_H
+
+#include "libavutil/pixfmt.h"
+
+typedef struct AVLumaCoefficients {
+double cr, cg, cb;
+} AVLumaCoefficients;
+
+typedef struct AVPrimaryCoefficients {
+double xr, yr, xg, yg, xb, yb;
+} AVPrimaryCoefficients;
+
+typedef struct AVWhitepointCoefficients {
+double xw, yw;
+} AVWhitepointCoefficients;
As said, these should not be floating point.
Adding a new public API and changing it later is messy, this
should be changed before its made public
i now see you replaced some public by avpriv in the latest patch
but still i think this should be changed to fixed point or AVRational
first. Even as API between the libs its messy to change it later
it would require us to keep the double API when its changed until
the next major bump
I see some discussion related to this on the IRC log from when i was
sleeping. Maybe it would be better to keep this on the mailing list
Also iam not sure my concern was clearly worded so ill sort my argument
and concerns so its clearer below:
1. exactly representing values
if you have a 0.1 you can represent that exactly as AVRational 1/10 but
maybe shockingly a double cannot.
Try a printf %f of 0.1 and it will do 0.10 looks good but thats deception
try that with more precission %100.99f shows this:
0.155511151231257827021181583404541015625
so if we want to use exactly the values from the spec, doubles with a
unit/base of 1 do not work.
int or even doubles with a base/unit of 3 might work exactly if
AVRational is unpopular. 3 instead of 1 is for that one pesky 1/3
1b. the exact value that 0.1 has in float/double depends on the precission
IEEE float
0.10001490116119384765625
IEEE double
0.155511151231257827021181583404541015625
long double
0.100013552527156068805425093160010874271392822265625
So there will be slight differences if (intermediate) types anywhere arent
exactly the same
2. someone said, you need to pick a denominator when doing float -> rational
av_d2q() will pick the best denominator for you.
3. avpriv_ vs av_
avpriv is evil, it combines the pain of ABI/API compatibility while the
public cant use it
Not making it public allows us to not commit to a fixed design *now*.
Unless there's a clear need for this to be part of the public API, i
don't think it's a good idea to do so. This change is being made because
this API is afaict needed in lavc, and not by some project using lavu.
Removing/changing an avpriv symbol or struct is a matter or waiting for
the nearest major bump. No need for an arbitrary 2 year wait period,
compat wrappers, or anything crazy.
4. rounding, regressions and inexactness
doubles/floats have in the past broken regression tests. They do not
always but i suggest we avoid them when theres no clear advantage
like higher speed in speed relevant code or much simpler code
thx
[...]
___
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] avcodec/libx264: don't define X264_API_IMPORTS when compiling static
> -Original Message- > From: ffmpeg-devel On Behalf Of Timo > Rothenpieler > Sent: Friday, May 20, 2022 1:38 PM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH] avcodec/libx264: don't define > X264_API_IMPORTS when compiling static > > On 20/05/2022 12:39, Soft Works wrote: > > > > > >> -Original Message- > >> From: ffmpeg-devel On Behalf Of > Timo > >> Rothenpieler > >> Sent: Friday, May 20, 2022 12:18 PM > >> To: ffmpeg-devel@ffmpeg.org > >> Subject: Re: [FFmpeg-devel] [PATCH] avcodec/libx264: don't define > >> X264_API_IMPORTS when compiling static > >> > >> On 20/05/2022 00:52, softworkz wrote: > >>> From: softworkz > >>> > >>> The definition of X264_API_IMPORTS is required for shared linking > >>> (when MSVC is used) but it must not be defined in case of static > >>> builds as is stated in x264.h: > >> > >> This doesn't seem right. It's about shared or static linking of > >> libx264 > >> itself, not ffmpeg. > > > > How about some custom macro like DISABLE_X264_API_IMPORTS that one > > can set when desired? > > > > In that case there wouldn't be any logical irritation. > > > > I'm still quite confused what the actual issue here is. > Countless libraries ffmpeg depends on need those kind of macros to set > the correct function import preamble. > Why does x264 need special treatment? It correctly sets the desired > flag > via its pkg-config file. The current code is #if defined(_MSC_VER) #define X264_API_IMPORTS 1 #endif Which means that this macro is always set then building with MSVC. But the macro may only be set when linking to x264.dll, not when linking statically to libx264. (pkg-config can't do anything about that) This problem was introduced by this change in libx264: https://code.videolan.org/videolan/x264/-/commit/a615f027ed172e2dd5380e736d487aa858a0c4ff#98b74dd0a8bf575bfdf90bbccf5142a555f06d4f_56_69 Previously, they had this line #define X264_API __declspec(dllimport) Which handled the situation automatically by checking whether it's linked as dll or static lib. But after that change, you are required to set this yourself (as a consumer). But ffmpeg has no proper condition to set this only when linking to x264.dll. That's why the current code is essentially wrong: #if defined(_MSC_VER) #define X264_API_IMPORTS 1 #endif And besides that, I don't think that those things belong into a code file. > Is this some "pkg-config does not exist with msvc" thing? The "official" way of building ffmpeg with MSVC is to run configure and make from MSYS2/MinGW which then only calls cl.exe and link.exe from a Visual Studio installation. In this case pkg-config is used. I'm working with regular Visual Studio projects, though. Even dependencies like libx264 are compiled in their own VS projects. There's no MSYS2, no make, no pkg-conf involved. I _think_ that just nobody has ever tried to link libx264 statically when compiling in the "official way", which is probably rarely used anyway and even more rare that somebody would bother to link with x264 and once again even more rare that the one would on top of that decide to link to libx264 statically. That's my guess why nobody has complained about this during the past 3 years. Kind regards, softworkz ___ 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 v4] avutil/csp: create avpriv API for colorspace structs
On 5/20/22 06:28, Michael Niedermayer wrote: 2. someone said, you need to pick a denominator when doing float -> rational av_d2q() will pick the best denominator for you. av_d2q() requires you to pass a maximum denominator, which essentially determines precision of the conversion. You still have to make a (somewhat) arbitrary choice in that matter. - Leo Izen (thebombzen) ___ 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/libx264: don't define X264_API_IMPORTS when compiling static
On 5/20/2022 1:07 PM, Soft Works wrote: > I'm working with regular Visual Studio projects, though. > Even dependencies like libx264 are compiled in their own > VS projects. > There's no MSYS2, no make, no pkg-conf involved. Things should not be added to FFmpeg in support of non-standard build systems. As per Thilo's explanation, I think this is a more appropraite fix: http://ffmpeg.org/pipermail/ffmpeg-devel/2021-October/287426.html Matt sent that last October, but it seems to have been missed. - Derek ___ 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/libx264: don't define X264_API_IMPORTS when compiling static
On 5/20/2022 1:49 PM, Derek Buitenhuis wrote: > As per Thilo's explanation, I think this is a more appropraite fix: Apologies, I meant to type *Timo*. - Derek ___ 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/libx264: don't define X264_API_IMPORTS when compiling static
> -Original Message- > From: ffmpeg-devel On Behalf Of > Derek Buitenhuis > Sent: Friday, May 20, 2022 2:51 PM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH] avcodec/libx264: don't define > X264_API_IMPORTS when compiling static > > On 5/20/2022 1:49 PM, Derek Buitenhuis wrote: > > As per Thilo's explanation, I think this is a more appropraite fix: > > Apologies, I meant to type *Timo*. Still wrong. Because I had posted that link. softworkz ___ 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/libx264: don't define X264_API_IMPORTS when compiling static
On 5/20/2022 1:54 PM, Soft Works wrote: > Still wrong. Because I had posted that link. No, not wrong. I was alluding to: http://ffmpeg.org/pipermail/ffmpeg-devel/2022-May/296605.html - and not the link. In any case, thanks for the needlessly aggressive email. - Derek ___ 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/libx264: don't define X264_API_IMPORTS when compiling static
> -Original Message- > From: ffmpeg-devel On Behalf Of > Derek Buitenhuis > Sent: Friday, May 20, 2022 3:00 PM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH] avcodec/libx264: don't define > X264_API_IMPORTS when compiling static > > On 5/20/2022 1:54 PM, Soft Works wrote: > > Still wrong. Because I had posted that link. > > No, not wrong. I was alluding to: http://ffmpeg.org/pipermail/ffmpeg- > devel/2022-May/296605.html - and not the link. > > In any case, thanks for the needlessly aggressive email. I didn't mean to be aggressive. But you posted the exact link that I had posted 2 messages before: > As per Thilo's explanation, I think this is a more appropraite fix: > > http://ffmpeg.org/pipermail/ffmpeg-devel/2021-October/287426.html You say you think this is a more appropriate fix "as per Timo's explanation" because of the following message: > No, not wrong. I was alluding to: http://ffmpeg.org/pipermail/ffmpeg- > devel/2022-May/296605.html - and not the link. ...where Timo said "This doesn't seem right"? I'm not sure but I think this doesn't seem right, :-) Kind regards, softworkz ___ 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 v4] avutil/csp: create avpriv API for colorspace structs
Hi Ronald
On Fri, May 20, 2022 at 07:26:56AM -0400, Ronald S. Bultje wrote:
> Hi Michael,
>
> On Fri, May 20, 2022 at 6:28 AM Michael Niedermayer
> wrote:
>
> > 1. exactly representing values
> >
>
> This isn't actually what I meant when I made the argument. If the spec says
> "0.137", I'd expect to be able to git grep the source code for "0.137" and
> find where it's defined. This is lost with AVRational, where it becomes {
> 137, 1000 }. This may sound silly, but I find this helpful.
>
> One way to address this is to add the exact value in a comment, like
> "(AVRational) { 137, 1000 }, // 0.137". This isn't pretty but retains
> grep-discoverability.
We use fixed point numbers in multiple places without
loosing grep-discoverability
for example
libavcodec/mpegaudiotab.h:FIX(0.54119610014619701222),
libavcodec/mpegaudiotab.h:FIX(1.3065629648763763537),
libavcodec/mpegaudiotab.h:FIX(0.50979557910415917998),
libavcodec/mpegaudiotab.h:FIX(2.5629154477415054814),
libavcodec/mpegaudiotab.h:FIX(0.89997622313641556513),
libavcodec/mpegaudiotab.h:FIX(0.60134488693504528634),
libavcodec/mpegaudiotab.h:FIX(0.5024192861881556782),
libavcodec/jfdctint_template.c:#define FIX_0_765366865 FIX(0.765366865)
libavcodec/jfdctint_template.c:#define FIX_0_899976223 FIX(0.899976223)
libavcodec/jfdctint_template.c:#define FIX_1_175875602 FIX(1.175875602)
libavcodec/jfdctint_template.c:#define FIX_1_501321110 FIX(1.501321110)
libavcodec/jfdctint_template.c:#define FIX_1_847759065 FIX(1.847759065)
libavcodec/jfdctint_template.c:#define FIX_1_961570560 FIX(1.961570560)
libavcodec/dcadata.c:SCALE(0.01112466771), SCALE(0.43016362190),
SCALE(0.53190881014), SCALE(0.02678431384),
libavcodec/dcadata.c:SCALE(0.01170534454), SCALE(0.43572667241),
SCALE(0.52686679363), SCALE(0.02568206564),
libavcodec/dcadata.c:SCALE(0.01230939943), SCALE(0.44127810001),
SCALE(0.52177828550), SCALE(0.02461459488),
libavcodec/dcadata.c:SCALE(0.01293735672), SCALE(0.44681602716),
SCALE(0.51664537191), SCALE(0.02358125709),
libavcodec/dcadata.c:SCALE(0.01358995494), SCALE(0.45233830810),
SCALE(0.51147013903), SCALE(0.02258131653),
libavcodec/dcadata.c:SCALE(0.01426773332), SCALE(0.45784294605),
SCALE(0.50625455379), SCALE(0.02161412500),
tests/utils.c:lum[0] = (FIX(0.29900) * r + FIX(0.58700) * g +
tests/utils.c: FIX(0.11400) * b + ONE_HALF) >> SCALEBITS;
tests/utils.c:lum[1] = (FIX(0.29900) * r + FIX(0.58700) * g +
tests/utils.c: FIX(0.11400) * b + ONE_HALF) >> SCALEBITS;
tests/utils.c:lum[0] = (FIX(0.29900) * r + FIX(0.58700) * g +
tests/utils.c: FIX(0.11400) * b + ONE_HALF) >> SCALEBITS;
tests/utils.c:lum[1] = (FIX(0.29900) * r + FIX(0.58700) * g +
tests/utils.c: FIX(0.11400) * b + ONE_HALF) >> SCALEBITS;
such a macro could produce a AVRational too easily (if wanted)
the advantage over a comment is that a comment can be forgotten
the argument for a macro cannot be forgotten and still succeed build
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Freedom in capitalist society always remains about the same as it was in
ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin
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/libx264: don't define X264_API_IMPORTS when compiling static
On 5/20/2022 2:06 PM, Soft Works wrote: >> As per Thilo's explanation, I think this is a more appropraite fix: >> >> http://ffmpeg.org/pipermail/ffmpeg-devel/2021-October/287426.html > > You say you think this is a more appropriate fix "as per Timo's explanation" > because of the following message: > >> No, not wrong. I was alluding to: http://ffmpeg.org/pipermail/ffmpeg- >> devel/2022-May/296605.html - and not the link. > > > ...where Timo said "This doesn't seem right"? The fix he suggests, using pkg-config, is what the patch I linked does. I don't know why it is important who is 'right' here, so I'll conclude replying, as it is rather unpleasant. - Derek ___ 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/libx264: don't define X264_API_IMPORTS when compiling static
> -Original Message- > From: ffmpeg-devel On Behalf Of > Derek Buitenhuis > Sent: Friday, May 20, 2022 2:50 PM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH] avcodec/libx264: don't define > X264_API_IMPORTS when compiling static > > On 5/20/2022 1:07 PM, Soft Works wrote: > > I'm working with regular Visual Studio projects, though. > > Even dependencies like libx264 are compiled in their own > > VS projects. > > There's no MSYS2, no make, no pkg-conf involved. > > Things should not be added to FFmpeg in support of > non-standard build systems. > > As per Thilo's explanation, I think this is a more appropraite fix: > > http://ffmpeg.org/pipermail/ffmpeg-devel/2021-October/287426.html > > Matt sent that last October, but it seems to have been missed. Actually I had talked to Matt about it and he gave me permission to rebase and resubmit his patch. (due to lack of time) But here comes my dilemma: I can't submit something which I'm not working with usually and where I don't have sufficient experience to be confident that it's all well. There have ben several discussions about it last year, last one with Michael in December IIRC. I wouldn't be able to go through defending and evolving this patch, that's why I wanted to do something very very basic and simple. Kind regards, softworkz ___ 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 v4] avutil/csp: create avpriv API for colorspace structs
On Fri, May 20, 2022 at 08:53:35AM -0300, James Almer wrote:
>
>
> On 5/20/2022 7:28 AM, Michael Niedermayer wrote:
> > On Wed, May 18, 2022 at 08:27:48PM +0200, Michael Niedermayer wrote:
> > > On Wed, May 18, 2022 at 08:23:38PM +0200, Michael Niedermayer wrote:
> > > > On Wed, May 18, 2022 at 11:18:17AM -0400, Leo Izen wrote:
> > > > > This commit moves some of the functionality from avfilter/colorspace
> > > > > into avutil/csp and exposes it as an avpriv API so it can be used by
> > > > > libavcodec and/or libavformat.
> > > > [...]
> > > > > +#ifndef AVUTIL_CSP_H
> > > > > +#define AVUTIL_CSP_H
> > > > > +
> > > > > +#include "libavutil/pixfmt.h"
> > > > > +
> > > > > +typedef struct AVLumaCoefficients {
> > > > > +double cr, cg, cb;
> > > > > +} AVLumaCoefficients;
> > > > > +
> > > > > +typedef struct AVPrimaryCoefficients {
> > > > > +double xr, yr, xg, yg, xb, yb;
> > > > > +} AVPrimaryCoefficients;
> > > > > +
> > > > > +typedef struct AVWhitepointCoefficients {
> > > > > +double xw, yw;
> > > > > +} AVWhitepointCoefficients;
> > > >
> > > > As said, these should not be floating point.
> > > > Adding a new public API and changing it later is messy, this
> > > > should be changed before its made public
> > >
> > > i now see you replaced some public by avpriv in the latest patch
> > > but still i think this should be changed to fixed point or AVRational
> > > first. Even as API between the libs its messy to change it later
> > > it would require us to keep the double API when its changed until
> > > the next major bump
> >
> > I see some discussion related to this on the IRC log from when i was
> > sleeping. Maybe it would be better to keep this on the mailing list
> >
> > Also iam not sure my concern was clearly worded so ill sort my argument
> > and concerns so its clearer below:
> >
> > 1. exactly representing values
> > if you have a 0.1 you can represent that exactly as AVRational 1/10 but
> > maybe shockingly a double cannot.
> > Try a printf %f of 0.1 and it will do 0.10 looks good but thats
> > deception
> > try that with more precission %100.99f shows this:
> >
> > 0.155511151231257827021181583404541015625
> > so if we want to use exactly the values from the spec, doubles with a
> > unit/base of 1 do not work.
> > int or even doubles with a base/unit of 3 might work exactly if
> > AVRational is unpopular. 3 instead of 1 is for that one pesky
> > 1/3
> > 1b. the exact value that 0.1 has in float/double depends on the precission
> > IEEE float
> >
> > 0.10001490116119384765625
> > IEEE double
> >
> > 0.155511151231257827021181583404541015625
> > long double
> >
> > 0.100013552527156068805425093160010874271392822265625
> > So there will be slight differences if (intermediate) types anywhere
> > arent
> > exactly the same
> >
> > 2. someone said, you need to pick a denominator when doing float -> rational
> > av_d2q() will pick the best denominator for you.
> >
> > 3. avpriv_ vs av_
> > avpriv is evil, it combines the pain of ABI/API compatibility while the
> > public cant use it
>
> Not making it public allows us to not commit to a fixed design *now*.
> Unless there's a clear need for this to be part of the public API, i don't
> think it's a good idea to do so. This change is being made because this API
> is afaict needed in lavc, and not by some project using lavu.
>
> Removing/changing an avpriv symbol or struct is a matter or waiting for the
> nearest major bump. No need for an arbitrary 2 year wait period, compat
> wrappers, or anything crazy.
yes but is that really easier ?
sheduling changes to happen during a bump vs adding a warper+deprecating
at any time with a automatic #if removing code
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
If you fake or manipulate statistics in a paper in physics you will never
get a job again.
If you fake or manipulate statistics in a paper in medicin you will get
a job for life at the pharma industry.
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/libx264: don't define X264_API_IMPORTS when compiling static
> -Original Message- > From: ffmpeg-devel On Behalf Of > Derek Buitenhuis > Sent: Friday, May 20, 2022 3:13 PM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH] avcodec/libx264: don't define > X264_API_IMPORTS when compiling static > > On 5/20/2022 2:06 PM, Soft Works wrote: > >> As per Thilo's explanation, I think this is a more appropraite fix: > >> > >> http://ffmpeg.org/pipermail/ffmpeg-devel/2021- > October/287426.html > > > > You say you think this is a more appropriate fix "as per Timo's > explanation" > > because of the following message: > > > >> No, not wrong. I was alluding to: > http://ffmpeg.org/pipermail/ffmpeg- > >> devel/2022-May/296605.html - and not the link. > > > > > > ...where Timo said "This doesn't seem right"? > > The fix he suggests, using pkg-config, is what the patch I linked > does. > > I don't know why it is important who is 'right' here, so I'll conclude > replying, > as it is rather unpleasant. Let me be honest: you caught me with this: > Things should not be added to FFmpeg in support of > non-standard build systems. So many adaptions have been made over the years for whatever kind of platforms and builds to work, but as soon as it's about MS, even making a super-trivial macro definition configurable is already too much and it's "non-standard"... Though, I apologize for my slightly overdriven reaction. sw ___ 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] mfenc: Dynamically load MFPlat.DLL
Allow builds of FFmpeg with MediaFoundation to work under N editions of
Windows which are without MediaFoundation by default.
Signed-off-by: Trystan Mata
---
configure | 4 +-
libavcodec/mf_utils.c | 26 ++--
libavcodec/mf_utils.h | 16 ++--
libavcodec/mfenc.c| 92 +--
4 files changed, 108 insertions(+), 30 deletions(-)
diff --git a/configure b/configure
index f115b21064..f16fbb320a 100755
--- a/configure
+++ b/configure
@@ -3129,8 +3129,8 @@ wmv3_vaapi_hwaccel_select="vc1_vaapi_hwaccel"
wmv3_vdpau_hwaccel_select="vc1_vdpau_hwaccel"
# hardware-accelerated codecs
-mediafoundation_deps="mftransform_h MFCreateAlignedMemoryBuffer"
-mediafoundation_extralibs="-lmfplat -lmfuuid -lole32 -lstrmiids"
+mediafoundation_deps="LoadLibrary mftransform_h
MFCreateAlignedMemoryBuffer"
+mediafoundation_extralibs="-lole32 -lstrmiids"
omx_deps="libdl pthreads"
omx_rpi_select="omx"
qsv_deps="libmfx"
diff --git a/libavcodec/mf_utils.c b/libavcodec/mf_utils.c
index eeabd0ce0b..9a83f71692 100644
--- a/libavcodec/mf_utils.c
+++ b/libavcodec/mf_utils.c
@@ -106,19 +106,20 @@ char *ff_hr_str_buf(char *buf, size_t size,
HRESULT hr)
// If fill_data!=NULL, initialize the buffer and set the length. (This
is a
// subtle but important difference: some decoders want CurrentLength==0 on
// provided output buffers.)
-IMFSample *ff_create_memory_sample(void *fill_data, size_t size, size_t
align)
+IMFSample *ff_create_memory_sample(const MFSymbols *symbols, void
*fill_data,
+ size_t size, size_t align)
{
HRESULT hr;
IMFSample *sample;
IMFMediaBuffer *buffer;
-hr = MFCreateSample(&sample);
+hr = symbols->MFCreateSample(&sample);
if (FAILED(hr))
return NULL;
align = FFMAX(align, 16); // 16 is "recommended", even if not
required
-hr = MFCreateAlignedMemoryBuffer(size, align - 1, &buffer);
+hr = symbols->MFCreateAlignedMemoryBuffer(size, align - 1, &buffer);
if (FAILED(hr))
return NULL;
@@ -548,7 +549,7 @@ const CLSID *ff_codec_to_mf_subtype(enum AVCodecID
codec)
}
}
-static int init_com_mf(void *log)
+static int init_com_mf(const MFSymbols *symbols, void *log)
{
HRESULT hr;
@@ -561,7 +562,7 @@ static int init_com_mf(void *log)
return AVERROR(ENOSYS);
}
-hr = MFStartup(MF_VERSION, MFSTARTUP_FULL);
+hr = symbols->MFStartup(MF_VERSION, MFSTARTUP_FULL);
if (FAILED(hr)) {
av_log(log, AV_LOG_ERROR, "could not initialize
MediaFoundation\n");
CoUninitialize();
@@ -571,15 +572,16 @@ static int init_com_mf(void *log)
return 0;
}
-static void uninit_com_mf(void)
+static void uninit_com_mf(const MFSymbols *symbols)
{
-MFShutdown();
+symbols->MFShutdown();
CoUninitialize();
}
// Find and create a IMFTransform with the given input/output types.
When done,
// you should use ff_free_mf() to destroy it, which will also uninit COM.
-int ff_instantiate_mf(void *log,
+int ff_instantiate_mf(const MFSymbols *symbols,
+ void *log,
GUID category,
MFT_REGISTER_TYPE_INFO *in_type,
MFT_REGISTER_TYPE_INFO *out_type,
@@ -594,7 +596,7 @@ int ff_instantiate_mf(void *log,
IMFActivate *winner = 0;
UINT32 flags;
-ret = init_com_mf(log);
+ret = init_com_mf(symbols, log);
if (ret < 0)
return ret;
@@ -667,14 +669,14 @@ int ff_instantiate_mf(void *log,
return 0;
error_uninit_mf:
-uninit_com_mf();
+uninit_com_mf(symbols);
return AVERROR(ENOSYS);
}
-void ff_free_mf(IMFTransform **mft)
+void ff_free_mf(const MFSymbols *symbols, IMFTransform **mft)
{
if (*mft)
IMFTransform_Release(*mft);
*mft = NULL;
-uninit_com_mf();
+uninit_com_mf(symbols);
}
diff --git a/libavcodec/mf_utils.h b/libavcodec/mf_utils.h
index d514723c3b..1b1041bb29 100644
--- a/libavcodec/mf_utils.h
+++ b/libavcodec/mf_utils.h
@@ -41,6 +41,15 @@
#include "avcodec.h"
+typedef struct MFSymbols {
+HRESULT (*MFCreateSample) (IMFSample **ppIMFSample);
+HRESULT (*MFCreateAlignedMemoryBuffer) (DWORD cbMaxLength, DWORD
cbAligment,
+IMFMediaBuffer **ppBuffer);
+HRESULT (*MFStartup) (ULONG Version, DWORD dwFlags);
+HRESULT (*MFShutdown) (void);
+HRESULT (*MFCreateMediaType) (IMFMediaType **ppMFType);
+} MFSymbols;
+
// These functions do exist in mfapi.h, but are only available within
// __cplusplus ifdefs.
HRESULT ff_MFGetAttributeSize(IMFAttributes *pattr, REFGUID guid,
@@ -150,7 +159,8 @@ char *ff_hr_str_buf(char *buf, size_t size, HRESULT hr);
#define FF_VAL_VT_UI4(v) FF_VARIANT_VALUE(VT_UI4, .ulVal = (v))
#define FF_VAL_VT_BOOL(v) FF_VARIANT_VALUE(VT_BOOL, .boolVal = (v))
-IMFSample *ff_create_memory_sample(void *fill_data, size_t size,
size_t align);
+IMFSample *ff_create_memory_sample(const MFSymbo
Re: [FFmpeg-devel] [PATCH v4] avutil/csp: create avpriv API for colorspace structs
Hi,
On Fri, May 20, 2022 at 9:11 AM Michael Niedermayer
wrote:
> On Fri, May 20, 2022 at 07:26:56AM -0400, Ronald S. Bultje wrote:
> > On Fri, May 20, 2022 at 6:28 AM Michael Niedermayer <
> mich...@niedermayer.cc>
> > > 1. exactly representing values
> >
> > This isn't actually what I meant when I made the argument. If the spec
> says
> > "0.137", I'd expect to be able to git grep the source code for "0.137"
> and
> > find where it's defined. This is lost with AVRational, where it becomes {
> > 137, 1000 }. This may sound silly, but I find this helpful.
> >
> > One way to address this is to add the exact value in a comment, like
> > "(AVRational) { 137, 1000 }, // 0.137". This isn't pretty but retains
> > grep-discoverability.
>
> We use fixed point numbers in multiple places without
> loosing grep-discoverability
> for example
>
[..]
> the advantage over a comment is that a comment can be forgotten
> the argument for a macro cannot be forgotten and still succeed build
>
That sounds reasonable, I'm OK with this approach.
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".
[FFmpeg-devel] [PATCH] fftools/ffmpeg: reuse the encoding code for flushing encoders
---
Fixed 2pass breakage.
Sent a separate patchset adding tests for 2pass
---
fftools/ffmpeg.c | 84
1 file changed, 21 insertions(+), 63 deletions(-)
diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 0e62c39522..9b69a5a2c9 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -836,12 +836,12 @@ static int encode_frame(OutputFile *of, OutputStream
*ost, AVFrame *frame)
AVCodecContext *enc = ost->enc_ctx;
AVPacket *pkt = ost->pkt;
const char *type_desc = av_get_media_type_string(enc->codec_type);
+const char*action = frame ? "encode" : "flush";
int ret;
+if (frame) {
ost->frames_encoded++;
-update_benchmark(NULL);
-
if (debug_ts) {
av_log(NULL, AV_LOG_INFO, "encoder <- type:%s "
"frame_pts:%s frame_pts_time:%s time_base:%d/%d\n",
@@ -849,9 +849,12 @@ static int encode_frame(OutputFile *of, OutputStream *ost,
AVFrame *frame)
av_ts2str(frame->pts), av_ts2timestr(frame->pts,
&enc->time_base),
enc->time_base.num, enc->time_base.den);
}
+}
+
+update_benchmark(NULL);
ret = avcodec_send_frame(enc, frame);
-if (ret < 0) {
+if (ret < 0 && !(ret == AVERROR_EOF && !frame)) {
av_log(NULL, AV_LOG_ERROR, "Error submitting %s frame to the
encoder\n",
type_desc);
return ret;
@@ -859,11 +862,20 @@ static int encode_frame(OutputFile *of, OutputStream
*ost, AVFrame *frame)
while (1) {
ret = avcodec_receive_packet(enc, pkt);
-update_benchmark("encode_%s %d.%d", type_desc,
+update_benchmark("%s_%s %d.%d", action, type_desc,
ost->file_index, ost->index);
-if (ret == AVERROR(EAGAIN))
+
+/* if two pass, output log on success and EOF */
+if ((ret >= 0 || ret == AVERROR_EOF) && ost->logfile && enc->stats_out)
+fprintf(ost->logfile, "%s", enc->stats_out);
+
+if (ret == AVERROR(EAGAIN)) {
+av_assert0(frame); // should never happen during flushing
return 0;
-else if (ret < 0) {
+} else if (ret == AVERROR_EOF) {
+output_packet(of, pkt, ost, 1);
+return ret;
+} else if (ret < 0) {
av_log(NULL, AV_LOG_ERROR, "%s encoding failed\n", type_desc);
return ret;
}
@@ -894,10 +906,6 @@ static int encode_frame(OutputFile *of, OutputStream *ost,
AVFrame *frame)
do_video_stats(ost, pkt->size);
output_packet(of, pkt, ost, 0);
-
-/* if two pass, output log */
-if (ost->logfile && enc->stats_out)
-fprintf(ost->logfile, "%s", enc->stats_out);
}
av_assert0(0);
@@ -1762,59 +1770,9 @@ static void flush_encoders(void)
if (enc->codec_type != AVMEDIA_TYPE_VIDEO && enc->codec_type !=
AVMEDIA_TYPE_AUDIO)
continue;
-for (;;) {
-const char *desc = NULL;
-AVPacket *pkt = ost->pkt;
-int pkt_size;
-
-switch (enc->codec_type) {
-case AVMEDIA_TYPE_AUDIO:
-desc = "audio";
-break;
-case AVMEDIA_TYPE_VIDEO:
-desc = "video";
-break;
-default:
-av_assert0(0);
-}
-
-update_benchmark(NULL);
-
-while ((ret = avcodec_receive_packet(enc, pkt)) ==
AVERROR(EAGAIN)) {
-ret = avcodec_send_frame(enc, NULL);
-if (ret < 0) {
-av_log(NULL, AV_LOG_FATAL, "%s encoding failed: %s\n",
- desc,
- av_err2str(ret));
-exit_program(1);
-}
-}
-
-update_benchmark("flush_%s %d.%d", desc, ost->file_index,
ost->index);
-if (ret < 0 && ret != AVERROR_EOF) {
-av_log(NULL, AV_LOG_FATAL, "%s encoding failed: %s\n",
- desc,
- av_err2str(ret));
-exit_program(1);
-}
-if (ost->logfile && enc->stats_out) {
-fprintf(ost->logfile, "%s", enc->stats_out);
-}
-if (ret == AVERROR_EOF) {
-output_packet(of, pkt, ost, 1);
-break;
-}
-if (ost->finished & MUXER_FINISHED) {
-av_packet_unref(pkt);
-continue;
-}
-av_packet_rescale_ts(pkt, enc->time_base, ost->mux_timebase);
-pkt_size = pkt->size;
-output_packet(of, pkt, ost, 0);
-if (ost->enc_ctx->codec_type == AVMEDIA_TYPE_VIDEO &&
vstats_filename) {
-do_video_stats(ost, pkt_size);
-}
-}
+ret = encode_frame(of, ost, NULL);
+if (ret != AVERROR_EOF)
+exit_program(1);
}
}
--
2.34.1
__
[FFmpeg-devel] [PATCH v2] avcodec/libx264: allow to disable definition of X264_API_IMPORTS macro
From: softworkz When MSVC is used, the definition of X264_API_IMPORTS is required for shared linking to libx264.dll, but it must not be defined in case of statically linking to libx264. Defining DISABLE_X264_API_IMPORTS allows to disable the definition of X264_API_IMPORTS for those cases. This has become necessary due to: https://code.videolan.org/videolan/x264/-/blob/ bfc87b7a330f75f5c9a21e56081e4b20344f139e/x264.h#L63-67 A better fix would eventually be this one: http://ffmpeg.org/pipermail/ffmpeg-devel/2021-October/287426.html But there has been disagreement and the issue stalled. This patch is intended to be a stop-gap solution until the mention fix will have been worked out. Signed-off-by: softworkz --- avcodec/libx264: don't define X264_API_IMPORTS when compiling static The definition of X264_API_IMPORTS is required for shared linking (when MSVC is used) but it must not be defined in case of static builds as is stated in x264.h: v2 use custom macro for control, so there's no mechanism imposed and no change as long as that macro isn't explicitly defined Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-29%2Fsoftworkz%2Fsubmit_x264_api_imports-v2 Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-29/softworkz/submit_x264_api_imports-v2 Pull-Request: https://github.com/ffstaging/FFmpeg/pull/29 Range-diff vs v1: 1: 640a17b84f ! 1: bc86a3e903 avcodec/libx264: don't define X264_API_IMPORTS when compiling static @@ Metadata Author: softworkz ## Commit message ## -avcodec/libx264: don't define X264_API_IMPORTS when compiling static +avcodec/libx264: allow to disable definition of X264_API_IMPORTS macro -The definition of X264_API_IMPORTS is required for shared linking -(when MSVC is used) but it must not be defined in case of static -builds as is stated in x264.h: +When MSVC is used, the definition of X264_API_IMPORTS is +required for shared linking to libx264.dll, but it must +not be defined in case of statically linking to libx264. + +Defining DISABLE_X264_API_IMPORTS allows to disable the +definition of X264_API_IMPORTS for those cases. + +This has become necessary due to: https://code.videolan.org/videolan/x264/-/blob/ bfc87b7a330f75f5c9a21e56081e4b20344f139e/x264.h#L63-67 -This commit adds a check for the definition of _LIB which indicates -static linking. +A better fix would eventually be this one: +http://ffmpeg.org/pipermail/ffmpeg-devel/2021-October/287426.html + +But there has been disagreement and the issue stalled. + +This patch is intended to be a stop-gap solution until +the mention fix will have been worked out. Signed-off-by: softworkz @@ libavcodec/libx264.c #include "sei.h" -#if defined(_MSC_VER) -+#if defined(_MSC_VER) && !defined(_LIB) ++#if defined(_MSC_VER) && !defined(DISABLE_X264_API_IMPORTS) #define X264_API_IMPORTS 1 #endif libavcodec/libx264.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c index 4ce3791ae8..adeaf0f52f 100644 --- a/libavcodec/libx264.c +++ b/libavcodec/libx264.c @@ -37,7 +37,7 @@ #include "atsc_a53.h" #include "sei.h" -#if defined(_MSC_VER) +#if defined(_MSC_VER) && !defined(DISABLE_X264_API_IMPORTS) #define X264_API_IMPORTS 1 #endif base-commit: 41a558fea06cc0a23b8d2d0dfb03ef6a25cf5100 -- ffmpeg-codebot ___ 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 v4] avutil/csp: create avpriv API for colorspace structs
Quoting James Almer (2022-05-20 13:53:35) > > > > 3. avpriv_ vs av_ > > avpriv is evil, it combines the pain of ABI/API compatibility while the > > public cant use it > > Not making it public allows us to not commit to a fixed design *now*. > Unless there's a clear need for this to be part of the public API, i > don't think it's a good idea to do so. This change is being made because > this API is afaict needed in lavc, and not by some project using lavu. The commit message claims it will be used in lavc and lavf. Seems to me like a strong sign that it really should be public. As far as I'm concerned, avpriv should not exist at all. -- 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".
[FFmpeg-devel] [PATCH 0/1] [WIP] avutil/csp changes
This patch is a work in progress example for swapping these structs from doubles to AVRationals. There's two main discussions here to be had - Is this API to be exposed as avpriv_ or av_? - Should these structs use AVRational or double values? I don't believe a consensus has been reached on this yet, but I've attached an AVRational version of it so we can see the pros/cons. Leo Izen (1): avutil/csp: create public API for colorspace structs libavfilter/colorspace.c| 143 libavfilter/colorspace.h| 31 +--- libavfilter/fflcms2.c | 25 --- libavfilter/fflcms2.h | 4 +- libavfilter/vf_colorspace.c | 37 +- libavfilter/vf_iccdetect.c | 5 +- libavfilter/vf_tonemap.c| 17 + libavutil/Makefile | 2 + libavutil/csp.c | 121 ++ libavutil/csp.h | 49 libavutil/version.h | 4 +- 11 files changed, 250 insertions(+), 188 deletions(-) create mode 100644 libavutil/csp.c create mode 100644 libavutil/csp.h -- 2.36.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 1/1] avutil/csp: create public API for colorspace structs
This commit moves some of the functionality from avfilter/colorspace
into avutil/csp and exposes it as a public API so it can be used by
libavcodec and/or libavformat. It also converts those structs from
double values to AVRational to make regression testing easier and
more consistent.
---
libavfilter/colorspace.c| 143
libavfilter/colorspace.h| 31 +---
libavfilter/fflcms2.c | 25 ---
libavfilter/fflcms2.h | 4 +-
libavfilter/vf_colorspace.c | 37 +-
libavfilter/vf_iccdetect.c | 5 +-
libavfilter/vf_tonemap.c| 17 +
libavutil/Makefile | 2 +
libavutil/csp.c | 121 ++
libavutil/csp.h | 49
libavutil/version.h | 4 +-
11 files changed, 250 insertions(+), 188 deletions(-)
create mode 100644 libavutil/csp.c
create mode 100644 libavutil/csp.h
diff --git a/libavfilter/colorspace.c b/libavfilter/colorspace.c
index 8d7b882375..3d125da6aa 100644
--- a/libavfilter/colorspace.c
+++ b/libavfilter/colorspace.c
@@ -65,24 +65,28 @@ void ff_matrix_mul_3x3(double dst[3][3],
/*
* see e.g. http://www.brucelindbloom.com/index.html?Eqn_RGB_XYZ_Matrix.html
*/
-void ff_fill_rgb2xyz_table(const struct PrimaryCoefficients *coeffs,
- const struct WhitepointCoefficients *wp,
+void ff_fill_rgb2xyz_table(const AVPrimaryCoefficients *coeffs,
+ const AVWhitepointCoefficients *wp,
double rgb2xyz[3][3])
{
double i[3][3], sr, sg, sb, zw;
-
-rgb2xyz[0][0] = coeffs->xr / coeffs->yr;
-rgb2xyz[0][1] = coeffs->xg / coeffs->yg;
-rgb2xyz[0][2] = coeffs->xb / coeffs->yb;
+double xr = av_q2d(coeffs->xr), yr = av_q2d(coeffs->yr);
+double xg = av_q2d(coeffs->xg), yg = av_q2d(coeffs->yg);
+double xb = av_q2d(coeffs->xb), yb = av_q2d(coeffs->yb);
+double xw = av_q2d(wp->xw), yw = av_q2d(wp->yw);
+
+rgb2xyz[0][0] = xr / yr;
+rgb2xyz[0][1] = xg / yg;
+rgb2xyz[0][2] = xb / yb;
rgb2xyz[1][0] = rgb2xyz[1][1] = rgb2xyz[1][2] = 1.0;
-rgb2xyz[2][0] = (1.0 - coeffs->xr - coeffs->yr) / coeffs->yr;
-rgb2xyz[2][1] = (1.0 - coeffs->xg - coeffs->yg) / coeffs->yg;
-rgb2xyz[2][2] = (1.0 - coeffs->xb - coeffs->yb) / coeffs->yb;
+rgb2xyz[2][0] = (1.0 - xr - yr) / yr;
+rgb2xyz[2][1] = (1.0 - xg - yg) / yg;
+rgb2xyz[2][2] = (1.0 - xb - yb) / yb;
ff_matrix_invert_3x3(rgb2xyz, i);
-zw = 1.0 - wp->xw - wp->yw;
-sr = i[0][0] * wp->xw + i[0][1] * wp->yw + i[0][2] * zw;
-sg = i[1][0] * wp->xw + i[1][1] * wp->yw + i[1][2] * zw;
-sb = i[2][0] * wp->xw + i[2][1] * wp->yw + i[2][2] * zw;
+zw = 1.0 - xw - yw;
+sr = i[0][0] * xw + i[0][1] * yw + i[0][2] * zw;
+sg = i[1][0] * xw + i[1][1] * yw + i[1][2] * zw;
+sb = i[2][0] * xw + i[2][1] * yw + i[2][2] * zw;
rgb2xyz[0][0] *= sr;
rgb2xyz[0][1] *= sg;
rgb2xyz[0][2] *= sb;
@@ -107,119 +111,32 @@ static const double gbr_matrix[3][3] =
{ 0.5, -0.5, 0 },
};
-/*
- * All constants explained in e.g.
https://linuxtv.org/downloads/v4l-dvb-apis/ch02s06.html
- * The older ones (bt470bg/m) are also explained in their respective ITU docs
- * (e.g.
https://www.itu.int/dms_pubrec/itu-r/rec/bt/R-REC-BT.470-5-199802-S!!PDF-E.pdf)
- * whereas the newer ones can typically be copied directly from wikipedia :)
- */
-static const struct LumaCoefficients luma_coefficients[AVCOL_SPC_NB] = {
-[AVCOL_SPC_FCC]= { 0.30, 0.59, 0.11 },
-[AVCOL_SPC_BT470BG]= { 0.299, 0.587, 0.114 },
-[AVCOL_SPC_SMPTE170M] = { 0.299, 0.587, 0.114 },
-[AVCOL_SPC_BT709] = { 0.2126, 0.7152, 0.0722 },
-[AVCOL_SPC_SMPTE240M] = { 0.212, 0.701, 0.087 },
-[AVCOL_SPC_YCOCG] = { 0.25, 0.5,0.25 },
-[AVCOL_SPC_RGB]= { 1, 1, 1 },
-[AVCOL_SPC_BT2020_NCL] = { 0.2627, 0.6780, 0.0593 },
-[AVCOL_SPC_BT2020_CL] = { 0.2627, 0.6780, 0.0593 },
-};
-
-const struct LumaCoefficients *ff_get_luma_coefficients(enum AVColorSpace csp)
-{
-const struct LumaCoefficients *coeffs;
-
-if (csp >= AVCOL_SPC_NB)
-return NULL;
-coeffs = &luma_coefficients[csp];
-if (!coeffs->cr)
-return NULL;
-
-return coeffs;
-}
-
-#define WP_D65 { 0.3127, 0.3290 }
-#define WP_C { 0.3100, 0.3160 }
-#define WP_DCI { 0.3140, 0.3510 }
-#define WP_E { 1/3.0f, 1/3.0f }
-
-static const struct ColorPrimaries color_primaries[AVCOL_PRI_NB] = {
-[AVCOL_PRI_BT709] = { WP_D65, { 0.640, 0.330, 0.300, 0.600, 0.150,
0.060 } },
-[AVCOL_PRI_BT470M]= { WP_C, { 0.670, 0.330, 0.210, 0.710, 0.140,
0.080 } },
-[AVCOL_PRI_BT470BG] = { WP_D65, { 0.640, 0.330, 0.290, 0.600, 0.150,
0.060 } },
-[AVCOL_PRI_SMPTE170M] = { WP_D65, { 0.630, 0.340, 0.310, 0.595, 0.155,
0.070 } },
-[AVCOL_PRI_SMPTE240M] = { WP_D65, { 0.630, 0.340, 0.310, 0.595, 0.155,
0.070 } },
-[AVCOL_PRI_SMPTE428] =
Re: [FFmpeg-devel] [PATCH 1/1] avutil/csp: create public API for colorspace structs
Leo Izen:
> This commit moves some of the functionality from avfilter/colorspace
> into avutil/csp and exposes it as a public API so it can be used by
> libavcodec and/or libavformat. It also converts those structs from
> double values to AVRational to make regression testing easier and
> more consistent.
> ---
> libavfilter/colorspace.c| 143
> libavfilter/colorspace.h| 31 +---
> libavfilter/fflcms2.c | 25 ---
> libavfilter/fflcms2.h | 4 +-
> libavfilter/vf_colorspace.c | 37 +-
> libavfilter/vf_iccdetect.c | 5 +-
> libavfilter/vf_tonemap.c| 17 +
> libavutil/Makefile | 2 +
> libavutil/csp.c | 121 ++
> libavutil/csp.h | 49
> libavutil/version.h | 4 +-
> 11 files changed, 250 insertions(+), 188 deletions(-)
> create mode 100644 libavutil/csp.c
> create mode 100644 libavutil/csp.h
>
> diff --git a/libavfilter/colorspace.c b/libavfilter/colorspace.c
> index 8d7b882375..3d125da6aa 100644
> --- a/libavfilter/colorspace.c
> +++ b/libavfilter/colorspace.c
> @@ -65,24 +65,28 @@ void ff_matrix_mul_3x3(double dst[3][3],
> /*
> * see e.g. http://www.brucelindbloom.com/index.html?Eqn_RGB_XYZ_Matrix.html
> */
> -void ff_fill_rgb2xyz_table(const struct PrimaryCoefficients *coeffs,
> - const struct WhitepointCoefficients *wp,
> +void ff_fill_rgb2xyz_table(const AVPrimaryCoefficients *coeffs,
> + const AVWhitepointCoefficients *wp,
> double rgb2xyz[3][3])
> {
> double i[3][3], sr, sg, sb, zw;
> -
> -rgb2xyz[0][0] = coeffs->xr / coeffs->yr;
> -rgb2xyz[0][1] = coeffs->xg / coeffs->yg;
> -rgb2xyz[0][2] = coeffs->xb / coeffs->yb;
> +double xr = av_q2d(coeffs->xr), yr = av_q2d(coeffs->yr);
> +double xg = av_q2d(coeffs->xg), yg = av_q2d(coeffs->yg);
> +double xb = av_q2d(coeffs->xb), yb = av_q2d(coeffs->yb);
> +double xw = av_q2d(wp->xw), yw = av_q2d(wp->yw);
> +
> +rgb2xyz[0][0] = xr / yr;
> +rgb2xyz[0][1] = xg / yg;
> +rgb2xyz[0][2] = xb / yb;
> rgb2xyz[1][0] = rgb2xyz[1][1] = rgb2xyz[1][2] = 1.0;
> -rgb2xyz[2][0] = (1.0 - coeffs->xr - coeffs->yr) / coeffs->yr;
> -rgb2xyz[2][1] = (1.0 - coeffs->xg - coeffs->yg) / coeffs->yg;
> -rgb2xyz[2][2] = (1.0 - coeffs->xb - coeffs->yb) / coeffs->yb;
> +rgb2xyz[2][0] = (1.0 - xr - yr) / yr;
> +rgb2xyz[2][1] = (1.0 - xg - yg) / yg;
> +rgb2xyz[2][2] = (1.0 - xb - yb) / yb;
> ff_matrix_invert_3x3(rgb2xyz, i);
> -zw = 1.0 - wp->xw - wp->yw;
> -sr = i[0][0] * wp->xw + i[0][1] * wp->yw + i[0][2] * zw;
> -sg = i[1][0] * wp->xw + i[1][1] * wp->yw + i[1][2] * zw;
> -sb = i[2][0] * wp->xw + i[2][1] * wp->yw + i[2][2] * zw;
> +zw = 1.0 - xw - yw;
> +sr = i[0][0] * xw + i[0][1] * yw + i[0][2] * zw;
> +sg = i[1][0] * xw + i[1][1] * yw + i[1][2] * zw;
> +sb = i[2][0] * xw + i[2][1] * yw + i[2][2] * zw;
> rgb2xyz[0][0] *= sr;
> rgb2xyz[0][1] *= sg;
> rgb2xyz[0][2] *= sb;
> @@ -107,119 +111,32 @@ static const double gbr_matrix[3][3] =
> { 0.5, -0.5, 0 },
> };
>
> -/*
> - * All constants explained in e.g.
> https://linuxtv.org/downloads/v4l-dvb-apis/ch02s06.html
> - * The older ones (bt470bg/m) are also explained in their respective ITU docs
> - * (e.g.
> https://www.itu.int/dms_pubrec/itu-r/rec/bt/R-REC-BT.470-5-199802-S!!PDF-E.pdf)
> - * whereas the newer ones can typically be copied directly from wikipedia :)
> - */
> -static const struct LumaCoefficients luma_coefficients[AVCOL_SPC_NB] = {
> -[AVCOL_SPC_FCC]= { 0.30, 0.59, 0.11 },
> -[AVCOL_SPC_BT470BG]= { 0.299, 0.587, 0.114 },
> -[AVCOL_SPC_SMPTE170M] = { 0.299, 0.587, 0.114 },
> -[AVCOL_SPC_BT709] = { 0.2126, 0.7152, 0.0722 },
> -[AVCOL_SPC_SMPTE240M] = { 0.212, 0.701, 0.087 },
> -[AVCOL_SPC_YCOCG] = { 0.25, 0.5,0.25 },
> -[AVCOL_SPC_RGB]= { 1, 1, 1 },
> -[AVCOL_SPC_BT2020_NCL] = { 0.2627, 0.6780, 0.0593 },
> -[AVCOL_SPC_BT2020_CL] = { 0.2627, 0.6780, 0.0593 },
> -};
> -
> -const struct LumaCoefficients *ff_get_luma_coefficients(enum AVColorSpace
> csp)
> -{
> -const struct LumaCoefficients *coeffs;
> -
> -if (csp >= AVCOL_SPC_NB)
> -return NULL;
> -coeffs = &luma_coefficients[csp];
> -if (!coeffs->cr)
> -return NULL;
> -
> -return coeffs;
> -}
> -
> -#define WP_D65 { 0.3127, 0.3290 }
> -#define WP_C { 0.3100, 0.3160 }
> -#define WP_DCI { 0.3140, 0.3510 }
> -#define WP_E { 1/3.0f, 1/3.0f }
> -
> -static const struct ColorPrimaries color_primaries[AVCOL_PRI_NB] = {
> -[AVCOL_PRI_BT709] = { WP_D65, { 0.640, 0.330, 0.300, 0.600, 0.150,
> 0.060 } },
> -[AVCOL_PRI_BT470M]= { WP_C, { 0.670, 0.330, 0.210, 0.710, 0.140,
> 0.080 } },
> -[AVCOL_PRI_BT470BG] = { WP_D65, { 0.640, 0.
Re: [FFmpeg-devel] [PATCH v4] avutil/csp: create avpriv API for colorspace structs
On 5/20/22 11:50, Anton Khirnov wrote: The commit message claims it will be used in lavc and lavf. Seems to me like a strong sign that it really should be public. As far as I'm concerned, avpriv should not exist at all. The intention of this patch is a precursor to using it in avcodec. Specifically I intend to call this API so I can tell the libjxl encoder what primaries/etc. to use when it converts the pixel data to XYB internally. I don't know if it will be used in avformat, but it could be if needed. - Leo Izen (thebombzen) ___ 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/1] avutil/csp: create public API for colorspace structs
On 5/20/22 12:01, Andreas Rheinhardt wrote:
Leo Izen:
This commit moves some of the functionality from avfilter/colorspace
into avutil/csp and exposes it as a public API so it can be used by
libavcodec and/or libavformat. It also converts those structs from
double values to AVRational to make regression testing easier and
more consistent.
---
+
+#include
+
+#include "attributes.h"
+#include "csp.h"
+#include "pixfmt.h"
+#include "rational.h"
+
+#define AVR(d) { (int)(d * 3), 3 }
Does this really lead to the intended values? After all, the cast does
not round to nearest, instead it just discards the fractional part (i.e.
rounds towards zero). You should probably use (int)(d * 3 + 0.5) to
compensate for that.
I could change it to do that. That said, I modeled this after the FIX
macro in libavcodec/mpegaudio.h on line 60, which doesn't do that. Is
that macro also incorrect, or is there a caveat here that makes these
scenarios different?
- Leo Izen (thebombzen)
___
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/libx264: allow to disable definition of X264_API_IMPORTS macro
On 5/20/2022 4:23 PM, softworkz wrote: > A better fix would eventually be this one: > http://ffmpeg.org/pipermail/ffmpeg-devel/2021-October/287426.html > > But there has been disagreement and the issue stalled. I did not see a single person disagree with Matt's patch? If someone did, I missed it. - Derek ___ 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 v4] avutil/csp: create avpriv API for colorspace structs
On Fri, 20 May 2022 08:53:35 -0300 James Almer wrote:
>
>
> On 5/20/2022 7:28 AM, Michael Niedermayer wrote:
> > On Wed, May 18, 2022 at 08:27:48PM +0200, Michael Niedermayer wrote:
> >> On Wed, May 18, 2022 at 08:23:38PM +0200, Michael Niedermayer wrote:
> >>> On Wed, May 18, 2022 at 11:18:17AM -0400, Leo Izen wrote:
> This commit moves some of the functionality from avfilter/colorspace
> into avutil/csp and exposes it as an avpriv API so it can be used by
> libavcodec and/or libavformat.
> >>> [...]
> +#ifndef AVUTIL_CSP_H
> +#define AVUTIL_CSP_H
> +
> +#include "libavutil/pixfmt.h"
> +
> +typedef struct AVLumaCoefficients {
> +double cr, cg, cb;
> +} AVLumaCoefficients;
> +
> +typedef struct AVPrimaryCoefficients {
> +double xr, yr, xg, yg, xb, yb;
> +} AVPrimaryCoefficients;
> +
> +typedef struct AVWhitepointCoefficients {
> +double xw, yw;
> +} AVWhitepointCoefficients;
> >>>
> >>> As said, these should not be floating point.
> >>> Adding a new public API and changing it later is messy, this
> >>> should be changed before its made public
> >>
> >> i now see you replaced some public by avpriv in the latest patch
> >> but still i think this should be changed to fixed point or AVRational
> >> first. Even as API between the libs its messy to change it later
> >> it would require us to keep the double API when its changed until
> >> the next major bump
> >
> > I see some discussion related to this on the IRC log from when i was
> > sleeping. Maybe it would be better to keep this on the mailing list
> >
> > Also iam not sure my concern was clearly worded so ill sort my argument
> > and concerns so its clearer below:
> >
> > 1. exactly representing values
> > if you have a 0.1 you can represent that exactly as AVRational 1/10 but
> > maybe shockingly a double cannot.
> >
> > Try a printf %f of 0.1 and it will do 0.10 looks good but thats
> > deception
> > try that with more precission %100.99f shows this:
> >
> > 0.155511151231257827021181583404541015625
> >
> > so if we want to use exactly the values from the spec, doubles with a
> > unit/base of 1 do not work.
> > int or even doubles with a base/unit of 3 might work exactly if
> > AVRational is unpopular. 3 instead of 1 is for that one pesky
> > 1/3
> >
> > 1b. the exact value that 0.1 has in float/double depends on the precission
> > IEEE float
> >
> > 0.10001490116119384765625
> > IEEE double
> >
> > 0.155511151231257827021181583404541015625
> > long double
> >
> > 0.100013552527156068805425093160010874271392822265625
> > So there will be slight differences if (intermediate) types anywhere
> > arent
> > exactly the same
> >
> > 2. someone said, you need to pick a denominator when doing float -> rational
> > av_d2q() will pick the best denominator for you.
> >
> > 3. avpriv_ vs av_
> > avpriv is evil, it combines the pain of ABI/API compatibility while the
> > public cant use it
>
> Not making it public allows us to not commit to a fixed design *now*.
> Unless there's a clear need for this to be part of the public API, i
> don't think it's a good idea to do so. This change is being made because
> this API is afaict needed in lavc, and not by some project using lavu.
I see it being useful outside FFmpeg.
>
> Removing/changing an avpriv symbol or struct is a matter or waiting for
> the nearest major bump. No need for an arbitrary 2 year wait period,
> compat wrappers, or anything crazy.
>
> >
> > 4. rounding, regressions and inexactness
> > doubles/floats have in the past broken regression tests. They do not
> > always but i suggest we avoid them when theres no clear advantage
> > like higher speed in speed relevant code or much simpler code
> >
> > thx
> >
> >
> > [...]
> >
> >
> > ___
> > 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...@ffm
Re: [FFmpeg-devel] [PATCH v2] avcodec/libx264: allow to disable definition of X264_API_IMPORTS macro
On Fri, May 20, 2022 at 6:23 PM Derek Buitenhuis wrote: > > On 5/20/2022 4:23 PM, softworkz wrote: > > A better fix would eventually be this one: > > http://ffmpeg.org/pipermail/ffmpeg-devel/2021-October/287426.html > > > > But there has been disagreement and the issue stalled. > > I did not see a single person disagree with Matt's patch? If someone did, > I missed it. > Me neither, it just didn't seem to get any feedback and was overlooked. Using pkg-config and letting CFLAGS control this through the appropriate define x264 itself created for it is the far better solution then inventing your own that noone knows about (soon we have one for every library then, where does it end?). - 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 v2] avcodec/libx264: allow to disable definition of X264_API_IMPORTS macro
> -Original Message- > From: ffmpeg-devel On Behalf Of > Derek Buitenhuis > Sent: Friday, May 20, 2022 6:23 PM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH v2] avcodec/libx264: allow to > disable definition of X264_API_IMPORTS macro > > On 5/20/2022 4:23 PM, softworkz wrote: > > A better fix would eventually be this one: > > http://ffmpeg.org/pipermail/ffmpeg-devel/2021-October/287426.html > > > > But there has been disagreement and the issue stalled. > > I did not see a single person disagree with Matt's patch? If someone > did, > I missed it. It continued here: https://master.gitmailbox.com/ffmpegdev/CAHVN4mhatDXNUe+=Z+TXfhyQB=avpzocphzthhrzt4ismrp...@mail.gmail.com/ But if Matt's patch would be agreeable, then that would surely be the best outcome. I can rebase and resubmit his patch if you would find it agreeable. Thanks, softworkz ___ 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/libx264: allow to disable definition of X264_API_IMPORTS macro
On 5/20/2022 5:37 PM, Soft Works wrote: > But if Matt's patch would be agreeable, then that would surely be > the best outcome. > > I can rebase and resubmit his patch if you would find it agreeable. Ah - that was not clear to me. If Ubuntu LTS does indeed ship such an old x264, the fallback discussed there may be indeed needed, but it's pretty simple to add. It does seem a tad silly given that the define only affects Windows, but I get where Michael is coming from. I think Matt's patch + fallback for older versions seems reasonable, if, in fact needed. Ideally we'd just drop support for older x264, though. Not sure which most people would prefer. - Derek ___ 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/libx264: allow to disable definition of X264_API_IMPORTS macro
On Fri, 20 May 2022, Derek Buitenhuis wrote: On 5/20/2022 5:37 PM, Soft Works wrote: But if Matt's patch would be agreeable, then that would surely be the best outcome. I can rebase and resubmit his patch if you would find it agreeable. Ah - that was not clear to me. If Ubuntu LTS does indeed ship such an old x264, the fallback discussed there may be indeed needed, but it's pretty simple to add. It does seem a tad silly given that the define only affects Windows, but I get where Michael is coming from. I think Matt's patch + fallback for older versions seems reasonable, if, in fact needed. Ideally we'd just drop support for older x264, though. Not sure which most people would prefer. Maybe just drop support for older versions when on Windows? That should cover those cases (even if ffmpeg is built with msvc but x264 with mingw, or vice versa) quite well, while still not bothering other platforms at all. // Martin ___ 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/libx264: allow to disable definition of X264_API_IMPORTS macro
On 5/20/2022 5:51 PM, Martin Storsjö wrote: > Maybe just drop support for older versions when on Windows? That should > cover those cases (even if ffmpeg is built with msvc but x264 with mingw, > or vice versa) quite well, while still not bothering other platforms at > all. Yeah, that seems reasonable to me. - Derek ___ 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/1] avutil/csp: create public API for colorspace structs
Leo Izen:
> On 5/20/22 12:01, Andreas Rheinhardt wrote:
>> Leo Izen:
>>> This commit moves some of the functionality from avfilter/colorspace
>>> into avutil/csp and exposes it as a public API so it can be used by
>>> libavcodec and/or libavformat. It also converts those structs from
>>> double values to AVRational to make regression testing easier and
>>> more consistent.
>>> ---
>>> +
>>> +#include
>>> +
>>> +#include "attributes.h"
>>> +#include "csp.h"
>>> +#include "pixfmt.h"
>>> +#include "rational.h"
>>> +
>>> +#define AVR(d) { (int)(d * 3), 3 }
>>
>> Does this really lead to the intended values? After all, the cast does
>> not round to nearest, instead it just discards the fractional part (i.e.
>> rounds towards zero). You should probably use (int)(d * 3 + 0.5) to
>> compensate for that.
>>
>
> I could change it to do that. That said, I modeled this after the FIX
> macro in libavcodec/mpegaudio.h on line 60, which doesn't do that. Is
> that macro also incorrect, or is there a caveat here that makes these
> scenarios different?
>
The values for mpegaudio.h are surely not rounded to nearest. I don't
know whether this is intended or not, but AFAIK the codecs using this
are not intended to be bitexact (as in: the concept of bitexactness does
not apply to these codecs because the output of a decoder is not exactly
defined anyway).
While some values are indeed rounded down when converting to double,
multiplying by 3 seems to produce the intended values somehow. I
would nevertheless add 0.5.
- 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 v4] avutil/csp: create avpriv API for colorspace structs
On Fri, 20 May 2022 12:28:15 +0200 Michael Niedermayer
wrote:
> On Wed, May 18, 2022 at 08:27:48PM +0200, Michael Niedermayer wrote:
> > On Wed, May 18, 2022 at 08:23:38PM +0200, Michael Niedermayer wrote:
> > > On Wed, May 18, 2022 at 11:18:17AM -0400, Leo Izen wrote:
> > > > This commit moves some of the functionality from avfilter/colorspace
> > > > into avutil/csp and exposes it as an avpriv API so it can be used by
> > > > libavcodec and/or libavformat.
> > > [...]
> > > > +#ifndef AVUTIL_CSP_H
> > > > +#define AVUTIL_CSP_H
> > > > +
> > > > +#include "libavutil/pixfmt.h"
> > > > +
> > > > +typedef struct AVLumaCoefficients {
> > > > +double cr, cg, cb;
> > > > +} AVLumaCoefficients;
> > > > +
> > > > +typedef struct AVPrimaryCoefficients {
> > > > +double xr, yr, xg, yg, xb, yb;
> > > > +} AVPrimaryCoefficients;
> > > > +
> > > > +typedef struct AVWhitepointCoefficients {
> > > > +double xw, yw;
> > > > +} AVWhitepointCoefficients;
> > >
> > > As said, these should not be floating point.
> > > Adding a new public API and changing it later is messy, this
> > > should be changed before its made public
> >
> > i now see you replaced some public by avpriv in the latest patch
> > but still i think this should be changed to fixed point or AVRational
> > first. Even as API between the libs its messy to change it later
> > it would require us to keep the double API when its changed until
> > the next major bump
>
> I see some discussion related to this on the IRC log from when i was
> sleeping. Maybe it would be better to keep this on the mailing list
>
> Also iam not sure my concern was clearly worded so ill sort my argument
> and concerns so its clearer below:
>
> 1. exactly representing values
> if you have a 0.1 you can represent that exactly as AVRational 1/10 but
> maybe shockingly a double cannot.
>
> Try a printf %f of 0.1 and it will do 0.10 looks good but thats
> deception
> try that with more precission %100.99f shows this:
>
> 0.155511151231257827021181583404541015625
>
> so if we want to use exactly the values from the spec, doubles with a
> unit/base of 1 do not work.
> int or even doubles with a base/unit of 3 might work exactly if
> AVRational is unpopular. 3 instead of 1 is for that one pesky 1/3
>
> 1b. the exact value that 0.1 has in float/double depends on the precission
> IEEE float
>
> 0.10001490116119384765625
> IEEE double
>
> 0.155511151231257827021181583404541015625
> long double
>
> 0.100013552527156068805425093160010874271392822265625
> So there will be slight differences if (intermediate) types anywhere arent
> exactly the same
It's worth pointing out that these values are already, in many cases,
arbitrarily rounded. D65, for example, has many different definitions:
- ISO/CIE provide a version rounded to 6 decimal digits but also
provides the full tabulated spectral curves (sampled at 10nm
intervals) from which you can derive a higher precision version
- ITU-R always rounds to 4 digits, but newer ITU-R standards (e.g.
BT.2100) reference the aforementioned ISO document for the definition,
making it technically ambiguous.
- Older EBU documents even round to 3 digits
- There exist other sources that round to 5 digits also
In my mind, this weakens somewhat the case of making sure these values
have exact representations, if the underlying physical constant being
represented does not have an exact decimal representation to begin with.
>
> 2. someone said, you need to pick a denominator when doing float -> rational
> av_d2q() will pick the best denominator for you.
>
> 3. avpriv_ vs av_
> avpriv is evil, it combines the pain of ABI/API compatibility while the
> public cant use it
>
> 4. rounding, regressions and inexactness
> doubles/floats have in the past broken regression tests. They do not
> always but i suggest we avoid them when theres no clear advantage
> like higher speed in speed relevant code or much simpler code
>
> thx
>
>
> [...]
>
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> The day soldiers stop bringing you their problems is the day you have stopped
> leading them. They have either lost confidence that you can help or concluded
> you do not care. Either case is a failure of leadership. - Colin Powell
> ___
> 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
Re: [FFmpeg-devel] [PATCH v2] avcodec/libx264: allow to disable definition of X264_API_IMPORTS macro
> -Original Message-
> From: ffmpeg-devel On Behalf Of
> Derek Buitenhuis
> Sent: Friday, May 20, 2022 6:55 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v2] avcodec/libx264: allow to
> disable definition of X264_API_IMPORTS macro
>
> On 5/20/2022 5:51 PM, Martin Storsjö wrote:
> > Maybe just drop support for older versions when on Windows? That
> should
> > cover those cases (even if ffmpeg is built with msvc but x264 with
> mingw,
> > or vice versa) quite well, while still not bothering other platforms
> at
> > all.
>
> Yeah, that seems reasonable to me.
To me as well!
This is the code in configure after (fixing and) applying Matt's patch:
enabled libwebp && {
enabled libwebp_encoder && require_pkg_config libwebp "libwebp >=
0.2.0" webp/encode.h WebPGetEncoderVersion
enabled libwebp_anim_encoder && check_pkg_config libwebp_anim_encoder
"libwebpmux >= 0.4.0" webp/mux.h WebPAnimEncoderOptionsInit; }
enabled libx264 && check_pkg_config libx264 x264 "stdint.h x264.h"
x264_encoder_encode &&
require_cpp_condition libx264 x264.h "X264_BUILD
>= 158"
enabled libx265 && require_pkg_config libx265 x265 x265.h
x265_api_get &&
require_cpp_condition libx265 x265.h "X265_BUILD
>= 70"
How would the check for this need to look like when we want to require version
158 only in
case of msvc compilation?
And what should be the minimum for non-MSVC builds? Keep 118 like it was?
Thanks,
sw
___
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 1/2] avutil/wchar_filename, file_open: Support long file names on Windows
> Yes, that's true. But there are hundreds of other things someone could > define which makes compilation fail. Doesn't mean that yet another such thing should be added by incorrectly redefining structs already defined correctly by system headers. > Probably you didn't spot it. It's already there: > # define stat win32_stat I'm actually wondering how does it even compile. All stat structs in code become struct win32_stat, and all calls to stat become calls to win32_stat, which in turn wraps _wstati64. But _wstati64 does not accept struct win32_stat*, it accepts struct _stati64*. Content of these structs is probably identical, but it should not matter: C is typed nominally, not structurally. > I don't want to say that I'd consider this to be a great solution. > But the problem is that the function and struct names are identical > and when we want to re-define/map the function, we also need to > provide a struct of that name because the macro-replacement can't > work selectively. Doesn't mean that the should be named identically in FFmpeg code. Naming a struct stat and a function avpriv_stat is a reasonable choice. You can even define avpriv_stat with parameters the way fstat is defined: #ifdef _WIN32 #define avpriv_stat(f,s) win32_stat((f), (s)) #else #define avpriv_stat(f,s) stat((f), (s)) #endif ___ 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 1/2] avutil/wchar_filename, file_open: Support long file names on Windows
> -Original Message- > From: ffmpeg-devel On Behalf Of nil- > admir...@mailo.com > Sent: Friday, May 20, 2022 7:52 PM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH v2 1/2] avutil/wchar_filename, > file_open: Support long file names on Windows > > > Yes, that's true. But there are hundreds of other things someone could > > define which makes compilation fail. > > Doesn't mean that yet another such thing should be added by incorrectly > redefining structs already defined correctly by system headers. > > > Probably you didn't spot it. It's already there: > > # define stat win32_stat > > I'm actually wondering how does it even compile. All stat structs in code > become struct win32_stat, and all calls to stat become calls to > win32_stat, > which in turn wraps _wstati64. But _wstati64 does not accept struct > win32_stat*, > it accepts struct _stati64*. Content of these structs is probably > identical, but > it should not matter: C is typed nominally, not structurally. > > > I don't want to say that I'd consider this to be a great solution. > > But the problem is that the function and struct names are identical > > and when we want to re-define/map the function, we also need to > > provide a struct of that name because the macro-replacement can't > > work selectively. > > Doesn't mean that the should be named identically in FFmpeg code. > Naming a struct stat and a function avpriv_stat is a reasonable choice. > You can even define avpriv_stat with parameters the way fstat is defined: > > #ifdef _WIN32 > #define avpriv_stat(f,s) win32_stat((f), (s)) > #else > #define avpriv_stat(f,s) stat((f), (s)) > #endif I thought the purpose of all those re-mappings would be that plain Posix functions can still be used..? It's already the Posix declaration where the function name is the same as the structure name (stat). I'm unbiased. avpriv_stat() would be ok from my POV as well, but I'm not sure whether all others would agree when plain stat could no longer be used (without breaking long filename support)? Thanks, softworkz ___ 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] opt_common: note D and T type streams for completeness.
On 2022-05-20 12:16 pm, Paul B Mahol wrote: lgtm Pushed as 9ab20b1614194280b862d98dfcdb7b1bcff03329 Thanks, 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 v2] avcodec/mfenc: Dynamically load MFPlat.DLL
Allow builds of FFmpeg with MediaFoundation to work under N editions of
Windows which are without MediaFoundation by default.
Signed-off-by: Trystan Mata
---
configure | 4 +-
libavcodec/mf_utils.c | 26 +++--
libavcodec/mf_utils.h | 16 ++--
libavcodec/mfenc.c| 91 ---
4 files changed, 107 insertions(+), 30 deletions(-)
diff --git a/configure b/configure
index f115b21064..f16fbb320a 100755
--- a/configure
+++ b/configure
@@ -3129,8 +3129,8 @@ wmv3_vaapi_hwaccel_select="vc1_vaapi_hwaccel"
wmv3_vdpau_hwaccel_select="vc1_vdpau_hwaccel"
# hardware-accelerated codecs
-mediafoundation_deps="mftransform_h MFCreateAlignedMemoryBuffer"
-mediafoundation_extralibs="-lmfplat -lmfuuid -lole32 -lstrmiids"
+mediafoundation_deps="LoadLibrary mftransform_h
MFCreateAlignedMemoryBuffer"
+mediafoundation_extralibs="-lole32 -lstrmiids"
omx_deps="libdl pthreads"
omx_rpi_select="omx"
qsv_deps="libmfx"
diff --git a/libavcodec/mf_utils.c b/libavcodec/mf_utils.c
index eeabd0ce0b..9a83f71692 100644
--- a/libavcodec/mf_utils.c
+++ b/libavcodec/mf_utils.c
@@ -106,19 +106,20 @@ char *ff_hr_str_buf(char *buf, size_t size,
HRESULT hr)
// If fill_data!=NULL, initialize the buffer and set the length. (This
is a
// subtle but important difference: some decoders want CurrentLength==0 on
// provided output buffers.)
-IMFSample *ff_create_memory_sample(void *fill_data, size_t size, size_t
align)
+IMFSample *ff_create_memory_sample(const MFSymbols *symbols, void
*fill_data,
+ size_t size, size_t align)
{
HRESULT hr;
IMFSample *sample;
IMFMediaBuffer *buffer;
-hr = MFCreateSample(&sample);
+hr = symbols->MFCreateSample(&sample);
if (FAILED(hr))
return NULL;
align = FFMAX(align, 16); // 16 is "recommended", even if not
required
-hr = MFCreateAlignedMemoryBuffer(size, align - 1, &buffer);
+hr = symbols->MFCreateAlignedMemoryBuffer(size, align - 1, &buffer);
if (FAILED(hr))
return NULL;
@@ -548,7 +549,7 @@ const CLSID *ff_codec_to_mf_subtype(enum AVCodecID
codec)
}
}
-static int init_com_mf(void *log)
+static int init_com_mf(const MFSymbols *symbols, void *log)
{
HRESULT hr;
@@ -561,7 +562,7 @@ static int init_com_mf(void *log)
return AVERROR(ENOSYS);
}
-hr = MFStartup(MF_VERSION, MFSTARTUP_FULL);
+hr = symbols->MFStartup(MF_VERSION, MFSTARTUP_FULL);
if (FAILED(hr)) {
av_log(log, AV_LOG_ERROR, "could not initialize
MediaFoundation\n");
CoUninitialize();
@@ -571,15 +572,16 @@ static int init_com_mf(void *log)
return 0;
}
-static void uninit_com_mf(void)
+static void uninit_com_mf(const MFSymbols *symbols)
{
-MFShutdown();
+symbols->MFShutdown();
CoUninitialize();
}
// Find and create a IMFTransform with the given input/output types.
When done,
// you should use ff_free_mf() to destroy it, which will also uninit COM.
-int ff_instantiate_mf(void *log,
+int ff_instantiate_mf(const MFSymbols *symbols,
+ void *log,
GUID category,
MFT_REGISTER_TYPE_INFO *in_type,
MFT_REGISTER_TYPE_INFO *out_type,
@@ -594,7 +596,7 @@ int ff_instantiate_mf(void *log,
IMFActivate *winner = 0;
UINT32 flags;
-ret = init_com_mf(log);
+ret = init_com_mf(symbols, log);
if (ret < 0)
return ret;
@@ -667,14 +669,14 @@ int ff_instantiate_mf(void *log,
return 0;
error_uninit_mf:
-uninit_com_mf();
+uninit_com_mf(symbols);
return AVERROR(ENOSYS);
}
-void ff_free_mf(IMFTransform **mft)
+void ff_free_mf(const MFSymbols *symbols, IMFTransform **mft)
{
if (*mft)
IMFTransform_Release(*mft);
*mft = NULL;
-uninit_com_mf();
+uninit_com_mf(symbols);
}
diff --git a/libavcodec/mf_utils.h b/libavcodec/mf_utils.h
index d514723c3b..1b1041bb29 100644
--- a/libavcodec/mf_utils.h
+++ b/libavcodec/mf_utils.h
@@ -41,6 +41,15 @@
#include "avcodec.h"
+typedef struct MFSymbols {
+HRESULT (*MFCreateSample) (IMFSample **ppIMFSample);
+HRESULT (*MFCreateAlignedMemoryBuffer) (DWORD cbMaxLength, DWORD
cbAligment,
+IMFMediaBuffer **ppBuffer);
+HRESULT (*MFStartup) (ULONG Version, DWORD dwFlags);
+HRESULT (*MFShutdown) (void);
+HRESULT (*MFCreateMediaType) (IMFMediaType **ppMFType);
+} MFSymbols;
+
// These functions do exist in mfapi.h, but are only available within
// __cplusplus ifdefs.
HRESULT ff_MFGetAttributeSize(IMFAttributes *pattr, REFGUID guid,
@@ -150,7 +159,8 @@ char *ff_hr_str_buf(char *buf, size_t size, HRESULT hr);
#define FF_VAL_VT_UI4(v) FF_VARIANT_VALUE(VT_UI4, .ulVal = (v))
#define FF_VAL_VT_BOOL(v) FF_VARIANT_VALUE(VT_BOOL, .boolVal = (v))
-IMFSample *ff_create_memory_sample(void *fill_data, size_t size,
size_t align);
+IMFSample *ff_create_memory_sample(const MFSymb
[FFmpeg-devel] [PATCH v3] avcodec/mfenc: Dynamically load MFPlat.DLL
From 2bdef1bdb93efa40b7d3fe21270f9f23465bee90 Mon Sep 17 00:00:00 2001
From: Trystan Mata
Date: Fri, 20 May 2022 14:26:49 +0200
Subject: [PATCH] avcodec/mfenc: Dynamically load MFPlat.DLL
Allow builds of FFmpeg with MediaFoundation to work under N editions of
Windows which are without MediaFoundation by default.
Signed-off-by: Trystan Mata
---
configure | 4 +-
libavcodec/mf_utils.c | 26 +++--
libavcodec/mf_utils.h | 16 ++--
libavcodec/mfenc.c| 91 ---
4 files changed, 107 insertions(+), 30 deletions(-)
diff --git a/configure b/configure
index f115b21064..f16fbb320a 100755
--- a/configure
+++ b/configure
@@ -3129,8 +3129,8 @@ wmv3_vaapi_hwaccel_select="vc1_vaapi_hwaccel"
wmv3_vdpau_hwaccel_select="vc1_vdpau_hwaccel"
# hardware-accelerated codecs
-mediafoundation_deps="mftransform_h MFCreateAlignedMemoryBuffer"
-mediafoundation_extralibs="-lmfplat -lmfuuid -lole32 -lstrmiids"
+mediafoundation_deps="LoadLibrary mftransform_h MFCreateAlignedMemoryBuffer"
+mediafoundation_extralibs="-lole32 -lstrmiids"
omx_deps="libdl pthreads"
omx_rpi_select="omx"
qsv_deps="libmfx"
diff --git a/libavcodec/mf_utils.c b/libavcodec/mf_utils.c
index eeabd0ce0b..9a83f71692 100644
--- a/libavcodec/mf_utils.c
+++ b/libavcodec/mf_utils.c
@@ -106,19 +106,20 @@ char *ff_hr_str_buf(char *buf, size_t size, HRESULT hr)
// If fill_data!=NULL, initialize the buffer and set the length. (This is a
// subtle but important difference: some decoders want CurrentLength==0 on
// provided output buffers.)
-IMFSample *ff_create_memory_sample(void *fill_data, size_t size, size_t align)
+IMFSample *ff_create_memory_sample(const MFSymbols *symbols, void *fill_data,
+ size_t size, size_t align)
{
HRESULT hr;
IMFSample *sample;
IMFMediaBuffer *buffer;
-hr = MFCreateSample(&sample);
+hr = symbols->MFCreateSample(&sample);
if (FAILED(hr))
return NULL;
align = FFMAX(align, 16); // 16 is "recommended", even if not required
-hr = MFCreateAlignedMemoryBuffer(size, align - 1, &buffer);
+hr = symbols->MFCreateAlignedMemoryBuffer(size, align - 1, &buffer);
if (FAILED(hr))
return NULL;
@@ -548,7 +549,7 @@ const CLSID *ff_codec_to_mf_subtype(enum AVCodecID codec)
}
}
-static int init_com_mf(void *log)
+static int init_com_mf(const MFSymbols *symbols, void *log)
{
HRESULT hr;
@@ -561,7 +562,7 @@ static int init_com_mf(void *log)
return AVERROR(ENOSYS);
}
-hr = MFStartup(MF_VERSION, MFSTARTUP_FULL);
+hr = symbols->MFStartup(MF_VERSION, MFSTARTUP_FULL);
if (FAILED(hr)) {
av_log(log, AV_LOG_ERROR, "could not initialize MediaFoundation\n");
CoUninitialize();
@@ -571,15 +572,16 @@ static int init_com_mf(void *log)
return 0;
}
-static void uninit_com_mf(void)
+static void uninit_com_mf(const MFSymbols *symbols)
{
-MFShutdown();
+symbols->MFShutdown();
CoUninitialize();
}
// Find and create a IMFTransform with the given input/output types. When done,
// you should use ff_free_mf() to destroy it, which will also uninit COM.
-int ff_instantiate_mf(void *log,
+int ff_instantiate_mf(const MFSymbols *symbols,
+ void *log,
GUID category,
MFT_REGISTER_TYPE_INFO *in_type,
MFT_REGISTER_TYPE_INFO *out_type,
@@ -594,7 +596,7 @@ int ff_instantiate_mf(void *log,
IMFActivate *winner = 0;
UINT32 flags;
-ret = init_com_mf(log);
+ret = init_com_mf(symbols, log);
if (ret < 0)
return ret;
@@ -667,14 +669,14 @@ int ff_instantiate_mf(void *log,
return 0;
error_uninit_mf:
-uninit_com_mf();
+uninit_com_mf(symbols);
return AVERROR(ENOSYS);
}
-void ff_free_mf(IMFTransform **mft)
+void ff_free_mf(const MFSymbols *symbols, IMFTransform **mft)
{
if (*mft)
IMFTransform_Release(*mft);
*mft = NULL;
-uninit_com_mf();
+uninit_com_mf(symbols);
}
diff --git a/libavcodec/mf_utils.h b/libavcodec/mf_utils.h
index d514723c3b..1b1041bb29 100644
--- a/libavcodec/mf_utils.h
+++ b/libavcodec/mf_utils.h
@@ -41,6 +41,15 @@
#include "avcodec.h"
+typedef struct MFSymbols {
+HRESULT (*MFCreateSample) (IMFSample **ppIMFSample);
+HRESULT (*MFCreateAlignedMemoryBuffer) (DWORD cbMaxLength, DWORD cbAligment,
+IMFMediaBuffer **ppBuffer);
+HRESULT (*MFStartup) (ULONG Version, DWORD dwFlags);
+HRESULT (*MFShutdown) (void);
+HRESULT (*MFCreateMediaType) (IMFMediaType **ppMFType);
+} MFSymbols;
+
// These functions do exist in mfapi.h, but are only available within
// __cplusplus ifdefs.
HRESULT ff_MFGetAttributeSize(IMFAttributes *pattr, REFGUID guid,
@@ -150,7 +159,8 @@ char *ff_hr_str_buf(char *buf, size_t size, HRESULT hr);
#define FF_VAL_VT_UI4(v) FF_VARIANT_VALUE(VT_UI4, .ulVal = (v))
#define FF_V
Re: [FFmpeg-devel] [PATCH v3] avcodec/mfenc: Dynamically load MFPlat.DLL
On Fri, 20 May 2022, Trystan Mata wrote: From 2bdef1bdb93efa40b7d3fe21270f9f23465bee90 Mon Sep 17 00:00:00 2001 From: Trystan Mata Date: Fri, 20 May 2022 14:26:49 +0200 Subject: [PATCH] avcodec/mfenc: Dynamically load MFPlat.DLL Allow builds of FFmpeg with MediaFoundation to work under N editions of Windows which are without MediaFoundation by default. Signed-off-by: Trystan Mata --- configure | 4 +- libavcodec/mf_utils.c | 26 +++-- libavcodec/mf_utils.h | 16 ++-- libavcodec/mfenc.c| 91 --- 4 files changed, 107 insertions(+), 30 deletions(-) This patch on its own breaks builds for UWP targets (where LoadLibrary isn't available). Or more precisely, previously mfenc was enabled, now it no longer is available. For such targets, one can unconditionally assume that the functions are available and can link directly against them. See ff_MFTEnumEx in mf_utils.c how that's done so far. Doing the same instead of LoadLibrary probably shouldn't be an issue for you, but I think we'd need to drop the dependency on LoadLibrary in configure. (I think that's fine, we don't really lose anything by doing that.) Btw, when sending updates to an earlier posted patch, it'd be nice if you'd mention somewhere what changed compared to the previous revision of the patch. // Martin ___ 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 v3] avcodec/mfenc: Dynamically load MFPlat.DLL
Firstly, I'm sorry about those patches without changes mentioned. As changes, there was just a whitespace that I removed. I'm not used at all to sending matches through mailing list. And Patchwork failed to apply my patches, so I resend it trying to see if the way I sent the patch was wrong. If I understood correctly, I need to change my patch to link FFmpeg against MFPlat.DLL on UWP targets. And for the symbols, I try to "load" them à la ff_MFTEnumEx. And so dynamically load MFPlat.DLL on non-UWP targets. Thank you for your review and you time. // Trystan ___ 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] image2 decoder: Add support for -skip_initial_bytes
Relevant questions from the patch submission checklist:
Explanation why it changes things like it does:
Sometimes input data may have custom headers that ffmpeg is not expected to
understand. It seems to me, this is why `-skip_initial_bytes` exists. From
the documentation it sounds like this option should be supported on all
muxers and demuxers but it doesn't seem to work with image2.
Summary of user visible advantages:
Users wanting to use the image2 format for files with custom headers can
now do just that.
Example using this change:
`ffmpeg -y -skip_initial_bytes 32 -f image2 -c:v rawvideo -pix_fmt rgb24
-s:v 64x64 -ts_from_file 2 -pattern_type glob -i "*.RGB8" output.mp4`
>From within the example.zip I've attached. This zip file also
includes `expected_output.mp4` for what the output should look like and
`corrupt_output.mp4` for what I get without this patch.
The patch contect itself:
---
Add support for -skip_initial_bytes when using the image2 decoder.
skip_initial_bytes is useful when the input data contains a custom header.
Previously this was not supported when using the image2 format.
Signed-off-by: Thomas Newton
---
libavformat/img2dec.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/libavformat/img2dec.c b/libavformat/img2dec.c
index 5f9d1f094f..19461aa200 100644
--- a/libavformat/img2dec.c
+++ b/libavformat/img2dec.c
@@ -474,6 +474,8 @@ int ff_img_read_packet(AVFormatContext *s1, AVPacket
*pkt)
ifmt = av_probe_input_format3(&pd, 1, &score);
if (ifmt && ifmt->read_packet == ff_img_read_packet &&
ifmt->raw_codec_id)
par->codec_id = ifmt->raw_codec_id;
+} else {
+avio_skip(f[i], s1->skip_initial_bytes);
}
if (par->codec_id == AV_CODEC_ID_RAWVIDEO && !par->width)
--
2.36.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 1/3] fftools: Stop using av_fopen_utf8
Provide a header based inline reimplementation of it.
Using av_fopen_utf8 doesn't work outside of the libraries when built
with MSVC as shared libraries (in the default configuration, where
each DLL gets a separate statically linked CRT).
---
fftools/ffmpeg_opt.c | 3 +-
fftools/fopen_utf8.h | 71
2 files changed, 73 insertions(+), 1 deletion(-)
create mode 100644 fftools/fopen_utf8.h
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 47e8b9b7bd..a5cd989d35 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -28,6 +28,7 @@
#endif
#include "ffmpeg.h"
+#include "fopen_utf8.h"
#include "cmdutils.h"
#include "opt_common.h"
@@ -1882,7 +1883,7 @@ static OutputStream *new_video_stream(OptionsContext *o,
AVFormatContext *oc, in
video_enc->stats_in = logbuffer;
}
if (video_enc->flags & AV_CODEC_FLAG_PASS1) {
-f = av_fopen_utf8(logfilename, "wb");
+f = fopen_utf8(logfilename, "wb");
if (!f) {
av_log(NULL, AV_LOG_FATAL,
"Cannot write log file '%s' for pass-1
encoding: %s\n",
diff --git a/fftools/fopen_utf8.h b/fftools/fopen_utf8.h
new file mode 100644
index 00..db57fcaec4
--- /dev/null
+++ b/fftools/fopen_utf8.h
@@ -0,0 +1,71 @@
+/*
+ * 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 FFTOOLS_FOPEN_UTF8_H
+#define FFTOOLS_FOPEN_UTF8_H
+
+#include
+
+/* The fopen_utf8 function here is essentially equivalent to av_fopen_utf8,
+ * except that it doesn't set O_CLOEXEC, and that it isn't exported
+ * from a different library. (On Windows, each DLL might use a different
+ * CRT, and FILE* handles can't be shared across them.) */
+
+#ifdef _WIN32
+#include "libavutil/wchar_filename.h"
+
+static inline FILE *fopen_utf8(const char *path_utf8, const char *mode)
+{
+wchar_t *path_w, *mode_w;
+FILE *f;
+
+/* convert UTF-8 to wide chars */
+if (utf8towchar(path_utf8, &path_w)) /* This sets errno on error. */
+return NULL;
+if (!path_w)
+goto fallback;
+
+if (utf8towchar(mode, &mode_w))
+return NULL;
+if (!mode_w) {
+/* If failing to interpret the mode string as utf8, it is an invalid
+ * parameter. */
+av_freep(&path_w);
+errno = EINVAL;
+return NULL;
+}
+
+f = _wfopen(path_w, mode_w);
+av_freep(&path_w);
+av_freep(&mode_w);
+
+return f;
+fallback:
+/* path may be in CP_ACP */
+return fopen(path_utf8, mode);
+}
+
+#else
+
+static inline FILE *fopen_utf8(const char *path, const char *mode)
+{
+return fopen(path, mode);
+}
+#endif
+
+#endif /* FFTOOLS_FOPEN_UTF8_H */
--
2.32.0 (Apple Git-132)
___
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/3] libavutil: Deprecate av_fopen_utf8, provide an avpriv version
Since every DLL can use an individual CRT on Windows, having
an exported function that opens a FILE* won't work if that
FILE* is going to be used from a different DLL (or from user
application code).
Internally within the libraries, the issue can be worked around
by duplicating the function in all libraries (this already happened
implicitly because the function resided in file_open.c) and renaming
the function to ff_fopen_utf8 (so that it doesn't end up exported from
the DLLs) and duplicating it in all libraries that use it.
That mechanism doesn't work for external users, thus deprecate the
existing function.
---
doc/APIchanges | 3 +++
fftools/fopen_utf8.h| 2 +-
libavfilter/Makefile| 1 +
libavfilter/file_open.c | 1 +
libavutil/avutil.h | 6 ++
libavutil/file_open.c | 9 -
libavutil/internal.h| 8
libavutil/version.h | 1 +
tests/ref/fate/source | 1 +
9 files changed, 30 insertions(+), 2 deletions(-)
create mode 100644 libavfilter/file_open.c
diff --git a/doc/APIchanges b/doc/APIchanges
index 1a9f0a303e..c3a1649079 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -14,6 +14,9 @@ libavutil: 2021-04-27
API changes, most recent first:
+2022-xx-xx - x - lavu 57.xx.xxx - avutil.h
+ Deprecate av_fopen_utf8() without replacement.
+
2022-03-16 - xx - all libraries - version_major.h
Add lib/version_major.h as new installed headers, which only
contain the major version number (and corresponding API deprecation
diff --git a/fftools/fopen_utf8.h b/fftools/fopen_utf8.h
index db57fcaec4..cd18fe8ce1 100644
--- a/fftools/fopen_utf8.h
+++ b/fftools/fopen_utf8.h
@@ -21,7 +21,7 @@
#include
-/* The fopen_utf8 function here is essentially equivalent to av_fopen_utf8,
+/* The fopen_utf8 function here is essentially equivalent to avpriv_fopen_utf8,
* except that it doesn't set O_CLOEXEC, and that it isn't exported
* from a different library. (On Windows, each DLL might use a different
* CRT, and FILE* handles can't be shared across them.) */
diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index ee2ea51e69..78ccfa37d3 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -23,6 +23,7 @@ OBJS = allfilters.o
\
version.o\
video.o \
+OBJS-$(HAVE_LIBC_MSVCRT) += file_open.o
OBJS-$(HAVE_THREADS) += pthread.o
# subsystems
diff --git a/libavfilter/file_open.c b/libavfilter/file_open.c
new file mode 100644
index 00..494a5d37a4
--- /dev/null
+++ b/libavfilter/file_open.c
@@ -0,0 +1 @@
+#include "libavutil/file_open.c"
diff --git a/libavutil/avutil.h b/libavutil/avutil.h
index 4d633156d1..64b68bdbd3 100644
--- a/libavutil/avutil.h
+++ b/libavutil/avutil.h
@@ -331,12 +331,18 @@ unsigned av_int_list_length_for_size(unsigned elsize,
#define av_int_list_length(list, term) \
av_int_list_length_for_size(sizeof(*(list)), list, term)
+#if FF_API_AV_FOPEN_UTF8
/**
* Open a file using a UTF-8 filename.
* The API of this function matches POSIX fopen(), errors are returned through
* errno.
+ * @deprecated Avoid using it, as on Windows, the FILE* allocated by this
+ * function may be allocated with a different CRT than the caller
+ * who uses the FILE*. No replacement provided in public API.
*/
+attribute_deprecated
FILE *av_fopen_utf8(const char *path, const char *mode);
+#endif
/**
* Return the fractional representation of the internal time base.
diff --git a/libavutil/file_open.c b/libavutil/file_open.c
index cc302f2f76..fb64c2e4ee 100644
--- a/libavutil/file_open.c
+++ b/libavutil/file_open.c
@@ -155,7 +155,7 @@ int avpriv_tempfile(const char *prefix, char **filename,
int log_offset, void *l
return fd; /* success */
}
-FILE *av_fopen_utf8(const char *path, const char *mode)
+FILE *avpriv_fopen_utf8(const char *path, const char *mode)
{
int fd;
int access;
@@ -188,3 +188,10 @@ FILE *av_fopen_utf8(const char *path, const char *mode)
return NULL;
return fdopen(fd, mode);
}
+
+#if FF_API_AV_FOPEN_UTF8
+FILE *av_fopen_utf8(const char *path, const char *mode)
+{
+return avpriv_fopen_utf8(path, mode);
+}
+#endif
diff --git a/libavutil/internal.h b/libavutil/internal.h
index 79c2130be0..b44cbaaa7b 100644
--- a/libavutil/internal.h
+++ b/libavutil/internal.h
@@ -37,6 +37,7 @@
#include
#include
#include
+#include
#include "config.h"
#include "attributes.h"
#include "timer.h"
@@ -183,8 +184,10 @@ void avpriv_request_sample(void *avc,
#pragma comment(linker, "/include:" EXTERN_PREFIX "avpriv_snprintf")
#endif
+#define avpriv_fopen_utf8 ff_fopen_utf8
#define avpriv_open ff_open
#define avpriv_tempfile ff_tempfile
+
#define PTRDIFF_SPECIFIER "Id"
#define SIZE_SPECIFIER "Iu"
#else
@@ -256,6 +259,11
[FFmpeg-devel] [PATCH 3/3] Switch uses of av_fopen_utf8 to avpriv_fopen_utf8
---
libavfilter/af_arnndn.c | 2 +-
libavfilter/opencl.c | 2 +-
libavfilter/vf_curves.c | 2 +-
libavfilter/vf_dnn_classify.c | 2 +-
libavfilter/vf_dnn_detect.c | 2 +-
libavfilter/vf_fieldhint.c| 2 +-
libavfilter/vf_lut3d.c| 4 ++--
libavfilter/vf_nnedi.c| 2 +-
libavfilter/vf_paletteuse.c | 2 +-
libavformat/ipfsgateway.c | 2 +-
10 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/libavfilter/af_arnndn.c b/libavfilter/af_arnndn.c
index fe562e81a1..5e3403ca6a 100644
--- a/libavfilter/af_arnndn.c
+++ b/libavfilter/af_arnndn.c
@@ -1478,7 +1478,7 @@ static int open_model(AVFilterContext *ctx, RNNModel
**model)
if (!s->model_name)
return AVERROR(EINVAL);
-f = av_fopen_utf8(s->model_name, "r");
+f = avpriv_fopen_utf8(s->model_name, "r");
if (!f) {
av_log(ctx, AV_LOG_ERROR, "Failed to open model file: %s\n",
s->model_name);
return AVERROR(EINVAL);
diff --git a/libavfilter/opencl.c b/libavfilter/opencl.c
index 70d5edb78c..c8e7e6e1a5 100644
--- a/libavfilter/opencl.c
+++ b/libavfilter/opencl.c
@@ -210,7 +210,7 @@ int ff_opencl_filter_load_program_from_file(AVFilterContext
*avctx,
const char *src_const;
int err;
-file = av_fopen_utf8(filename, "r");
+file = avpriv_fopen_utf8(filename, "r");
if (!file) {
av_log(avctx, AV_LOG_ERROR, "Unable to open program "
"source file \"%s\".\n", filename);
diff --git a/libavfilter/vf_curves.c b/libavfilter/vf_curves.c
index 22a1f8aa70..82e2753f01 100644
--- a/libavfilter/vf_curves.c
+++ b/libavfilter/vf_curves.c
@@ -416,7 +416,7 @@ static int dump_curves(const char *fname, uint16_t
*graph[NB_COMP + 1],
AVBPrint buf;
const double scale = 1. / (lut_size - 1);
static const char * const colors[] = { "red", "green", "blue", "#404040",
};
-FILE *f = av_fopen_utf8(fname, "w");
+FILE *f = avpriv_fopen_utf8(fname, "w");
av_assert0(FF_ARRAY_ELEMS(colors) == NB_COMP + 1);
diff --git a/libavfilter/vf_dnn_classify.c b/libavfilter/vf_dnn_classify.c
index c612ba8e80..852f5ddcee 100644
--- a/libavfilter/vf_dnn_classify.c
+++ b/libavfilter/vf_dnn_classify.c
@@ -131,7 +131,7 @@ static int read_classify_label_file(AVFilterContext
*context)
FILE *file;
DnnClassifyContext *ctx = context->priv;
-file = av_fopen_utf8(ctx->labels_filename, "r");
+file = avpriv_fopen_utf8(ctx->labels_filename, "r");
if (!file){
av_log(context, AV_LOG_ERROR, "failed to open file %s\n",
ctx->labels_filename);
return AVERROR(EINVAL);
diff --git a/libavfilter/vf_dnn_detect.c b/libavfilter/vf_dnn_detect.c
index dd4507250f..68bd2cd0c3 100644
--- a/libavfilter/vf_dnn_detect.c
+++ b/libavfilter/vf_dnn_detect.c
@@ -244,7 +244,7 @@ static int read_detect_label_file(AVFilterContext *context)
FILE *file;
DnnDetectContext *ctx = context->priv;
-file = av_fopen_utf8(ctx->labels_filename, "r");
+file = avpriv_fopen_utf8(ctx->labels_filename, "r");
if (!file){
av_log(context, AV_LOG_ERROR, "failed to open file %s\n",
ctx->labels_filename);
return AVERROR(EINVAL);
diff --git a/libavfilter/vf_fieldhint.c b/libavfilter/vf_fieldhint.c
index e7afac1116..e6061c6d3c 100644
--- a/libavfilter/vf_fieldhint.c
+++ b/libavfilter/vf_fieldhint.c
@@ -73,7 +73,7 @@ static av_cold int init(AVFilterContext *ctx)
av_log(ctx, AV_LOG_ERROR, "Hint file must be set.\n");
return AVERROR(EINVAL);
}
-s->hint = av_fopen_utf8(s->hint_file_str, "r");
+s->hint = avpriv_fopen_utf8(s->hint_file_str, "r");
if (!s->hint) {
ret = AVERROR(errno);
av_log(ctx, AV_LOG_ERROR, "%s: %s\n", s->hint_file_str,
av_err2str(ret));
diff --git a/libavfilter/vf_lut3d.c b/libavfilter/vf_lut3d.c
index a5190b6688..c7ecb7018e 100644
--- a/libavfilter/vf_lut3d.c
+++ b/libavfilter/vf_lut3d.c
@@ -1243,7 +1243,7 @@ static av_cold int lut3d_init(AVFilterContext *ctx)
return set_identity_matrix(ctx, 32);
}
-f = av_fopen_utf8(lut3d->file, "r");
+f = avpriv_fopen_utf8(lut3d->file, "r");
if (!f) {
ret = AVERROR(errno);
av_log(ctx, AV_LOG_ERROR, "%s: %s\n", lut3d->file, av_err2str(ret));
@@ -2134,7 +2134,7 @@ static av_cold int lut1d_init(AVFilterContext *ctx)
return 0;
}
-f = av_fopen_utf8(lut1d->file, "r");
+f = avpriv_fopen_utf8(lut1d->file, "r");
if (!f) {
ret = AVERROR(errno);
av_log(ctx, AV_LOG_ERROR, "%s: %s\n", lut1d->file, av_err2str(ret));
diff --git a/libavfilter/vf_nnedi.c b/libavfilter/vf_nnedi.c
index 370f643678..e5a16918bd 100644
--- a/libavfilter/vf_nnedi.c
+++ b/libavfilter/vf_nnedi.c
@@ -957,7 +957,7 @@ static av_cold int init(AVFilterContext *ctx)
size_t bytes_read;
int ret = 0;
-weights_file = av_fopen_utf8(s->weights_file, "rb");
+weights_file = avpriv_fopen_utf8(s->weights_file, "rb");
if (!weights_file) {
av_log(c
[FFmpeg-devel] [PATCH] libx264: Set min build version to 158
From: Matt Oliver
Was "[PATCH] libx264: Do not explicitly set X264_API_IMPORTS"
Setting X264_API_IMPORTS only affects msvc builds and it breaks
linking to static builds (although is required for shared builds).
This flag is set by x264 in its pkgconfig as required since build
158 (a615f027ed172e2dd5380e736d487aa858a0c4ff) from July 2019.
So this patch updates configure to require a newer x264 build that
correctly sets the imports flag.
Alternatively we can detect the x264 build version in configure
and keep the fallback of manually setting the flag on older x264
builds that arent using pkgconfig (to keep the old behaviour) but
that requires some complex configure changes.
Submitted-by: softworkz
Signed-off-by: Matt Oliver
---
libx264: Set min build version to 158
I'm submitting this patch on behalf of Matt with his permission.
There seemed to be agreement that the 158 version limit should be
applied to MSVC builds only. For the latter I'd need some help, because
I can't test this and I'm not familiar enough with the configure script
logic to make a change with sufficient confidence.
Thanks, softworkz
Published-As:
https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-30%2Fsoftworkz%2Fsubmit_x264_api_imports_matt-v1
Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg
pr-ffstaging-30/softworkz/submit_x264_api_imports_matt-v1
Pull-Request: https://github.com/ffstaging/FFmpeg/pull/30
configure| 7 ++-
libavcodec/libx264.c | 4
2 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/configure b/configure
index f115b21064..d977a97397 100755
--- a/configure
+++ b/configure
@@ -6656,11 +6656,8 @@ enabled libvpx&& {
enabled libwebp && {
enabled libwebp_encoder && require_pkg_config libwebp "libwebp >=
0.2.0" webp/encode.h WebPGetEncoderVersion
enabled libwebp_anim_encoder && check_pkg_config libwebp_anim_encoder
"libwebpmux >= 0.4.0" webp/mux.h WebPAnimEncoderOptionsInit; }
-enabled libx264 && { check_pkg_config libx264 x264 "stdint.h x264.h"
x264_encoder_encode ||
- { require libx264 "stdint.h x264.h"
x264_encoder_encode "-lx264 $pthreads_extralibs $libm_extralibs" &&
- warn "using libx264 without pkg-config"; } }
&&
- require_cpp_condition libx264 x264.h "X264_BUILD
>= 118" &&
- check_cpp_condition libx262 x264.h "X264_MPEG2"
+enabled libx264 && check_pkg_config libx264 x264 "stdint.h x264.h"
x264_encoder_encode &&
+ require_cpp_condition libx264 x264.h "X264_BUILD
>= 158"
enabled libx265 && require_pkg_config libx265 x265 x265.h
x265_api_get &&
require_cpp_condition libx265 x265.h "X265_BUILD
>= 70"
enabled libxavs && require libxavs "stdint.h xavs.h"
xavs_encoder_encode "-lxavs $pthreads_extralibs $libm_extralibs"
diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
index 4ce3791ae8..14177b3016 100644
--- a/libavcodec/libx264.c
+++ b/libavcodec/libx264.c
@@ -37,10 +37,6 @@
#include "atsc_a53.h"
#include "sei.h"
-#if defined(_MSC_VER)
-#define X264_API_IMPORTS 1
-#endif
-
#include
#include
#include
base-commit: 41a558fea06cc0a23b8d2d0dfb03ef6a25cf5100
--
ffmpeg-codebot
___
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/3] fftools: Stop using av_fopen_utf8
> -Original Message-
> From: ffmpeg-devel On Behalf Of Martin
> Storsjö
> Sent: Friday, May 20, 2022 11:13 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH 1/3] fftools: Stop using av_fopen_utf8
>
> Provide a header based inline reimplementation of it.
>
> Using av_fopen_utf8 doesn't work outside of the libraries when built
> with MSVC as shared libraries (in the default configuration, where
> each DLL gets a separate statically linked CRT).
> ---
> fftools/ffmpeg_opt.c | 3 +-
> fftools/fopen_utf8.h | 71
> 2 files changed, 73 insertions(+), 1 deletion(-)
> create mode 100644 fftools/fopen_utf8.h
>
> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> index 47e8b9b7bd..a5cd989d35 100644
> --- a/fftools/ffmpeg_opt.c
> +++ b/fftools/ffmpeg_opt.c
> @@ -28,6 +28,7 @@
> #endif
>
> #include "ffmpeg.h"
> +#include "fopen_utf8.h"
> #include "cmdutils.h"
> #include "opt_common.h"
>
> @@ -1882,7 +1883,7 @@ static OutputStream *new_video_stream(OptionsContext
> *o, AVFormatContext *oc, in
> video_enc->stats_in = logbuffer;
> }
> if (video_enc->flags & AV_CODEC_FLAG_PASS1) {
> -f = av_fopen_utf8(logfilename, "wb");
> +f = fopen_utf8(logfilename, "wb");
> if (!f) {
> av_log(NULL, AV_LOG_FATAL,
> "Cannot write log file '%s' for pass-1
> encoding: %s\n",
> diff --git a/fftools/fopen_utf8.h b/fftools/fopen_utf8.h
> new file mode 100644
> index 00..db57fcaec4
> --- /dev/null
> +++ b/fftools/fopen_utf8.h
> @@ -0,0 +1,71 @@
> +/*
> + * 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 FFTOOLS_FOPEN_UTF8_H
> +#define FFTOOLS_FOPEN_UTF8_H
> +
> +#include
> +
> +/* The fopen_utf8 function here is essentially equivalent to
> av_fopen_utf8,
> + * except that it doesn't set O_CLOEXEC, and that it isn't exported
> + * from a different library. (On Windows, each DLL might use a different
> + * CRT, and FILE* handles can't be shared across them.) */
> +
> +#ifdef _WIN32
> +#include "libavutil/wchar_filename.h"
> +
> +static inline FILE *fopen_utf8(const char *path_utf8, const char *mode)
> +{
> +wchar_t *path_w, *mode_w;
> +FILE *f;
> +
> +/* convert UTF-8 to wide chars */
> +if (utf8towchar(path_utf8, &path_w)) /* This sets errno on error. */
> +return NULL;
> +if (!path_w)
> +goto fallback;
> +
> +if (utf8towchar(mode, &mode_w))
> +return NULL;
> +if (!mode_w) {
> +/* If failing to interpret the mode string as utf8, it is an
> invalid
> + * parameter. */
> +av_freep(&path_w);
> +errno = EINVAL;
> +return NULL;
> +}
> +
> +f = _wfopen(path_w, mode_w);
> +av_freep(&path_w);
> +av_freep(&mode_w);
> +
> +return f;
> +fallback:
> +/* path may be in CP_ACP */
> +return fopen(path_utf8, mode);
> +}
> +
> +#else
> +
> +static inline FILE *fopen_utf8(const char *path, const char *mode)
> +{
> +return fopen(path, mode);
> +}
> +#endif
> +
> +#endif /* FFTOOLS_FOPEN_UTF8_H */
> --
LGTM. (all three)
Tested with VS project build (full static linkage, though).
Thank you,
sw
___
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 00/10] libavformat/asf: fix handling of byte array length values
The spec allows attachment sizes of up to UINT32_MAX while we can handle only sizes up to INT32_MAX (in downstream code) The debug.assert in get_tag didn't really address this, and truncating the value_len in calling methods cannot be used because the length value is required in order to continue parsing. This adds a check with log message in ff_asf_handle_byte_array to handle those (rare) cases. v2: Rebased & PING v3: Adjustments suggested by Michael v4: 1 of 11 merged, 10 to go.. v5: adjusted commit message of 4/10 as requested softworkz (10): libavformat/asf: fix handling of byte array length values libavformat/asfdec: fix get_value return type and add checks for libavformat/asfdec: fix type of value_len libavformat/asfdec: fixing get_tag libavformat/asfdec: implement parsing of GUID values libavformat/asfdec: avoid clang warnings libavformat/asfdec: remove variable redefinition in inner scope libavformat/asfdec: ensure variables are initialized libavformat/asfdec: fix parameter type in asf_read_stream_propertie() libavformat/asfdec: fix variable types and add checks for unsupported values libavformat/asf.c | 8 +- libavformat/asf.h | 2 +- libavformat/asfdec_f.c | 338 +++-- 3 files changed, 229 insertions(+), 119 deletions(-) base-commit: 9ab20b1614194280b862d98dfcdb7b1bcff03329 Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-12%2Fsoftworkz%2Fmaster-upstream_asf_4-v5 Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-12/softworkz/master-upstream_asf_4-v5 Pull-Request: https://github.com/ffstaging/FFmpeg/pull/12 Range-diff vs v4: 1: 60966b7907 = 1: 7505ffa3c5 libavformat/asf: fix handling of byte array length values 2: 5acab7b52b = 2: f2d0b72bf0 libavformat/asfdec: fix get_value return type and add checks for 3: 97e0d765c9 = 3: 99660db6ef libavformat/asfdec: fix type of value_len 4: 025123f72d = 4: 8aaab15e8b libavformat/asfdec: fixing get_tag 5: 2d01e4dff5 = 5: ba31d01715 libavformat/asfdec: implement parsing of GUID values 6: 33b3d163df ! 6: d171cd5184 libavformat/asfdec: fix macro definition and use @@ Metadata Author: softworkz ## Commit message ## -libavformat/asfdec: fix macro definition and use +libavformat/asfdec: avoid clang warnings + +such as: +- bugprone-macro-parentheses +- wextra-semi-stmt Signed-off-by: softworkz 7: 1509b83f47 = 7: 0d032d9d4c libavformat/asfdec: remove variable redefinition in inner scope 8: fd31b0be2e = 8: 6bdb2d8bec libavformat/asfdec: ensure variables are initialized 9: f8728b1c51 = 9: d510093ed6 libavformat/asfdec: fix parameter type in asf_read_stream_propertie() 10: 78ed5aeb38 = 10: a05986d76b libavformat/asfdec: fix variable types and add checks for unsupported values -- ffmpeg-codebot ___ 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 01/10] libavformat/asf: fix handling of byte array length values
From: softworkz
The spec allows attachment sizes of up to UINT32_MAX while
we can handle only sizes up to INT32_MAX (in downstream
code)
The debug.assert in get_tag didn't really address this,
and truncating the value_len in calling methods cannot
be used because the length value is required in order to
continue parsing. This adds a check with log message in
ff_asf_handle_byte_array to handle those (rare) cases.
Signed-off-by: softworkz
---
libavformat/asf.c | 8 +++-
libavformat/asf.h | 2 +-
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/libavformat/asf.c b/libavformat/asf.c
index 1285062220..bec7db0c7e 100644
--- a/libavformat/asf.c
+++ b/libavformat/asf.c
@@ -139,12 +139,18 @@ static int get_id3_tag(AVFormatContext *s, int len)
}
int ff_asf_handle_byte_array(AVFormatContext *s, const char *name,
- int val_len)
+ uint32_t val_len)
{
+if (val_len > INT32_MAX) {
+av_log(s, AV_LOG_VERBOSE, "Unable to handle byte arrays > INT32_MAX
in tag %s.\n", name);
+return 1;
+}
+
if (!strcmp(name, "WM/Picture")) // handle cover art
return asf_read_picture(s, val_len);
else if (!strcmp(name, "ID3")) // handle ID3 tag
return get_id3_tag(s, val_len);
+av_log(s, AV_LOG_DEBUG, "Unsupported byte array in tag %s.\n", name);
return 1;
}
diff --git a/libavformat/asf.h b/libavformat/asf.h
index 01cc4f7a46..4d28560f56 100644
--- a/libavformat/asf.h
+++ b/libavformat/asf.h
@@ -111,7 +111,7 @@ extern const AVMetadataConv ff_asf_metadata_conv[];
* is unsupported by this function and 0 otherwise.
*/
int ff_asf_handle_byte_array(AVFormatContext *s, const char *name,
- int val_len);
+ uint32_t val_len);
#define ASF_PACKET_FLAG_ERROR_CORRECTION_PRESENT 0x80 //1000
--
ffmpeg-codebot
___
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 02/10] libavformat/asfdec: fix get_value return type and add checks for
From: softworkz
unsupported values
get_value had a return type of int, which means that reading
QWORDS (case 4) was broken due to truncation of the result from
avio_rl64().
Signed-off-by: softworkz
---
libavformat/asfdec_f.c | 57 +++---
1 file changed, 43 insertions(+), 14 deletions(-)
diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
index 4770a812db..c7c4ba55d6 100644
--- a/libavformat/asfdec_f.c
+++ b/libavformat/asfdec_f.c
@@ -203,7 +203,7 @@ static int asf_probe(const AVProbeData *pd)
/* size of type 2 (BOOL) is 32bit for "Extended Content Description Object"
* but 16 bit for "Metadata Object" and "Metadata Library Object" */
-static int get_value(AVIOContext *pb, int type, int type2_size)
+static uint64_t get_value(AVIOContext *pb, int type, int type2_size)
{
switch (type) {
case ASF_BOOL:
@@ -549,6 +549,8 @@ static int asf_read_ext_content_desc(AVFormatContext *s)
{
AVIOContext *pb = s->pb;
ASFContext *asf = s->priv_data;
+uint64_t dar_num = 0;
+uint64_t dar_den = 0;
int desc_count, i, ret;
desc_count = avio_rl16(pb);
@@ -568,14 +570,27 @@ static int asf_read_ext_content_desc(AVFormatContext *s)
/* My sample has that stream set to 0 maybe that mean the container.
* ASF stream count starts at 1. I am using 0 to the container value
* since it's unused. */
-if (!strcmp(name, "AspectRatioX"))
-asf->dar[0].num = get_value(s->pb, value_type, 32);
-else if (!strcmp(name, "AspectRatioY"))
-asf->dar[0].den = get_value(s->pb, value_type, 32);
+if (!strcmp(name, "AspectRatioX")) {
+dar_num = get_value(s->pb, value_type, 32);
+if (dar_num > INT64_MAX) {
+av_log(s, AV_LOG_DEBUG, "Unsupported AspectRatioX value:
%"PRIu64"\n", dar_num);
+return AVERROR(ENOTSUP);
+}
+}
+else if (!strcmp(name, "AspectRatioY")) {
+dar_den = get_value(s->pb, value_type, 32);
+if (dar_den > INT64_MAX) {
+av_log(s, AV_LOG_DEBUG, "Unsupported AspectRatioY value:
%"PRIu64"\n", dar_den);
+return AVERROR(ENOTSUP);
+}
+}
else
get_tag(s, name, value_type, value_len, 32);
}
+if (dar_num && dar_den)
+av_reduce(&asf->dar[0].num, &asf->dar[0].den, dar_num, dar_den,
INT_MAX);
+
return 0;
}
@@ -603,6 +618,8 @@ static int asf_read_metadata(AVFormatContext *s)
{
AVIOContext *pb = s->pb;
ASFContext *asf = s->priv_data;
+uint64_t dar_num[128] = {0};
+uint64_t dar_den[128] = {0};
int n, stream_num, name_len_utf16, name_len_utf8, value_len;
int ret, i;
n = avio_rl16(pb);
@@ -630,17 +647,29 @@ static int asf_read_metadata(AVFormatContext *s)
av_log(s, AV_LOG_TRACE, "%d stream %d name_len %2d type %d len %4d
<%s>\n",
i, stream_num, name_len_utf16, value_type, value_len, name);
-if (!strcmp(name, "AspectRatioX")){
-int aspect_x = get_value(s->pb, value_type, 16);
-if(stream_num < 128)
-asf->dar[stream_num].num = aspect_x;
-} else if(!strcmp(name, "AspectRatioY")){
-int aspect_y = get_value(s->pb, value_type, 16);
-if(stream_num < 128)
-asf->dar[stream_num].den = aspect_y;
-} else {
+if (!strcmp(name, "AspectRatioX") && stream_num < 128) {
+dar_num[stream_num] = get_value(s->pb, value_type, 16);
+if (dar_num[stream_num] > INT64_MAX) {
+av_log(s, AV_LOG_DEBUG, "Unsupported AspectRatioX value:
%"PRIu64"\n", dar_num[stream_num]);
+return AVERROR(ENOTSUP);
+}
+}
+else if (!strcmp(name, "AspectRatioY") && stream_num < 128) {
+dar_den[stream_num] = get_value(s->pb, value_type, 16);
+if (dar_den[stream_num] > INT64_MAX) {
+av_log(s, AV_LOG_DEBUG, "Unsupported AspectRatioY value:
%"PRIu64"\n", dar_den[stream_num]);
+return AVERROR(ENOTSUP);
+}
+} else
get_tag(s, name, value_type, value_len, 16);
+
+
+if (stream_num < 128 && dar_num[stream_num] && dar_den[stream_num]) {
+av_reduce(&asf->dar[stream_num].num, &asf->dar[stream_num].den,
dar_num[stream_num], dar_den[stream_num], INT_MAX);
+dar_num[stream_num] = 0;
+dar_den[stream_num] = 0;
}
+
av_freep(&name);
}
--
ffmpeg-codebot
___
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 03/10] libavformat/asfdec: fix type of value_len
From: softworkz
The value_len is an uint32 not an int32 per spec. That
value must not be truncated, neither by casting to int, nor by any
conditional checks, because at the end of get_tag, this value is
needed to move forward in parsing. When the len value gets
modified, the parsing may break.
Signed-off-by: softworkz
---
libavformat/asfdec_f.c | 24 +++-
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
index c7c4ba55d6..eda7175c96 100644
--- a/libavformat/asfdec_f.c
+++ b/libavformat/asfdec_f.c
@@ -219,7 +219,7 @@ static uint64_t get_value(AVIOContext *pb, int type, int
type2_size)
}
}
-static void get_tag(AVFormatContext *s, const char *key, int type, int len,
int type2_size)
+static void get_tag(AVFormatContext *s, const char *key, int type, uint32_t
len, int type2_size)
{
ASFContext *asf = s->priv_data;
char *value = NULL;
@@ -529,7 +529,7 @@ static int asf_read_ext_stream_properties(AVFormatContext
*s)
static int asf_read_content_desc(AVFormatContext *s)
{
AVIOContext *pb = s->pb;
-int len1, len2, len3, len4, len5;
+uint32_t len1, len2, len3, len4, len5;
len1 = avio_rl16(pb);
len2 = avio_rl16(pb);
@@ -620,25 +620,23 @@ static int asf_read_metadata(AVFormatContext *s)
ASFContext *asf = s->priv_data;
uint64_t dar_num[128] = {0};
uint64_t dar_den[128] = {0};
-int n, stream_num, name_len_utf16, name_len_utf8, value_len;
+int n, name_len_utf8;
+uint16_t stream_num, name_len_utf16, value_type;
+uint32_t value_len;
int ret, i;
n = avio_rl16(pb);
for (i = 0; i < n; i++) {
uint8_t *name;
-int value_type;
avio_rl16(pb); // lang_list_index
-stream_num = avio_rl16(pb);
-name_len_utf16 = avio_rl16(pb);
-value_type = avio_rl16(pb); /* value_type */
-value_len = avio_rl32(pb);
+stream_num = (uint16_t)avio_rl16(pb);
+name_len_utf16 = (uint16_t)avio_rl16(pb);
+value_type = (uint16_t)avio_rl16(pb); /* value_type */
+value_len = avio_rl32(pb);
-if (value_len < 0 || value_len > UINT16_MAX)
-return AVERROR_INVALIDDATA;
-
-name_len_utf8 = 2*name_len_utf16 + 1;
-name = av_malloc(name_len_utf8);
+name_len_utf8 = 2 * name_len_utf16 + 1;
+name = av_malloc(name_len_utf8);
if (!name)
return AVERROR(ENOMEM);
--
ffmpeg-codebot
___
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 04/10] libavformat/asfdec: fixing get_tag
From: softworkz
These three are closely related and can't be separated easily:
In get_tag, the code was adding 22 bytes (in order to allow
it to hold 64bit numbers as string) to the value len for creating
creating a buffer. This was unnecessarily imposing a
size-constraint on the value_len parameter.
The code in get_tag, was limiting the maximum value_len to
half the size of INT32. This was applied for all value types, even
though it is required only in case of ASF_UNICODE, not for any
other ones (like ASCII).
get_tag was always allocating a buffer regardless of the
datatype, even though this isn't required in case of ASF_BYTE_ARRAY
The check for the return value from ff_asf_handle_byte_array()
being >0 is removed here because the log message is emitted
by the function itself now.
Signed-off-by: softworkz
---
libavformat/asfdec_f.c | 54 +++---
1 file changed, 40 insertions(+), 14 deletions(-)
diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
index eda7175c96..cb7da2d679 100644
--- a/libavformat/asfdec_f.c
+++ b/libavformat/asfdec_f.c
@@ -222,37 +222,63 @@ static uint64_t get_value(AVIOContext *pb, int type, int
type2_size)
static void get_tag(AVFormatContext *s, const char *key, int type, uint32_t
len, int type2_size)
{
ASFContext *asf = s->priv_data;
-char *value = NULL;
int64_t off = avio_tell(s->pb);
-#define LEN 22
-
-av_assert0((unsigned)len < (INT_MAX - LEN) / 2);
+char *value = NULL;
+uint64_t required_bufferlen;
+int buffer_len;
if (!asf->export_xmp && !strncmp(key, "xmp", 3))
goto finish;
-value = av_malloc(2 * len + LEN);
+switch (type) {
+case ASF_UNICODE:
+required_bufferlen = (uint64_t)len * 2 + 1;
+break;
+case -1: // ASCII
+required_bufferlen = (uint64_t)len + 1;
+break;
+case ASF_BYTE_ARRAY:
+ff_asf_handle_byte_array(s, key, len);
+goto finish;
+case ASF_BOOL:
+case ASF_DWORD:
+case ASF_QWORD:
+case ASF_WORD:
+required_bufferlen = 22;
+break;
+case ASF_GUID:
+required_bufferlen = 33;
+break;
+default:
+required_bufferlen = len;
+break;
+}
+
+if (required_bufferlen > INT32_MAX) {
+av_log(s, AV_LOG_VERBOSE, "Unable to handle values > INT32_MAX in tag
%s.\n", key);
+goto finish;
+}
+
+buffer_len = (int)required_bufferlen;
+
+value = av_malloc(buffer_len);
if (!value)
goto finish;
switch (type) {
case ASF_UNICODE:
-avio_get_str16le(s->pb, len, value, 2 * len + 1);
+avio_get_str16le(s->pb, len, value, buffer_len);
break;
-case -1: // ASCI
-avio_read(s->pb, value, len);
-value[len]=0;
+case -1: // ASCII
+avio_read(s->pb, value, buffer_len - 1);
+value[buffer_len - 1] = 0;
break;
-case ASF_BYTE_ARRAY:
-if (ff_asf_handle_byte_array(s, key, len) > 0)
-av_log(s, AV_LOG_VERBOSE, "Unsupported byte array in tag %s.\n",
key);
-goto finish;
case ASF_BOOL:
case ASF_DWORD:
case ASF_QWORD:
case ASF_WORD: {
uint64_t num = get_value(s->pb, type, type2_size);
-snprintf(value, LEN, "%"PRIu64, num);
+snprintf(value, buffer_len, "%"PRIu64, num);
break;
}
case ASF_GUID:
--
ffmpeg-codebot
___
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 05/10] libavformat/asfdec: implement parsing of GUID values
From: softworkz
Signed-off-by: softworkz
---
libavformat/asfdec_f.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
index cb7da2d679..81a29f99d5 100644
--- a/libavformat/asfdec_f.c
+++ b/libavformat/asfdec_f.c
@@ -281,9 +281,12 @@ static void get_tag(AVFormatContext *s, const char *key,
int type, uint32_t len,
snprintf(value, buffer_len, "%"PRIu64, num);
break;
}
-case ASF_GUID:
-av_log(s, AV_LOG_DEBUG, "Unsupported GUID value in tag %s.\n", key);
-goto finish;
+case ASF_GUID: {
+ff_asf_guid g;
+ff_get_guid(s->pb, &g);
+snprintf(value, buffer_len, "%x", g[0]);
+break;
+}
default:
av_log(s, AV_LOG_DEBUG,
"Unsupported value type %d in tag %s.\n", type, key);
--
ffmpeg-codebot
___
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 06/10] libavformat/asfdec: avoid clang warnings
From: softworkz
such as:
- bugprone-macro-parentheses
- wextra-semi-stmt
Signed-off-by: softworkz
---
libavformat/asfdec_f.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
index 81a29f99d5..91c3874ac7 100644
--- a/libavformat/asfdec_f.c
+++ b/libavformat/asfdec_f.c
@@ -906,21 +906,21 @@ static int asf_read_header(AVFormatContext *s)
}
#define DO_2BITS(bits, var, defval) \
-switch (bits & 3) { \
+switch ((bits) & 3) { \
case 3: \
-var = avio_rl32(pb);\
+(var) = avio_rl32(pb); \
rsize += 4; \
break; \
case 2: \
-var = avio_rl16(pb);\
+(var) = avio_rl16(pb); \
rsize += 2; \
break; \
case 1: \
-var = avio_r8(pb); \
+(var) = avio_r8(pb);\
rsize++;\
break; \
default:\
-var = defval; \
+(var) = (defval); \
break; \
}
@@ -1003,9 +1003,9 @@ static int asf_get_packet(AVFormatContext *s, AVIOContext
*pb)
asf->packet_flags= c;
asf->packet_property = d;
-DO_2BITS(asf->packet_flags >> 5, packet_length, s->packet_size);
-DO_2BITS(asf->packet_flags >> 1, padsize, 0); // sequence ignored
-DO_2BITS(asf->packet_flags >> 3, padsize, 0); // padding length
+DO_2BITS(asf->packet_flags >> 5, packet_length, s->packet_size)
+DO_2BITS(asf->packet_flags >> 1, padsize, 0) // sequence ignored
+DO_2BITS(asf->packet_flags >> 3, padsize, 0) // padding length
// the following checks prevent overflows and infinite loops
if (!packet_length || packet_length >= (1U << 29)) {
@@ -1066,9 +1066,9 @@ static int asf_read_frame_header(AVFormatContext *s,
AVIOContext *pb)
asf->stream_index = asf->asfid2avid[num & 0x7f];
asfst = &asf->streams[num & 0x7f];
// sequence should be ignored!
-DO_2BITS(asf->packet_property >> 4, asf->packet_seq, 0);
-DO_2BITS(asf->packet_property >> 2, asf->packet_frag_offset, 0);
-DO_2BITS(asf->packet_property, asf->packet_replic_size, 0);
+DO_2BITS(asf->packet_property >> 4, asf->packet_seq, 0)
+DO_2BITS(asf->packet_property >> 2, asf->packet_frag_offset, 0)
+DO_2BITS(asf->packet_property, asf->packet_replic_size, 0)
av_log(asf, AV_LOG_TRACE, "key:%d stream:%d seq:%d offset:%d
replic_size:%d num:%X packet_property %X\n",
asf->packet_key_frame, asf->stream_index, asf->packet_seq,
asf->packet_frag_offset, asf->packet_replic_size, num,
asf->packet_property);
@@ -1144,7 +1144,7 @@ static int asf_read_frame_header(AVFormatContext *s,
AVIOContext *pb)
return AVERROR_INVALIDDATA;
}
if (asf->packet_flags & 0x01) {
-DO_2BITS(asf->packet_segsizetype >> 6, asf->packet_frag_size, 0); // 0
is illegal
+DO_2BITS(asf->packet_segsizetype >> 6, asf->packet_frag_size, 0) // 0
is illegal
if (rsize > asf->packet_size_left) {
av_log(s, AV_LOG_ERROR, "packet_replic_size is invalid\n");
return AVERROR_INVALIDDATA;
--
ffmpeg-codebot
___
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 07/10] libavformat/asfdec: remove variable redefinition in inner scope
From: softworkz
Signed-off-by: softworkz
---
libavformat/asfdec_f.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
index 91c3874ac7..fae15d9b05 100644
--- a/libavformat/asfdec_f.c
+++ b/libavformat/asfdec_f.c
@@ -1191,7 +1191,7 @@ static int asf_parse_packet(AVFormatContext *s,
AVIOContext *pb, AVPacket *pkt)
return AVERROR_EOF;
if (asf->packet_size_left < FRAME_HEADER_SIZE ||
asf->packet_segments < 1 && asf->packet_time_start == 0) {
-int ret = asf->packet_size_left + asf->packet_padsize;
+ret = asf->packet_size_left + asf->packet_padsize;
if (asf->packet_size_left && asf->packet_size_left <
FRAME_HEADER_SIZE)
av_log(s, AV_LOG_WARNING, "Skip due to FRAME_HEADER_SIZE\n");
@@ -1260,7 +1260,6 @@ static int asf_parse_packet(AVFormatContext *s,
AVIOContext *pb, AVPacket *pkt)
if (asf_st->pkt.size != asf_st->packet_obj_size ||
// FIXME is this condition sufficient?
asf_st->frag_offset + asf->packet_frag_size > asf_st->pkt.size) {
-int ret;
if (asf_st->pkt.data) {
av_log(s, AV_LOG_INFO,
--
ffmpeg-codebot
___
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 08/10] libavformat/asfdec: ensure variables are initialized
From: softworkz
Signed-off-by: softworkz
---
libavformat/asfdec_f.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
index fae15d9b05..cb396cccfe 100644
--- a/libavformat/asfdec_f.c
+++ b/libavformat/asfdec_f.c
@@ -978,6 +978,7 @@ static int asf_get_packet(AVFormatContext *s, AVIOContext
*pb)
avio_seek(pb, -1, SEEK_CUR); // FIXME
}
} else {
+d = e = 0;
c = avio_r8(pb);
if (c & 0x80) {
rsize ++;
--
ffmpeg-codebot
___
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 09/10] libavformat/asfdec: fix parameter type in asf_read_stream_propertie()
From: softworkz
Signed-off-by: softworkz
---
libavformat/asfdec_f.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
index cb396cccfe..95cab8b960 100644
--- a/libavformat/asfdec_f.c
+++ b/libavformat/asfdec_f.c
@@ -324,7 +324,7 @@ static int asf_read_file_properties(AVFormatContext *s)
return 0;
}
-static int asf_read_stream_properties(AVFormatContext *s, int64_t size)
+static int asf_read_stream_properties(AVFormatContext *s, uint64_t size)
{
ASFContext *asf = s->priv_data;
AVIOContext *pb = s->pb;
--
ffmpeg-codebot
___
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 10/10] libavformat/asfdec: fix variable types and add checks for unsupported values
From: softworkz
Signed-off-by: softworkz
---
libavformat/asfdec_f.c | 168 ++---
1 file changed, 108 insertions(+), 60 deletions(-)
diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
index 95cab8b960..d50682b901 100644
--- a/libavformat/asfdec_f.c
+++ b/libavformat/asfdec_f.c
@@ -333,9 +333,9 @@ static int asf_read_stream_properties(AVFormatContext *s,
uint64_t size)
ASFStream *asf_st;
ff_asf_guid g;
enum AVMediaType type;
-int type_specific_size, sizeX;
-unsigned int tag1;
-int64_t pos1, pos2, start_time;
+unsigned int tag1, type_specific_size, sizeX;
+int64_t pos1, pos2;
+uint32_t start_time;
int test_for_ext_stream_audio, is_dvr_ms_audio = 0;
if (s->nb_streams == ASF_MAX_STREAMS) {
@@ -404,7 +404,14 @@ static int asf_read_stream_properties(AVFormatContext *s,
uint64_t size)
st->codecpar->codec_type = type;
if (type == AVMEDIA_TYPE_AUDIO) {
-int ret = ff_get_wav_header(s, pb, st->codecpar, type_specific_size,
0);
+int ret;
+
+if (type_specific_size > INT32_MAX) {
+av_log(s, AV_LOG_DEBUG, "Unsupported WAV header size (>
INT32_MAX)\n");
+return AVERROR(ENOTSUP);
+}
+
+ret = ff_get_wav_header(s, pb, st->codecpar, (int)type_specific_size,
0);
if (ret < 0)
return ret;
if (is_dvr_ms_audio) {
@@ -434,21 +441,32 @@ static int asf_read_stream_properties(AVFormatContext *s,
uint64_t size)
}
} else if (type == AVMEDIA_TYPE_VIDEO &&
size - (avio_tell(pb) - pos1 + 24) >= 51) {
+unsigned int width, height;
avio_rl32(pb);
avio_rl32(pb);
avio_r8(pb);
avio_rl16(pb);/* size */
-sizeX = avio_rl32(pb); /* size */
-st->codecpar->width = avio_rl32(pb);
-st->codecpar->height = avio_rl32(pb);
+sizeX = avio_rl32(pb); /* size */
+width = avio_rl32(pb);
+height = avio_rl32(pb);
+
+if (width > INT32_MAX || height > INT32_MAX) {
+av_log(s, AV_LOG_DEBUG, "Unsupported video size %dx%d\n", width,
height);
+return AVERROR(ENOTSUP);
+}
+
+st->codecpar->width = (int)width;
+st->codecpar->height = (int)height;
/* not available for asf */
avio_rl16(pb); /* panes */
st->codecpar->bits_per_coded_sample = avio_rl16(pb); /* depth */
tag1 = avio_rl32(pb);
avio_skip(pb, 20);
if (sizeX > 40) {
-if (size < sizeX - 40 || sizeX - 40 > INT_MAX -
AV_INPUT_BUFFER_PADDING_SIZE)
-return AVERROR_INVALIDDATA;
+if (size < sizeX - 40 || sizeX - 40 > INT_MAX -
AV_INPUT_BUFFER_PADDING_SIZE) {
+av_log(s, AV_LOG_DEBUG, "Unsupported extradata size\n");
+return AVERROR(ENOTSUP);
+}
st->codecpar->extradata_size = ffio_limit(pb, sizeX - 40);
st->codecpar->extradata =
av_mallocz(st->codecpar->extradata_size +
AV_INPUT_BUFFER_PADDING_SIZE);
@@ -500,9 +518,9 @@ static int asf_read_ext_stream_properties(AVFormatContext
*s)
ASFContext *asf = s->priv_data;
AVIOContext *pb = s->pb;
ff_asf_guid g;
-int ext_len, payload_ext_ct, stream_ct, i;
-uint32_t leak_rate, stream_num;
-unsigned int stream_languageid_index;
+uint16_t payload_ext_ct, stream_ct, i;
+uint32_t leak_rate, ext_len;
+uint16_t stream_languageid_index, stream_num;
avio_rl64(pb); // starttime
avio_rl64(pb); // endtime
@@ -514,15 +532,15 @@ static int asf_read_ext_stream_properties(AVFormatContext
*s)
avio_rl32(pb); // alt-init-bucket-fullness
avio_rl32(pb); // max-object-size
avio_rl32(pb); // flags
(reliable,seekable,no_cleanpoints?,resend-live-cleanpoints, rest of bits
reserved)
-stream_num = avio_rl16(pb); // stream-num
+stream_num = (uint16_t)avio_rl16(pb); // stream-num
-stream_languageid_index = avio_rl16(pb); // stream-language-id-index
+stream_languageid_index = (uint16_t)avio_rl16(pb); //
stream-language-id-index
if (stream_num < 128)
asf->streams[stream_num].stream_language_index =
stream_languageid_index;
avio_rl64(pb); // avg frametime in 100ns units
-stream_ct = avio_rl16(pb); // stream-name-count
-payload_ext_ct = avio_rl16(pb); // payload-extension-system-count
+stream_ct = (uint16_t)avio_rl16(pb); // stream-name-count
+payload_ext_ct = (uint16_t)avio_rl16(pb); // payload-extension-system-count
if (stream_num < 128) {
asf->stream_bitrates[stream_num] = leak_rate;
@@ -536,12 +554,10 @@ static int asf_read_ext_stream_properties(AVFormatContext
*s)
}
for (i = 0; i < payload_ext_ct; i++) {
-int size;
+uint16_t size;
ff_get_guid(pb, &g);
-size = avio_
