Re: [FFmpeg-devel] [PATCH] libavformat/mov.c: export vendor id as metadata
On Tue, Nov 17, 2020 at 9:54 PM Gyan Doshi wrote:
>
>
> On 18-11-2020 02:41 am, Thierry Foucu wrote:
> > ---
> > libavformat/mov.c | 24 ++--
> > 1 file changed, 22 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavformat/mov.c b/libavformat/mov.c
> > index 2b90e31170..1f9163d658 100644
> > --- a/libavformat/mov.c
> > +++ b/libavformat/mov.c
> > @@ -2095,6 +2095,8 @@ static void mov_parse_stsd_video(MOVContext *c,
> AVIOContext *pb,
> > uint8_t codec_name[32] = { 0 };
> > int64_t stsd_start;
> > unsigned int len;
> > +int32_t id = 0;
> > +char vendor_id[5];
> >
> > /* The first 16 bytes of the video sample description are already
> >* read in ff_mov_read_stsd_entries() */
> > @@ -2102,7 +2104,15 @@ static void mov_parse_stsd_video(MOVContext *c,
> AVIOContext *pb,
> >
> > avio_rb16(pb); /* version */
> > avio_rb16(pb); /* revision level */
> > -avio_rb32(pb); /* vendor */
> > +id = avio_rb32(pb); /* vendor */
> > +if (id != 0) {
> > +vendor_id[0] = (id >> 24) & 0xff;
> > +vendor_id[1] = (id >> 16) & 0xff;
> > +vendor_id[2] = (id >> 8) & 0xff;
> > +vendor_id[3] = (id >> 0) & 0xff;
> > +vendor_id[4] = 0;
> > +av_dict_set(&st->metadata, "vendor_id", vendor_id, 0);
> > +}
> > avio_rb32(pb); /* temporal quality */
> > avio_rb32(pb); /* spatial quality */
> >
> > @@ -2150,10 +2160,20 @@ static void mov_parse_stsd_audio(MOVContext *c,
> AVIOContext *pb,
> > {
> > int bits_per_sample, flags;
> > uint16_t version = avio_rb16(pb);
> > +int32_t id = 0;
> > +char vendor_id[5];
> > AVDictionaryEntry *compatible_brands =
> av_dict_get(c->fc->metadata, "compatible_brands", NULL, AV_DICT_MATCH_CASE);
> >
> > avio_rb16(pb); /* revision level */
> > -avio_rb32(pb); /* vendor */
> > +id = avio_rb32(pb); /* vendor */
> > +if (id != 0) {
> > +vendor_id[0] = (id >> 24) & 0xff;
> > +vendor_id[1] = (id >> 16) & 0xff;
> > +vendor_id[2] = (id >> 8) & 0xff;
> > +vendor_id[3] = (id >> 0) & 0xff;
> > +vendor_id[4] = 0;
> > +av_dict_set(&st->metadata, "vendor_id", vendor_id, 0);
> > +}
>
> Seems fine.
>
> But you likely have to update many FATE refs. FATE fails. See
> https://patchwork.ffmpeg.org/check/20508/
> This automated check stops with the first failure. So check for all
> tests with '-k'
>
Thanks.
I did look at the error but it does not seem related to this patch.
Looking at it, i'm not sure how adding vendor ID could break a RGB24 in
matroska.
--- ./tests/ref/fate/rgb24-mkv 2020-10-29 20:05:36.014929264 +
+++ tests/data/fate/rgb24-mkv 2020-11-17 21:56:50.624249364 +
@@ -1,5 +1,5 @@
-fde8903c4df0ba8235dafcfd8a2f368c *tests/data/fate/rgb24-mkv.matroska
-58216 tests/data/fate/rgb24-mkv.matroska
+6244b8750d4155d3c9357bab26396ef9 *tests/data/fate/rgb24-mkv.matroska
+58245 tests/data/fate/rgb24-mkv.matroska
#tb 0: 1/10
#media_type 0: video
#codec_id 0: rawvideo
Test rgb24-mkv failed. Look at tests/data/fate/rgb24-mkv.err for details.
tests/Makefile:255: recipe for target 'fate-rgb24-mkv' failed
I will run fate and double check
> 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".
--
Thierry Foucu
___
ffmpeg-devel mailing list
ffmpeg-devel@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] libavformat/mov.c: export vendor id as metadata
On 18-11-2020 02:41 am, Thierry Foucu wrote:
---
libavformat/mov.c | 24 ++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 2b90e31170..1f9163d658 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -2095,6 +2095,8 @@ static void mov_parse_stsd_video(MOVContext *c,
AVIOContext *pb,
uint8_t codec_name[32] = { 0 };
int64_t stsd_start;
unsigned int len;
+int32_t id = 0;
+char vendor_id[5];
/* The first 16 bytes of the video sample description are already
* read in ff_mov_read_stsd_entries() */
@@ -2102,7 +2104,15 @@ static void mov_parse_stsd_video(MOVContext *c,
AVIOContext *pb,
avio_rb16(pb); /* version */
avio_rb16(pb); /* revision level */
-avio_rb32(pb); /* vendor */
+id = avio_rb32(pb); /* vendor */
+if (id != 0) {
+vendor_id[0] = (id >> 24) & 0xff;
+vendor_id[1] = (id >> 16) & 0xff;
+vendor_id[2] = (id >> 8) & 0xff;
+vendor_id[3] = (id >> 0) & 0xff;
+vendor_id[4] = 0;
+av_dict_set(&st->metadata, "vendor_id", vendor_id, 0);
+}
avio_rb32(pb); /* temporal quality */
avio_rb32(pb); /* spatial quality */
@@ -2150,10 +2160,20 @@ static void mov_parse_stsd_audio(MOVContext *c, AVIOContext *pb,
{
int bits_per_sample, flags;
uint16_t version = avio_rb16(pb);
+int32_t id = 0;
+char vendor_id[5];
AVDictionaryEntry *compatible_brands = av_dict_get(c->fc->metadata,
"compatible_brands", NULL, AV_DICT_MATCH_CASE);
avio_rb16(pb); /* revision level */
-avio_rb32(pb); /* vendor */
+id = avio_rb32(pb); /* vendor */
+if (id != 0) {
+vendor_id[0] = (id >> 24) & 0xff;
+vendor_id[1] = (id >> 16) & 0xff;
+vendor_id[2] = (id >> 8) & 0xff;
+vendor_id[3] = (id >> 0) & 0xff;
+vendor_id[4] = 0;
+av_dict_set(&st->metadata, "vendor_id", vendor_id, 0);
+}
Seems fine.
But you likely have to update many FATE refs. FATE fails. See
https://patchwork.ffmpeg.org/check/20508/
This automated check stops with the first failure. So check for all
tests with '-k'
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] Support HDR10+ metadata for HEVC.
On Sun, Nov 15, 2020 at 12:19 AM Andreas Rheinhardt < andreas.rheinha...@gmail.com> wrote: > Mohammad Izadi: > > From: Mohammad Izadi > > > > HDR10+ is dynamic metadata (A/341 Amendment - SMPTE2094-40) that needs > to be decoded from ITU-T T.35 in HEVC bitstream. The HDR10+ is transferred > to side data packet to be used or passed through. > > > > The fate test file can be found here: > https://drive.google.com/file/d/1Hadzc24-RsgnRqS-B0bxLmzDeTwEOhtE/view?usp=sharing > > > > The video file needs to be copied to fate-suite/mov/ > > --- > > 1. The comments regarding fate don't belong in the commit message, they > belong here below the --- (the --annotate option is useful for this when > using git send-email). > Done. > 2. This file is huge. Why don't you use a file with a smaller > bitrate/resolution? > Took another video with ~1mb. It seems 1080p is the lowest res we can capture HDR10+ by Samsung S10. Removed audio as well. Added the link to the 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". ___ ffmpeg-devel mailing list ffmpeg-devel@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] Support HDR10+ metadata for HEVC.
From: Mohammad Izadi
HDR10+ is dynamic metadata (A/341 Amendment - SMPTE2094-40) that needs to be
decoded from ITU-T T.35 in HEVC bitstream. The HDR10+ is transferred to side
data packet to be used or passed through.
---
The fate test file can be found here:
https://drive.google.com/file/d/1vcT0ohzpoyVFcxQN32jKIhUrBaBwZweT/view?usp=sharing
The video file needs to be copied to fate-suite/mov/
configure | 1 +
fftools/ffprobe.c | 106 +
libavcodec/Makefile | 3 +
libavcodec/avpacket.c | 1 +
libavcodec/decode.c | 1 +
libavcodec/dynamic_hdr10_plus.c | 223 ++
.../dynamic_hdr10_plus.h | 22 +-
libavcodec/hevc_sei.c | 62 -
libavcodec/hevc_sei.h | 5 +
libavcodec/hevcdec.c | 18 ++
libavcodec/packet.h | 8 +
libavcodec/version.h | 2 +-
libavfilter/vf_showinfo.c | 106 -
libavutil/Makefile| 2 -
libavutil/hdr_dynamic_metadata.c | 47
tests/fate/mov.mak| 3 +
tests/ref/fate/mov-hdr10-plus-metadata| 90 +++
17 files changed, 629 insertions(+), 71 deletions(-)
create mode 100644 libavcodec/dynamic_hdr10_plus.c
rename libavutil/hdr_dynamic_metadata.h => libavcodec/dynamic_hdr10_plus.h
(95%)
delete mode 100644 libavutil/hdr_dynamic_metadata.c
create mode 100644 tests/ref/fate/mov-hdr10-plus-metadata
diff --git a/configure b/configure
index 51e43fbf66..a9f12a421e 100755
--- a/configure
+++ b/configure
@@ -2360,6 +2360,7 @@ CONFIG_EXTRA="
dirac_parse
dnn
dvprofile
+dynamic_hdr10_plus
exif
faandct
faanidct
diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
index 86bd23d36d..4cee4e8ec3 100644
--- a/fftools/ffprobe.c
+++ b/fftools/ffprobe.c
@@ -35,6 +35,7 @@
#include "libavutil/bprint.h"
#include "libavutil/display.h"
#include "libavutil/hash.h"
+#include "libavutil/hdr_dynamic_metadata.h"
#include "libavutil/mastering_display_metadata.h"
#include "libavutil/dovi_meta.h"
#include "libavutil/opt.h"
@@ -1860,6 +1861,105 @@ static inline int show_tags(WriterContext *w,
AVDictionary *tags, int section_id
return ret;
}
+static void print_dynamic_hdr10_plus(WriterContext *w, AVDynamicHDRPlus
*metadata)
+{
+if (!metadata)
+return;
+print_int("application version", metadata->application_version);
+print_int("num_windows", metadata->num_windows);
+for (int n = 1; n < metadata->num_windows; n++) {
+AVHDRPlusColorTransformParams *params = &metadata->params[n];
+print_q("window_upper_left_corner_x",
+params->window_upper_left_corner_x,'/');
+print_q("window_upper_left_corner_y",
+params->window_upper_left_corner_y,'/');
+print_q("window_lower_right_corner_x",
+params->window_lower_right_corner_x,'/');
+print_q("window_lower_right_corner_y",
+params->window_lower_right_corner_y,'/');
+print_q("window_upper_left_corner_x",
+params->window_upper_left_corner_x,'/');
+print_q("window_upper_left_corner_y",
+params->window_upper_left_corner_y,'/');
+print_int("center_of_ellipse_x",
+ params->center_of_ellipse_x ) ;
+print_int("center_of_ellipse_y",
+ params->center_of_ellipse_y );
+print_int("rotation_angle",
+ params->rotation_angle);
+print_int("semimajor_axis_internal_ellipse",
+ params->semimajor_axis_internal_ellipse);
+print_int("semimajor_axis_external_ellipse",
+ params->semimajor_axis_external_ellipse);
+print_int("semiminor_axis_external_ellipse",
+ params->semiminor_axis_external_ellipse);
+print_int("overlap_process_option",
+ params->overlap_process_option);
+}
+print_q("targeted_system_display_maximum_luminance",
+metadata->targeted_system_display_maximum_luminance,'/');
+if (metadata->targeted_system_display_actual_peak_luminance_flag) {
+print_int("num_rows_targeted_system_display_actual_peak_luminance",
+
metadata->num_rows_targeted_system_display_actual_peak_luminance);
+print_int("num_cols_targeted_system_display_actual_peak_luminance",
+
metadata->num_cols_targeted_system_display_actual_peak_luminance);
+for (int i = 0; i <
metadata->num_rows_targeted_system_display_actual_peak_luminance; i++) {
+for (int j = 0; j <
metadata->num_cols_targeted_system_display_actual_peak_luminance; j++) {
+
Re: [FFmpeg-devel] [PATCH] fate: Convert the musepack8 test to an oneoff test
On Thu, 12 Nov 2020, Martin Storsjö wrote: On Thu, 12 Nov 2020, Andreas Rheinhardt wrote: Martin Storsjö: This fixes tests if built for x86 with x87 FPU. --- This requires someone to upload a reference file. --- tests/fate/mpc.mak | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/fate/mpc.mak b/tests/fate/mpc.mak index bb1c03d250..cde6e55177 100644 --- a/tests/fate/mpc.mak +++ b/tests/fate/mpc.mak @@ -12,7 +12,9 @@ fate-musepack7: REF = $(SAMPLES)/musepack/inside-mp7.pcm FATE_MPC-$(call ALLYES, FILE_PROTOCOL MPC8_DEMUXER MPC8_DECODER \ ARESAMPLE_FILTER PCM_S16LE_ENCODER \ FRAMECRC_MUXER PIPE_PROTOCOL) += fate-musepack8 -fate-musepack8: CMD = framecrc -i $(TARGET_SAMPLES)/musepack/inside-mp8.mpc -ss 8.4 -af aresample -c:a pcm_s16le +fate-musepack8: CMD = pcm -i $(TARGET_SAMPLES)/musepack/inside-mp8.mpc -ss 8.4 -af aresample +fate-musepack8: CMP = oneoff +fate-musepack8: REF = $(SAMPLES)/musepack/inside-mp8.pcm FATE_SAMPLES_AVCONV += $(FATE_MPC-yes) fate-mpc: $(FATE_MPC-yes) To quote myself: "The test uses the framecrc output, because Musepack SV8 is an encoder that returns multiple frames for a single packet, so that timing information in the test output is valueable. Output seeking has been used in order to limit the size of the ref file as well as to test this codepath for the first time." The timing information would be lost with your patch; output seeking would only be implicitly tested (by looking at the first sample that is retained). And of course your ref file would be way bigger. So is there no better way? Well one way would be to convert the floating point bits of the decoder to fixed point, or create some combination of pcm/oneoff + framecrc for timing info but without the actual crcs. Or build something like framecrc with actual output data instead of hashes, with a matching oneoff test tool to compare the outputs... But as things are right now, any crc tests for anything involving floats (at least, anything that does more than one single trivial arithmetic operation on floats before storing them) are bound to break. Concluded on irc that this probably is the only sensible option, and a sample has been uploaded a couple hours ago, thus pushing this one. // 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".
[FFmpeg-devel] [PATCH] libavformat/mov.c: export vendor id as metadata
---
libavformat/mov.c | 24 ++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 2b90e31170..1f9163d658 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -2095,6 +2095,8 @@ static void mov_parse_stsd_video(MOVContext *c,
AVIOContext *pb,
uint8_t codec_name[32] = { 0 };
int64_t stsd_start;
unsigned int len;
+int32_t id = 0;
+char vendor_id[5];
/* The first 16 bytes of the video sample description are already
* read in ff_mov_read_stsd_entries() */
@@ -2102,7 +2104,15 @@ static void mov_parse_stsd_video(MOVContext *c,
AVIOContext *pb,
avio_rb16(pb); /* version */
avio_rb16(pb); /* revision level */
-avio_rb32(pb); /* vendor */
+id = avio_rb32(pb); /* vendor */
+if (id != 0) {
+vendor_id[0] = (id >> 24) & 0xff;
+vendor_id[1] = (id >> 16) & 0xff;
+vendor_id[2] = (id >> 8) & 0xff;
+vendor_id[3] = (id >> 0) & 0xff;
+vendor_id[4] = 0;
+av_dict_set(&st->metadata, "vendor_id", vendor_id, 0);
+}
avio_rb32(pb); /* temporal quality */
avio_rb32(pb); /* spatial quality */
@@ -2150,10 +2160,20 @@ static void mov_parse_stsd_audio(MOVContext *c,
AVIOContext *pb,
{
int bits_per_sample, flags;
uint16_t version = avio_rb16(pb);
+int32_t id = 0;
+char vendor_id[5];
AVDictionaryEntry *compatible_brands = av_dict_get(c->fc->metadata,
"compatible_brands", NULL, AV_DICT_MATCH_CASE);
avio_rb16(pb); /* revision level */
-avio_rb32(pb); /* vendor */
+id = avio_rb32(pb); /* vendor */
+if (id != 0) {
+vendor_id[0] = (id >> 24) & 0xff;
+vendor_id[1] = (id >> 16) & 0xff;
+vendor_id[2] = (id >> 8) & 0xff;
+vendor_id[3] = (id >> 0) & 0xff;
+vendor_id[4] = 0;
+av_dict_set(&st->metadata, "vendor_id", vendor_id, 0);
+}
st->codecpar->channels = avio_rb16(pb); /* channel count */
st->codecpar->bits_per_coded_sample = avio_rb16(pb); /* sample size */
--
2.29.2.299.gdc1121823c-goog
___
ffmpeg-devel mailing list
ffmpeg-devel@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] http: support retry on connection error
Another ping... would be great to have some feedback on this... Eran From: Eran Kornblau Sent: Sunday, November 8, 2020 8:59 AM To: FFmpeg development discussions and patches Subject: RE: [PATCH] http: support retry on connection error Pinging again... Thanks, Eran From: Eran Kornblau Sent: Sunday, October 25, 2020 3:40 PM To: FFmpeg development discussions and patches mailto:ffmpeg-devel@ffmpeg.org>> Subject: [PATCH] http: support retry on connection error Hi, This patch adds 2 options to http: - reconnect_on_status - a list of http status codes that should be retried. the list can contain explicit status codes or the strings 4xx/5xx. - reconnect_on_err - reconnects on arbitrary errors during connect, e.g. ECONNRESET/ETIMEDOUT. The retry employs the same exponential backoff logic as the existing reconnect/reconnect_at_eof flags. Related tickets: https://trac.ffmpeg.org/ticket/6066 https://trac.ffmpeg.org/ticket/7768 Thanks! Eran ___ ffmpeg-devel mailing list ffmpeg-devel@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] tools/target_dec_fuzzer: Adjust maxpixels for G2M
Fixes: Timeout (50sec -> 3sec)
Fixes:
27383/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_G2M_fuzzer-5196953666977792
Found-by: continuous fuzzing process
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer
---
tools/target_dec_fuzzer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/target_dec_fuzzer.c b/tools/target_dec_fuzzer.c
index 4eb59bd296..21da41cab0 100644
--- a/tools/target_dec_fuzzer.c
+++ b/tools/target_dec_fuzzer.c
@@ -149,7 +149,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t
size) {
case AV_CODEC_ID_DST: maxsamples /= 1<<20; break;
case AV_CODEC_ID_DXV: maxpixels /= 32;break;
case AV_CODEC_ID_FFWAVESYNTH: maxsamples /= 16384; break;
-case AV_CODEC_ID_G2M: maxpixels /= 64;break;
+case AV_CODEC_ID_G2M: maxpixels /= 1024; break;
case AV_CODEC_ID_GDV: maxpixels /= 512; break;
case AV_CODEC_ID_GIF: maxpixels /= 16;break;
case AV_CODEC_ID_HAP: maxpixels /= 128; break;
--
2.17.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] Moves yuv2yuvX_sse3 to yasm, unrolls main loop and other small optimizations for ~20% speedup.
On Mon, Nov 16, 2020 at 11:03 AM Alan Kelly wrote: > +cglobal yuv2yuvX, 6, 7, 16, filter, filterSize, dest, dstW, dither, offset, > src Only 8 xmm registers are used, so 8 should be used instead of 16 here. Otherwise it causes unnecessary spilling of registers on 64-bit Windows. > +%if ARCH_X86_64 > +%define ptr_size 8 [...] > +%else > +%define ptr_size 4 The predefined variable gprsize already exists for this purpose, so that can be used instead. > +movq xmm3, [ditherq] If vpbroadcastq m3, [ditherq] is used for AVX2 here, then the following > +vperm2i128 m3, m3, m3, 0 instruction can be eliminated. > +punpcklwdm1, m1 > +punpckldqm1, m1 Can be replaced with pshuflw m1, m1, q >+mov srcq, [filterSizeq] >+test srcd, srcd test srcq, srcq should be used here, since the lower 32 bits of a valid pointer could randomly happen to be zero on a 64-bit system. > +REP_RET Since non-temporal stores are being used, this should be replaced with sfence RET to guarantee proper memory ordering semantics in multi-threaded use cases. Things will usually work fine without it, but may potentially break in "fun to debug" ways. ___ ffmpeg-devel mailing list ffmpeg-devel@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 2/3] libavcodec: add a new AV_CODEC_EXPORT_DATA_FILM_GRAIN flag and option
Nov 17, 2020, 10:58 by an...@khirnov.net: > Quoting Lynne (2020-11-17 00:21:43) > >> Nov 16, 2020, 15:56 by ffm...@haasn.xyz: >> >> > In terms of an API user needing this functionality, the way I see it >> > working is that presence of the side-data will imply that it still needs >> > to be applied, with the side data being removed the moment grain >> > actually is applied. So that means that decoders applying film grain >> > must never export this side data, and decoders not applying film grain >> > must always export this side data. >> > >> > A decoder both applying grain and exporting the side data is >> > nonsensical, and would only result in double-grain-application. In terms >> > of the capability checking, I don't need you to tell me whether or not >> > the decoder can apply/export side data or not, rather, I need to tell >> > *you* whether or not I can apply film grain myself. This is what I >> > understand AV_CODEC_EXPORT_DATA_FILM_GRAIN to be. Whether or not it's >> > actually exported when that flag is present is irrelevant to me, all I'm >> > trying to do is signaling that I'd prefer film grain to be exported >> > rather than applied. >> > >> >> Yup, that's my idea of how this should work as well. >> Maybe even analyzers should want to skip applying film grain in a majority >> of cases or apply it independently. So I still think of having an export >> flag control the decoder behavior is acceptable in this case. >> > > It's not about majority vs. minority, the question is > "is there EVER a valid reason to apply AND export?" > If the answer is yes, then there needs to be a way to do that. The flag > you're proposing explicitly says you can do either one or the other, but > never both. > That's a kinda black and white view. We have both APIs that don't allow very common cases to be handled and are creating us problems and we have APIs which allow for completely unreasonable things to happen and are creating us problems as well. I don't think there's a reasonable non-contrived case in which you'd want to both export and apply film grain. But I also didn't think there's a reasonable case to ever need to chain demuxers and decoders. ___ ffmpeg-devel mailing list ffmpeg-devel@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] avformat/mov: adjust skip_samples according to seek timestamp
On Mon, Nov 09, 2020 at 06:26:46PM +0100, Matthieu Bouron wrote:
> On Mon, Nov 02, 2020 at 12:42:19PM +0100, Matthieu Bouron wrote:
> > Currently skip_samples is set to start_pad if sample_time is lesser or
> > equal to 0. This can cause issues if the stream starts with packets that
> > have negative pts. Calling avformat_seek_file() with ts set to 0 on such
> > streams makes the mov demuxer return the right corresponding packets
> > (near the 0 timestamp) but set skip_samples to start_pad which is
> > incorrect as the audio decoder will discard the returned samples
> > according to skip_samples from the first packet it receives (which has
> > its timestamp near 0).
> >
> > For example, considering the following audio stream with start_pad=1344:
> >
> > [PKT pts=-1344] [PKT pts=-320] [PKT pts=704] [PKT pts=1728] [...]
> >
> > Calling avformat_seek_file() with ts=0 makes the next call to
> > av_read_frame() return the packet with pts=-320 and a skip samples
> > side data set to 1344 (start_pad). This makes the audio decoder
> > incorrectly discard (1344 - 320) samples.
> >
> > This commit makes the move demuxer adjust skip_samples according to the
> > stream start_pad, seek timestamp and first sample timestamp.
> >
> > The above example will now result in av_read_frame() still returning the
> > packet with pts=-320 but with a skip samples side data set to 320
> > (src_pad - (seek_timestamp - first_timestamp)). This makes the audio
> > decoder only discard 320 samples (from pts=-320 to pts=0).
> > ---
> > libavformat/mov.c | 18 --
> > 1 file changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavformat/mov.c b/libavformat/mov.c
> > index dd0db6bca79..f99cb0df25a 100644
> > --- a/libavformat/mov.c
> > +++ b/libavformat/mov.c
> > @@ -8098,20 +8098,34 @@ static int mov_read_seek(AVFormatContext *s, int
> > stream_index, int64_t sample_ti
> > return sample;
> >
> > if (mc->seek_individually) {
> > +MOVStreamContext *sc = s->streams[stream_index]->priv_data;
> > +
> > /* adjust seek timestamp to found sample timestamp */
> > +int64_t first_timestamp = st->internal->index_entries[0].timestamp;
> > int64_t seek_timestamp =
> > st->internal->index_entries[sample].timestamp;
> >
> > +/* adjust skip samples according to stream start_pad, seek
> > timestamp and first timestamp */
> > +int64_t skip_samples = FFMAX(sc->start_pad - (seek_timestamp -
> > first_timestamp), 0);
> > +st->internal->skip_samples = skip_samples;
> > +
> > for (i = 0; i < s->nb_streams; i++) {
> > int64_t timestamp;
> > MOVStreamContext *sc = s->streams[i]->priv_data;
> > st = s->streams[i];
> > -st->internal->skip_samples = (sample_time <= 0) ?
> > sc->start_pad : 0;
> >
> > if (stream_index == i)
> > continue;
> >
> > timestamp = av_rescale_q(seek_timestamp,
> > s->streams[stream_index]->time_base, st->time_base);
> > -mov_seek_stream(s, st, timestamp, flags);
> > +sample = mov_seek_stream(s, st, timestamp, flags);
> > +if (sample >= 0) {
> > +first_timestamp = st->internal->index_entries[0].timestamp;
> > +seek_timestamp =
> > st->internal->index_entries[sample].timestamp;
> > +
> > +/* adjust skip samples according to stream start_pad, seek
> > timestamp and first timestamp */
> > +skip_samples = FFMAX(sc->start_pad - (seek_timestamp -
> > first_timestamp), 0);
> > +st->internal->skip_samples = skip_samples;
> > +}
> > }
> > } else {
> > for (i = 0; i < s->nb_streams; i++) {
> > --
> > 2.29.2
> >
>
> Ping.
>
Ping.
--
Matthieu B.
___
ffmpeg-devel mailing list
ffmpeg-devel@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 10/18] lavf: move AVStream.*index_entries* to AVStreamInternal
On 11/17/2020 3:43 AM, Andreas Rheinhardt wrote: James Almer: On 11/16/2020 12:25 PM, Hendrik Leppkes wrote: On Mon, Nov 16, 2020 at 4:20 PM James Almer wrote: On 11/16/2020 3:01 AM, Anton Khirnov wrote: Quoting Xiang, Haihao (2020-11-16 06:16:55) This change breaks the compiling of gst-libav ( https://gitlab.freedesktop.org/gstreamer/gst-libav), I filed https://trac.ffmpeg.org/ticket/8988 to track this regression. This is not a regression, it's a bug in gst-libav. These fields are private and have always been private, applications accessing them are broken. Looking at that ticket, it seems gst-libav wants the timestamp for a given index entry. Unless this can already be achieved in some other way, it could maybe be implemented cleanly with a new public function (We have av_index_search_timestamp() to fetch an index from a timestamp after all). I also like being able to access the index, it can be used by players to efficiently offer seeking straight to keyframes (eg. index entries), which basically requires iterating over the full index and getting the full information (timestamp, flags at least) - Hendrik What do you suggest, implement something like what you added to you LAVFilters fork? I don't know if returning a pointer to a given AVIndexEntry is ideal. If that were the case, then it wouldn't be an internal field. Perhaps something like int av_index_get_entries_count(AVStream *st); int av_index_get_timestamp(AVStream *st, int64_t *timestamp, int *flags, int idx); Where timestamp and flags are output parameters in the latter, containing the relevant AVIndexEntry values. You missed pos and size which should also be exported; pos and size sound effectively like internal values only lavf should care about, for seeking purposes. Why would the library user care about the offset of seek points in a stream described within an AVStream? They are using lavf API to handle demuxing, after all. Timestamps for seek points and the fact they are keyframes or not are useful for the user to make certain choices, but i have doubts about the rest. I don't know whether the same applies to min_distance as I don't really see what this is good for (seems to be only used by mov). And this already leads me to the next point: We might want to change what we export in the future and using pointer arguments for the each field is not good for this. Using a struct with version information (or a flags parameter for av_index_get_timestamp()) would solve this problem; too bad AVIndexEntry is not available as name for this struct (btw: the current AVIndexEntry struct should be made private after the appropriate deprecation period). If we need a public struct then AVIndexEntry can remain as such, and any field in it we don't want to export can be deprecated and then removed. The current AVIndexEntry can then be copied/renamed to FFIndexEntry or similar, and moved to internal.h - Andreas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2 2/3] libavcodec: add a new AV_CODEC_EXPORT_DATA_FILM_GRAIN flag and option
Quoting Lynne (2020-11-17 00:21:43) > Nov 16, 2020, 15:56 by ffm...@haasn.xyz: > > > In terms of an API user needing this functionality, the way I see it > > working is that presence of the side-data will imply that it still needs > > to be applied, with the side data being removed the moment grain > > actually is applied. So that means that decoders applying film grain > > must never export this side data, and decoders not applying film grain > > must always export this side data. > > > > A decoder both applying grain and exporting the side data is > > nonsensical, and would only result in double-grain-application. In terms > > of the capability checking, I don't need you to tell me whether or not > > the decoder can apply/export side data or not, rather, I need to tell > > *you* whether or not I can apply film grain myself. This is what I > > understand AV_CODEC_EXPORT_DATA_FILM_GRAIN to be. Whether or not it's > > actually exported when that flag is present is irrelevant to me, all I'm > > trying to do is signaling that I'd prefer film grain to be exported > > rather than applied. > > > > Yup, that's my idea of how this should work as well. > Maybe even analyzers should want to skip applying film grain in a majority > of cases or apply it independently. So I still think of having an export > flag control the decoder behavior is acceptable in this case. It's not about majority vs. minority, the question is "is there EVER a valid reason to apply AND export?" If the answer is yes, then there needs to be a way to do that. The flag you're proposing explicitly says you can do either one or the other, but never both. -- 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 5/7] avformat/wavdec: Avoid zeroing written to array
Quoting Michael Niedermayer (2020-11-16 22:28:48) > On Mon, Nov 16, 2020 at 07:07:21AM +0100, Anton Khirnov wrote: > > Quoting Michael Niedermayer (2020-11-16 01:05:07) > > > On Sat, Nov 14, 2020 at 11:12:15AM +0100, Anton Khirnov wrote: > > > > Quoting Michael Niedermayer (2020-11-10 00:04:54) > > > > > Fixes: OOM > > > > > Fixes: > > > > > 26934/clusterfuzz-testcase-minimized-ffmpeg_dem_W64_fuzzer-5996784213819392 > > > > > > > > > > Found-by: continuous fuzzing process > > > > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > > > > Signed-off-by: Michael Niedermayer > > > > > --- > > > > > libavformat/wavdec.c | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/libavformat/wavdec.c b/libavformat/wavdec.c > > > > > index a81f2c7a67..6e5f4ccc12 100644 > > > > > --- a/libavformat/wavdec.c > > > > > +++ b/libavformat/wavdec.c > > > > > @@ -920,7 +920,7 @@ static int w64_read_header(AVFormatContext *s) > > > > > if (chunk_size == UINT32_MAX || (filesize >= 0 && > > > > > chunk_size > filesize)) > > > > > return AVERROR_INVALIDDATA; > > > > > > > > > > -value = av_mallocz(chunk_size + 1); > > > > > +value = av_malloc(chunk_size + 1); > > > > > > > > This looks highly suspicious as a fix for anything other than > > > > performance. > > > > > > if iam not mistaken: > > > The allocation doesnzt trigger OOM as no physical memory is allocated > > > but once it is written to "z" it does and then OOMs > > > if OTOH its written too while data is read from somewhere then a > > > EOF ends writing and no OOM would happen > > > > That is highly dependent on your OS and its configuration (e.g. you can > > disable overcommit on Linux). > > yes thats correct > If the OS supports this then a OOM can be avoided, otherwise it will OOM > Avoiding the OOM in the remaining cases seems too messy to me. OTOH > this here is a rather simple change ... I am okay with the change, I object to it being committed as a (security) fix, since security fixes for platform-agnostic issues should not be so highly platform-dependent. Feel free to push if the commit message stops claiming it fixes OOM. -- 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/1] avfilter/vf_lut3d: fix sanitizef INF handling
LGTM
On Mon, Nov 9, 2020 at 2:12 AM wrote:
> From: Mark Reid
>
> ---
> libavfilter/vf_lut3d.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/libavfilter/vf_lut3d.c b/libavfilter/vf_lut3d.c
> index 988f6c8b55..172d6df0c8 100644
> --- a/libavfilter/vf_lut3d.c
> +++ b/libavfilter/vf_lut3d.c
> @@ -107,7 +107,7 @@ typedef struct ThreadData {
>
> #define EXPONENT_MASK 0x7F80
> #define MANTISSA_MASK 0x007F
> -#define SIGN_MASK 0x7FFF
> +#define SIGN_MASK 0x8000
>
> static inline float sanitizef(float f)
> {
> @@ -120,7 +120,7 @@ static inline float sanitizef(float f)
> return 0.0f;
> } else if (t.i & SIGN_MASK) {
> // -INF
> -return FLT_MIN;
> +return -FLT_MAX;
> } else {
> // +INF
> return FLT_MAX;
> --
> 2.27.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 1/1] avcodec/exr: use lookuptable for alpha if there is no trc_func
LGTM
On Mon, Nov 9, 2020 at 4:37 AM wrote:
> From: Mark Reid
>
> ---
> libavcodec/exr.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavcodec/exr.c b/libavcodec/exr.c
> index cf7824402a..e907c5c464 100644
> --- a/libavcodec/exr.c
> +++ b/libavcodec/exr.c
> @@ -1203,7 +1203,7 @@ static int decode_block(AVCodecContext *avctx, void
> *tdata,
> }
> } else if (s->pixel_type == EXR_HALF) {
> // 16-bit
> -if (c < 3) {
> +if (c < 3 || !trc_func) {
> for (x = 0; x < xsize; x++) {
> *ptr_x++ =
> s->gamma_table[bytestream_get_le16(&src)];
> }
> --
> 2.27.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] http: Check for AVERROR_EOF in the check for premature end
On Fri, 13 Nov 2020, Martin Storsjö wrote:
When the check was added (in 3668701f9600, in 2015), some IO
functions returned 0 on EOF (in particular, the TCP protocol
did, but the TLS protocol returned AVERROR_EOF). Since
0e1f771d2200d in 2017, the TCP protocol also returns AVERROR_EOF
instead of 0, making the check for premature end never have the
intended effect.
---
libavformat/http.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/libavformat/http.c b/libavformat/http.c
index 3d25d652d3..2d24c00e18 100644
--- a/libavformat/http.c
+++ b/libavformat/http.c
@@ -1436,7 +1436,8 @@ static int http_buf_read(URLContext *h, uint8_t *buf, int
size)
if ((!s->willclose || s->chunksize == UINT64_MAX) && s->off >=
target_end)
return AVERROR_EOF;
len = ffurl_read(s->hd, buf, size);
-if (!len && (!s->willclose || s->chunksize == UINT64_MAX) && s->off <
target_end) {
+if ((!len || len == AVERROR_EOF) &&
+(!s->willclose || s->chunksize == UINT64_MAX) && s->off <
target_end) {
av_log(h, AV_LOG_ERROR,
"Stream ends prematurely at %"PRIu64", should be
%"PRIu64"\n",
s->off, target_end
--
2.24.3 (Apple Git-128)
Ping?
// 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 10/18] lavf: move AVStream.*index_entries* to AVStreamInternal
On Tue, Nov 17, 2020 at 1:46 AM James Almer wrote: > > On 11/16/2020 12:25 PM, Hendrik Leppkes wrote: > > On Mon, Nov 16, 2020 at 4:20 PM James Almer wrote: > >> > >> On 11/16/2020 3:01 AM, Anton Khirnov wrote: > >>> Quoting Xiang, Haihao (2020-11-16 06:16:55) > > This change breaks the compiling of gst-libav ( > https://gitlab.freedesktop.org/gstreamer/gst-libav), I filed > https://trac.ffmpeg.org/ticket/8988 to track this regression. > >>> > >>> This is not a regression, it's a bug in gst-libav. These fields are > >>> private and have always been private, applications accessing them are > >>> broken. > >> > >> Looking at that ticket, it seems gst-libav wants the timestamp for a > >> given index entry. Unless this can already be achieved in some other > >> way, it could maybe be implemented cleanly with a new public function > >> (We have av_index_search_timestamp() to fetch an index from a timestamp > >> after all). > > > > I also like being able to access the index, it can be used by players > > to efficiently offer seeking straight to keyframes (eg. index > > entries), which basically requires iterating over the full index and > > getting the full information (timestamp, flags at least) > > > > - Hendrik > > What do you suggest, implement something like what you added to you > LAVFilters fork? I don't know if returning a pointer to a given > AVIndexEntry is ideal. If that were the case, then it wouldn't be an > internal field. > I agree that just giving out the field is not a good public API, but I just needed a quick fix. :) A proper get_count + accessor that takes the index is much more sensible, as you suggested. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
