[FFmpeg-devel] [PATCH v2] libavfilter/vf_overlay_qsv: Use format of first input to set output format for overlay_qsv

2022-07-06 Thread Wenbin Chen
overlay_qsv hard coded to use nv12 as output format. Now use the format
of the first input to set output format.

For detailed information of supported format on different platform,
please see the "composition" rows in "Video Processing Features" at
below link:
https://www.intel.com/content/www/us/en/develop/documentation/media-capabilities-of-intel-hardware/top.html

Signed-off-by: Wenbin Chen 
---
 libavfilter/vf_overlay_qsv.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavfilter/vf_overlay_qsv.c b/libavfilter/vf_overlay_qsv.c
index 7e76b39aa9..d947a1faa1 100644
--- a/libavfilter/vf_overlay_qsv.c
+++ b/libavfilter/vf_overlay_qsv.c
@@ -276,6 +276,7 @@ static int config_output(AVFilterLink *outlink)
 int ret;
 
 av_log(ctx, AV_LOG_DEBUG, "Output is of %s.\n", 
av_get_pix_fmt_name(outlink->format));
+vpp->qsv_param.out_sw_format = in0->format;
 if ((in0->format == AV_PIX_FMT_QSV && in1->format != AV_PIX_FMT_QSV) ||
 (in0->format != AV_PIX_FMT_QSV && in1->format == AV_PIX_FMT_QSV)) {
 av_log(ctx, AV_LOG_ERROR, "Mixing hardware and software pixel formats 
is not supported.\n");
@@ -288,6 +289,7 @@ static int config_output(AVFilterLink *outlink)
 av_log(ctx, AV_LOG_ERROR, "Inputs with different underlying QSV 
devices are forbidden.\n");
 return AVERROR(EINVAL);
 }
+vpp->qsv_param.out_sw_format = hw_frame0->sw_format;
 }
 
 outlink->w  = vpp->var_values[VAR_MW];
-- 
2.32.0

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

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


[FFmpeg-devel] [PATCH] Add metadatareader filter.

2022-07-06 Thread Raymond Cheng
FFmpeg has a handy set of filters, metadata/ametadata, which allow us
to add, print or save per-frame metadata to a file. I will use these
filters when I want to save the results from speech transcription,
for instance. What is missing is a way to round-trip the saved
metadata, so I decided to add filters which go in the opposite
direction: metadatareader/ametadatareader. These filters inject
per-frame metadata into the filter graph from a previously saved
metadata/ametadata file.

If you had a speech transcription filter called speech_recognition
which produced per-frame metadata using the key "transcription",
here is how you might use the metadata reader to burn hard subtitles:

Pass 1: ffmpeg -i input -vn -sn -af 
speech_recognition,ametadata=mode=print:file=transcription.txt -f null /dev/null
Pass 2: ffmpeg -i input -vf 
metadatareader=file=transcription.txt,drawtext=fontsize=30:x=30:y=30:fontcolor=yellow:text="'%{metadata\:transcription}'"
 out.mp4

It should be noted in the example above that the metadata crossed
over from audio to video. It was saved from audio in Pass 1, and
applied to video on Pass 2. This is perfectly valid, as well as
non-crossover applications.

Signed-off-by: Raymond Cheng 
---
 Changelog |   1 +
 configure |   2 +
 libavfilter/Makefile  |   2 +
 libavfilter/allfilters.c  |   2 +
 libavfilter/f_metadatareader.c| 455 +
 libavutil/error_tracing.h | 126 +
 tests/fate-run.sh |   3 +
 tests/fate/filter-audio.mak   |   7 +
 tests/fate/filter-video.mak   |   6 +
 tests/ref/fate/filter-ametadatareader | 690 ++
 tests/ref/fate/filter-metadatareader  |  14 +
 11 files changed, 1308 insertions(+)
 create mode 100644 libavfilter/f_metadatareader.c
 create mode 100644 libavutil/error_tracing.h
 create mode 100644 tests/ref/fate/filter-ametadatareader
 create mode 100644 tests/ref/fate/filter-metadatareader

diff --git a/Changelog b/Changelog
index ba679aa64e..646eac87e9 100644
--- a/Changelog
+++ b/Changelog
@@ -23,6 +23,7 @@ version 5.1:
 - virtualbass audio filter
 - VDPAU AV1 hwaccel
 - PHM image format support
+- metadatareader video and ametadatareader audio filter
 
 
 version 5.0:
diff --git a/configure b/configure
index fea512e8ef..8262f8d708 100755
--- a/configure
+++ b/configure
@@ -3614,6 +3614,7 @@ libzmq_protocol_select="network"
 
 # filters
 ametadata_filter_deps="avformat"
+ametadatareader_filter_deps="avformat"
 amovie_filter_deps="avcodec avformat"
 aresample_filter_deps="swresample"
 asr_filter_deps="pocketsphinx"
@@ -3679,6 +3680,7 @@ libplacebo_filter_deps="libplacebo vulkan"
 lv2_filter_deps="lv2"
 mcdeint_filter_deps="avcodec gpl"
 metadata_filter_deps="avformat"
+metadatareader_filter_deps="avformat"
 movie_filter_deps="avcodec avformat"
 mpdecimate_filter_deps="gpl"
 mpdecimate_filter_select="pixelutils"
diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index 22b0a0ca15..15de9b3815 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -71,6 +71,7 @@ OBJS-$(CONFIG_ALLPASS_FILTER)+= af_biquads.o
 OBJS-$(CONFIG_ALOOP_FILTER)  += f_loop.o
 OBJS-$(CONFIG_AMERGE_FILTER) += af_amerge.o
 OBJS-$(CONFIG_AMETADATA_FILTER)  += f_metadata.o
+OBJS-$(CONFIG_AMETADATAREADER_FILTER)+= f_metadatareader.o
 OBJS-$(CONFIG_AMIX_FILTER)   += af_amix.o
 OBJS-$(CONFIG_AMULTIPLY_FILTER)  += af_amultiply.o
 OBJS-$(CONFIG_ANEQUALIZER_FILTER)+= af_anequalizer.o
@@ -365,6 +366,7 @@ OBJS-$(CONFIG_MEDIAN_FILTER) += vf_median.o
 OBJS-$(CONFIG_MERGEPLANES_FILTER)+= vf_mergeplanes.o framesync.o
 OBJS-$(CONFIG_MESTIMATE_FILTER)  += vf_mestimate.o 
motion_estimation.o
 OBJS-$(CONFIG_METADATA_FILTER)   += f_metadata.o
+OBJS-$(CONFIG_METADATAREADER_FILTER) += f_metadatareader.o
 OBJS-$(CONFIG_MIDEQUALIZER_FILTER)   += vf_midequalizer.o framesync.o
 OBJS-$(CONFIG_MINTERPOLATE_FILTER)   += vf_minterpolate.o 
motion_estimation.o
 OBJS-$(CONFIG_MIX_FILTER)+= vf_mix.o framesync.o
diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
index ec70feef11..3e58e52ea7 100644
--- a/libavfilter/allfilters.c
+++ b/libavfilter/allfilters.c
@@ -58,6 +58,7 @@ extern const AVFilter ff_af_allpass;
 extern const AVFilter ff_af_aloop;
 extern const AVFilter ff_af_amerge;
 extern const AVFilter ff_af_ametadata;
+extern const AVFilter ff_af_ametadatareader;
 extern const AVFilter ff_af_amix;
 extern const AVFilter ff_af_amultiply;
 extern const AVFilter ff_af_anequalizer;
@@ -346,6 +347,7 @@ extern const AVFilter ff_vf_median;
 extern const AVFilter ff_vf_mergeplanes;
 extern const AVFilter ff_vf_mestimate;
 extern const AVFilter ff_vf_metadata;
+extern const AVFilter ff_vf_metadatareader;
 extern const AVFilter 

[FFmpeg-devel] [PATCH] Support bidirectional metadata in drawtext filter

2022-07-06 Thread Raymond Cheng
The drawtext filter supports static bidi text via a function called
shape_text(). Fixed so that it calls shape_text() when rendering
non-static text from metadata (so that bidi text is rendered properly).

As an example, "Hello world" is "مرحبا بالعالم" in Arabic. The
following command line worked just fine before, and still works
after this change:

  ffmpeg -i input -vf 
drawtext=fontsize=30:x=30:y=30:fontcolor=yellow:text='مرحبا بالعالم' out.mp4

However, this command line did NOT work:

  ffmpeg -i input -vf metadata=mode=add:key=transcription:value='مرحبا 
بالعالم',drawtext=fontsize=30:x=30:y=30:fontcolor=yellow:text="'From metadata\: 
%{metadata\:transcription}'" out.mp4

This commit fixes it so that this second command line now works.

NOTE that the above command lines are for example only. They render
the proper text, but improperly justified (left-justified instead of
right-justified). For one-line transcriptions, this is easily fixed
by replacing x=30 with x=\(700-tw\). Two-line transcriptions do not
have a simple solution.

Signed-off-by: Raymond Cheng 
---
 libavfilter/vf_drawtext.c | 57 +--
 1 file changed, 37 insertions(+), 20 deletions(-)

diff --git a/libavfilter/vf_drawtext.c b/libavfilter/vf_drawtext.c
index feb6898848..9e4a63b7fd 100644
--- a/libavfilter/vf_drawtext.c
+++ b/libavfilter/vf_drawtext.c
@@ -608,7 +608,7 @@ static int load_textfile(AVFilterContext *ctx)
 }
 
 #if CONFIG_LIBFRIBIDI
-static int shape_text(AVFilterContext *ctx)
+static int shape_text_arg(AVFilterContext *ctx, char **ppText)
 {
 DrawTextContext *s = ctx->priv;
 uint8_t *tmp;
@@ -625,12 +625,15 @@ static int shape_text(AVFilterContext *ctx)
 FriBidiCharType *bidi_types = NULL;
 FriBidiStrIndex i,j;
 
-len = strlen(s->text);
+if (!s->text_shaping)
+return 0; // Do nothing
+
+len = strlen(*ppText);
 if (!(unicodestr = av_malloc_array(len, sizeof(*unicodestr {
 goto out;
 }
 len = fribidi_charset_to_unicode(FRIBIDI_CHAR_SET_UTF8,
- s->text, len, unicodestr);
+ *ppText, len, unicodestr);
 
 bidi_types = av_malloc_array(len, sizeof(*bidi_types));
 if (!bidi_types) {
@@ -676,14 +679,14 @@ static int shape_text(AVFilterContext *ctx)
 unicodestr[j++] = unicodestr[i];
 len = j;
 
-if (!(tmp = av_realloc(s->text, (len * 4 + 1) * sizeof(*s->text {
+if (!(tmp = av_realloc(*ppText, (len * 4 + 1) * sizeof(**ppText {
 /* Use len * 4, as a unicode character can be up to 4 bytes in UTF-8 */
 goto out;
 }
 
-s->text = tmp;
+*ppText = tmp;
 len = fribidi_unicode_to_charset(FRIBIDI_CHAR_SET_UTF8,
- unicodestr, len, s->text);
+ unicodestr, len, *ppText);
 ret = 0;
 
 out:
@@ -693,8 +696,19 @@ out:
 av_free(bidi_types);
 return ret;
 }
+#else
+static int shape_text_arg(AVFilterContext *ctx, char **ppText)
+{
+return 0;
+}
 #endif
 
+static int shape_text(AVFilterContext *ctx)
+{
+DrawTextContext *s = ctx->priv;
+return shape_text_arg(ctx, (char **)>text);
+}
+
 static enum AVFrameSideDataType text_source_string_parse(const char 
*text_source_string)
 {
 av_assert0(text_source_string);
@@ -771,11 +785,8 @@ static av_cold int init(AVFilterContext *ctx)
 return AVERROR(EINVAL);
 }
 
-#if CONFIG_LIBFRIBIDI
-if (s->text_shaping)
-if ((err = shape_text(ctx)) < 0)
-return err;
-#endif
+if ((err = shape_text(ctx)) < 0)
+return err;
 
 if ((err = FT_Init_FreeType(&(s->library {
 av_log(ctx, AV_LOG_ERROR,
@@ -1034,11 +1045,19 @@ static int func_metadata(AVFilterContext *ctx, AVBPrint 
*bp,
 {
 DrawTextContext *s = ctx->priv;
 AVDictionaryEntry *e = av_dict_get(s->metadata, argv[0], NULL, 0);
+int err;
+
+if (e && e->value) {
+if ((err = shape_text_arg(ctx, >value)) < 0)
+return err;
 
-if (e && e->value)
 av_bprintf(bp, "%s", e->value);
-else if (argc >= 2)
+} else if (argc >= 2) {
+if ((err = shape_text_arg(ctx, [1])) < 0)
+return err;
+
 av_bprintf(bp, "%s", argv[1]);
+}
 return 0;
 }
 
@@ -1634,13 +1653,11 @@ static int filter_frame(AVFilterLink *inlink, AVFrame 
*frame)
 av_frame_free();
 return ret;
 }
-#if CONFIG_LIBFRIBIDI
-if (s->text_shaping)
-if ((ret = shape_text(ctx)) < 0) {
-av_frame_free();
-return ret;
-}
-#endif
+
+if ((ret = shape_text(ctx)) < 0) {
+av_frame_free();
+return ret;
+}
 }
 
 s->var_values[VAR_N] = inlink->frame_count_out + s->start_number;
-- 
2.34.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org

Re: [FFmpeg-devel] [PATCH v2 2/2] avcodec/libopusdec: Enable FEC/PLC

2022-07-06 Thread Lynne
Jul 4, 2022, 16:13 by philip-dylan.gleo...@savoirfairelinux.com:

> Adds FEC/PLC support to libopus. The lost packets are detected as a
> discontinuity in the audio stream. When a discontinuity is used, this
> patch tries to decode the FEC data. If FEC data is present in the
> packet, it is decoded, otherwise audio is re-created through PLC.
>
> This patch is based on Steinar H. Gunderson contribution, and corrects
> the pts computation: all pts are expressed in samples instead of time.
> This patch also adds an option "decode_fec" which enables or disables
> FEC decoding. This option is disabled by default to keep consistent
> behaviour with former versions.
>
> A number of checks are made to ensure compatibility with different
> containers. Indeed, video containers seem to have a pts expressed in ms
> while it is expressed in samples for audio containers. It also manages
> the cases where pkt->duration is 0, in some RTP streams. This patch
> ignores data it can not reconstruct, i.e. packets received twice and
> packets with a length that is not a multiple of 2.5ms.
>
> Signed-off-by: Philip-Dylan Gleonec 
> 
> Co-developed-by: Steinar H. Gunderson 
> ---
>  libavcodec/libopusdec.c | 105 +++-
>  1 file changed, 94 insertions(+), 11 deletions(-)
>
> diff --git a/libavcodec/libopusdec.c b/libavcodec/libopusdec.c
> index 316ab0f2a7..f5d0e95fc8 100644
> --- a/libavcodec/libopusdec.c
> +++ b/libavcodec/libopusdec.c
> @@ -44,10 +44,15 @@ struct libopus_context {
>  #ifdef OPUS_SET_PHASE_INVERSION_DISABLED_REQUEST
>  int apply_phase_inv;
>  #endif
> +int decode_fec;
> +int64_t expected_next_pts;
>  };
>  
>  #define OPUS_HEAD_SIZE 19
>  
> +// Sample rate is constant as libopus always output at 48kHz
> +static const AVRational opus_timebase = { 1, 48000 };
> +
>  static av_cold int libopus_decode_init(AVCodecContext *avc)
>  {
>  struct libopus_context *opus = avc->priv_data;
> @@ -140,6 +145,8 @@ static av_cold int libopus_decode_init(AVCodecContext 
> *avc)
>  /* Decoder delay (in samples) at 48kHz */
>  avc->delay = avc->internal->skip_samples = opus->pre_skip;
>  
> +opus->expected_next_pts = AV_NOPTS_VALUE;
> +
>  return 0;
>  }
>  
> @@ -160,25 +167,100 @@ static int libopus_decode(AVCodecContext *avc, AVFrame 
> *frame,
>  int *got_frame_ptr, AVPacket *pkt)
>  {
>  struct libopus_context *opus = avc->priv_data;
> -int ret, nb_samples;
> +uint8_t *outptr;
> +int ret, nb_samples = 0, nb_lost_samples = 0, nb_samples_left;
> +
> +// If FEC is enabled, calculate number of lost samples
> +if (opus->decode_fec &&
> +opus->expected_next_pts != AV_NOPTS_VALUE &&
> +pkt->pts != AV_NOPTS_VALUE &&
> +pkt->pts != opus->expected_next_pts) {
> +// Cap at recovering 120 ms of lost audio.
> +nb_lost_samples = pkt->pts - opus->expected_next_pts;
> +nb_lost_samples = FFMIN(nb_lost_samples, MAX_FRAME_SIZE);
> +// pts is expressed in ms for some containers (e.g. mkv)
> +// FEC only works for SILK frames (> 10ms)
> +// Detect if nb_lost_samples is in ms, and convert in samples if it 
> is
> +if (nb_lost_samples > 0) {
> +if (avc->pkt_timebase.den != 48000) {
> +nb_lost_samples = av_rescale_q(nb_lost_samples, 
> avc->pkt_timebase, opus_timebase);
> +}
> +// For FEC/PLC, frame_size has to be to have a multiple of 2.5 ms
> +if (nb_lost_samples % (5LL * opus_timebase.den / 2000)) {
> +nb_lost_samples -= nb_lost_samples % (5LL * 
> opus_timebase.den / 2000);
>

I counted not two, but three different coding styles used in both patches. Fix 
it.

The lost samples count is very wrong for Ogg Opus files, and in general
it's simply incorrect. You *always* need to convert PTS into samples
properly via the timebase, and you even hardcode a random timebase that
happens to correspond to samples in some conditions.
Corrupt ogg files have a duration that's 10x longer than the input, whilst
disabling FEC or using our native decoder outputs a correct number of samples.

You can use
ffmpeg -i test.opus -c:a copy -bsf:a noise=amount=0:dropamount=2 -y test2.opus
to corrupt Ogg Opus files and test yourself. I used 100% packet loss percentage
for my tests.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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


[FFmpeg-devel] [PATCH] avfilter: add vsrc_ddagrab

2022-07-06 Thread Timo Rothenpieler
---
 Changelog  |   1 +
 configure  |   7 +
 doc/filters.texi   |  68 ++
 libavfilter/Makefile   |   1 +
 libavfilter/allfilters.c   |   1 +
 libavfilter/version.h  |   2 +-
 libavfilter/vsrc_ddagrab.c | 973 +
 libavfilter/vsrc_ddagrab_shaders.h | 120 
 8 files changed, 1172 insertions(+), 1 deletion(-)
 create mode 100644 libavfilter/vsrc_ddagrab.c
 create mode 100644 libavfilter/vsrc_ddagrab_shaders.h

diff --git a/Changelog b/Changelog
index ba679aa64e..1a9fdcb412 100644
--- a/Changelog
+++ b/Changelog
@@ -23,6 +23,7 @@ version 5.1:
 - virtualbass audio filter
 - VDPAU AV1 hwaccel
 - PHM image format support
+- ddagrab (Desktop Duplication) video source filter
 
 
 version 5.0:
diff --git a/configure b/configure
index fea512e8ef..12ce83f07d 100755
--- a/configure
+++ b/configure
@@ -2309,6 +2309,7 @@ SYSTEM_FUNCS="
 SetDllDirectory
 setmode
 setrlimit
+SetThreadDpiAwarenessContext
 Sleep
 strerror_r
 sysconf
@@ -2352,6 +2353,7 @@ TOOLCHAIN_FEATURES="
 "
 
 TYPES_LIST="
+IDXGIOutput5
 kCMVideoCodecType_HEVC
 kCMVideoCodecType_HEVCWithAlpha
 kCMVideoCodecType_VP9
@@ -3154,6 +3156,8 @@ overlay_cuda_filter_deps="ffnvcodec"
 overlay_cuda_filter_deps_any="cuda_nvcc cuda_llvm"
 sharpen_npp_filter_deps="ffnvcodec libnpp"
 
+ddagrab_filter_deps="d3d11va IDXGIOutput1"
+
 amf_deps_any="libdl LoadLibrary"
 nvenc_deps="ffnvcodec"
 nvenc_deps_any="libdl LoadLibrary"
@@ -6388,10 +6392,13 @@ check_struct "sys/time.h sys/resource.h" "struct 
rusage" ru_maxrss
 check_type "windows.h dxva.h" "DXVA_PicParams_AV1" 
-DWINAPI_FAMILY=WINAPI_FAMILY_DESKTOP_APP -D_CRT_BUILD_DESKTOP_APP=0
 check_type "windows.h dxva.h" "DXVA_PicParams_HEVC" 
-DWINAPI_FAMILY=WINAPI_FAMILY_DESKTOP_APP -D_CRT_BUILD_DESKTOP_APP=0
 check_type "windows.h dxva.h" "DXVA_PicParams_VP9" 
-DWINAPI_FAMILY=WINAPI_FAMILY_DESKTOP_APP -D_CRT_BUILD_DESKTOP_APP=0
+check_type "windows.h dxgi1_2.h" "IDXGIOutput1"
+check_type "windows.h dxgi1_5.h" "IDXGIOutput5"
 check_type "windows.h d3d11.h" "ID3D11VideoDecoder"
 check_type "windows.h d3d11.h" "ID3D11VideoContext"
 check_type "d3d9.h dxva2api.h" DXVA2_ConfigPictureDecode -D_WIN32_WINNT=0x0602
 check_func_headers mfapi.h MFCreateAlignedMemoryBuffer -lmfplat
+check_func_headers windows.h SetThreadDpiAwarenessContext -D_WIN32_WINNT=0x0605
 
 check_type "vdpau/vdpau.h" "VdpPictureInfoHEVC"
 check_type "vdpau/vdpau.h" "VdpPictureInfoVP9"
diff --git a/doc/filters.texi b/doc/filters.texi
index 5a889895c6..8a872084ee 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -26366,6 +26366,74 @@ need for a nullsrc video source.
 @end itemize
 
 
+@section ddagrab
+
+Captures the Windows Desktop via Desktop Duplication API.
+
+The filter exclusively returns D3D11 Hardware Frames, for on-gpu encoding
+or processing. So an explicit @ref{hwdownload} is needed for any kind of
+software processing.
+
+It accepts the following options:
+
+@table @option
+@item output_idx
+DXGI Output Index to capture.
+
+Usually corresponds to the index Windows has given the screen minus one,
+so it's starting at 0.
+
+Defaults to output 0.
+
+@item draw_mouse
+Whether to draw the mouse cursor.
+
+Defaults to true.
+
+Only affects hardware cursors. If a game or application renders its own cursor,
+it'll always be captured.
+
+@item framerate
+Framerate at which the desktop will be captured.
+
+Defaults to 30 FPS.
+
+@item video_size
+Specify the size of the captured video.
+
+Defaults to the full size of the screen.
+
+Cropped from the bottom/right if smaller than screen size.
+
+@item offset_x
+Horizontal offset of the captured video.
+
+@item offset_y
+Vertical offset of the captured video.
+
+@end table
+
+@subsection Examples
+
+Capture primary screen and encode using nvenc:
+@example
+ffmpeg -f lavfi -i ddagrab -c:v h264_nvenc -cq 18 output.mp4
+@end example
+
+You can also skip the lavfi device and directly use the filter.
+Also demonstrates downloading the frame and encoding with libx264.
+Explicit output format specification is required in this case:
+@example
+ffmpeg -filter_complex 
ddagrab=output_idx=1:framerate=60,hwdownload,format=bgra -c:v libx264 -crf 18 
output.mp4
+@end example
+
+If you want to capture only a subsection of the desktop, this can be achieved
+by specifying a smaller size and its offsets into the screen:
+@example
+ddagrab=video_size=800x600:offset_x=100:offset_y=100
+@end example
+
+
 @section gradients
 Generate several gradients.
 
diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index 22b0a0ca15..6356c3ae59 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -553,6 +553,7 @@ OBJS-$(CONFIG_COLOR_FILTER)  += 
vsrc_testsrc.o
 OBJS-$(CONFIG_COLORCHART_FILTER) += vsrc_testsrc.o
 OBJS-$(CONFIG_COLORSPECTRUM_FILTER)  += vsrc_testsrc.o
 OBJS-$(CONFIG_COREIMAGESRC_FILTER)   += 

Re: [FFmpeg-devel] [PATCH 3/3] avformat/mov: disallow a zero sample size in trun atoms

2022-07-06 Thread Marton Balint



On Tue, 28 Jun 2022, "zhilizhao(赵志立)" wrote:





On Jun 28, 2022, at 4:02 AM, Marton Balint  wrote:

In order to not generate 0 sized packets or create a huge index table
needlessly.

Fixes: Timeout
Fixes: 
43717/clusterfuzz-testcase-minimized-ffmpeg_IO_DEMUXER_fuzzer-5206008287330304
Fixes: 
45738/clusterfuzz-testcase-minimized-ffmpeg_IO_DEMUXER_fuzzer-6142535657979904

Signed-off-by: Marton Balint 
---
libavformat/mov.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index c6fbe511c0..d7ef6ba6d6 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -5179,6 +5179,8 @@ static int mov_read_trun(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)
distance++;
if (av_sat_add64(dts, sample_duration) != dts + 
(uint64_t)sample_duration)
return AVERROR_INVALIDDATA;
+if (!sample_size)
+return AVERROR_INVALIDDATA;
dts += sample_duration;
offset += sample_size;
sc->data_size += sample_size;


LGTM.


Thanks, applied the series.

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

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


[FFmpeg-devel] [PATCH 2/2] avutil/hwcontext_d3d11va: fix texture_infos writes on non-fixed-size pools

2022-07-06 Thread Timo Rothenpieler
---
 libavutil/hwcontext_d3d11va.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/libavutil/hwcontext_d3d11va.c b/libavutil/hwcontext_d3d11va.c
index e5afcb2a9d..6355bd1e29 100644
--- a/libavutil/hwcontext_d3d11va.c
+++ b/libavutil/hwcontext_d3d11va.c
@@ -166,6 +166,17 @@ static AVBufferRef *wrap_texture_buf(AVHWFramesContext 
*ctx, ID3D11Texture2D *te
 return NULL;
 }
 
+if (s->nb_surfaces <= s->nb_surfaces_used) {
+frames_hwctx->texture_infos = av_realloc_f(frames_hwctx->texture_infos,
+   s->nb_surfaces_used + 1,
+   
sizeof(*frames_hwctx->texture_infos));
+if (!frames_hwctx->texture_infos) {
+ID3D11Texture2D_Release(tex);
+return NULL;
+}
+s->nb_surfaces = s->nb_surfaces_used + 1;
+}
+
 frames_hwctx->texture_infos[s->nb_surfaces_used].texture = tex;
 frames_hwctx->texture_infos[s->nb_surfaces_used].index = index;
 s->nb_surfaces_used++;
@@ -284,7 +295,7 @@ static int d3d11va_frames_init(AVHWFramesContext *ctx)
 }
 }
 
-hwctx->texture_infos = av_calloc(ctx->initial_pool_size, 
sizeof(*hwctx->texture_infos));
+hwctx->texture_infos = av_realloc_f(NULL, ctx->initial_pool_size, 
sizeof(*hwctx->texture_infos));
 if (!hwctx->texture_infos)
 return AVERROR(ENOMEM);
 s->nb_surfaces = ctx->initial_pool_size;
-- 
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 1/2] avutil/hwcontext_d3d11va: fix mixed declaration and code

2022-07-06 Thread Timo Rothenpieler
---
 libavutil/hwcontext_d3d11va.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libavutil/hwcontext_d3d11va.c b/libavutil/hwcontext_d3d11va.c
index 904d14bbc8..e5afcb2a9d 100644
--- a/libavutil/hwcontext_d3d11va.c
+++ b/libavutil/hwcontext_d3d11va.c
@@ -397,6 +397,7 @@ static int d3d11va_transfer_data(AVHWFramesContext *ctx, 
AVFrame *dst,
 D3D11_TEXTURE2D_DESC desc;
 D3D11_MAPPED_SUBRESOURCE map;
 HRESULT hr;
+int res;
 
 if (frame->hw_frames_ctx->data != (uint8_t *)ctx || other->format != 
ctx->sw_format)
 return AVERROR(EINVAL);
@@ -405,7 +406,7 @@ static int d3d11va_transfer_data(AVHWFramesContext *ctx, 
AVFrame *dst,
 
 if (!s->staging_texture) {
 ID3D11Texture2D_GetDesc((ID3D11Texture2D *)texture, );
-int res = d3d11va_create_staging_texture(ctx, desc.Format);
+res = d3d11va_create_staging_texture(ctx, desc.Format);
 if (res < 0)
 return res;
 }
-- 
2.34.1

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

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


Re: [FFmpeg-devel] [PATCH] avcodec: add Radiance HDR image format support

2022-07-06 Thread Paul B Mahol
On Wed, Jul 6, 2022 at 8:50 PM James Almer  wrote:

> On 7/6/2022 3:47 PM, Paul B Mahol wrote:
> > Added parser.
> > The test will be added after this is merged.
>
> [...]
>
> > diff --git a/libavformat/img2.c b/libavformat/img2.c
> > index 870d2ebbc5..b03075f3b0 100644
> > --- a/libavformat/img2.c
> > +++ b/libavformat/img2.c
> > @@ -91,6 +91,7 @@ const IdStrMap ff_img_tags[] = {
> >  { AV_CODEC_ID_VBN,"vbn"  },
> >  { AV_CODEC_ID_JPEGXL, "jxl"  },
> >  { AV_CODEC_ID_QOI,"qoi"  },
> > +{ AV_CODEC_ID_HDR,"hdr"  },
>
> This could end up confusing people, so preferably use RHDR or some other
> combination for the codec id.
>

Take it or leave it.

IT is HDR, not going to change.

Confusing can not happen at all it is codec.

>
> >  { AV_CODEC_ID_NONE,   NULL   }
> >  };
>
> ___
> 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: add Radiance HDR image format support

2022-07-06 Thread Andreas Rheinhardt
James Almer:
> 
> 
> On 7/6/2022 4:09 PM, Andreas Rheinhardt wrote:
>> James Almer:
>>> On 7/6/2022 3:47 PM, Paul B Mahol wrote:
 Added parser.
 The test will be added after this is merged.
>>>
>>> [...]
>>>
 +static int hdr_parse(AVCodecParserContext *s, AVCodecContext *avctx,
 + const uint8_t **poutbuf, int *poutbuf_size,
 + const uint8_t *buf, int buf_size)
 +{
 +    HDRParseContext *ipc = s->priv_data;
 +    uint64_t state = ipc->pc.state64;
 +    int next = END_NOT_FOUND, i = 0;
 +
 +    s->pict_type = AV_PICTURE_TYPE_NONE;
>>>
>>> pict_type I, and key frame?
>>>
>>> [...]
>>>
 +static int hdr_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
 +    const AVFrame *frame, int *got_packet)
 +{
 +    HDREncContext *s = avctx->priv_data;
 +    int64_t packet_size;
 +    uint8_t *buf;
 +    int ret;
 +
 +    packet_size = avctx->width * avctx->height * 4LL + 1024LL;
 +    if ((ret = ff_get_encode_buffer(avctx, pkt, packet_size, 0)) < 0)
>>>
>>> Wont using worst case scenario like this result in massive packets?
>>> av_shrink_packet() only changes the size field, it doesn't realloc the
>>> buffer to shrink it.
>>>
>>> You could allocate a buffer in HDREncContext with
>>> av_fast_padded_malloc() here, use the bytestream2 API below, then
>>> allocate the packet with ff_get_encode_buffer() using
>>> bytestream2_tell_p() as size and do a memcpy at the end.
>>>
>>
>> If that is the aim, then there is no reason to do the memcpy ourselves
>> (or do the av_fast_padded_malloc directly): Just use ff_alloc_packet()
>> and the generic code will ensure that the packet data will be made
>> refcounted. And the temp buffer will be freed generically, too.
> 
> The idea is to keep the encoder working with user provided buffers,
> while not allocating worst case scenario packets.
> 

This could also be done generically:
https://github.com/mkver/FFmpeg/commit/4c06a1e457fd00f2ad14c1328587d29964c9fa11
You were against it because you wanted some kind of checks (I don't
remember the details); checks the current code does not have.

- 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: add Radiance HDR image format support

2022-07-06 Thread James Almer



On 7/6/2022 4:09 PM, Andreas Rheinhardt wrote:

James Almer:

On 7/6/2022 3:47 PM, Paul B Mahol wrote:

Added parser.
The test will be added after this is merged.


[...]


+static int hdr_parse(AVCodecParserContext *s, AVCodecContext *avctx,
+ const uint8_t **poutbuf, int *poutbuf_size,
+ const uint8_t *buf, int buf_size)
+{
+    HDRParseContext *ipc = s->priv_data;
+    uint64_t state = ipc->pc.state64;
+    int next = END_NOT_FOUND, i = 0;
+
+    s->pict_type = AV_PICTURE_TYPE_NONE;


pict_type I, and key frame?

[...]


+static int hdr_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
+    const AVFrame *frame, int *got_packet)
+{
+    HDREncContext *s = avctx->priv_data;
+    int64_t packet_size;
+    uint8_t *buf;
+    int ret;
+
+    packet_size = avctx->width * avctx->height * 4LL + 1024LL;
+    if ((ret = ff_get_encode_buffer(avctx, pkt, packet_size, 0)) < 0)


Wont using worst case scenario like this result in massive packets?
av_shrink_packet() only changes the size field, it doesn't realloc the
buffer to shrink it.

You could allocate a buffer in HDREncContext with
av_fast_padded_malloc() here, use the bytestream2 API below, then
allocate the packet with ff_get_encode_buffer() using
bytestream2_tell_p() as size and do a memcpy at the end.



If that is the aim, then there is no reason to do the memcpy ourselves
(or do the av_fast_padded_malloc directly): Just use ff_alloc_packet()
and the generic code will ensure that the packet data will be made
refcounted. And the temp buffer will be freed generically, too.


The idea is to keep the encoder working with user provided buffers, 
while not allocating worst case scenario packets.





+    return ret;
+
+    buf = pkt->data;
+    bytestream_put_str(, "#?RADIANCE\n");
+    bytestream_put_str(, "SOFTWARE=lavc\n");
+    ret = snprintf(buf, 32, "PIXASPECT=%f\n",
av_q2d(av_inv_q(avctx->sample_aspect_ratio)));
+    if (ret > 0)
+    buf += ret;
+    bytestream_put_str(, "FORMAT=32-bit_rle_rgbe\n\n");
+    ret = snprintf(buf, 32, "-Y %d +X %d\n", avctx->height,
avctx->width);
+    if (ret > 0)
+    buf += ret;
+
+    for (int y = 0; y < avctx->height; y++) {
+    const float *red   = (const float *)(frame->data[2] + y *
frame->linesize[2]);
+    const float *green = (const float *)(frame->data[0] + y *
frame->linesize[0]);
+    const float *blue  = (const float *)(frame->data[1] + y *
frame->linesize[1]);
+
+    if (avctx->width < 8 || avctx->width > 0x7fff) {
+    for (int x = 0; x < avctx->width; x++) {
+    float2rgbe(buf, red[x], green[x], blue[x]);
+    buf += 4;
+    }
+    } else {
+    bytestream_put_byte(, 2);
+    bytestream_put_byte(, 2);
+    bytestream_put_byte(, avctx->width >> 8);
+    bytestream_put_byte(, avctx->width & 0xFF);
+
+    for (int x = 0; x < avctx->width; x++)
+    float2rgbe(s->scanline + 4 * x, red[x], green[x],
blue[x]);
+    for (int p = 0; p < 4; p++)
+    rle(, s->scanline + p, avctx->width);
+    }
+    }
+
+    pkt->flags |= AV_PKT_FLAG_KEY;
+
+    av_shrink_packet(pkt, buf - pkt->data);
+
+    *got_packet = 1;
+
+    return 0;
+}


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

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


___
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: add Radiance HDR image format support

2022-07-06 Thread Andreas Rheinhardt
James Almer:
> On 7/6/2022 3:47 PM, Paul B Mahol wrote:
>> Added parser.
>> The test will be added after this is merged.
> 
> [...]
> 
>> +static int hdr_parse(AVCodecParserContext *s, AVCodecContext *avctx,
>> + const uint8_t **poutbuf, int *poutbuf_size,
>> + const uint8_t *buf, int buf_size)
>> +{
>> +    HDRParseContext *ipc = s->priv_data;
>> +    uint64_t state = ipc->pc.state64;
>> +    int next = END_NOT_FOUND, i = 0;
>> +
>> +    s->pict_type = AV_PICTURE_TYPE_NONE;
> 
> pict_type I, and key frame?
> 
> [...]
> 
>> +static int hdr_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
>> +    const AVFrame *frame, int *got_packet)
>> +{
>> +    HDREncContext *s = avctx->priv_data;
>> +    int64_t packet_size;
>> +    uint8_t *buf;
>> +    int ret;
>> +
>> +    packet_size = avctx->width * avctx->height * 4LL + 1024LL;
>> +    if ((ret = ff_get_encode_buffer(avctx, pkt, packet_size, 0)) < 0)
> 
> Wont using worst case scenario like this result in massive packets?
> av_shrink_packet() only changes the size field, it doesn't realloc the
> buffer to shrink it.
> 
> You could allocate a buffer in HDREncContext with
> av_fast_padded_malloc() here, use the bytestream2 API below, then
> allocate the packet with ff_get_encode_buffer() using
> bytestream2_tell_p() as size and do a memcpy at the end.
> 

If that is the aim, then there is no reason to do the memcpy ourselves
(or do the av_fast_padded_malloc directly): Just use ff_alloc_packet()
and the generic code will ensure that the packet data will be made
refcounted. And the temp buffer will be freed generically, too.

>> +    return ret;
>> +
>> +    buf = pkt->data;
>> +    bytestream_put_str(, "#?RADIANCE\n");
>> +    bytestream_put_str(, "SOFTWARE=lavc\n");
>> +    ret = snprintf(buf, 32, "PIXASPECT=%f\n",
>> av_q2d(av_inv_q(avctx->sample_aspect_ratio)));
>> +    if (ret > 0)
>> +    buf += ret;
>> +    bytestream_put_str(, "FORMAT=32-bit_rle_rgbe\n\n");
>> +    ret = snprintf(buf, 32, "-Y %d +X %d\n", avctx->height,
>> avctx->width);
>> +    if (ret > 0)
>> +    buf += ret;
>> +
>> +    for (int y = 0; y < avctx->height; y++) {
>> +    const float *red   = (const float *)(frame->data[2] + y *
>> frame->linesize[2]);
>> +    const float *green = (const float *)(frame->data[0] + y *
>> frame->linesize[0]);
>> +    const float *blue  = (const float *)(frame->data[1] + y *
>> frame->linesize[1]);
>> +
>> +    if (avctx->width < 8 || avctx->width > 0x7fff) {
>> +    for (int x = 0; x < avctx->width; x++) {
>> +    float2rgbe(buf, red[x], green[x], blue[x]);
>> +    buf += 4;
>> +    }
>> +    } else {
>> +    bytestream_put_byte(, 2);
>> +    bytestream_put_byte(, 2);
>> +    bytestream_put_byte(, avctx->width >> 8);
>> +    bytestream_put_byte(, avctx->width & 0xFF);
>> +
>> +    for (int x = 0; x < avctx->width; x++)
>> +    float2rgbe(s->scanline + 4 * x, red[x], green[x],
>> blue[x]);
>> +    for (int p = 0; p < 4; p++)
>> +    rle(, s->scanline + p, avctx->width);
>> +    }
>> +    }
>> +
>> +    pkt->flags |= AV_PKT_FLAG_KEY;
>> +
>> +    av_shrink_packet(pkt, buf - pkt->data);
>> +
>> +    *got_packet = 1;
>> +
>> +    return 0;
>> +}
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

___
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: add Radiance HDR image format support

2022-07-06 Thread James Almer

On 7/6/2022 3:47 PM, Paul B Mahol wrote:

Added parser.
The test will be added after this is merged.


[...]


+static int hdr_parse(AVCodecParserContext *s, AVCodecContext *avctx,
+ const uint8_t **poutbuf, int *poutbuf_size,
+ const uint8_t *buf, int buf_size)
+{
+HDRParseContext *ipc = s->priv_data;
+uint64_t state = ipc->pc.state64;
+int next = END_NOT_FOUND, i = 0;
+
+s->pict_type = AV_PICTURE_TYPE_NONE;


pict_type I, and key frame?

[...]


+static int hdr_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
+const AVFrame *frame, int *got_packet)
+{
+HDREncContext *s = avctx->priv_data;
+int64_t packet_size;
+uint8_t *buf;
+int ret;
+
+packet_size = avctx->width * avctx->height * 4LL + 1024LL;
+if ((ret = ff_get_encode_buffer(avctx, pkt, packet_size, 0)) < 0)


Wont using worst case scenario like this result in massive packets? 
av_shrink_packet() only changes the size field, it doesn't realloc the 
buffer to shrink it.


You could allocate a buffer in HDREncContext with 
av_fast_padded_malloc() here, use the bytestream2 API below, then 
allocate the packet with ff_get_encode_buffer() using 
bytestream2_tell_p() as size and do a memcpy at the end.



+return ret;
+
+buf = pkt->data;
+bytestream_put_str(, "#?RADIANCE\n");
+bytestream_put_str(, "SOFTWARE=lavc\n");
+ret = snprintf(buf, 32, "PIXASPECT=%f\n", 
av_q2d(av_inv_q(avctx->sample_aspect_ratio)));
+if (ret > 0)
+buf += ret;
+bytestream_put_str(, "FORMAT=32-bit_rle_rgbe\n\n");
+ret = snprintf(buf, 32, "-Y %d +X %d\n", avctx->height, avctx->width);
+if (ret > 0)
+buf += ret;
+
+for (int y = 0; y < avctx->height; y++) {
+const float *red   = (const float *)(frame->data[2] + y * 
frame->linesize[2]);
+const float *green = (const float *)(frame->data[0] + y * 
frame->linesize[0]);
+const float *blue  = (const float *)(frame->data[1] + y * 
frame->linesize[1]);
+
+if (avctx->width < 8 || avctx->width > 0x7fff) {
+for (int x = 0; x < avctx->width; x++) {
+float2rgbe(buf, red[x], green[x], blue[x]);
+buf += 4;
+}
+} else {
+bytestream_put_byte(, 2);
+bytestream_put_byte(, 2);
+bytestream_put_byte(, avctx->width >> 8);
+bytestream_put_byte(, avctx->width & 0xFF);
+
+for (int x = 0; x < avctx->width; x++)
+float2rgbe(s->scanline + 4 * x, red[x], green[x], blue[x]);
+for (int p = 0; p < 4; p++)
+rle(, s->scanline + p, avctx->width);
+}
+}
+
+pkt->flags |= AV_PKT_FLAG_KEY;
+
+av_shrink_packet(pkt, buf - pkt->data);
+
+*got_packet = 1;
+
+return 0;
+}


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

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


Re: [FFmpeg-devel] [PATCH] avcodec: add Radiance HDR image format support

2022-07-06 Thread James Almer

On 7/6/2022 3:47 PM, Paul B Mahol wrote:

Added parser.
The test will be added after this is merged.


[...]


diff --git a/libavformat/img2.c b/libavformat/img2.c
index 870d2ebbc5..b03075f3b0 100644
--- a/libavformat/img2.c
+++ b/libavformat/img2.c
@@ -91,6 +91,7 @@ const IdStrMap ff_img_tags[] = {
 { AV_CODEC_ID_VBN,"vbn"  },
 { AV_CODEC_ID_JPEGXL, "jxl"  },
 { AV_CODEC_ID_QOI,"qoi"  },
+{ AV_CODEC_ID_HDR,"hdr"  },


This could end up confusing people, so preferably use RHDR or some other 
combination for the codec id.



 { AV_CODEC_ID_NONE,   NULL   }
 };


___
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: add Radiance HDR image format support

2022-07-06 Thread Paul B Mahol
Added parser.
The test will be added after this is merged.
From 3558348c59e22ead8978fdaaa98e0bca361c772f Mon Sep 17 00:00:00 2001
From: Paul B Mahol 
Date: Sun, 3 Jul 2022 23:50:05 +0200
Subject: [PATCH] avcodec: add Radiance HDR image format support

Signed-off-by: Paul B Mahol 
---
 doc/general_contents.texi |   2 +
 libavcodec/Makefile   |   3 +
 libavcodec/allcodecs.c|   2 +
 libavcodec/codec_desc.c   |   7 ++
 libavcodec/codec_id.h |   1 +
 libavcodec/hdr_parser.c   |  78 ++
 libavcodec/hdrdec.c   | 222 ++
 libavcodec/hdrenc.c   | 189 
 libavcodec/parsers.c  |   1 +
 libavformat/Makefile  |   1 +
 libavformat/allformats.c  |   1 +
 libavformat/img2.c|   1 +
 libavformat/img2dec.c |   8 ++
 libavformat/img2enc.c |   2 +-
 14 files changed, 517 insertions(+), 1 deletion(-)
 create mode 100644 libavcodec/hdr_parser.c
 create mode 100644 libavcodec/hdrdec.c
 create mode 100644 libavcodec/hdrenc.c

diff --git a/doc/general_contents.texi b/doc/general_contents.texi
index b1d3e3aa05..f25c784d3b 100644
--- a/doc/general_contents.texi
+++ b/doc/general_contents.texi
@@ -749,6 +749,8 @@ following image formats are supported:
 @tab OpenEXR
 @item FITS @tab X @tab X
 @tab Flexible Image Transport System
+@item HDR  @tab X @tab X
+@tab Radiance HDR RGBE Image format
 @item IMG  @tab   @tab X
 @tab GEM Raster image
 @item JPEG @tab X @tab X
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 457ec58377..ef2318438b 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -403,6 +403,8 @@ OBJS-$(CONFIG_HAP_DECODER) += hapdec.o hap.o
 OBJS-$(CONFIG_HAP_ENCODER) += hapenc.o hap.o
 OBJS-$(CONFIG_HCA_DECODER) += hcadec.o
 OBJS-$(CONFIG_HCOM_DECODER)+= hcom.o
+OBJS-$(CONFIG_HDR_DECODER) += hdrdec.o
+OBJS-$(CONFIG_HDR_ENCODER) += hdrenc.o
 OBJS-$(CONFIG_HEVC_DECODER)+= hevcdec.o hevc_mvs.o \
   hevc_cabac.o hevc_refs.o hevcpred.o\
   hevcdsp.o hevc_filter.o hevc_data.o \
@@ -1142,6 +1144,7 @@ OBJS-$(CONFIG_H261_PARSER) += h261_parser.o
 OBJS-$(CONFIG_H263_PARSER) += h263_parser.o
 OBJS-$(CONFIG_H264_PARSER) += h264_parser.o h264_sei.o h264data.o
 OBJS-$(CONFIG_HEVC_PARSER) += hevc_parser.o hevc_data.o
+OBJS-$(CONFIG_HDR_PARSER)  += hdr_parser.o
 OBJS-$(CONFIG_IPU_PARSER)  += ipu_parser.o
 OBJS-$(CONFIG_JPEG2000_PARSER) += jpeg2000_parser.o
 OBJS-$(CONFIG_MJPEG_PARSER)+= mjpeg_parser.o
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index bdfc2f6f45..31d2c5979c 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -469,6 +469,8 @@ extern const FFCodec ff_gsm_decoder;
 extern const FFCodec ff_gsm_ms_decoder;
 extern const FFCodec ff_hca_decoder;
 extern const FFCodec ff_hcom_decoder;
+extern const FFCodec ff_hdr_encoder;
+extern const FFCodec ff_hdr_decoder;
 extern const FFCodec ff_iac_decoder;
 extern const FFCodec ff_ilbc_decoder;
 extern const FFCodec ff_imc_decoder;
diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
index 44ad2d1fe8..eeea15b1ef 100644
--- a/libavcodec/codec_desc.c
+++ b/libavcodec/codec_desc.c
@@ -1893,6 +1893,13 @@ static const AVCodecDescriptor codec_descriptors[] = {
 .long_name = NULL_IF_CONFIG_SMALL("PHM (Portable HalfFloatMap) image"),
 .props = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSLESS,
 },
+{
+.id= AV_CODEC_ID_HDR,
+.type  = AVMEDIA_TYPE_VIDEO,
+.name  = "hdr",
+.long_name = NULL_IF_CONFIG_SMALL("HDR (Radiance RGBE format) image"),
+.props = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSY,
+},
 
 /* various PCM "codecs" */
 {
diff --git a/libavcodec/codec_id.h b/libavcodec/codec_id.h
index 81fb316cff..005efa6334 100644
--- a/libavcodec/codec_id.h
+++ b/libavcodec/codec_id.h
@@ -312,6 +312,7 @@ enum AVCodecID {
 AV_CODEC_ID_JPEGXL,
 AV_CODEC_ID_QOI,
 AV_CODEC_ID_PHM,
+AV_CODEC_ID_HDR,
 
 /* various PCM "codecs" */
 AV_CODEC_ID_FIRST_AUDIO = 0x1, ///< A dummy id pointing at the start of audio codecs
diff --git a/libavcodec/hdr_parser.c b/libavcodec/hdr_parser.c
new file mode 100644
index 00..e0027c8b81
--- /dev/null
+++ b/libavcodec/hdr_parser.c
@@ -0,0 +1,78 @@
+/*
+ * Radiance HDR parser
+ * Copyright (c) 2022 Paul B Mahol
+ *
+ * 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 

Re: [FFmpeg-devel] [PATCH 2/2] avcodec/smacker: Optimize constant 16bit audio output

2022-07-06 Thread Tomas Härdin
ons 2022-07-06 klockan 19:11 +0200 skrev Michael Niedermayer:
> On Wed, May 04, 2022 at 11:39:54AM +0200, Tomas Härdin wrote:
> > tis 2022-05-03 klockan 18:30 +0200 skrev Michael Niedermayer:
> > > 
> > > +    } else if (stereo) {
> > > +    val  = 256*values[1] + values[0];
> > > +    val2 = 256*values[3] + values[2];
> > > +    for(; i < unp_size; i+=2) {
> > > +    pred[0] += val;
> > > +    pred[1] += val2;
> > > +    *samples++ = pred[0];
> > > +    *samples++ = pred[1];
> > > +    }
> > > +    } else {
> > > +    val = 256*values[1] + values[0];
> > > +    for(; i < unp_size; i++) {
> > > +    pred[0] += val;
> > > +    *samples++ = pred[0];
> > > +    }
> > > +    }
> > 
> > Got any numbers on how much faster this is? Just out of curiosity
> 
> With the fuzzed sample:
> before:
> 3263902379 decicycles in ABBB, 128 runs,  0 skips
> 
> after:
> 398977744 decicycles in ABBB,    1024 runs,  0 skips
> 
> the first times out after 128 runs which is why the runs differ

Cool. Well, looks good

/Tomas

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

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


Re: [FFmpeg-devel] [PATCH 2/2] avcodec/smacker: Optimize constant 16bit audio output

2022-07-06 Thread Paul B Mahol
lgtm
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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


Re: [FFmpeg-devel] [PATCH 2/2] avcodec/smacker: Optimize constant 16bit audio output

2022-07-06 Thread Michael Niedermayer
On Wed, May 04, 2022 at 11:39:54AM +0200, Tomas Härdin wrote:
> tis 2022-05-03 klockan 18:30 +0200 skrev Michael Niedermayer:
> > 
> > +    } else if (stereo) {
> > +    val  = 256*values[1] + values[0];
> > +    val2 = 256*values[3] + values[2];
> > +    for(; i < unp_size; i+=2) {
> > +    pred[0] += val;
> > +    pred[1] += val2;
> > +    *samples++ = pred[0];
> > +    *samples++ = pred[1];
> > +    }
> > +    } else {
> > +    val = 256*values[1] + values[0];
> > +    for(; i < unp_size; i++) {
> > +    pred[0] += val;
> > +    *samples++ = pred[0];
> > +    }
> > +    }
> 
> Got any numbers on how much faster this is? Just out of curiosity

With the fuzzed sample:
before:
3263902379 decicycles in ABBB, 128 runs,  0 skips

after:
398977744 decicycles in ABBB,1024 runs,  0 skips

the first times out after 128 runs which is why the runs differ

thx

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Opposition brings concord. Out of discord comes the fairest harmony.
-- Heraclitus


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 5/8] avformat/matroskaenc: Use av_fast_realloc_array for index entries

2022-07-06 Thread Tomas Härdin
ons 2022-07-06 klockan 17:10 +0200 skrev Andreas Rheinhardt:
> Tomas Härdin:
> > tis 2022-07-05 klockan 22:26 +0200 skrev Andreas Rheinhardt:
> > > 
> > > -    entries = av_realloc_array(entries, cues->num_entries + 1,
> > > sizeof(mkv_cuepoint));
> > > -    if (!entries)
> > > -    return AVERROR(ENOMEM);
> > > -    cues->entries = entries;
> > > +    ret = av_fast_realloc_array(>entries, 
> > > > allocated_entries,
> > > +    cues->num_entries + 1,
> > > +    MAX_SUPPORTED_EBML_LENGTH /
> > > MIN_CUETRACKPOS_SIZE,
> > 
> > Looks fine since MAX_SUPPORTED_EBML_LENGTH <= INT_MAX. Even
> > SIZE_MAX /
> > MIN_CUETRACKPOS_SIZE would work. Maybe we can could switch
> > MAX_SUPPORTED_EBML_LENGTH to
> > 
> >  #define MAX_SUPPORTED_EBML_LENGTH FFMIN(MAX_EBML_LENGTH, SIZE_MAX)
> > 
> > ?
> > 
> 
> To quote the comment for MAX_SUPPORTED_EBML_LENGTH:
> "/* The dynamic buffer API we rely upon has a limit of INT_MAX;
>  * and so has avio_write(). */"
> And I don't get why MAX_SUPPORTED_EBML_LENGTH <= INT_MAX is even
> relevant here. (Do you worry that MAX_SUPPORTED_EBML_LENGTH /
> MIN_CUETRACKPOS_SIZE might not be representable in a size_t? Thinking
> about this, defining it as FFMIN3(MAX_EBML_LENGTH, INT_MAX, SIZE_MAX)
> is
> better.)

INT_MAX <= SIZE_MAX on all platforms I am aware of. Just thought we
might want to support absolutely gargantuan .mkv files. Leaving it as-
is is fine

/Tomas

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

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


Re: [FFmpeg-devel] [PATCH 5/8] avformat/matroskaenc: Use av_fast_realloc_array for index entries

2022-07-06 Thread Andreas Rheinhardt
Tomas Härdin:
> tis 2022-07-05 klockan 22:26 +0200 skrev Andreas Rheinhardt:
>>
>> -    entries = av_realloc_array(entries, cues->num_entries + 1,
>> sizeof(mkv_cuepoint));
>> -    if (!entries)
>> -    return AVERROR(ENOMEM);
>> -    cues->entries = entries;
>> +    ret = av_fast_realloc_array(>entries, 
>>> allocated_entries,
>> +    cues->num_entries + 1,
>> +    MAX_SUPPORTED_EBML_LENGTH /
>> MIN_CUETRACKPOS_SIZE,
> 
> Looks fine since MAX_SUPPORTED_EBML_LENGTH <= INT_MAX. Even SIZE_MAX /
> MIN_CUETRACKPOS_SIZE would work. Maybe we can could switch
> MAX_SUPPORTED_EBML_LENGTH to
> 
>  #define MAX_SUPPORTED_EBML_LENGTH FFMIN(MAX_EBML_LENGTH, SIZE_MAX)
> 
> ?
> 

To quote the comment for MAX_SUPPORTED_EBML_LENGTH:
"/* The dynamic buffer API we rely upon has a limit of INT_MAX;
 * and so has avio_write(). */"
And I don't get why MAX_SUPPORTED_EBML_LENGTH <= INT_MAX is even
relevant here. (Do you worry that MAX_SUPPORTED_EBML_LENGTH /
MIN_CUETRACKPOS_SIZE might not be representable in a size_t? Thinking
about this, defining it as FFMIN3(MAX_EBML_LENGTH, INT_MAX, SIZE_MAX) is
better.)

- 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 6/8] avcodec/movtextenc: Use av_fast_realloc_array

2022-07-06 Thread Tomas Härdin
tis 2022-07-05 klockan 22:26 +0200 skrev Andreas Rheinhardt:
> 
> -    if (s->count + 1 > FFMIN(SIZE_MAX / sizeof(*s-
> >style_attributes), UINT16_MAX) ||
> -    !(tmp = av_fast_realloc(s->style_attributes,
> -    
> >style_attributes_bytes_allocated,
> -    (s->count + 1) * sizeof(*s-
> >style_attributes {
> +    ret = av_fast_realloc_array(>style_attributes, 
> >style_attributes_allocated,
> +    s->count + 1, UINT16_MAX,
> sizeof(*s->style_attributes));

Looks simple enough. Should ever s->count == UINT16_MAX then this will
fail, as it should

/Tomas

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

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


Re: [FFmpeg-devel] [PATCH 4/8] avformat/flvenc: Use array instead of linked list for index

2022-07-06 Thread Andreas Rheinhardt
Tomas Härdin:
> tis 2022-07-05 klockan 22:26 +0200 skrev Andreas Rheinhardt:
>> Using a linked list had very much overhead (the pointer to the next
>> entry increased the size of the index entry struct from 16 to 24
>> bytes,
>> not to mention the overhead of having separate allocations), so it is
>> better to (re)allocate a continuous array for the index.
>> av_fast_realloc_array() is used for this purpose, in order not to
>> reallocate the array for each entry. It's feature of allowing to
>> restrict the number of elements of the array is put to good use here:
>> Each entry will lead to 18 bytes being written and the array is
>> contained in an element whose size field has a length of three bytes,
>> so that more than (2^24 - 1) / 18 entries make no sense.
>>
> 
>> +    /* The filepositions array is part of a metadata element whose
>> size field
>> + * is three bytes long; so bound the number of filepositions in
>> order not
>> + * to allocate more than could ever be written. */
>> +    int ret = av_fast_realloc_array(>filepositions,
>> +    >filepositions_allocated,
>> +    flv->filepositions_count + 1,
>> +    (((1 << 24) - 1) - 10) /
> 
> Why -10? I see it bumps max_nb down from 932067 to 932066, but it make
> no difference keeping below 1<<24
> 

This was designed to make the result of the following computation
(performed in shift_data) to always fit into three bytes:

metadata_size = flv->filepositions_count * 9 * 2 + 10

But now upon rereading I see that there is some change added to this
unconditionally after that. I will also account for that.
(Notice that it is not intended to account for flv->metadata_totalsize
which is added later.)

- 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 5/8] avformat/matroskaenc: Use av_fast_realloc_array for index entries

2022-07-06 Thread Tomas Härdin
tis 2022-07-05 klockan 22:26 +0200 skrev Andreas Rheinhardt:
> 
> -    entries = av_realloc_array(entries, cues->num_entries + 1,
> sizeof(mkv_cuepoint));
> -    if (!entries)
> -    return AVERROR(ENOMEM);
> -    cues->entries = entries;
> +    ret = av_fast_realloc_array(>entries, 
> >allocated_entries,
> +    cues->num_entries + 1,
> +    MAX_SUPPORTED_EBML_LENGTH /
> MIN_CUETRACKPOS_SIZE,

Looks fine since MAX_SUPPORTED_EBML_LENGTH <= INT_MAX. Even SIZE_MAX /
MIN_CUETRACKPOS_SIZE would work. Maybe we can could switch
MAX_SUPPORTED_EBML_LENGTH to

 #define MAX_SUPPORTED_EBML_LENGTH FFMIN(MAX_EBML_LENGTH, SIZE_MAX)

?

/Tomas

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

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


Re: [FFmpeg-devel] [PATCH 4/8] avformat/flvenc: Use array instead of linked list for index

2022-07-06 Thread Tomas Härdin
tis 2022-07-05 klockan 22:26 +0200 skrev Andreas Rheinhardt:
> Using a linked list had very much overhead (the pointer to the next
> entry increased the size of the index entry struct from 16 to 24
> bytes,
> not to mention the overhead of having separate allocations), so it is
> better to (re)allocate a continuous array for the index.
> av_fast_realloc_array() is used for this purpose, in order not to
> reallocate the array for each entry. It's feature of allowing to
> restrict the number of elements of the array is put to good use here:
> Each entry will lead to 18 bytes being written and the array is
> contained in an element whose size field has a length of three bytes,
> so that more than (2^24 - 1) / 18 entries make no sense.
> 

> +    /* The filepositions array is part of a metadata element whose
> size field
> + * is three bytes long; so bound the number of filepositions in
> order not
> + * to allocate more than could ever be written. */
> +    int ret = av_fast_realloc_array(>filepositions,
> +    >filepositions_allocated,
> +    flv->filepositions_count + 1,
> +    (((1 << 24) - 1) - 10) /

Why -10? I see it bumps max_nb down from 932067 to 932066, but it make
no difference keeping below 1<<24

Looks like the rest of the patch makes the code faster and easier to
read. Very nice.

/Tomas


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

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


Re: [FFmpeg-devel] [PATCH 3/8] avutil/mem: Add av_fast_realloc_array()

2022-07-06 Thread Tomas Härdin
ons 2022-07-06 klockan 16:46 +0200 skrev Andreas Rheinhardt:
> Tomas Härdin:
> > tis 2022-07-05 klockan 22:26 +0200 skrev Andreas Rheinhardt:
> > > From: Andreas Rheinhardt 
> > > 
> > > This is an array-equivalent of av_fast_realloc(). Its advantages
> > > compared to using av_fast_realloc() for allocating arrays are as
> > > follows:
> > > 
> > > a) It performs its own overflow checks for the multiplication
> > > that is
> > > implicit in array allocations. (And it only needs to perform
> > > these
> > > checks (as well as the multiplication itself) in case the array
> > > needs
> > > to
> > > be reallocated.)
> > > b) It allows to limit the number of elements to an upper bound
> > > given
> > > by the caller. This allows to restrict the number of allocated
> > > elements
> > > to fit into an int and therefore makes this function usable with
> > > counters of this type. It can also be used to avoid overflow
> > > checks
> > > in
> > > the caller: E.g. setting it to UINT_MAX - 1 elements makes it
> > > safe to
> > > increase the desired number of elements in steps of one. And it
> > > avoids
> > > overallocations in situations where one already has an upper
> > > bound.
> > > c) av_fast_realloc_array() will always allocate in multiples of
> > > array
> > > elements; no memory is wasted with partial elements.
> > > d) By returning an int, av_fast_realloc_array() can distinguish
> > > between
> > > ordinary allocation failures (meriting AVERROR(ENOMEM)) and
> > > failures
> > > because of allocation limits (by returning AVERROR(ERANGE)).
> > > e) It is no longer possible for the user to accidentally lose the
> > > pointer by using ptr = av_fast_realloc(ptr, ...).
> > 
> > If you add an option for clearing the newly allocated memory then
> > this
> > could work for my av_fast_recalloc() use case in the jpeg2000
> > decoder.
> > Or we could have two functions.
> > 
> 
> I'd prefer it if the zeroing function were a wrapper around the
> non-zeroing function.

That should work, the change in allocated entries can be detected

> 
> > Small bikeshed: since the function takes a pointer to a pointer as
> > argument, av_fast_realloc_arrayp() might be a better name. I had in
> > mind to similarly rename av_fast_recalloc() to av_fast_recallocp().
> > 
> > 
> > > +
> > > +    nb = min_nb + (min_nb + 14) / 16;
> > 
> > Not +15? Or +0?
> 
> "av_fast_realloc_array() instead allocates nb + (nb + 14) / 16.
> Rounding
> up is done in order not to reallocate in steps of one if the current
> number is < 16; adding 14 instead of 15 has the effect of only
> allocating one element if one element is desired."

Wups, I actually read that part of the commit message then forgot about
it. I blame sickness

/Tomas

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

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


Re: [FFmpeg-devel] [PATCH 3/8] avutil/mem: Add av_fast_realloc_array()

2022-07-06 Thread Andreas Rheinhardt
Tomas Härdin:
> tis 2022-07-05 klockan 22:26 +0200 skrev Andreas Rheinhardt:
>> From: Andreas Rheinhardt 
>>
>> This is an array-equivalent of av_fast_realloc(). Its advantages
>> compared to using av_fast_realloc() for allocating arrays are as
>> follows:
>>
>> a) It performs its own overflow checks for the multiplication that is
>> implicit in array allocations. (And it only needs to perform these
>> checks (as well as the multiplication itself) in case the array needs
>> to
>> be reallocated.)
>> b) It allows to limit the number of elements to an upper bound given
>> by the caller. This allows to restrict the number of allocated
>> elements
>> to fit into an int and therefore makes this function usable with
>> counters of this type. It can also be used to avoid overflow checks
>> in
>> the caller: E.g. setting it to UINT_MAX - 1 elements makes it safe to
>> increase the desired number of elements in steps of one. And it
>> avoids
>> overallocations in situations where one already has an upper bound.
>> c) av_fast_realloc_array() will always allocate in multiples of array
>> elements; no memory is wasted with partial elements.
>> d) By returning an int, av_fast_realloc_array() can distinguish
>> between
>> ordinary allocation failures (meriting AVERROR(ENOMEM)) and failures
>> because of allocation limits (by returning AVERROR(ERANGE)).
>> e) It is no longer possible for the user to accidentally lose the
>> pointer by using ptr = av_fast_realloc(ptr, ...).
> 
> If you add an option for clearing the newly allocated memory then this
> could work for my av_fast_recalloc() use case in the jpeg2000 decoder.
> Or we could have two functions.
> 

I'd prefer it if the zeroing function were a wrapper around the
non-zeroing function.

> Small bikeshed: since the function takes a pointer to a pointer as
> argument, av_fast_realloc_arrayp() might be a better name. I had in
> mind to similarly rename av_fast_recalloc() to av_fast_recallocp().
> 
> 
>> +
>> +    nb = min_nb + (min_nb + 14) / 16;
> 
> Not +15? Or +0?

"av_fast_realloc_array() instead allocates nb + (nb + 14) / 16. Rounding
up is done in order not to reallocate in steps of one if the current
number is < 16; adding 14 instead of 15 has the effect of only
allocating one element if one element is desired."

> 
>> +
>> +    /* If min_nb is so big that the above calculation overflowed,
>> + * just allocate as much as we are allowed to. */
>> +    nb = nb < min_nb ? max_nb : FFMIN(nb, max_nb);
> 
> Looks OK, but an explicit check for overflow is easier to verify
> 
>> +
>> +    memcpy(, ptr, sizeof(array));
>> +
>> +    array = av_realloc(array, nb * elsize);
>> +    if (!array)
>> +    return AVERROR(ENOMEM);
>> +
>> +    memcpy(ptr, , sizeof(array));
> 
> An optional memset() here would be useful for me
> 
> Else it looks OK
> 
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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


Re: [FFmpeg-devel] [PATCH 3/8] avutil/mem: Add av_fast_realloc_array()

2022-07-06 Thread Tomas Härdin
tis 2022-07-05 klockan 22:26 +0200 skrev Andreas Rheinhardt:
> From: Andreas Rheinhardt 
> 
> This is an array-equivalent of av_fast_realloc(). Its advantages
> compared to using av_fast_realloc() for allocating arrays are as
> follows:
> 
> a) It performs its own overflow checks for the multiplication that is
> implicit in array allocations. (And it only needs to perform these
> checks (as well as the multiplication itself) in case the array needs
> to
> be reallocated.)
> b) It allows to limit the number of elements to an upper bound given
> by the caller. This allows to restrict the number of allocated
> elements
> to fit into an int and therefore makes this function usable with
> counters of this type. It can also be used to avoid overflow checks
> in
> the caller: E.g. setting it to UINT_MAX - 1 elements makes it safe to
> increase the desired number of elements in steps of one. And it
> avoids
> overallocations in situations where one already has an upper bound.
> c) av_fast_realloc_array() will always allocate in multiples of array
> elements; no memory is wasted with partial elements.
> d) By returning an int, av_fast_realloc_array() can distinguish
> between
> ordinary allocation failures (meriting AVERROR(ENOMEM)) and failures
> because of allocation limits (by returning AVERROR(ERANGE)).
> e) It is no longer possible for the user to accidentally lose the
> pointer by using ptr = av_fast_realloc(ptr, ...).

If you add an option for clearing the newly allocated memory then this
could work for my av_fast_recalloc() use case in the jpeg2000 decoder.
Or we could have two functions.

Small bikeshed: since the function takes a pointer to a pointer as
argument, av_fast_realloc_arrayp() might be a better name. I had in
mind to similarly rename av_fast_recalloc() to av_fast_recallocp().


> +
> +    nb = min_nb + (min_nb + 14) / 16;

Not +15? Or +0?

> +
> +    /* If min_nb is so big that the above calculation overflowed,
> + * just allocate as much as we are allowed to. */
> +    nb = nb < min_nb ? max_nb : FFMIN(nb, max_nb);

Looks OK, but an explicit check for overflow is easier to verify

> +
> +    memcpy(, ptr, sizeof(array));
> +
> +    array = av_realloc(array, nb * elsize);
> +    if (!array)
> +    return AVERROR(ENOMEM);
> +
> +    memcpy(ptr, , sizeof(array));

An optional memset() here would be useful for me

Else it looks OK

/Tomas

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

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


Re: [FFmpeg-devel] [PATCH 1/8] avutil/mem: Handle fast allocations near UINT_MAX properly

2022-07-06 Thread Andreas Rheinhardt
Tomas Härdin:
> tis 2022-07-05 klockan 22:09 +0200 skrev Andreas Rheinhardt:
>> av_fast_realloc and av_fast_mallocz? store the size of
>> the objects they allocate in an unsigned. Yet they overallocate
>> and currently they can allocate more than UINT_MAX bytes
>> in case a user has requested a size of about UINT_MAX * 16 / 17
>> or more if SIZE_MAX > UINT_MAX.
> 
> I think you mean if max_alloc_size > UINT_MAX
> 

Both are correct. I should probably add a note to the commit message
that this whole issue can only be encountered if one has increased the
allocation limit by calling av_max_alloc() before that.

>> In this case it is impossible
>> to store the true size of the buffer via the unsigned*;
>> future requests are likely to use the (re)allocation codepath
>> even if the buffer is actually large enough because of
>> the incorrect size.
>>
>> Fix this by ensuring that the actually allocated size
>> always fits into an unsigned. (This entails erroring out
>> in case the user requested more than UINT_MAX.)
> 
> Who decided unsigned was a good idea in these functions anyway?
> 

git log will tell you.

>>
>> Signed-off-by: Andreas Rheinhardt 
>> ---
>>  libavutil/mem.c | 4 
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/libavutil/mem.c b/libavutil/mem.c
>> index a0c9a42849..18aff5291f 100644
>> --- a/libavutil/mem.c
>> +++ b/libavutil/mem.c
>> @@ -510,6 +510,8 @@ void *av_fast_realloc(void *ptr, unsigned int
>> *size, size_t min_size)
>>  return ptr;
>>  
>>  max_size = atomic_load_explicit(_alloc_size,
>> memory_order_relaxed);
>> +    /* *size is an unsigned, so the real maximum is <= UINT_MAX. */
>> +    max_size = FFMIN(max_size, UINT_MAX);
>>  
>>  if (min_size > max_size) {
>>  *size = 0;
>> @@ -542,6 +544,8 @@ static inline void fast_malloc(void *ptr,
>> unsigned int *size, size_t min_size, i
>>  }
>>  
>>  max_size = atomic_load_explicit(_alloc_size,
>> memory_order_relaxed);
>> +    /* *size is an unsigned, so the real maximum is <= UINT_MAX. */
>> +    max_size = FFMIN(max_size, UINT_MAX);
>>  
>>  if (min_size > max_size) {
>>  av_freep(ptr);
> 
> Looks OK. This is also why I decided to do formal verification on my
> av_fast_recalloc() patch. I only verify part of it, so it's vulnerable
> to this also.
> 
> This is inspiring me to rework my patch to use size_t instead of
> unsigned for *size

See also 3/8.

- 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 1/8] avutil/mem: Handle fast allocations near UINT_MAX properly

2022-07-06 Thread Tomas Härdin
tis 2022-07-05 klockan 22:09 +0200 skrev Andreas Rheinhardt:
> av_fast_realloc and av_fast_mallocz? store the size of
> the objects they allocate in an unsigned. Yet they overallocate
> and currently they can allocate more than UINT_MAX bytes
> in case a user has requested a size of about UINT_MAX * 16 / 17
> or more if SIZE_MAX > UINT_MAX.

I think you mean if max_alloc_size > UINT_MAX

> In this case it is impossible
> to store the true size of the buffer via the unsigned*;
> future requests are likely to use the (re)allocation codepath
> even if the buffer is actually large enough because of
> the incorrect size.
> 
> Fix this by ensuring that the actually allocated size
> always fits into an unsigned. (This entails erroring out
> in case the user requested more than UINT_MAX.)

Who decided unsigned was a good idea in these functions anyway?

> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavutil/mem.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/libavutil/mem.c b/libavutil/mem.c
> index a0c9a42849..18aff5291f 100644
> --- a/libavutil/mem.c
> +++ b/libavutil/mem.c
> @@ -510,6 +510,8 @@ void *av_fast_realloc(void *ptr, unsigned int
> *size, size_t min_size)
>  return ptr;
>  
>  max_size = atomic_load_explicit(_alloc_size,
> memory_order_relaxed);
> +    /* *size is an unsigned, so the real maximum is <= UINT_MAX. */
> +    max_size = FFMIN(max_size, UINT_MAX);
>  
>  if (min_size > max_size) {
>  *size = 0;
> @@ -542,6 +544,8 @@ static inline void fast_malloc(void *ptr,
> unsigned int *size, size_t min_size, i
>  }
>  
>  max_size = atomic_load_explicit(_alloc_size,
> memory_order_relaxed);
> +    /* *size is an unsigned, so the real maximum is <= UINT_MAX. */
> +    max_size = FFMIN(max_size, UINT_MAX);
>  
>  if (min_size > max_size) {
>  av_freep(ptr);

Looks OK. This is also why I decided to do formal verification on my
av_fast_recalloc() patch. I only verify part of it, so it's vulnerable
to this also.

This is inspiring me to rework my patch to use size_t instead of
unsigned for *size

/Tomas

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

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


Re: [FFmpeg-devel] [PATCH v2 1/6] fflcms2: move to libavcodec

2022-07-06 Thread Andreas Rheinhardt
Niklas Haas:
> From: Niklas Haas 
> 
> We will need this helper inside libavcodec in the future, so move it
> there, leaving behind an #include to the raw source file in its old
> location in libvfilter. This approach is inspired by the handling of
> vulkan.c, and avoids us needing to expose any of it publicly (or
> semi-publicly) in e.g. libavutil, thus avoiding any ABI headaches.

This is only correct if you work under the assumption that when building
with static libraries, all the static libraries come from the same
commit. If you e.g. allow to build with an older libavfilter and a newer
libavcodec (like with shared libs), this assumption breaks down: if the
newer version of this file exports another function, you get linker
(ODR) errors because both versions might be pulled in.
Behaviour/signature changes by any function or modifications to any of
the structs would be similarly catastrophic.

A way out of this mess would be to version everything in the header like so:
#define FFLCMS2_VERSION 1

void AV_JOIN(ff_icc_context_uninit, FFLCMS2_VERSION)(FFIccContext *s);

(of course, there should be a dedicated macro for this to reduce typing.)

A patch that makes any of the "catastrophic" modifications described
above would need to bump the version. If one uses compatible versions,
the files would be deduplicated for static builds.
It would of course have the downside that these macros would be
everywhere where it is used, not only in fflcms2.[ch].

> 
> It's debatable whether the actual code belongs in libavcodec or
> libavfilter, but I decided to put it into libavcodec because it
> conceptually deals with encoding and decoding ICC profiles, and will be
> used to decode embedded ICC profiles in image files.
> 
> Signed-off-by: Niklas Haas 
> ---
>  libavcodec/Makefile   |   1 +
>  libavcodec/fflcms2.c  | 311 ++
>  libavcodec/fflcms2.h  |  87 
>  libavfilter/fflcms2.c | 294 +--
>  libavfilter/fflcms2.h |  65 +
>  5 files changed, 401 insertions(+), 357 deletions(-)
>  create mode 100644 libavcodec/fflcms2.c
>  create mode 100644 libavcodec/fflcms2.h
> 

___
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 to libavc/opus to create extradata if missing

2022-07-06 Thread Tristan Matthews
On Wed, Jul 6, 2022 at 10:10 AM Tristan Matthews 
wrote:

> Hi,
>
> On Sun, Jan 3, 2021 at 8:09 PM Andreas Rheinhardt <
> andreas.rheinha...@gmail.com> wrote:
>
>> Jonathan Baudanza:
>> > On Sun, Jan 3, 2021, at 3:34 PM, Andreas Rheinhardt wrote:
>> >> Lynne:
>> >>>
>> >>> Apart from that LGTM.
>> >>
>> >> +1 if the case of more than two channels has been properly tested.
>> >>
>> >
>> > I tested this by creating an (invalid) SDP file with channels set to 3.
>> In this case, the rtp demuxer fails with the following message:
>> >
>> >[sdp @ 0x7fe40280b800] Error creating opus extradata: Invalid data
>> found when processing input
>> >
>> > It might be more descriptive if we added a log warning about the
>> channel count. WDYT?
>>
>> It's ok as-is.
>>
>> - Andreas
>>
>
> Could this land? I found another case that it fixes (specifically
> streamcopy + muxing a specific mkv file that was captured via RTP).
>

Sorry for the noise, I missed in the thread that the code in question moved
from the opus decoder to the RTP parser and landed there.

Best,
Tristan
___
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 to libavc/opus to create extradata if missing

2022-07-06 Thread Tristan Matthews
Hi,

On Sun, Jan 3, 2021 at 8:09 PM Andreas Rheinhardt <
andreas.rheinha...@gmail.com> wrote:

> Jonathan Baudanza:
> > On Sun, Jan 3, 2021, at 3:34 PM, Andreas Rheinhardt wrote:
> >> Lynne:
> >>>
> >>> Apart from that LGTM.
> >>
> >> +1 if the case of more than two channels has been properly tested.
> >>
> >
> > I tested this by creating an (invalid) SDP file with channels set to 3.
> In this case, the rtp demuxer fails with the following message:
> >
> >[sdp @ 0x7fe40280b800] Error creating opus extradata: Invalid data
> found when processing input
> >
> > It might be more descriptive if we added a log warning about the channel
> count. WDYT?
>
> It's ok as-is.
>
> - Andreas
>

Could this land? I found another case that it fixes (specifically
streamcopy + muxing a specific mkv file that was captured via RTP).

Best,
Tristan
___
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 6/6] avcodec/encode:: generate ICC profiles

2022-07-06 Thread Anton Khirnov
Quoting Niklas Haas (2022-06-29 12:12:51)
> From: Niklas Haas 
> 
> Only if requested, and only if the codec signals support for ICC
> profiles. Implementation roughly matches the functionality of the
> existing vf_iccgen filter, albeit with some reduced flexibility and no
> caching.
> 
> Ideally, we'd also only do this on the first frame (e.g. mjpeg, apng),
> but there's no meaningful way for us to distinguish between this case
> and e.g. somebody using the image2 muxer, in which case we'd want to
> attach ICC profiles to every frame in the stream.

AV_CODEC_FLAG_GLOBAL_HEADER?

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

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


Re: [FFmpeg-devel] [PATCH v2 3/6] avcodec: add API for automatic handling of icc profiles

2022-07-06 Thread Anton Khirnov
Quoting Niklas Haas (2022-06-29 12:12:48)
> From: Niklas Haas 
> 
> This functionally already exists, but as pointed out in #9672 and #9673,
> requiring users to manually include filters is clumsy, error-prone and
> hard to use together with tools like ffplay.
> 
> To streamline ICC profile support, add a new AVCodecContext option to
> globally enable reading and writing ICC profiles, automatically, for all
> appropriate media types.
> 
> Note that this commit only includes the new API. The implementation is
> split off to separate commits for readability.
> 
> Signed-off-by: Niklas Haas 
> ---
>  doc/APIchanges   |  4 
>  libavcodec/avcodec.h | 10 ++
>  libavcodec/version.h |  2 +-
>  3 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 20b944933a..b8acf86352 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -14,6 +14,10 @@ libavutil: 2021-04-27
>  
>  API changes, most recent first:
>  
> +2022-06-28 - x - lavc 59.35.100 - avcodec.h
> +  Add the icc_profiles option to AVCodecContext, to enable automatic reading
> +  and writing of embedded ICC profiles in image files.
> +
>  2022-06-12 - xx - lavf 59.25.100 - avio.h
>Add avio_vprintf(), similar to avio_printf() but allow to use it
>from within a function taking a variable argument list as input.
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 4dae23d06e..8ee9be22d0 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -979,6 +979,16 @@ typedef struct AVCodecContext {
>   */
>  enum AVChromaLocation chroma_sample_location;
>  
> +/**
> + * Whether to decode/encode ICC profiles. If set, libavcodec will
> + * generate/parse ICC profiles as appropriate for the type of file. No
> + * effect on codecs which cannot contain embedded ICC profiles, or when
> + * compiled without support for lcms2.
> + * - encoding: Set by user
> + * - decoding: Set by user
> + */
> +int icc_profiles;

This is an ABI break. You could also just add a new AV_CODEC_FLAG
instead.

Also, why should this not be enabled by default?

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

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


Re: [FFmpeg-devel] [PATCH 1/8] avutil/mem: Handle fast allocations near UINT_MAX properly

2022-07-06 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2022-07-06 15:08:05)
> Anton Khirnov:
> > Quoting Andreas Rheinhardt (2022-07-05 22:09:37)
> >> av_fast_realloc and av_fast_mallocz? store the size of
> >> the objects they allocate in an unsigned. Yet they overallocate
> >> and currently they can allocate more than UINT_MAX bytes
> >> in case a user has requested a size of about UINT_MAX * 16 / 17
> >> or more if SIZE_MAX > UINT_MAX. In this case it is impossible
> >> to store the true size of the buffer via the unsigned*;
> >> future requests are likely to use the (re)allocation codepath
> >> even if the buffer is actually large enough because of
> >> the incorrect size.
> >>
> >> Fix this by ensuring that the actually allocated size
> >> always fits into an unsigned. (This entails erroring out
> >> in case the user requested more than UINT_MAX.)
> > 
> > I really dislike this av_fast_* naming.
> > 
> > Given that all these functions use unsigned int for something that
> > should really be size_t, how about we deprecate them all and replace
> > with something that has a sane naming convention and uses proper types?
> > 
> 
> What name do you suggest?

I suggested av_?alloc*_reuse() in a recent thread, since those function
are "fast" by reusing the buffer when possible.

> And what's your opinion of the actual patch?

Seems straightforwardly ok.

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

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


Re: [FFmpeg-devel] [PATCH 1/8] avutil/mem: Handle fast allocations near UINT_MAX properly

2022-07-06 Thread Andreas Rheinhardt
Anton Khirnov:
> Quoting Andreas Rheinhardt (2022-07-05 22:09:37)
>> av_fast_realloc and av_fast_mallocz? store the size of
>> the objects they allocate in an unsigned. Yet they overallocate
>> and currently they can allocate more than UINT_MAX bytes
>> in case a user has requested a size of about UINT_MAX * 16 / 17
>> or more if SIZE_MAX > UINT_MAX. In this case it is impossible
>> to store the true size of the buffer via the unsigned*;
>> future requests are likely to use the (re)allocation codepath
>> even if the buffer is actually large enough because of
>> the incorrect size.
>>
>> Fix this by ensuring that the actually allocated size
>> always fits into an unsigned. (This entails erroring out
>> in case the user requested more than UINT_MAX.)
> 
> I really dislike this av_fast_* naming.
> 
> Given that all these functions use unsigned int for something that
> should really be size_t, how about we deprecate them all and replace
> with something that has a sane naming convention and uses proper types?
> 

What name do you suggest?
And what's your opinion of the actual patch?

- 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 1/8] avutil/mem: Handle fast allocations near UINT_MAX properly

2022-07-06 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2022-07-05 22:09:37)
> av_fast_realloc and av_fast_mallocz? store the size of
> the objects they allocate in an unsigned. Yet they overallocate
> and currently they can allocate more than UINT_MAX bytes
> in case a user has requested a size of about UINT_MAX * 16 / 17
> or more if SIZE_MAX > UINT_MAX. In this case it is impossible
> to store the true size of the buffer via the unsigned*;
> future requests are likely to use the (re)allocation codepath
> even if the buffer is actually large enough because of
> the incorrect size.
> 
> Fix this by ensuring that the actually allocated size
> always fits into an unsigned. (This entails erroring out
> in case the user requested more than UINT_MAX.)

I really dislike this av_fast_* naming.

Given that all these functions use unsigned int for something that
should really be size_t, how about we deprecate them all and replace
with something that has a sane naming convention and uses proper types?

-- 
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] libavcodec/qsvenc: Use parameter from AVCodecContext to reset qsv codec

2022-07-06 Thread Wenbin Chen
Using parameter from AVCodecContext to reset qsv codec is more suitable
for MFXVideoENCODE_Reset()'s usage. Per-frame metadata is more suitable
for the usage of mfxEncodeCtrl being passed to
MFXVideoENCODE_EncodeFrameAsync(). Now change it to use the value
from AVCodecContext.

Signed-off-by: Wenbin Chen 
---
 doc/encoders.texi   |  5 ++---
 libavcodec/qsvenc.c | 26 --
 2 files changed, 10 insertions(+), 21 deletions(-)

diff --git a/doc/encoders.texi b/doc/encoders.texi
index 02a91ffe96..bf5fafd3fe 100644
--- a/doc/encoders.texi
+++ b/doc/encoders.texi
@@ -3337,10 +3337,9 @@ For encoders set this flag to ON to reduce power 
consumption and GPU usage.
 Following options can be used durning qsv encoding.
 
 @table @option
-@item @var{qsv_config_qp}
+@item @var{global_quality}
 Supported in h264_qsv and hevc_qsv.
-This option can be set in per-frame metadata. QP parameter can be dynamically
-changed when encoding in CQP mode.
+Change this value to reset qsv codec's qp configuration.
 @end table
 
 @subsection H264 options
diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
index 2382c2f5f7..82cb5a4865 100644
--- a/libavcodec/qsvenc.c
+++ b/libavcodec/qsvenc.c
@@ -1625,34 +1625,24 @@ static int update_qp(AVCodecContext *avctx, 
QSVEncContext *q,
  const AVFrame *frame)
 {
 int updated = 0, qp = 0, new_qp;
-char *tail;
-AVDictionaryEntry *entry = NULL;
 
 if (avctx->codec_id != AV_CODEC_ID_H264 && avctx->codec_id != 
AV_CODEC_ID_HEVC)
 return 0;
 
-entry = av_dict_get(frame->metadata, "qsv_config_qp", NULL, 0);
-if (entry && q->param.mfx.RateControlMethod == MFX_RATECONTROL_CQP) {
-qp = strtol(entry->value, , 10);
-if (*tail) {
-av_log(avctx, AV_LOG_WARNING, "Invalid qsv_config_qp string. 
Ignore this metadata\n");
-return 0;
-}
-if (qp < 0 || qp > 51) {
-av_log(avctx, AV_LOG_WARNING, "Invalid qp, clip to 0 ~ 51\n");
-qp = av_clip(qp, 0, 51);
-}
-av_log(avctx, AV_LOG_DEBUG, "Configure qp: %d\n",qp);
-UPDATE_PARAM(q->param.mfx.QPP, qp);
+if (q->param.mfx.RateControlMethod == MFX_RATECONTROL_CQP) {
+qp = avctx->global_quality / FF_QP2LAMBDA;
+new_qp = av_clip(qp, 0, 51);
+UPDATE_PARAM(q->param.mfx.QPP, new_qp);
 new_qp = av_clip(qp * fabs(avctx->i_quant_factor) +
 avctx->i_quant_offset, 0, 51);
 UPDATE_PARAM(q->param.mfx.QPI, new_qp);
 new_qp = av_clip(qp * fabs(avctx->b_quant_factor) +
 avctx->b_quant_offset, 0, 51);
 UPDATE_PARAM(q->param.mfx.QPB, new_qp);
-av_log(avctx, AV_LOG_DEBUG,
-"using fixed qp = %d/%d/%d for idr/p/b frames\n",
-q->param.mfx.QPI, q->param.mfx.QPP, q->param.mfx.QPB);
+if (updated)
+av_log(avctx, AV_LOG_DEBUG,
+   "using fixed qp = %d/%d/%d for idr/p/b frames\n",
+   q->param.mfx.QPI, q->param.mfx.QPP, q->param.mfx.QPB);
 }
 return updated;
 }
-- 
2.32.0

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

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


Re: [FFmpeg-devel] [PATCH 14/18] avcodec/hevcdec: Don't allocate redundant HEVCContexts

2022-07-06 Thread Andreas Rheinhardt
Michael Niedermayer:
> On Sat, Jul 02, 2022 at 08:32:06AM +0200, Andreas Rheinhardt wrote:
>> Michael Niedermayer:
>>> On Fri, Jul 01, 2022 at 12:29:45AM +0200, Andreas Rheinhardt wrote:
 The HEVC decoder has both HEVCContext and HEVCLocalContext
 structures. The latter is supposed to be the structure
 containing the per-slicethread state.

 Yet up until now that is not how it is handled in practice:
 Each HEVCLocalContext has a unique HEVCContext allocated for it
 and each of these coincides except in exactly one field: The
 corresponding HEVCLocalContext. This makes it possible to pass
 the HEVCContext everywhere where logically a HEVCLocalContext
 should be used. And up until recently, this is how it has been done.

 Yet the preceding patches changed this, making it possible
 to avoid allocating redundant HEVCContexts.

 Signed-off-by: Andreas Rheinhardt 
 ---
  libavcodec/hevcdec.c | 40 
  libavcodec/hevcdec.h |  2 --
  2 files changed, 16 insertions(+), 26 deletions(-)

 diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
 index 9d1241f293..048fcc76b4 100644
 --- a/libavcodec/hevcdec.c
 +++ b/libavcodec/hevcdec.c
 @@ -2548,13 +2548,12 @@ static int hls_decode_entry_wpp(AVCodecContext 
 *avctxt, void *hevc_lclist,
  {
  HEVCLocalContext *lc = ((HEVCLocalContext**)hevc_lclist)[self_id];
  const HEVCContext *const s = lc->parent;
 -HEVCContext *s1  = avctxt->priv_data;
 -int ctb_size= 1<< s1->ps.sps->log2_ctb_size;
 +int ctb_size= 1 << s->ps.sps->log2_ctb_size;
  int more_data   = 1;
  int ctb_row = job;
 -int ctb_addr_rs = s1->sh.slice_ctb_addr_rs + ctb_row * 
 ((s1->ps.sps->width + ctb_size - 1) >> s1->ps.sps->log2_ctb_size);
 -int ctb_addr_ts = s1->ps.pps->ctb_addr_rs_to_ts[ctb_addr_rs];
 -int thread = ctb_row % s1->threads_number;
 +int ctb_addr_rs = s->sh.slice_ctb_addr_rs + ctb_row * 
 ((s->ps.sps->width + ctb_size - 1) >> s->ps.sps->log2_ctb_size);
 +int ctb_addr_ts = s->ps.pps->ctb_addr_rs_to_ts[ctb_addr_rs];
 +int thread = ctb_row % s->threads_number;
  int ret;
  
  if(ctb_row) {
 @@ -2572,7 +2571,7 @@ static int hls_decode_entry_wpp(AVCodecContext 
 *avctxt, void *hevc_lclist,
  
  ff_thread_await_progress2(s->avctx, ctb_row, thread, 
 SHIFT_CTB_WPP);
  
 -if (atomic_load(>wpp_err)) {
 +if (atomic_load(>wpp_err)) {
  ff_thread_report_progress2(s->avctx, ctb_row , thread, 
 SHIFT_CTB_WPP);
>>>
>>> the consts in "const HEVCContext *const " make clang version 6.0.0-1ubuntu2 
>>> unhappy
>>> (this was building shared libs)
>>>
>>>
>>> CC  libavcodec/hevcdec.o
>>> src/libavcodec/hevcdec.c:2574:13: error: address argument to atomic 
>>> operation must be a pointer to non-const _Atomic type ('const atomic_int *' 
>>> (aka 'const _Atomic(int) *') invalid)
>>> if (atomic_load(>wpp_err)) {
>>> ^   ~~~
>>> /usr/lib/llvm-6.0/lib/clang/6.0.0/include/stdatomic.h:134:29: note: 
>>> expanded from macro 'atomic_load'
>>> #define atomic_load(object) __c11_atomic_load(object, __ATOMIC_SEQ_CST)
>>> ^ ~~
>>> 1 error generated.
>>> src/ffbuild/common.mak:81: recipe for target 'libavcodec/hevcdec.o' failed
>>> make: *** [libavcodec/hevcdec.o] Error 1
>>>
>>> thx
>>>
>>
>> Thanks for testing this. atomic_load is indeed declared without const in
>> 7.17.7.2:
>>
>> C atomic_load(volatile A *object);
>>
>> Upon reflection this makes sense, because if atomics are implemented via
>> mutexes, even a read may involve a preceding write. So I'll cast const
>> away here, too, and add a comment. (It works when casting const away,
>> doesn't it?)
> 
> This doesnt feel "right". These pointers should not be coming from a const
> if they are written to
> 

The HEVCContext is not const because the underlying object is const; the
HEVCContext is const when accessed from any part of the code that may be
run from slice threads, because if a slice thread modifies it, you have
a data race in case any of the other slice threads reads this field or
modifies it itself. But this is by definition not true for atomic
operations, so casting const away for them is fine.

> The compiler accepts it with an explicit cast though. With an implicit cast
> it produces a warning
> 
___
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 v3] avformat/mov: discard data streams with all zero sample_delta

2022-07-06 Thread Zhao Zhili
From: Zhao Zhili 

Streams with all zero sample_delta in 'stts' have all zero dts.
They have higher chance be chose by mov_find_next_sample(), which
leads to seek again and again.

For example, GoPro created a 'GoPro SOS' stream:
  Stream #0:4[0x5](eng): Data: none (fdsc / 0x63736466), 13 kb/s (default)
Metadata:
  creation_time   : 2022-06-21T08:49:19.00Z
  handler_name: GoPro SOS

With 'ffprobe -show_frames http://example.com/gopro.mp4', ffprobe
blocks until all samples in 'GoPro SOS' stream are consumed first.

Signed-off-by: Zhao Zhili 
---
v3: 'for ffmpeg' -> 'for ffmpeg command.'
v2: suggest how to override discard flag in log message.
 libavformat/mov.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 88669faa70..9a24e5effa 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -3062,6 +3062,21 @@ static int mov_read_stts(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)
 st->nb_frames= total_sample_count;
 if (duration)
 st->duration= FFMIN(st->duration, duration);
+
+// All samples have zero duration. They have higher chance be chose by
+// mov_find_next_sample, which leads to seek again and again.
+//
+// It's AVERROR_INVALIDDATA actually, but such files exist in the wild.
+// So only mark data stream as discarded for safety.
+if (!duration && sc->stts_count &&
+st->codecpar->codec_type == AVMEDIA_TYPE_DATA) {
+av_log(c->fc, AV_LOG_WARNING,
+   "All samples in data stream index:id [%d:%d] have zero "
+   "duration, stream set to be discarded by default. Override "
+   "using AVStream->discard or -discard for ffmpeg command.\n",
+   st->index, st->id);
+st->discard = AVDISCARD_ALL;
+}
 sc->track_end = duration;
 return 0;
 }
-- 
2.35.3

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

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


[FFmpeg-devel] [PATCH v2] avformat/mov: discard data streams with all zero sample_delta

2022-07-06 Thread Zhao Zhili
From: Zhao Zhili 

Streams with all zero sample_delta in 'stts' have all zero dts.
They have higher chance be chose by mov_find_next_sample(), which
leads to seek again and again.

For example, GoPro created a 'GoPro SOS' stream:
  Stream #0:4[0x5](eng): Data: none (fdsc / 0x63736466), 13 kb/s (default)
Metadata:
  creation_time   : 2022-06-21T08:49:19.00Z
  handler_name: GoPro SOS

With 'ffprobe -show_frames http://example.com/gopro.mp4', ffprobe
blocks until all samples in 'GoPro SOS' stream are consumed first.

Signed-off-by: Zhao Zhili 
---
v2: suggest how to override discard flag in log message.

 libavformat/mov.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 88669faa70..a3e4f05481 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -3062,6 +3062,21 @@ static int mov_read_stts(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)
 st->nb_frames= total_sample_count;
 if (duration)
 st->duration= FFMIN(st->duration, duration);
+
+// All samples have zero duration. They have higher chance be chose by
+// mov_find_next_sample, which leads to seek again and again.
+//
+// It's AVERROR_INVALIDDATA actually, but such files exist in the wild.
+// So only mark data stream as discarded for safety.
+if (!duration && sc->stts_count &&
+st->codecpar->codec_type == AVMEDIA_TYPE_DATA) {
+av_log(c->fc, AV_LOG_WARNING,
+   "All samples in data stream index:id [%d:%d] have zero "
+   "duration, stream set to be discarded by default. Override "
+   "using AVStream->discard or -discard for ffmpeg\n",
+   st->index, st->id);
+st->discard = AVDISCARD_ALL;
+}
 sc->track_end = duration;
 return 0;
 }
-- 
2.35.3

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

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


Re: [FFmpeg-devel] [PATCH 2/2] fftools/ffmpeg_filter: Fix audio_drift_threshold check

2022-07-06 Thread Anton Khirnov
Quoting Michael Niedermayer (2022-07-06 00:36:25)
> On Tue, Jul 05, 2022 at 06:58:28PM +0200, Anton Khirnov wrote:
> > Quoting Michael Niedermayer (2022-07-02 14:56:50)
> > > On Sat, Jul 02, 2022 at 10:38:26AM +0200, Anton Khirnov wrote:
> > > > Quoting Michael Niedermayer (2022-07-01 21:53:00)
> > > > > On Thu, Jun 30, 2022 at 10:55:46AM +0200, Anton Khirnov wrote:
> > > > > > Variant 2 is less bad, but the whole check seems hacky to me, since 
> > > > > > it
> > > > > > seems to make assumptions about swr defaults
> > > > > > 
> > > > > > Won't setting this unconditionally have the same effect?
> > > > > 
> > > > > it has the same effect but its not so nice to the user to recommand 
> > > > > extra
> > > > > arguments which make no difference
> > > > 
> > > > Sorry, I don't follow. What is recommending any arguments to the user
> > > > here?
> > > 
> > > i meant this thing here:
> > > ./ffmpeg   -i matrixbench_mpeg2.mpg -async 1 -f null -  2>&1 | grep async
> > > 
> > > -async is forwarded to lavfi similarly to -af 
> > > aresample=async=1:min_hard_comp=0.10:first_pts=0.
> > > 
> > > vs.
> > > 
> > > -async is forwarded to lavfi similarly to -af 
> > > aresample=async=1:first_pts=0.
> > 
> > I don't see a problem - why would the user care how it is forwarded?
> 
> The user may want to perform the equivalent operation inside a filter 
> chain/graph
> or with a tool different from ffmpeg or even via the API
> its really a very minor thing if these defaults are displayed too, it just
> feels a bit cleaner to skip them

Then the correct thing to do is retrieve the swr default value via the
AVOption API. Though IMO it's extra complexity for marginal gain.

I also wonder if this option should exist at all, given that it does
nothing but set a swr option. It is also global, so you can't have
different values for different streams.

-- 
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".