Re: [FFmpeg-devel] [PATCH 2/5] avcodec/amfenc: Set all color metadata for AMF
On Thu, 13 Jan 2022, Michael Fabian 'Xaymar' Dirks wrote:
From: Michael Fabian 'Xaymar' Dirks
Fixes the color information in the output from the AMD encoder
being wrong, which led to weird transcoding results. This
problem appeared out of thin air, and I've been unable to tie
it to a specific driver that supports my hardware. Unfortunately
this requires AMF SDK version 1.4.23 or later, but it should
still work fine on older drivers.
Theoretical support for HDR encoding is also now possible,
although the implementation is not complete. I have no clear
idea on how to generate AMFs HDR metadata structure, or where
to even take this information from.
---
libavcodec/amfenc.h | 1 +
libavcodec/amfenc_h264.c | 48 +-
libavcodec/amfenc_hevc.c | 50
3 files changed, 98 insertions(+), 1 deletion(-)
diff --git a/libavcodec/amfenc.h b/libavcodec/amfenc.h
index 358b2ef778..951e529362 100644
--- a/libavcodec/amfenc.h
+++ b/libavcodec/amfenc.h
@@ -21,6 +21,7 @@
#include
+#include
#include
#include
diff --git a/libavcodec/amfenc_h264.c b/libavcodec/amfenc_h264.c
index aeca99f7c6..009378e9f1 100644
--- a/libavcodec/amfenc_h264.c
+++ b/libavcodec/amfenc_h264.c
@@ -137,6 +137,8 @@ static av_cold int amf_encode_init_h264(AVCodecContext
*avctx)
AMFRate framerate;
AMFSize framesize = AMFConstructSize(avctx->width,
avctx->height);
int deblocking_filter = (avctx->flags &
AV_CODEC_FLAG_LOOP_FILTER) ? 1 : 0;
+amf_int64color_depth;
+amf_int64color_profile;
if (avctx->framerate.num > 0 && avctx->framerate.den > 0) {
framerate = AMFConstructRate(avctx->framerate.num,
avctx->framerate.den);
@@ -194,10 +196,54 @@ static av_cold int amf_encode_init_h264(AVCodecContext
*avctx)
AMF_ASSIGN_PROPERTY_RATIO(res, ctx->encoder,
AMF_VIDEO_ENCODER_ASPECT_RATIO, ratio);
}
-/// Color Range (Partial/TV/MPEG or Full/PC/JPEG)
+// Color Metadata
+/// Color Range (Support for older Drivers)
if (avctx->color_range == AVCOL_RANGE_JPEG) {
AMF_ASSIGN_PROPERTY_BOOL(res, ctx->encoder,
AMF_VIDEO_ENCODER_FULL_RANGE_COLOR, 1);
+} else {
+AMF_ASSIGN_PROPERTY_BOOL(res, ctx->encoder,
AMF_VIDEO_ENCODER_FULL_RANGE_COLOR, 0);
+}
+/// Color Space & Depth
+color_depth = AMF_COLOR_BIT_DEPTH_8;
+color_profile = AMF_VIDEO_CONVERTER_COLOR_PROFILE_UNKNOWN;
+switch (avctx->colorspace) {
+case AVCOL_SPC_SMPTE170M:
+if (avctx->color_range == AVCOL_RANGE_JPEG) {
+color_profile = AMF_VIDEO_CONVERTER_COLOR_PROFILE_FULL_601;
+} else {
+color_profile = AMF_VIDEO_CONVERTER_COLOR_PROFILE_601;
+}
+break;
+case AVCOL_SPC_BT709:
+if (avctx->color_range == AVCOL_RANGE_JPEG) {
+color_profile = AMF_VIDEO_CONVERTER_COLOR_PROFILE_FULL_709;
+} else {
+color_profile = AMF_VIDEO_CONVERTER_COLOR_PROFILE_709;
+}
+break;
+case AVCOL_SPC_BT2020_NCL:
+case AVCOL_SPC_BT2020_CL:
+// !FIXME: Verify that this is correct on Hardware supporting it.
+// !FIXME: Figure out how to decide on bit depth.
+if (avctx->color_range == AVCOL_RANGE_JPEG) {
+color_profile = AMF_VIDEO_CONVERTER_COLOR_PROFILE_FULL_2020;
+color_depth = AMF_COLOR_BIT_DEPTH_10;
+} else {
+color_profile = AMF_VIDEO_CONVERTER_COLOR_PROFILE_2020;
+color_depth = AMF_COLOR_BIT_DEPTH_10;
+}
What does this do? Force a 10 bit output even if the input surfaces are 8
bit? I am uneasy about setting output bit depth based on colorspace.
Considering that only 8 bit source formats are supported in amfenc.c
format_map at the moment... So maybe in this patch it is better to
keep everything 8 bit for now?
+break;
}
+AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
AMF_VIDEO_ENCODER_COLOR_BIT_DEPTH, color_depth);
+AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
AMF_VIDEO_ENCODER_INPUT_COLOR_PROFILE, color_profile);
+AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
AMF_VIDEO_ENCODER_OUTPUT_COLOR_PROFILE, color_profile);
+/// Color Transfer Characteristics (AMF matches ISO/IEC)
+AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
AMF_VIDEO_ENCODER_INPUT_TRANSFER_CHARACTERISTIC, (amf_int64)avctx->color_trc);
+AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
AMF_VIDEO_ENCODER_OUTPUT_TRANSFER_CHARACTERISTIC, (amf_int64)avctx->color_trc);
+/// Color Primaries (AMF matches ISO/IEC)
+AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
AMF_VIDEO_ENCODER_INPUT_COLOR_PRIMARIES, (amf_int64)avctx->color_primaries);
+AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
AMF_VIDEO_ENCODER_OUTPUT_COLOR_PRIMARIES, (amf_int64)avctx->color_primaries);
+/// !TODO: AMF HDR Metadata generation
// autodet
Re: [FFmpeg-devel] [PATCH 5/5] avcodec/amfenc: Add missing profiles
On Thu, 13 Jan 2022, Michael Fabian 'Xaymar' Dirks wrote:
From: Michael Fabian 'Xaymar' Dirks
Adds the missing profiles to the '-help encoder=xxx_amf' list,
even if the user may never need these.
---
libavcodec/amfenc_h264.c | 11 ++-
libavcodec/amfenc_hevc.c | 1 +
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/libavcodec/amfenc_h264.c b/libavcodec/amfenc_h264.c
index 87a3bb6a73..ae21c60357 100644
--- a/libavcodec/amfenc_h264.c
+++ b/libavcodec/amfenc_h264.c
@@ -37,11 +37,12 @@ static const AVOption options[] = {
{ "lowlatency_highqquality", "",0,
AV_OPT_TYPE_CONST, { .i64 = AMF_VIDEO_ENCODER_USAGE_LOW_LATENCY_HIGH_QUALITY }, 0, 0, VE, "usage" },
/// Profile,
-{ "profile","Profile", OFFSET(profile),AV_OPT_TYPE_INT, {
.i64 = AMF_VIDEO_ENCODER_PROFILE_MAIN }, AMF_VIDEO_ENCODER_PROFILE_BASELINE,
AMF_VIDEO_ENCODER_PROFILE_CONSTRAINED_HIGH, VE, "profile" },
-{ "main", "", 0, AV_OPT_TYPE_CONST, {
.i64 = AMF_VIDEO_ENCODER_PROFILE_MAIN }, 0, 0, VE, "profile" },
-{ "high", "", 0, AV_OPT_TYPE_CONST, {
.i64 = AMF_VIDEO_ENCODER_PROFILE_HIGH }, 0, 0, VE, "profile" },
-{ "constrained_baseline", "", 0, AV_OPT_TYPE_CONST, {
.i64 = AMF_VIDEO_ENCODER_PROFILE_CONSTRAINED_BASELINE }, 0, 0, VE, "profile" },
-{ "constrained_high", "", 0, AV_OPT_TYPE_CONST, {
.i64 = AMF_VIDEO_ENCODER_PROFILE_CONSTRAINED_HIGH }, 0, 0, VE, "profile" },
+{ "profile","Profile", OFFSET(profile), AV_OPT_TYPE_INT, {
.i64 = AMF_VIDEO_ENCODER_PROFILE_MAIN }, AMF_VIDEO_ENCODER_PROFILE_BASELINE,
AMF_VIDEO_ENCODER_PROFILE_CONSTRAINED_HIGH, VE, "profile" },
+{ "baseline", "", 0, AV_OPT_TYPE_CONST, {
.i64 = AMF_VIDEO_ENCODER_PROFILE_BASELINE }, 0, 0, VE, "profile" },
+{ "main", "", 0, AV_OPT_TYPE_CONST, {
.i64 = AMF_VIDEO_ENCODER_PROFILE_MAIN }, 0, 0, VE, "profile" },
+{ "high", "", 0, AV_OPT_TYPE_CONST, {
.i64 = AMF_VIDEO_ENCODER_PROFILE_HIGH }, 0, 0, VE, "profile" },
+{ "constrained_baseline", "", 0, AV_OPT_TYPE_CONST, {
.i64 = AMF_VIDEO_ENCODER_PROFILE_CONSTRAINED_BASELINE }, 0, 0, VE, "profile" },
+{ "constrained_high", "", 0, AV_OPT_TYPE_CONST, {
.i64 = AMF_VIDEO_ENCODER_PROFILE_CONSTRAINED_HIGH }, 0, 0, VE, "profile" },
/// Profile Level
{ "level", "Profile Level",OFFSET(level), AV_OPT_TYPE_INT, { .i64
= 0 }, 0, 62, VE, "level" },
diff --git a/libavcodec/amfenc_hevc.c b/libavcodec/amfenc_hevc.c
index 565be9bad9..a69f37e7b1 100644
--- a/libavcodec/amfenc_hevc.c
+++ b/libavcodec/amfenc_hevc.c
@@ -34,6 +34,7 @@ static const AVOption options[] = {
{ "profile","Set the profile (default main)", OFFSET(profile),
AV_OPT_TYPE_INT,{ .i64 = AMF_VIDEO_ENCODER_HEVC_PROFILE_MAIN }, AMF_VIDEO_ENCODER_HEVC_PROFILE_MAIN,
AMF_VIDEO_ENCODER_HEVC_PROFILE_MAIN, VE, "profile" },
{ "main", "", 0, AV_OPT_TYPE_CONST,{ .i64 =
AMF_VIDEO_ENCODER_HEVC_PROFILE_MAIN }, 0, 0, VE, "profile" },
+{ "main10", "", 0, AV_OPT_TYPE_CONST,{ .i64 =
AMF_VIDEO_ENCODER_HEVC_PROFILE_MAIN_10 }, 0, 0, VE, "profile" },
Tha maximum is not increased for the new profile.
Regards,
Marton
{ "profile_tier", "Set the profile tier (default main)", OFFSET(tier),
AV_OPT_TYPE_INT,{ .i64 = AMF_VIDEO_ENCODER_HEVC_TIER_MAIN }, AMF_VIDEO_ENCODER_HEVC_TIER_MAIN,
AMF_VIDEO_ENCODER_HEVC_TIER_HIGH, VE, "tier" },
{ "main", "", 0, AV_OPT_TYPE_CONST, { .i64 =
AMF_VIDEO_ENCODER_HEVC_TIER_MAIN }, 0, 0, VE, "tier" },
--
2.34.1.windows.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 1/3] avcodec/pcm-dvdenc: Remove unused extra_sample(s|_count)
Signed-off-by: Andreas Rheinhardt
---
libavcodec/pcm-dvdenc.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/libavcodec/pcm-dvdenc.c b/libavcodec/pcm-dvdenc.c
index 6bab1e0aaa..c7f1d46ba3 100644
--- a/libavcodec/pcm-dvdenc.c
+++ b/libavcodec/pcm-dvdenc.c
@@ -31,8 +31,6 @@ typedef struct PCMDVDContext {
int block_size; // Size of a block of samples in bytes
int samples_per_block; // Number of samples per channel per block
int groups_per_block;// Number of 20/24-bit sample groups per block
-uint8_t *extra_samples; // Pointer to leftover samples from a frame
-int extra_sample_count; // Number of leftover samples in the buffer
} PCMDVDContext;
static av_cold int pcm_dvd_encode_init(AVCodecContext *avctx)
--
2.34.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 2/3] avcodec/pcm-dvdenc: Fix encoding 24bit samples
The earlier code ignored the lower 16 bits and instead used
the highest 8 bits twice.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/pcm-dvdenc.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/libavcodec/pcm-dvdenc.c b/libavcodec/pcm-dvdenc.c
index c7f1d46ba3..0881697c17 100644
--- a/libavcodec/pcm-dvdenc.c
+++ b/libavcodec/pcm-dvdenc.c
@@ -146,8 +146,8 @@ static int pcm_dvd_encode_frame(AVCodecContext *avctx,
AVPacket *avpkt,
for (int i = 2; i; i--) {
bytestream2_put_be16(&pb, src32[0] >> 16);
bytestream2_put_be16(&pb, src32[1] >> 16);
-bytestream2_put_byte(&pb, (*src32++) >> 24);
-bytestream2_put_byte(&pb, (*src32++) >> 24);
+bytestream2_put_byte(&pb, (uint8_t)((*src32++) >> 8));
+bytestream2_put_byte(&pb, (uint8_t)((*src32++) >> 8));
}
} while (--blocks);
} else {
@@ -157,10 +157,10 @@ static int pcm_dvd_encode_frame(AVCodecContext *avctx,
AVPacket *avpkt,
bytestream2_put_be16(&pb, src32[1] >> 16);
bytestream2_put_be16(&pb, src32[2] >> 16);
bytestream2_put_be16(&pb, src32[3] >> 16);
-bytestream2_put_byte(&pb, (*src32++) >> 24);
-bytestream2_put_byte(&pb, (*src32++) >> 24);
-bytestream2_put_byte(&pb, (*src32++) >> 24);
-bytestream2_put_byte(&pb, (*src32++) >> 24);
+bytestream2_put_byte(&pb, (uint8_t)((*src32++) >> 8));
+bytestream2_put_byte(&pb, (uint8_t)((*src32++) >> 8));
+bytestream2_put_byte(&pb, (uint8_t)((*src32++) >> 8));
+bytestream2_put_byte(&pb, (uint8_t)((*src32++) >> 8));
}
} while (--blocks);
}
--
2.34.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 3/3] fate/pcm: Add pcm_dvd transcode tests
Signed-off-by: Andreas Rheinhardt --- I could also move these tests to a file of their own if desired. tests/fate/pcm.mak | 31 ++ tests/ref/fate/pcm_dvd-16-1-48000 | 11 tests/ref/fate/pcm_dvd-16-1-96000 | 26 tests/ref/fate/pcm_dvd-16-2-48000 | 26 tests/ref/fate/pcm_dvd-16-5.1-48000 | 65 tests/ref/fate/pcm_dvd-16-5.1-96000 | 51 tests/ref/fate/pcm_dvd-16-7.1-48000 | 69 + tests/ref/fate/pcm_dvd-24-1-48000 | 59 ++ tests/ref/fate/pcm_dvd-24-2-48000 | 36 +++ tests/ref/fate/pcm_dvd-24-5.1-48000 | 94 + tests/ref/fate/pcm_dvd-24-7.1-48000 | 69 + 11 files changed, 537 insertions(+) create mode 100644 tests/ref/fate/pcm_dvd-16-1-48000 create mode 100644 tests/ref/fate/pcm_dvd-16-1-96000 create mode 100644 tests/ref/fate/pcm_dvd-16-2-48000 create mode 100644 tests/ref/fate/pcm_dvd-16-5.1-48000 create mode 100644 tests/ref/fate/pcm_dvd-16-5.1-96000 create mode 100644 tests/ref/fate/pcm_dvd-16-7.1-48000 create mode 100644 tests/ref/fate/pcm_dvd-24-1-48000 create mode 100644 tests/ref/fate/pcm_dvd-24-2-48000 create mode 100644 tests/ref/fate/pcm_dvd-24-5.1-48000 create mode 100644 tests/ref/fate/pcm_dvd-24-7.1-48000 diff --git a/tests/fate/pcm.mak b/tests/fate/pcm.mak index e3e2674034..4e79fc7c41 100644 --- a/tests/fate/pcm.mak +++ b/tests/fate/pcm.mak @@ -27,6 +27,37 @@ fate-dcinema-encode: tests/data/asynth-96000-6.wav fate-dcinema-encode: SRC = tests/data/asynth-96000-6.wav fate-dcinema-encode: CMD = enc_dec_pcm daud framemd5 s16le $(SRC) -c:a pcm_s24daud -frames:a 20 +FATE_SAMPLES_PCM-$(call TRANSCODE, PCM_DVD, MPEG2VOB MPEGPS, TRUEHD_DEMUXER TRUEHD_DECODER PCM_S24LE_ENCODER) += fate-pcm_dvd-24-7.1-48000 +fate-pcm_dvd-24-7.1-48000: CMD = transcode truehd $(TARGET_SAMPLES)/truehd/atmos.thd vob "-c:a pcm_dvd" "-c:a pcm_s24le" + +FATE_SAMPLES_PCM-$(call TRANSCODE, PCM_DVD, MPEG2VOB MPEGPS, MXF_DEMUXER PCM_S16LE_DECODER) += fate-pcm_dvd-16-7.1-48000 +fate-pcm_dvd-16-7.1-48000: CMD = transcode mxf $(TARGET_SAMPLES)/mxf/Sony-1.mxf vob "-map 0:a -c:a pcm_dvd" + +FATE_SAMPLES_PCM-$(call TRANSCODE, PCM_DVD, MPEG2VOB MPEGPS, DAUD_DEMUXER PCM_S24DAUD_DECODER) += fate-pcm_dvd-16-5.1-96000 +fate-pcm_dvd-16-5.1-96000: CMD = transcode daud $(TARGET_SAMPLES)/d-cinema/THX_Science_FLT_1920-partial.302 vob "-c:a pcm_dvd" + +FATE_SAMPLES_PCM-$(call TRANSCODE, PCM_DVD, MPEG2VOB MPEGPS, TRUEHD_DEMUXER TRUEHD_DECODER PCM_S24LE_ENCODER) += fate-pcm_dvd-24-5.1-48000 +fate-pcm_dvd-24-5.1-48000: CMD = transcode truehd $(TARGET_SAMPLES)/lossless-audio/truehd_5.1.raw vob "-c:a pcm_dvd" "-c:a pcm_s24le -t 0.2" + +FATE_SAMPLES_PCM-$(call TRANSCODE, PCM_DVD, MPEG2VOB MPEGPS, MATROSKA_DEMUXER FLAC_DECODER) += fate-pcm_dvd-16-5.1-48000 +fate-pcm_dvd-16-5.1-48000: CMD = transcode matroska $(TARGET_SAMPLES)/mkv/flac_channel_layouts.mka vob "-map 0:a:1 -c:a pcm_dvd" "-t 0.2" + +FATE_SAMPLES_PCM-$(call TRANSCODE, PCM_DVD, MPEG2VOB MPEGPS, FLAC_DEMUXER FLAC_PARSER FLAC_DECODER PCM_S24LE_ENCODER) += fate-pcm_dvd-24-2-48000 +fate-pcm_dvd-24-2-48000: CMD = transcode flac $(TARGET_SAMPLES)/filter/seq-3341-7_seq-3342-5-24bit.flac vob "-c:a pcm_dvd" "-c:a pcm_s24le -t 0.2" + +FATE_SAMPLES_PCM-$(call TRANSCODE, PCM_DVD, MPEG2VOB MPEGPS, WAV_DEMUXER PCM_S16LE_DECODER) += fate-pcm_dvd-16-2-48000 +fate-pcm_dvd-16-2-48000: CMD = transcode wav $(TARGET_SAMPLES)/wav/200828-005.wav vob "-c:a pcm_dvd" "-t 0.2" + +FATE_SAMPLES_PCM-$(call TRANSCODE, PCM_DVD, MPEG2VOB MPEGPS, MXF_DEMUXER PCM_S24LE_DECODER PCM_S24LE_ENCODER) += fate-pcm_dvd-24-1-48000 +fate-pcm_dvd-24-1-48000: CMD = transcode mxf $(TARGET_SAMPLES)/mxf/omneon_8.3.0.0_xdcam_startc_footer.mxf vob "-map 0:a:0 -c:a pcm_dvd" "-c:a pcm_s24le" + +FATE_SAMPLES_PCM-$(call TRANSCODE, PCM_DVD, MPEG2VOB MPEGPS, MXF_DEMUXER PCM_S16LE_DECODER) += fate-pcm_dvd-16-1-48000 +fate-pcm_dvd-16-1-48000: CMD = transcode mxf $(TARGET_SAMPLES)/mxf/opatom_missing_index.mxf vob "-c:a pcm_dvd" + +FATE_PCM-$(call TRANSCODE, PCM_DVD, MPEG2VOB MPEGPS, WAV_DEMUXER PCM_S16LE_DECODER) += fate-pcm_dvd-16-1-96000 +fate-pcm_dvd-16-1-96000: tests/data/asynth-96000-1.wav +fate-pcm_dvd-16-1-96000: CMD = transcode wav $(TARGET_PATH)/tests/data/asynth-96000-1.wav vob "-c:a pcm_dvd" "-t 0.2" + FATE_FFMPEG += $(FATE_PCM-yes) FATE_SAMPLES_AVCONV += $(FATE_SAMPLES_PCM-yes) fate-pcm: $(FATE_PCM-yes) $(FATE_SAMPLES_PCM-yes) diff --git a/tests/ref/fate/pcm_dvd-16-1-48000 b/tests/ref/fate/pcm_dvd-16-1-48000 new file mode 100644 index 00..29f4e84d50 --- /dev/null +++ b/tests/ref/fate/pcm_dvd-16-1-48000 @@ -0,0 +1,11 @@ +af7b5ae365019ec64a9397bc6b33e18c *tests/data/fate/pcm_dvd-16-1-48000.vob +8192 tests/data/fate/pcm_dvd-16-1-48000.vob +#tb 0: 1/48000 +#media_type 0: audio +#codec_id 0: pcm_s16le +#sample_rate 0: 48000 +#channel_layout_name 0: mono +0, 0, 0, 997, 1994, 0xd416def
Re: [FFmpeg-devel] [PATCH 1/3] avcodec/pcm-dvdenc: Remove unused extra_sample(s|_count)
LGTM to whole set. ___ ffmpeg-devel mailing list ffmpeg-devel@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][GSoC'22] Chromakey CUDA
Applied with some minor stylistic changes and cleanups Thanks! ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3] ffmpeg: add option -isync
Quoting Gyan Doshi (2022-07-09 21:56:10) > > > On 2022-07-10 01:13 am, Paul B Mahol wrote: > > On Sat, Jul 9, 2022 at 8:28 PM Gyan Doshi wrote: > > > >> > >> On 2022-07-08 09:26 am, Gyan Doshi wrote: > >>> > >>> On 2022-07-07 03:11 pm, Anton Khirnov wrote: > Quoting Gyan Doshi (2022-07-04 18:29:12) > > This is a per-file input option that adjusts an input's timestamps > > with reference to another input, so that emitted packet timestamps > > account for the difference between the start times of the two inputs. > > > > Typical use case is to sync two or more live inputs such as from > > capture > > devices. Both the target and reference input source timestamps > > should be > > based on the same clock source. > > > > If not all inputs have timestamps, the wallclock times at the time of > > reception of inputs shall be used. FFmpeg must have been compiled with > > thread support for this last case. > I'm wondering if simply using the other input's InputFile.ts_offset > wouldn't achieve the same effect with much less complexity. > >>> That's what I initially did. But since the code can also use two other > >>> sources for start times (start_time_realtime, first_pkt_wallclock), > >>> those intervals may not exactly match the difference between > >>> fmctx->start_times so I use a generic calculation. > >> Plan to push on Monday, if no further changes. 5.1 is to be cut soon. > >> > >> > > Why big rush, its not so critical. > > Patch was first sent on 22nd June. Many patches wait for way longer than that. > Only one reviewer asked for changes. That entitles you to disregard me then? -- 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 v3] ffmpeg: add option -isync
Quoting Gyan Doshi (2022-07-08 05:56:21) > > > On 2022-07-07 03:11 pm, Anton Khirnov wrote: > > Quoting Gyan Doshi (2022-07-04 18:29:12) > >> This is a per-file input option that adjusts an input's timestamps > >> with reference to another input, so that emitted packet timestamps > >> account for the difference between the start times of the two inputs. > >> > >> Typical use case is to sync two or more live inputs such as from capture > >> devices. Both the target and reference input source timestamps should be > >> based on the same clock source. > >> > >> If not all inputs have timestamps, the wallclock times at the time of > >> reception of inputs shall be used. FFmpeg must have been compiled with > >> thread support for this last case. > > I'm wondering if simply using the other input's InputFile.ts_offset > > wouldn't achieve the same effect with much less complexity. > > That's what I initially did. But since the code can also use two other > sources for start times (start_time_realtime, first_pkt_wallclock), > those intervals may not exactly match the difference between > fmctx->start_times so I use a generic calculation. In what cases is it better to use either of those two other sources? As per the commit message, the timestamps of both inputs are supposed to come from the same clock. Then it seems to me that offsetting each of those streams by different amounts would break synchronization rather than improve it. -- 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 v3] ffmpeg: add option -isync
On 2022-07-10 10:46 pm, Anton Khirnov wrote: Quoting Gyan Doshi (2022-07-08 05:56:21) On 2022-07-07 03:11 pm, Anton Khirnov wrote: Quoting Gyan Doshi (2022-07-04 18:29:12) This is a per-file input option that adjusts an input's timestamps with reference to another input, so that emitted packet timestamps account for the difference between the start times of the two inputs. Typical use case is to sync two or more live inputs such as from capture devices. Both the target and reference input source timestamps should be based on the same clock source. If not all inputs have timestamps, the wallclock times at the time of reception of inputs shall be used. FFmpeg must have been compiled with thread support for this last case. I'm wondering if simply using the other input's InputFile.ts_offset wouldn't achieve the same effect with much less complexity. That's what I initially did. But since the code can also use two other sources for start times (start_time_realtime, first_pkt_wallclock), those intervals may not exactly match the difference between fmctx->start_times so I use a generic calculation. In what cases is it better to use either of those two other sources? As per the commit message, the timestamps of both inputs are supposed to come from the same clock. Then it seems to me that offsetting each of those streams by different amounts would break synchronization rather than improve it. The first preference, when available, stores the epoch time closest to time of capture. That would eliminate some jitter. The 2nd preference is the fmctx->start_time. The 3rd is the reception wallclock. It is a fallback. It will likely lead to the worst sync. Regards, Gyan ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v1] avformat/imfdec: preserve stream information
On Sun, Jul 3, 2022 at 2:13 PM Andreas Rheinhardt
wrote:
>
> Pierre-Anthony Lemieux:
> > On Sun, Jul 3, 2022 at 12:15 PM Andreas Rheinhardt
> > wrote:
> >>
> >> Pierre-Anthony Lemieux:
> >>> On Sun, Jul 3, 2022 at 11:28 AM Andreas Rheinhardt
> >>> wrote:
>
> p...@sandflow.com:
> > From: Pierre-Anthony Lemieux
> >
> > As discussed at https://trac.ffmpeg.org/ticket/9818, the IMF demuxer
> > does not
> > currently preserve stream information such as language in the case of
> > audio
> > streams. This patch is modeled on copy_stream_props() at
> > avformat/concatdec.c.
> >
> > ---
> > libavformat/imfdec.c | 5 +
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/libavformat/imfdec.c b/libavformat/imfdec.c
> > index 71dfb26958..7aa66a06bf 100644
> > --- a/libavformat/imfdec.c
> > +++ b/libavformat/imfdec.c
> > @@ -580,11 +580,16 @@ static int
> > set_context_streams_from_tracks(AVFormatContext *s)
> > return AVERROR(ENOMEM);
> > }
> > asset_stream->id = i;
> > +asset_stream->r_frame_rate =
> > first_resource_stream->r_frame_rate;
> > +asset_stream->avg_frame_rate =
> > first_resource_stream->avg_frame_rate;
> > +asset_stream->sample_aspect_ratio =
> > first_resource_stream->sample_aspect_ratio;
> > ret = avcodec_parameters_copy(asset_stream->codecpar,
> > first_resource_stream->codecpar);
> > if (ret < 0) {
> > av_log(s, AV_LOG_ERROR, "Could not copy stream
> > parameters\n");
> > return ret;
> > }
> > +av_dict_copy(&asset_stream->metadata,
> > first_resource_stream->metadata, 0);
> > +ff_stream_side_data_copy(asset_stream, first_resource_stream);
> > avpriv_set_pts_info(asset_stream,
> > first_resource_stream->pts_wrap_bits,
> > first_resource_stream->time_base.num,
>
> Seems to me like one should use ff_stream_encode_params_copy here. Of
> course, it would have to be renamed and moved if used in a demuxer.
> >>>
> >>> Would copy_stream_props() in concatdec.c need to be refactored as well?
> >>>
> >>
> >> I often wondered about this. The problem with copy_stream_props is that
> >> it is not only called during read_header, but lateron as well, but e.g.
> >> the documentation of AVStream.side_data says that it is populated when
> >> the stream is created and not later.
> >> This issue does of course not exist in your case.
> >
> > ff_stream_encode_params_copy() does not seem to set pts_wrap_bits,
> > i.e. it does not call avpriv_set_pts_info().
> >
>
> The reason for this is that pts_wrap_bits is irrelevant for muxing.
Do you have in mind:
(a) a single av_stream_params_copy(), which copies all parameters
(b) two separate functions av_stream_demux_params_copy() and
av_stream_mux_params_copy(), which copies only relevant parameters?
(c) something else?
In the case of (b), should both be defined now, or only
av_stream_demux_params_copy() be implemented now since avformat/imfdec
would only use av_stream_demux_params_copy()?
I have not seen anyone else chime in.
>
> >>
> >>> Note that, in the case of avformat/imfdec.c, AVStream::id is not
> >>> copied across, so ff_stream_encode_params_copy() would need to be
> >>> followed by asset_stream->id = i;
> >>>
> >>
> >> Yeah, I know.
> >>
> >> - Andreas
> >> ___
> >> ffmpeg-devel mailing list
> >> ffmpeg-devel@ffmpeg.org
> >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>
> >> To unsubscribe, visit link above, or email
> >> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
> > ___
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] avdevice/avdevice: fix return value of avdevice_list_devices()
According to API docs avdevice_list_devices(), avdevice_list_input_sources()
and avdevice_list_input_sinks() should return the number of autodetected
devices on success. This is redundant with AVDeviceInfoList->nb_devices so it
was not noticed earlier that none of the underlying device list functions work
like that.
Let's fix it in generic code to make it in line with the API docs.
Fixes ticket #9820.
Signed-off-by: Marton Balint
---
libavdevice/avdevice.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/libavdevice/avdevice.c b/libavdevice/avdevice.c
index b4fb272eb6..58996404b3 100644
--- a/libavdevice/avdevice.c
+++ b/libavdevice/avdevice.c
@@ -75,9 +75,11 @@ int avdevice_list_devices(AVFormatContext *s,
AVDeviceInfoList **device_list)
ret = s->oformat->get_device_list(s, *device_list);
else
ret = s->iformat->get_device_list(s, *device_list);
-if (ret < 0)
+if (ret < 0) {
avdevice_free_list_devices(device_list);
-return ret;
+return ret;
+}
+return (*device_list)->nb_devices;
}
static int list_devices_for_context(AVFormatContext *s, AVDictionary *options,
--
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 v3] ffmpeg: add option -isync
Quoting Gyan Doshi (2022-07-10 20:02:38) > > > On 2022-07-10 10:46 pm, Anton Khirnov wrote: > > Quoting Gyan Doshi (2022-07-08 05:56:21) > >> > >> On 2022-07-07 03:11 pm, Anton Khirnov wrote: > >>> Quoting Gyan Doshi (2022-07-04 18:29:12) > This is a per-file input option that adjusts an input's timestamps > with reference to another input, so that emitted packet timestamps > account for the difference between the start times of the two inputs. > > Typical use case is to sync two or more live inputs such as from capture > devices. Both the target and reference input source timestamps should be > based on the same clock source. > > If not all inputs have timestamps, the wallclock times at the time of > reception of inputs shall be used. FFmpeg must have been compiled with > thread support for this last case. > >>> I'm wondering if simply using the other input's InputFile.ts_offset > >>> wouldn't achieve the same effect with much less complexity. > >> That's what I initially did. But since the code can also use two other > >> sources for start times (start_time_realtime, first_pkt_wallclock), > >> those intervals may not exactly match the difference between > >> fmctx->start_times so I use a generic calculation. > > In what cases is it better to use either of those two other sources? > > > > As per the commit message, the timestamps of both inputs are supposed to > > come from the same clock. Then it seems to me that offsetting each of > > those streams by different amounts would break synchronization rather > > than improve it. > > The first preference, when available, stores the epoch time closest to > time of capture. That would eliminate some jitter. > The 2nd preference is the fmctx->start_time. The 3rd is the reception > wallclock. It is a fallback. It will likely lead to the worst sync. You did not answer my question. If both streams use the same clock, then how is offsetting them by different amounts improve sync? -- 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] avcodec/aarch64: Access externs via GOT with PIC
Android, starting from version 6 (API level 23, from the year 2015), requires all shared libraries to be position-independent, and refuses to load shared libraries (which are the most common native binary type on Android as in Android applications, native code is placed in shared libraries accessed via the Java Native Interface) containing dynamic relocations. To support PIC, all AArch64 assembly code in FFmpeg uses the `movrel` macro to obtain addresses of labels, such as lookup table constants, in a way that with CONFIG_PIC being 1, PC-relative addresses of labels are computed via the `adrp` instruction. This approach, however, is suitable only for labels defined in the same object file. For `adrp` to work directly between object files, the linker has to perform a text relocation. And in my scenario (libavcodec being a static library linked to a shared library, though I'm not sure if this is relevant), this resulted in the following LLVM linker errors, making my application not buildable for Android: "relocation R_AARCH64_ADR_PREL_PG_HI21 cannot be used against symbol ff_cos_32; recompile with -fPIC" "can't create dynamic relocation R_AARCH64_ADD_ABS_LO12_NC against symbol: ff_cos_32 in readonly segment; recompile object files with -fPIC or pass '-Wl,-z,notext' to allow text relocations in the output" This commit brings the solution that is already implemented in FFmpeg on AArch32 to AArch64 - a separate macro, `movrelx`, which emits instructions for computing the address of an external label through the Global Object Table (GOT). The same targets as `movrel` is implemented for are covered by this commit. For Apple targets, the instruction sequence is the one that is generated for referencing an extern variable in C code by Clang for the `aarch64-apple-darwin` target triple. For other targets (Linux), this is the sequence emitted by Clang for the `aarch64-unknown-linux-gnu` target, by GCC, and specified in the "Assembly expressions" section of the Arm Compiler Reference Guide. The hardware-assisted AddressSanitizer has no effect on the sequence - Clang generates the same `:got:` and `:got_lo12:` addressing regardless of whether `-fsanitize=hwaddress` is used. Windows has no concept of PIC, and Windows builds should be done with CONFIG_PIC being 0, so it doesn't need to be handled. The literal offset, however, is always applied using a separate `add` or `sub` instruction as the actual address is loaded indirectly for an extern object that is the whole lookup table itself. The only place where the offset is currently used with `movrelx` is VP9, with the offset being 256 or 512 bytes there. Unfortunately, that offset can't be moved to the positive immediate offset encoded in load/store instructions there without major restructuring, as the actual memory accesses are performed in a function that is common to different offset values, with the offset being pre-applied to one of its arguments instead. Without PIC though, `movrelx` is implemented exactly the same as `movrel`, with the offset applied directly to the `ldr` literal, so the non-PIC path is unaffected by this change. Testing was performed on my local build setup for Android based on ndk-build. Two things were tested: - Regression testing was done using the `libavcodec/tests/fft.c` test. `libavcodec` was built as a static library, and the test was built as a native executable (which, unlike shared libraries, isn't required to be position-independent). Both the executable without the changes and the executable with the new code were launched on a physical AArch64 device using Termux. As the length of the instruction sequences for `movrel` and `movrelx` without the offset is the same, comparing the two binaries in a diff tool has shown the expected 13 differences in the code - 12 in `fftN_neon` for different transform sizes, and 1 in `ff_fft_calc_neon`. The results for the FFT test were the same for both executables with different transform size values. - To check sufficiency and suitability for fixing the original issue, the `fft.c` test was converted into a shared library (with the `main` function renamed), and a proxy executable performing `dlopen` of the library and invoking the main test function from it via `dlsym`. Termux is built with `targetSdkVersion` 28, so the `dlopen` rule of Android API levels 23 and above disallowing dynamic relocations should apply to it. The testing device is running Android 11 (API level 30). The test was executed successfully, meaning that no relocations incompatible with PIC are required by libavcodec anymore. Signed-off-by: Triang3l --- libavcodec/aarch64/fft_neon.S | 4 ++-- libavcodec/aarch64/sbrdsp_neon.S | 2 +- libavcodec/aarch64/vp9mc_16bpp_neon.S | 4 ++-- libavcodec/aarch64/vp9mc_neon.S | 4 ++-- libavutil/aarch64/asm.S | 19 +++ 5 files changed, 26 insertions(+), 7 deletions(-
Re: [FFmpeg-devel] [PATCH 1/3] avfilter/vf_zscale: remove some unneeded initializations
On Fri, 8 Jul 2022, Marton Balint wrote:
Signed-off-by: Marton Balint
---
libavfilter/vf_zscale.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/libavfilter/vf_zscale.c b/libavfilter/vf_zscale.c
index 44cb4b9c61..6861eef278 100644
--- a/libavfilter/vf_zscale.c
+++ b/libavfilter/vf_zscale.c
@@ -149,12 +149,6 @@ static av_cold int init(AVFilterContext *ctx)
{
ZScaleContext *s = ctx->priv;
int ret;
-int i;
-for (i = 0; i < MAX_THREADS; i++) {
-s->tmp[i] = NULL;
-s->graph[i] = NULL;
-s->alpha_graph[i] = NULL;
-}
zimg_image_format_default(&s->src_format, ZIMG_API_VERSION);
zimg_image_format_default(&s->dst_format, ZIMG_API_VERSION);
zimg_image_format_default(&s->src_format_tmp, ZIMG_API_VERSION);
--
Will apply the series.
Regards,
Marton
2.35.3
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/3] avfilter/vf_zscale: remove some unneeded initializations
This was not critical at all, please revert. ___ ffmpeg-devel mailing list ffmpeg-devel@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/aarch64: Access externs via GOT with PIC
Hi, Thanks for your patch! While the patch probably is worthwhile to pursue, ffmpeg does work on Android 6 and newer (with the aarch64 assembly), so there's some gaps/misses in the reasoning in the patch description. On Sun, 10 Jul 2022, Triang3l wrote: To support PIC, all AArch64 assembly code in FFmpeg uses the `movrel` macro to obtain addresses of labels, such as lookup table constants, in a way that with CONFIG_PIC being 1, PC-relative addresses of labels are computed via the `adrp` instruction. This approach, however, is suitable only for labels defined in the same object file. For `adrp` to work directly between object files, the linker has to perform a text relocation. No, it doesn't. It's fully possible to build such libraries without text relocations currently. My memory of the situation was that we're linking our shared libraries with -Wl,-Bsymbolic, which means that those symbol references are bound at link time, so the offset from adrp to the target symbol is fixed at link time, and no text relocation is needed. However I did try to link such shared libraries, removing the -Wl,-Bsymbolic argument, and it still succeeds. So I'm a bit unsure what really makes it work for me when it isn't working for you. With Android NDK r24, I configured a build with "-target-os=android --arch=aarch64 --cc=aarch64-linux-android32-clang --enable-cross-compile --enable-shared", and successfully build that. The built libavcodec.so.59 does not have text relocations. Can you reproduce and confirm this bit? And in my scenario (libavcodec being a static library linked to a shared library, though I'm not sure if this is relevant), I think this might be quite relevant. Does adding -Wl,-Bsymbolic to the linker invocation when linking in the static libavcodec into your shared library fix the issue? (I'm not necessarily arguing that we shouldn't fix this issue - but I want to know exactly what differs in your setup and what detail exactly makes it work in our current default builds which is missing in yours.) Windows has no concept of PIC, and Windows builds should be done with CONFIG_PIC being 0, so it doesn't need to be handled. While Windows doesn't really have proper PIC, common builds of ffmpeg on Windows on aarch64 do end up with CONFIG_PIC set to 1, so please do handle that in the movrelx macro too, expanding to the same as the current movrel macro for those cases. // Martin ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/aarch64: Access externs via GOT with PIC
On Mon, 11 Jul 2022, Martin Storsjö wrote: Hi, Thanks for your patch! While the patch probably is worthwhile to pursue, ffmpeg does work on Android 6 and newer (with the aarch64 assembly), so there's some gaps/misses in the reasoning in the patch description. On Sun, 10 Jul 2022, Triang3l wrote: To support PIC, all AArch64 assembly code in FFmpeg uses the `movrel` macro to obtain addresses of labels, such as lookup table constants, in a way that with CONFIG_PIC being 1, PC-relative addresses of labels are computed via the `adrp` instruction. This approach, however, is suitable only for labels defined in the same object file. For `adrp` to work directly between object files, the linker has to perform a text relocation. No, it doesn't. It's fully possible to build such libraries without text relocations currently. My memory of the situation was that we're linking our shared libraries with -Wl,-Bsymbolic, which means that those symbol references are bound at link time, so the offset from adrp to the target symbol is fixed at link time, and no text relocation is needed. However I did try to link such shared libraries, removing the -Wl,-Bsymbolic argument, and it still succeeds. So I'm a bit unsure what really makes it work for me when it isn't working for you. Andreas Rheinhardt kindly reminded me that when linking libavcodec.so, we add -Wl,--version-script,libavcodec/libavcodec.ver, which makes e.g. ff_cos_32 a non-exported symbol, which means that it can't be interposed, and thus the relocation can be fixed at link time. If I remove that argument, I can reproduce the linker errors, and adding -Wl,-Bsymbolic fixes it. I wonder if we could mark these symbols as ELF hidden, so that they wouldn't need to be interposable at all, even when you link the static library into a separate shared library? // Martin ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3 0/4] DOVI: Add NLQ pivots to AVDOVIDataMapping
On 17/06/2022 16.35, quietvoid wrote:
The NLQ pivots are not documented but should be present
in the header for profile 7 RPU format.
It has been verified using Dolby's verification toolkit.
With the pivots parsed, the parsed values for
num_{x,y}_partitions are correct and usually equal to 1.
Changes in v3:
- Corrected AVDOVIDataMapping end offset for copy.
- Split logging patches and consolidated show_info logging into one call.
quietvoid (4):
libavutil/dovi_meta: Add nlq_pivots to AVDOVIDataMapping
fftools/ffprobe: Add DOVI nlq_pivots logging
libavfilter/vf_showinfo: Add DOVI nlq_pivots logging
fate: Add test to parse profile 7 DOVI RPU
fftools/ffprobe.c | 4 +
libavcodec/dovi_rpu.c | 9 +-
libavfilter/vf_showinfo.c | 6 +
libavutil/dovi_meta.h | 1 +
tests/fate/hevc.mak | 3 +
tests/ref/fate/hevc-dovi-profile7-rpu | 296 ++
6 files changed, 318 insertions(+), 1 deletion(-)
create mode 100644 tests/ref/fate/hevc-dovi-profile7-rpu
Ping on this patch set.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3 1/4] libavutil/dovi_meta: Add nlq_pivots to AVDOVIDataMapping
quietvoid:
> The NLQ pivots are not documented but should be present
> in the header for profile 7 RPU format.
> It has been verified using Dolby's verification toolkit.
>
> Signed-off-by: quietvoid
> ---
> libavcodec/dovi_rpu.c | 9 -
> libavutil/dovi_meta.h | 1 +
> 2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/libavcodec/dovi_rpu.c b/libavcodec/dovi_rpu.c
> index a87562c8a3..31e87cfbcc 100644
> --- a/libavcodec/dovi_rpu.c
> +++ b/libavcodec/dovi_rpu.c
> @@ -117,7 +117,7 @@ int ff_dovi_attach_side_data(DOVIContext *s, AVFrame
> *frame)
> /* Copy only the parts of these structs known to us at compiler-time. */
> #define COPY(t, a, b, last) memcpy(a, b, offsetof(t, last) +
> sizeof((b)->last))
> COPY(AVDOVIRpuDataHeader, av_dovi_get_header(dovi), &s->header,
> disable_residual_flag);
> -COPY(AVDOVIDataMapping, av_dovi_get_mapping(dovi), s->mapping,
> nlq[2].linear_deadzone_threshold);
> +COPY(AVDOVIDataMapping, av_dovi_get_mapping(dovi), s->mapping,
> nlq_pivots[1]);
Do we need the [1] here? I think not.
> COPY(AVDOVIColorMetadata, av_dovi_get_color(dovi), s->color,
> source_diagonal);
> return 0;
> }
> @@ -315,7 +315,14 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t
> *rpu, size_t rpu_size)
> }
>
> if (use_nlq) {
> +int nlq_pivot = 0;
> vdr->mapping.nlq_method_idc = get_bits(gb, 3);
> +
> +for (int i = 0; i < 2; i++) {
> +nlq_pivot += get_bits(gb, hdr->bl_bit_depth);
> +vdr->mapping.nlq_pivots[i] = av_clip_uint16(nlq_pivot);
> +}
> +
> /**
> * The patent mentions another legal value, NLQ_MU_LAW, but it's
> * not documented anywhere how to parse or apply that type of
> NLQ.
> diff --git a/libavutil/dovi_meta.h b/libavutil/dovi_meta.h
> index 3d11e02bff..6afc7b055a 100644
> --- a/libavutil/dovi_meta.h
> +++ b/libavutil/dovi_meta.h
> @@ -147,6 +147,7 @@ typedef struct AVDOVIDataMapping {
> uint32_t num_x_partitions;
> uint32_t num_y_partitions;
> AVDOVINLQParams nlq[3]; /* per component */
> +uint16_t nlq_pivots[2]; /* nlq_pred_pivot_value */
> } AVDOVIDataMapping;
>
> /**
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3 1/4] libavutil/dovi_meta: Add nlq_pivots to AVDOVIDataMapping
On 10/07/2022 19.12, Andreas Rheinhardt wrote:
quietvoid:
The NLQ pivots are not documented but should be present
in the header for profile 7 RPU format.
It has been verified using Dolby's verification toolkit.
Signed-off-by: quietvoid
---
libavcodec/dovi_rpu.c | 9 -
libavutil/dovi_meta.h | 1 +
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/libavcodec/dovi_rpu.c b/libavcodec/dovi_rpu.c
index a87562c8a3..31e87cfbcc 100644
--- a/libavcodec/dovi_rpu.c
+++ b/libavcodec/dovi_rpu.c
@@ -117,7 +117,7 @@ int ff_dovi_attach_side_data(DOVIContext *s, AVFrame *frame)
/* Copy only the parts of these structs known to us at compiler-time. */
#define COPY(t, a, b, last) memcpy(a, b, offsetof(t, last) +
sizeof((b)->last))
COPY(AVDOVIRpuDataHeader, av_dovi_get_header(dovi), &s->header,
disable_residual_flag);
-COPY(AVDOVIDataMapping, av_dovi_get_mapping(dovi), s->mapping,
nlq[2].linear_deadzone_threshold);
+COPY(AVDOVIDataMapping, av_dovi_get_mapping(dovi), s->mapping,
nlq_pivots[1]);
Do we need the [1] here? I think not.
It does indeed seem to be equivalent to just have `nlq_pivots`.
COPY(AVDOVIColorMetadata, av_dovi_get_color(dovi), s->color,
source_diagonal);
return 0;
}
@@ -315,7 +315,14 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu,
size_t rpu_size)
}
if (use_nlq) {
+int nlq_pivot = 0;
vdr->mapping.nlq_method_idc = get_bits(gb, 3);
+
+for (int i = 0; i < 2; i++) {
+nlq_pivot += get_bits(gb, hdr->bl_bit_depth);
+vdr->mapping.nlq_pivots[i] = av_clip_uint16(nlq_pivot);
+}
+
/**
* The patent mentions another legal value, NLQ_MU_LAW, but it's
* not documented anywhere how to parse or apply that type of NLQ.
diff --git a/libavutil/dovi_meta.h b/libavutil/dovi_meta.h
index 3d11e02bff..6afc7b055a 100644
--- a/libavutil/dovi_meta.h
+++ b/libavutil/dovi_meta.h
@@ -147,6 +147,7 @@ typedef struct AVDOVIDataMapping {
uint32_t num_x_partitions;
uint32_t num_y_partitions;
AVDOVINLQParams nlq[3]; /* per component */
+uint16_t nlq_pivots[2]; /* nlq_pred_pivot_value */
} AVDOVIDataMapping;
/**
___
ffmpeg-devel mailing list
ffmpeg-devel@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/aarch64: Access externs via GOT with PIC
Hi Martin, thanks for a quick and detailed review!Oops, Thunderbird (or
something else in the chain) has added an extraneous space in the beginning of
every line, though the +- deltas seem to be unaffected. I can try sending the
patch some other way if needed, like as a binary attachment, or via git
send-email, but possibly the "^ " (without quotes) per-line regular expression
should be fine for cleaning it up as well. This is my first patch submission to
this repository, and overall to a mailing list, so I'm just starting learning
the best practices… hi everyone!!!Yes, the original issue was likely a
consequence of us using a completely different build process than FFmpeg is
designed to be built with. We have our own Premake 5 scripts for FFmpeg
components, that are used for generation of Android ndk-build files via my
Premake generator, Triang3l/premake-androidndk on GitHub. So yes, we don't have
any link options imported from a file.I agree, it was very suprising to see
that dynamic relocations were done between object files within a single shared
library at all. I suspected ASLR at first, but I don't know if it can randomize
the locations of individual object files this way, that's probably not the case
though. I'll see what can be done to reproduce the effect of libavcodec.v —
since my shared library is the final application logic, not a library that's
actually shareable, none of the FFmpeg objects need to be exported from it at
all. However, static libraries barely have anything to configure overall as far
as I know, so disabling exports specifically for FFmpeg may be complicated —
but thankfully, we can (and even should, to reduce the file size) use
-fvisibility=hidden globally in our application, if that helps fix the issue.
-Wl,-Bsymbolic should be fine too, and possibly even less intrusive.If using
__attribute__((visibility("hidden"))) for the lookup tables prevents dynamic
relocations from being inserted, and if that doesn't break common usages of
libavcodec, yes, it should be a way better solution than introducing an
additional memory load at runtime. I'll check if that works for us tomorrow or
in a couple of days.If we're able to avoid using the global object table this
way though, maybe it could be desirable to also get rid of `movrelx` in the
AArch32 code as well?By the way, I'm also super confused by how the offset is
applied in the local `movrel` currently, it looks very inconsistent. The `adrp`
and `add` combination, as I understand its operation, should work for any
32-bit literal value, not specifically for addresses of known objects — `adrp`
accepting the upper 20 bits as a literal and adding them to the PC, and then
`add` just adding the lower 12 bits, the offset within the page, also taken as
a literal.While `movrelx` most likely requires the offset to be applied
explicitly using 0…4095 `add` or `sub` afterwards because it's first loaded
from a lookup table of addresses of, if I understand correctly, actual objects
in the code (I wouldn't expect it to be filled with object+8, object+16,
object+24 pointers in case of a large object for loading time reasons, though I
haven't researched this topic in detail), if everything `movrel` does is adding
the PC to the input literal… do we even need to emit `sub` for negative offsets
in it?The current Linux implementation of `movrel` just adds the offset
directly to the label using + regardless of whether it's positive or negative,
and there already are instances of negative offsets in the code
(`subpel_filters` minus 16 in vp8dsp_neon.S). The AArch32 version of this code
also doesn't use an offset parameter, rather, passing `subpel_filters-16`
directly as the label argument.However, the Apple AArch64 implementation does
have two paths for applying the offset — directly to the literal if it's
positive, but via a `sub` instruction for a negative one. This is also true for
the Windows implementation — whose existence overall is questionable, as
Windows DLLs use a different relocation method, and PIC doesn't apply to them
at all if I understand correctly; and also the `movrel` implementation for
Windows with a non-negative offset is identical to the Linux version, minus the
hardware-assisted ASan path on Linux.I'll likely play with the assembler later
to see how negative offsets are interpreted, but still, is there a reason to
emit the subtraction instruction that you can remember, or would it be safe to
possibly even remove the offset argument completely?For `movrelx`, however, if
it's needed at all, the offset argument is desirable as without PIC, it will be
applied to the label literal for free.Thank you!— Tri
On Mon, 11 Jul 2022, Martin Storsjö wrote:> Hi,>> Thanks for your patch! While
the patch probably is worthwhile to pursue, > ffmpeg does work on Android 6 and
newer (with the aarch64 assembly), so > there's some gaps/misses in the
reasoning in the patch description.>> On Sun, 1
Re: [FFmpeg-devel] [PATCH] avcodec/aarch64: Access externs via GOT with PIC
oh no… I have no idea what's happening, all the newlines in the previous
email were destroyed, sorry ugggh… let's try without disabling rich text in the
Samsung client this time :/Hi Martin, thanks for a quick and detailed
review!Oops, Thunderbird (or something else in the chain) has added an
extraneous space in the beginning of every line, though the +- deltas seem to
be unaffected. I can try sending the patch some other way if needed, like as a
binary attachment, or via git send-email, but possibly the "^ " (without
quotes) per-line regular expression should be fine for cleaning it up as well.
This is my first patch submission to this repository, and overall to a mailing
list, so I'm just starting learning the best practices… hi everyone!!!Yes, the
original issue was likely a consequence of us using a completely different
build process than FFmpeg is designed to be built with. We have our own Premake
5 scripts for FFmpeg components, that are used for generation of Android
ndk-build files via my Premake generator, Triang3l/premake-androidndk on
GitHub. So yes, we don't have any link options imported from a file.I agree, it
was very suprising to see that dynamic relocations were done between object
files within a single shared library at all. I suspected ASLR at first, but I
don't know if it can randomize the locations of individual object files this
way, that's probably not the case though. I'll see what can be done to
reproduce the effect of libavcodec.v — since my shared library is the final
application logic, not a library that's actually shareable, none of the FFmpeg
objects need to be exported from it at all. However, static libraries barely
have anything to configure overall as far as I know, so disabling exports
specifically for FFmpeg may be complicated — but thankfully, we can (and even
should, to reduce the file size) use -fvisibility=hidden globally in our
application, if that helps fix the issue. -Wl,-Bsymbolic should be fine too,
and possibly even less intrusive.If using __attribute__((visibility("hidden")))
for the lookup tables prevents dynamic relocations from being inserted, and if
that doesn't break common usages of libavcodec, yes, it should be a way better
solution than introducing an additional memory load at runtime. I'll check if
that works for us tomorrow or in a couple of days.If we're able to avoid using
the global object table this way though, maybe it could be desirable to also
get rid of `movrelx` in the AArch32 code as well?By the way, I'm also super
confused by how the offset is applied in the local `movrel` currently, it looks
very inconsistent. The `adrp` and `add` combination, as I understand its
operation, should work for any 32-bit literal value, not specifically for
addresses of known objects — `adrp` accepting the upper 20 bits as a literal
and adding them to the PC, and then `add` just adding the lower 12 bits, the
offset within the page, also taken as a literal.While `movrelx` most likely
requires the offset to be applied explicitly using 0…4095 `add` or `sub`
afterwards because it's first loaded from a lookup table of addresses of, if I
understand correctly, actual objects in the code (I wouldn't expect it to be
filled with object+8, object+16, object+24 pointers in case of a large object
for loading time reasons, though I haven't researched this topic in detail), if
everything `movrel` does is adding the PC to the input literal… do we even need
to emit `sub` for negative offsets in it?The current Linux implementation of
`movrel` just adds the offset directly to the label using + regardless of
whether it's positive or negative, and there already are instances of negative
offsets in the code (`subpel_filters` minus 16 in vp8dsp_neon.S). The AArch32
version of this code also doesn't use an offset parameter, rather, passing
`subpel_filters-16` directly as the label argument.However, the Apple AArch64
implementation does have two paths for applying the offset — directly to the
literal if it's positive, but via a `sub` instruction for a negative one. This
is also true for the Windows implementation — whose existence overall is
questionable, as Windows DLLs use a different relocation method, and PIC
doesn't apply to them at all if I understand correctly; and also the `movrel`
implementation for Windows with a non-negative offset is identical to the Linux
version, minus the hardware-assisted ASan path on Linux.I'll likely play with
the assembler later to see how negative offsets are interpreted, but still, is
there a reason to emit the subtraction instruction that you can remember, or
would it be safe to possibly even remove the offset argument completely?For
`movrelx`, however, if it's needed at all, the offset argument is desirable as
without PIC, it will be applied to the label literal for free.Thank you!— TriOn
Mon, 11 Jul 2022, Martin Storsjö wrote:> Hi,>> Thanks for your patch! While the
patch prob
[FFmpeg-devel] [PATCH 1/4] avformat/asfcrypt: Fix wrong array length in function declaration
multiswap_step() and multiswap_inv_step() both only require
six keys; in all current callers, these keys are part of
an array of twelve keys, yet in some of these callers the keys
given to these functions point to the second half of these
twelve keys, so that only six keys are available to these functions.
This led to -Wstringop-overread warnings when compiling with GCC 12.1.
Fix these by adapting the declaration of these functions.
Signed-off-by: Andreas Rheinhardt
---
libavformat/asfcrypt.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libavformat/asfcrypt.c b/libavformat/asfcrypt.c
index c77e37503e..ed68fb60ed 100644
--- a/libavformat/asfcrypt.c
+++ b/libavformat/asfcrypt.c
@@ -73,7 +73,7 @@ static void multiswap_invert_keys(uint32_t keys[12])
keys[i] = inverse(keys[i]);
}
-static uint32_t multiswap_step(const uint32_t keys[12], uint32_t v)
+static uint32_t multiswap_step(const uint32_t keys[6], uint32_t v)
{
int i;
v *= keys[0];
@@ -85,7 +85,7 @@ static uint32_t multiswap_step(const uint32_t keys[12],
uint32_t v)
return v;
}
-static uint32_t multiswap_inv_step(const uint32_t keys[12], uint32_t v)
+static uint32_t multiswap_inv_step(const uint32_t keys[6], uint32_t v)
{
int i;
v -= keys[5];
--
2.34.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 2/4] avcodec/snowenc: Don't pass int[2] as parameter declared as int[3]
check_block_inter() currently does this when calling check_block().
This leads to a -Wstringop-overflow= warning when compiling with
GCC 12.1.
Given that the main part of the body of check_block() consists
of an "if (intra) { ... } else { ... }" which is true iff
check_block() is not called from check_block_inter(),
it makes sense to fix this by just inlining check_block()
check_block_inter() and turning check_block() into a new
check_block_intra() (with the inter parts removed, of course).
This should also not make much of a difference for the generated code
given that both check_block() as well as check_block_inter()
are already marked as av_always_inline, so this commit follows
this route to fix the issue.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/snowenc.c | 64 +++-
1 file changed, 40 insertions(+), 24 deletions(-)
diff --git a/libavcodec/snowenc.c b/libavcodec/snowenc.c
index 207948675b..5e6b489c43 100644
--- a/libavcodec/snowenc.c
+++ b/libavcodec/snowenc.c
@@ -892,34 +892,23 @@ static int encode_subband(SnowContext *s, SubBand *b,
const IDWTELEM *src, const
//encode_subband_dzr(s, b, src, parent, stride, orientation);
}
-static av_always_inline int check_block(SnowContext *s, int mb_x, int mb_y,
int p[3], int intra, uint8_t (*obmc_edged)[MB_SIZE * 2], int *best_rd){
+static av_always_inline int check_block_intra(SnowContext *s, int mb_x, int
mb_y, int p[3],
+ uint8_t (*obmc_edged)[MB_SIZE *
2], int *best_rd)
+{
const int b_stride= s->b_width << s->block_max_depth;
BlockNode *block= &s->block[mb_x + mb_y * b_stride];
BlockNode backup= *block;
-unsigned value;
-int rd, index;
+int rd;
av_assert2(mb_x>=0 && mb_y>=0);
av_assert2(mb_xcolor[0] = p[0];
-block->color[1] = p[1];
-block->color[2] = p[2];
-block->type |= BLOCK_INTRA;
-}else{
-index= (p[0] + 31*p[1]) & (ME_CACHE_SIZE-1);
-value= s->me_cache_generation + (p[0]>>10) + (p[1]<<6) +
(block->ref<<12);
-if(s->me_cache[index] == value)
-return 0;
-s->me_cache[index]= value;
-
-block->mx= p[0];
-block->my= p[1];
-block->type &= ~BLOCK_INTRA;
-}
+block->color[0] = p[0];
+block->color[1] = p[1];
+block->color[2] = p[2];
+block->type |= BLOCK_INTRA;
-rd= get_block_rd(s, mb_x, mb_y, 0, obmc_edged) + s->intra_penalty *
!!intra;
+rd = get_block_rd(s, mb_x, mb_y, 0, obmc_edged) + s->intra_penalty;
//FIXME chroma
if(rd < *best_rd){
@@ -934,8 +923,35 @@ static av_always_inline int check_block(SnowContext *s,
int mb_x, int mb_y, int
/* special case for int[2] args we discard afterwards,
* fixes compilation problem with gcc 2.95 */
static av_always_inline int check_block_inter(SnowContext *s, int mb_x, int
mb_y, int p0, int p1, uint8_t (*obmc_edged)[MB_SIZE * 2], int *best_rd){
-int p[2] = {p0, p1};
-return check_block(s, mb_x, mb_y, p, 0, obmc_edged, best_rd);
+const int b_stride = s->b_width << s->block_max_depth;
+BlockNode *block = &s->block[mb_x + mb_y * b_stride];
+BlockNode backup = *block;
+unsigned value;
+int rd, index;
+
+av_assert2(mb_x >= 0 && mb_y >= 0);
+av_assert2(mb_x < b_stride);
+
+index = (p0 + 31 * p1) & (ME_CACHE_SIZE-1);
+value = s->me_cache_generation + (p0 >> 10) + (p1 << 6) + (block->ref <<
12);
+if (s->me_cache[index] == value)
+return 0;
+s->me_cache[index] = value;
+
+block->mx = p0;
+block->my = p1;
+block->type &= ~BLOCK_INTRA;
+
+rd = get_block_rd(s, mb_x, mb_y, 0, obmc_edged);
+
+//FIXME chroma
+if (rd < *best_rd) {
+*best_rd = rd;
+return 1;
+} else {
+*block = backup;
+return 0;
+}
}
static av_always_inline int check_4block_inter(SnowContext *s, int mb_x, int
mb_y, int p0, int p1, int ref, int *best_rd){
@@ -1092,7 +1108,7 @@ static void iterative_me(SnowContext *s){
// get previous score (cannot be cached due to OBMC)
if(pass > 0 && (block->type&BLOCK_INTRA)){
int color0[3]= {block->color[0], block->color[1],
block->color[2]};
-check_block(s, mb_x, mb_y, color0, 1, obmc_edged,
&best_rd);
+check_block_intra(s, mb_x, mb_y, color0, obmc_edged,
&best_rd);
}else
check_block_inter(s, mb_x, mb_y, block->mx, block->my,
obmc_edged, &best_rd);
@@ -1150,7 +1166,7 @@ static void iterative_me(SnowContext *s){
}
best_rd= ref_rd;
*block= ref_b;
-check_block(s, mb_x, mb_y, color, 1, obmc_edged, &best_rd);
+check_block_intra(s, mb_x, mb_y, color, obmc_edged, &best_rd);
//FIXME RD style color selection
if(!same_block(block, &backup)){
if(tb ) tb ->type &= ~BLOCK_
[FFmpeg-devel] [PATCH 3/4] avcodec/h264_loopfilter: Fix incorrect function parameter array size
filter_mb_mbaff_edgev() and filter_mb_mbaff_edgecv()
have a function parameter whose expected size depends upon
another parameter: It is 2 * bsi + 1 (with bsi always being 1 or 2).
This array is declared as const int16_t[7], yet some of the callers
with bsi == 1 call it with only an const int16_t[4] available.
This leads to -Wstringop-overread warnings from GCC 12.1.
This commit fixes these by replacing [7] with [/* 2 * bsi + 1 */],
so that the expected range and its dependence on bsi is immediately
visible.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/h264_loopfilter.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libavcodec/h264_loopfilter.c b/libavcodec/h264_loopfilter.c
index 2440cfa831..c164a289b7 100644
--- a/libavcodec/h264_loopfilter.c
+++ b/libavcodec/h264_loopfilter.c
@@ -143,7 +143,7 @@ static av_always_inline void filter_mb_edgecv(uint8_t *pix,
int stride,
static av_always_inline void filter_mb_mbaff_edgev(const H264Context *h,
uint8_t *pix,
int stride,
- const int16_t bS[7], int
bsi,
+ const int16_t bS[ /* 1 + 2
* bsi */ ], int bsi,
int qp, int a, int b,
int intra)
{
@@ -166,7 +166,7 @@ static av_always_inline void filter_mb_mbaff_edgev(const
H264Context *h, uint8_t
static av_always_inline void filter_mb_mbaff_edgecv(const H264Context *h,
uint8_t *pix, int stride,
-const int16_t bS[7],
+const int16_t bS[ /* 1 + 2
* bsi */ ],
int bsi, int qp, int a,
int b, int intra)
{
--
2.34.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 4/4] avcodec/svq1enc: Use unsigned for parameter >= 0 to workaround GCC bug
encode_block() in svq1enc.c looks like the following:
static int encode_block(int block[7][256], int level)
{
int best_score = 0;
for (unsigned x = 0; x < level; x++) {
int v = block[1][x];
block[level][x] = 0;
best_score += v * v;
}
if (level > 0 && best_score > 64) {
int score = 0;
score += encode_block(block, level - 1);
score += encode_block(block, level - 1);
if (score < best_score) {
best_score = score;
}
}
return best_score;
}
When called from outside of encode_block(), it is always called with
level == 5.
This triggers a bug [1] in GCC: On -O3, it creates eight clones of
encode_block with different values of level inlined into it. The clones
with negative values are of course useless*, but they also lead to
-Warray-bounds warnings, because they access block[-1].
This has been mitigated in GCC 12: It no longer creates clones
for parameters that it knows are impossible. Somehow switching levels
to unsigned makes GCC know this. Therefore this commit does this.
(For GCC 11, this changes the warning to "array subscript 4294967295 is
above array bounds" from "array subscript -1 is below array bounds".)
[1]: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102513
*: These clones can actually be discarded when compiling with
-ffunction-sections.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/svq1enc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavcodec/svq1enc.c b/libavcodec/svq1enc.c
index 3c2d594632..6072f8d07d 100644
--- a/libavcodec/svq1enc.c
+++ b/libavcodec/svq1enc.c
@@ -91,7 +91,7 @@ static int ssd_int8_vs_int16_c(const int8_t *pix1, const
int16_t *pix2,
}
static int encode_block(SVQ1EncContext *s, uint8_t *src, uint8_t *ref,
-uint8_t *decoded, int stride, int level,
+uint8_t *decoded, int stride, unsigned level,
int threshold, int lambda, int intra)
{
int count, y, x, i, j, split, best_mean, best_score, best_count;
--
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 v3] ffmpeg: add option -isync
On 2022-07-11 12:21 am, Anton Khirnov wrote: Quoting Gyan Doshi (2022-07-10 20:02:38) On 2022-07-10 10:46 pm, Anton Khirnov wrote: Quoting Gyan Doshi (2022-07-08 05:56:21) On 2022-07-07 03:11 pm, Anton Khirnov wrote: Quoting Gyan Doshi (2022-07-04 18:29:12) This is a per-file input option that adjusts an input's timestamps with reference to another input, so that emitted packet timestamps account for the difference between the start times of the two inputs. Typical use case is to sync two or more live inputs such as from capture devices. Both the target and reference input source timestamps should be based on the same clock source. If not all inputs have timestamps, the wallclock times at the time of reception of inputs shall be used. FFmpeg must have been compiled with thread support for this last case. I'm wondering if simply using the other input's InputFile.ts_offset wouldn't achieve the same effect with much less complexity. That's what I initially did. But since the code can also use two other sources for start times (start_time_realtime, first_pkt_wallclock), those intervals may not exactly match the difference between fmctx->start_times so I use a generic calculation. In what cases is it better to use either of those two other sources? As per the commit message, the timestamps of both inputs are supposed to come from the same clock. Then it seems to me that offsetting each of those streams by different amounts would break synchronization rather than improve it. The first preference, when available, stores the epoch time closest to time of capture. That would eliminate some jitter. The 2nd preference is the fmctx->start_time. The 3rd is the reception wallclock. It is a fallback. It will likely lead to the worst sync. You did not answer my question. If both streams use the same clock, then how is offsetting them by different amounts improve sync? Because the clocks can be different at different stages of stream conveyance i.e. capture -> encode -> network relay -> ffmpeg reception. As long as both use the same clock at a given stage, they represent the same sync relation but with some jitter in the mix added with each stage. The semantics of start_time_realtime is "pts=0 in the stream was captured at this real world time" (unix epoch). The fmctx start will usually be system timestamps at encode or mux. We should prefer the earliest stage available, which is what the patch does. Regards, Gyan ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
