Re: [FFmpeg-devel] fftools/ffmpeg_optc AVDictionary **opts, If memory allocation fails,

[Yy]

> 2021年12月3日 下午8:06，Yy  写道：
> 
> 
> 
>> 2021年12月3日 下午5:42，Andreas Rheinhardt > > 写道：
>> 
>> Yu Yang:
>>> Opts is assigned by setup_find_stream_info_opts(). Opts may be NULL.
>>> This situation is compatible in avformat_find_stream_info(). 
>>> Before av_dict_free(), the necessary checks were ignored.
>>> 
>>> // in fftools/ffmpeg_opt.c:1266
>>> 1067 static int open_input_file(OptionsContext *o, const char *filename)
>>> 1068 {
>>> ...
>>> 1191 AVDictionary **opts = setup_find_stream_info_opts(ic, 
>>> o->g->codec_opts);
>>> ...
>>> 1196 ret = avformat_find_stream_info(ic, opts);
>>> 1197 
>>> 1198 for (i = 0; i < orig_nb_streams; i++)
>>> 1199 av_dict_free(&opts[i]);
>>> ...
>>> 1342 }
>>> ```
>>> ```c
>>> // in libavutil/dict.c:203
>>> 203 void av_dict_free(AVDictionary **pm)
>>> 204 {
>>> 205 AVDictionary *m = *pm;
>>> ...
>>> 215 }
>>> 
>>> coredump backtrace info:
>>> ==6235==ERROR: AddressSanitizer: SEGV on unknown address 0x (pc 
>>> 0x06ba9c2f bp 0x7ffc3d5baa30 sp 0x7ffc3d5ba9a0 T0)
>>> ==6235==The signal is caused by a READ memory access.
>>> ==6235==Hint: address points to the zero page.
>>>   #0 0x6ba9c2f in av_dict_free 
>>> /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/libavutil/dict.c:205:23
>>>   #1 0x4ce5ac in open_input_file 
>>> /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg_opt.c:1199:13
>>>   #2 0x4c9dc0 in open_files 
>>> /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg_opt.c:3338:15
>>>   #3 0x4c9295 in ffmpeg_parse_options 
>>> /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg_opt.c:3378:11
>>>   #4 0x58f241 in main 
>>> /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:4988:11
>>>   #5 0x7fe35197f0b2 in __libc_start_main 
>>> /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16
>>>   #6 0x42033d in _start (/home/r1/ffmpeg/ffmpeg_4.4.1+0x42033d)
>>> 
>>> Reported-by: TOTE Robot 
>>> Signed-off-by: Yu Yang 
>>> ---
>>> fftools/ffmpeg_opt.c | 9 +
>>> 1 file changed, 5 insertions(+), 4 deletions(-)
>>> 
>>> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
>>> index a27263b879..a9fc54d948 100644
>>> --- a/fftools/ffmpeg_opt.c
>>> +++ b/fftools/ffmpeg_opt.c
>>> @@ -1197,10 +1197,11 @@ static int open_input_file(OptionsContext *o, const 
>>> char *filename)
>>>/* If not enough info to get the stream parameters, we decode the
>>>   first frames to get it. (used in mpeg case for example) */
>>>ret = avformat_find_stream_info(ic, opts);
>>> -
>>> -for (i = 0; i < orig_nb_streams; i++)
>>> -av_dict_free(&opts[i]);
>>> -av_freep(&opts);
>>> +if (opts){
>>> +for (i = 0; i < orig_nb_streams; i++)
>>> +av_dict_free(&opts[i]);
>>> +av_freep(&opts);
>>> +}
>>> 
>>>if (ret < 0) {
>>>av_log(NULL, AV_LOG_FATAL, "%s: could not find codec 
>>> parameters\n", filename);
>>> 
>> 
>> You should instead check setup_find_stream_info_opts() (either only call
>> it if orig_nb_streams is > 0 or modify it to return an error code given
>> that it can currently return NULL even on nonerror).
> Thanks for your advice, i will try to fix it again.
>> - Andreas
Hi, I try to fix this problem lastday and it not work. Today, I refer to your 
suggestion 
above to modify. But I think may be you misunderstood this bug. Because of no 
alloc memory for stream opts, ‘&opts[I]’ free caused crash.  And if 
orig_nb_streams == 0, 
crash won’t happen. So I think it is unnecessary to fix it call 
setup_find_stream_info_opts()
If orig_nb_streams is > 0. Opts == NULL when  orig_nb_streams == 0 or no alloc 
memory. 
I don’t think it is error. Because avformat_find_stream_info() allow opts == 
NULL. 
But we need to check if opts == NULL before releasing. May be this fix is right?
How do you think?   
Thx.
-Yu Yang
>> ___ 
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org 
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel 
>> 
>> 
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-requ...@ffmpeg.org  
>> with subject "unsubscribe".

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

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

Re: [FFmpeg-devel] [PATCH 1/2] avcodec/dvdsub: Don't dump images to disk based on DEBUG define

[Anton Khirnov]

Quoting Soft Works (2021-11-29 21:17:32)
> It's been a regular annoyance.
> Introduce a debug-only parameter for this.
> 
> Signed-off-by: softworkz 
> ---
>  libavcodec/dvdsubdec.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)

Does anyone actually find that code useful? Apparently it's been there
since 2005, and our standards for what is acceptable have changed
somewhat.

I would just drop 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] fate: add audio tests for Silicon Graphics Movie format

[Anton Khirnov]

Quoting Peter Ross (2021-12-06 08:48:08)
> Signed-off-by: Peter Ross 
> ---
> 
> Miminimal test vectors attached.
> Can somebody help upload them to $SAMPLES/mv folder on rsync server?
> 
>  tests/fate/audio.mak  | 12 
>  tests/ref/fate/mv-mono16bit   |  8 
>  tests/ref/fate/mv-mono8bit|  8 
>  tests/ref/fate/mv-stereo16bit |  8 
>  tests/ref/fate/mv-stereo8bit  |  8 
>  5 files changed, 44 insertions(+)
>  create mode 100644 tests/ref/fate/mv-mono16bit
>  create mode 100644 tests/ref/fate/mv-mono8bit
>  create mode 100644 tests/ref/fate/mv-stereo16bit
>  create mode 100644 tests/ref/fate/mv-stereo8bit
> 
> diff --git a/tests/fate/audio.mak b/tests/fate/audio.mak
> index fd9905ca0a..ed7ad9c220 100644
> --- a/tests/fate/audio.mak
> +++ b/tests/fate/audio.mak
> @@ -38,6 +38,18 @@ fate-imc: CMD = pcm -i $(TARGET_SAMPLES)/imc/imc.avi
>  fate-imc: CMP = oneoff
>  fate-imc: REF = $(SAMPLES)/imc/imc-201706.pcm
>  
> +FATE_SAMPLES_AUDIO-$(call DEMDEC, MV, PCM_S8) += fate-mv-mono8bit
> +fate-mv-mono8bit: CMD = framecrc -i 
> $(TARGET_SAMPLES)/mv/mono8bit-minimal.movie -vn -af aresample
> +
> +FATE_SAMPLES_AUDIO-$(call DEMDEC, MV, PCM_S16BE) += fate-mv-mono16bit
> +fate-mv-mono16bit: CMD = framecrc -i 
> $(TARGET_SAMPLES)/mv/mono16bit-minimal.movie -vn -af aresample
> +
> +FATE_SAMPLES_AUDIO-$(call DEMDEC, MV, PCM_S8) += fate-mv-stereo8bit
> +fate-mv-stereo8bit: CMD = framecrc -i 
> $(TARGET_SAMPLES)/mv/stereo8bit-minimal.movie -vn -af aresample
> +
> +FATE_SAMPLES_AUDIO-$(call DEMDEC, MV, PCM_S16BE) += fate-mv-stereo16bit
> +fate-mv-stereo16bit: CMD = framecrc -i 
> $(TARGET_SAMPLES)/mv/stereo16bit-minimal.movie -vn -af aresample

Why -af aresample? Is that even bitexact?

-- 
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/7] avformat/vivo: Do not use the general expression evaluator for parsing a floating point value

[Anton Khirnov]

Quoting Michael Niedermayer (2021-12-05 22:19:01)
> Fixes: Timeout
> Fixes: 
> 41564/clusterfuzz-testcase-minimized-ffmpeg_dem_VIVO_fuzzer-6309014024093696
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavformat/vivo.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/libavformat/vivo.c b/libavformat/vivo.c
> index b2904cd25a7..8e819d910b7 100644
> --- a/libavformat/vivo.c
> +++ b/libavformat/vivo.c
> @@ -206,11 +206,12 @@ static int vivo_read_header(AVFormatContext *s)
>  return AVERROR_INVALIDDATA;
>  value_used = 1;
>  } else if (!strcmp(key, "FPS")) {
> -AVRational tmp;
> +double d;
> +if (sscanf(value, "%f", &d) != 1)
> +return AVERROR_INVALIDDATA;
>  
>  value_used = 1;
> -if (!av_parse_ratio(&tmp, value, 1, AV_LOG_WARNING, s))
> -fps = av_inv_q(tmp);
> +fps = av_d2q(1/d, 1);
>  }
>  
>  if (!value_used)
> -- 
> 2.17.1

Won't this be inexact? You might also skip parsing the field if the
relevant key has already been seen.

-- 
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 4/4] avcodec/targa: Do not return images when there is no image in the tga

[Anton Khirnov]

Quoting Michael Niedermayer (2021-12-03 15:04:47)
> On Sat, Sep 18, 2021 at 06:27:15PM +0200, Michael Niedermayer wrote:
> > On Fri, Sep 17, 2021 at 08:06:48PM +0200, Paul B Mahol wrote:
> > > On Fri, Sep 17, 2021 at 8:00 PM Michael Niedermayer 
> > > 
> > > wrote:
> > > 
> > > > On Fri, Sep 17, 2021 at 07:35:32PM +0200, Paul B Mahol wrote:
> > > > > Please do not apply, This actually changes output to nothing.
> > > >
> > > > Do you have such a TGA file without an image in it?
> > > >
> > > > Why would that be relevant, comply with TGA specifications please.
> > > 
> > > thx
> > 
> > It is relevant as it would allow to check how such a file is handled
> > by various implementations
> > The specification does not specify that. It just says 
> > "No image Data included"
> > and our decoder returns a black image, the specification doesnt say that
> > means "black image" or i missed it when reading 
> > after the patch the decoder returns nothing instead of a black image
> > The other type this might reach is type 8, the specification i have doesnt
> > say anything about type 8 except that its reserved
> > 
> > Truevision TGAa
> > FILE FORMAT SPECIFICATION
> > Version 2.0
> > ...
> > Image Type - Field 3 (1 byte):
> > The TGA File Format can be used to store Pseudo-Color, True-Color and 
> > Direct-Color images of various
> > pixel depths. Truevision has currently defined seven image types:
> > image   Description
> > Type0   No image Data included
> 
> ping
> what do you prefer for this issue ?
> Id like to do something here not just ignore it even if the conclusion is
> that this is "not a bug but intended behavior"

A brief look at other opensource decoders suggest they don't support
type==0 at all. image-rs does, but it's not clear to be what its output
is. I'd say not returning a frame is reasonable.

-- 
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] qsvenc_hevc: Enable look ahead with ExtBRC

[Zhong Li]

>Xiang, Haihao  于2021年12月6日周一 上午11:09写道：
> Any comment for this patch ? Could someone help to merge this patch if no
> objection ?

LGTM, will apply.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 2/4] avcodec/avpacket: Perform fewer reallocations in repeated av_grow_packet()

[Anton Khirnov]

Quoting Michael Niedermayer (2021-12-04 22:32:56)
> Fixes: Timeout
> Fixes: 
> 41446/clusterfuzz-testcase-minimized-ffmpeg_dem_SAMI_fuzzer-4667644540747776
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/avpacket.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
> index d8d8fef3b9e..b10498bfc1d 100644
> --- a/libavcodec/avpacket.c
> +++ b/libavcodec/avpacket.c
> @@ -142,7 +142,12 @@ int av_grow_packet(AVPacket *pkt, int grow_by)
>  
>  if (new_size + data_offset > pkt->buf->size ||
>  !av_buffer_is_writable(pkt->buf)) {
> -int ret = av_buffer_realloc(&pkt->buf, new_size + data_offset);
> +int ret;
> +
> +if (new_size + data_offset < INT_MAX - new_size/16)
> +new_size += new_size/16;
> +
> +ret = av_buffer_realloc(&pkt->buf, new_size + data_offset);

This needs a comment, e.g.
// allocate slightly more than requested to avoid excessive
// reallocations

Flow control in this function is overly complicated as it is.

-- 
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] IMF demuxer ping

[Anton Khirnov]

Quoting Lynne (2021-12-05 11:13:17)
> 5 Dec 2021, 02:33 by p...@sandflow.com:
> 
> > Hi all,
> >
> > Quick ping re: libavformat/imf demuxer patch set.
> >
> > All outstanding feedback (thanks!) has been addressed as far as I know.
> >
> >  What are the next steps?
> >
> > It would be good to make progress while it is fresh in peoples' minds.
> >
> > Latest patchset at [1] and tracking PR at [2] (including memory leak 
> > checking):
> >
> > [1] http://ffmpeg.org/pipermail/ffmpeg-devel/2021-November/288173.html
> > [2] https://github.com/sandflow/ffmpeg-imf/pull/74
> >
> > Looking forward to (finally) adding support for IMF demuxing to ffmpeg.
> >
> 
> There are still many issues.
> Put your custom header beneath the standard ffmpeg header. Or
> remove it altogether if you're allowed to.
> You're not following our code style at all:
> All internal functions (except intra-file functions) must have
> an ff_ prefix. If it's a struct, the prefix is FF.

actually we do not require any prefixing for internal structs, since
they are not visible in the ABI.

-- 
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] fate: add audio tests for Silicon Graphics Movie format

[Andreas Rheinhardt]

Anton Khirnov:
> Quoting Peter Ross (2021-12-06 08:48:08)
>> Signed-off-by: Peter Ross 
>> ---
>>
>> Miminimal test vectors attached.
>> Can somebody help upload them to $SAMPLES/mv folder on rsync server?
>>
>>  tests/fate/audio.mak  | 12 
>>  tests/ref/fate/mv-mono16bit   |  8 
>>  tests/ref/fate/mv-mono8bit|  8 
>>  tests/ref/fate/mv-stereo16bit |  8 
>>  tests/ref/fate/mv-stereo8bit  |  8 
>>  5 files changed, 44 insertions(+)
>>  create mode 100644 tests/ref/fate/mv-mono16bit
>>  create mode 100644 tests/ref/fate/mv-mono8bit
>>  create mode 100644 tests/ref/fate/mv-stereo16bit
>>  create mode 100644 tests/ref/fate/mv-stereo8bit
>>
>> diff --git a/tests/fate/audio.mak b/tests/fate/audio.mak
>> index fd9905ca0a..ed7ad9c220 100644
>> --- a/tests/fate/audio.mak
>> +++ b/tests/fate/audio.mak
>> @@ -38,6 +38,18 @@ fate-imc: CMD = pcm -i $(TARGET_SAMPLES)/imc/imc.avi
>>  fate-imc: CMP = oneoff
>>  fate-imc: REF = $(SAMPLES)/imc/imc-201706.pcm
>>  
>> +FATE_SAMPLES_AUDIO-$(call DEMDEC, MV, PCM_S8) += fate-mv-mono8bit
>> +fate-mv-mono8bit: CMD = framecrc -i 
>> $(TARGET_SAMPLES)/mv/mono8bit-minimal.movie -vn -af aresample
>> +
>> +FATE_SAMPLES_AUDIO-$(call DEMDEC, MV, PCM_S16BE) += fate-mv-mono16bit
>> +fate-mv-mono16bit: CMD = framecrc -i 
>> $(TARGET_SAMPLES)/mv/mono16bit-minimal.movie -vn -af aresample
>> +
>> +FATE_SAMPLES_AUDIO-$(call DEMDEC, MV, PCM_S8) += fate-mv-stereo8bit
>> +fate-mv-stereo8bit: CMD = framecrc -i 
>> $(TARGET_SAMPLES)/mv/stereo8bit-minimal.movie -vn -af aresample
>> +
>> +FATE_SAMPLES_AUDIO-$(call DEMDEC, MV, PCM_S16BE) += fate-mv-stereo16bit
>> +fate-mv-stereo16bit: CMD = framecrc -i 
>> $(TARGET_SAMPLES)/mv/stereo16bit-minimal.movie -vn -af aresample
> 
> Why -af aresample? Is that even bitexact?
> 

A conversion of non-float formats is bitexact (if it isn't, it should be
treated as a bug). The pcm_s16be decoder outputs the same sample format
as the pcm_s16le encoder accepts, so aresample should be unnecessary in
these cases, but not for the pcm_s8 case. Of course, one could also just
copy the audio instead of decoding. This would also have the advantage
that the actual requirements of this test better match the stated
requirements: These tests currently all need the aresample filter, but
DEMDEC does not check for this. As a result, these tests will fail e.g.
with --disable-swscale.

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

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

Re: [FFmpeg-devel] [PATCH 2/4] avcodec/avpacket: Perform fewer reallocations in repeated av_grow_packet()

[Michael Niedermayer]

On Mon, Dec 06, 2021 at 11:04:09AM +0100, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2021-12-04 22:32:56)
> > Fixes: Timeout
> > Fixes: 
> > 41446/clusterfuzz-testcase-minimized-ffmpeg_dem_SAMI_fuzzer-4667644540747776
> > 
> > Found-by: continuous fuzzing process 
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavcodec/avpacket.c | 7 ++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
> > index d8d8fef3b9e..b10498bfc1d 100644
> > --- a/libavcodec/avpacket.c
> > +++ b/libavcodec/avpacket.c
> > @@ -142,7 +142,12 @@ int av_grow_packet(AVPacket *pkt, int grow_by)
> >  
> >  if (new_size + data_offset > pkt->buf->size ||
> >  !av_buffer_is_writable(pkt->buf)) {
> > -int ret = av_buffer_realloc(&pkt->buf, new_size + data_offset);
> > +int ret;
> > +
> > +if (new_size + data_offset < INT_MAX - new_size/16)
> > +new_size += new_size/16;
> > +
> > +ret = av_buffer_realloc(&pkt->buf, new_size + data_offset);
> 
> This needs a comment, e.g.
> // allocate slightly more than requested to avoid excessive
> // reallocations

above comment added

thx

> 
> Flow control in this function is overly complicated as it is.

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who are best at talking, realize last or never when they are wrong.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 2/4] avcodec/apedec: Use 64bit in 4/3 calculation in do_apply_filter()

[Anton Khirnov]

Quoting Michael Niedermayer (2021-12-03 18:19:54)
> Fixes: Integer overflow
> Fixes: 
> 40973/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_APE_fuzzer-6739312704618496
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/apedec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c
> index 9c723f29977..5c3261cf5cb 100644
> --- a/libavcodec/apedec.c
> +++ b/libavcodec/apedec.c
> @@ -1337,7 +1337,7 @@ static void do_apply_filter(APEContext *ctx, int 
> version, APEFilter *f,
>  absres = FFABSU(res);
>  if (absres)
>  *f->adaptcoeffs = APESIGN(res) *
> -  (8 << ((absres > f->avg * 3LL) + (absres > 
> (f->avg + f->avg / 3;
> +  (8 << ((absres > f->avg * 3LL) + (absres > 
> ((int64_t)f->avg + f->avg / 3;
>  /* equivalent to the following code
>  if (absres <= f->avg * 4 / 3)
>  *f->adaptcoeffs = APESIGN(res) * 8;
> -- 
> 2.17.1

Does this not assume a 32bit int?

-- 
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] hwcontext_vulkan: make 2 public functions always available

[Anton Khirnov]

The same issue seems to exist for hwcontext_videotoolbox.

How about putting all those stubs into hwcontext_stubs.c, which will
then contain only no-op stubs.

-- 
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] hwcontext_vulkan: make 2 public functions always available

[Anton Khirnov]

Quoting Hendrik Leppkes (2021-12-03 14:17:44)
> On Thu, Dec 2, 2021 at 7:13 PM Andreas Rheinhardt
>  wrote:
> >
> > Lynne:
> > > The issue is that if ffmpeg is compiled without Vulkan, and an API
> > > user includes and uses the functions exposed in hwcontext_vulkan.h,
> > > then a linking error will happen, as the hwcontext_vulkan.c file has
> > > not been compiled.
> > >
> > > Move the functions to another file, and make it always compiled. The
> > > functions do not use any Vulkan functions.
> > >
> > > Patch attached.
> > >
> > >
> >
> > 1. Isn't a linking error exactly what is intended in such cases?
> 
> Just do document what was already said on IRC:
> 
> I believe a stable ABI is important, and that includes always
> exporting the same symbols on any given platform, no matter the build
> configuration.
> That way you can use binary distributions of our libraries without
> needing to worry it was build just right, and use such optional
> features when they are functional.
> 
> It was suggested to me that I should just dlsym any "optional"
> features, but thats really forcing a lot of complexity on any user-app
> just to save a few bytes of stub functions to ensure linking or
> dynamic loading works.
> 
> I believe we have in the past included stubs for APIs that might not
> be functional at all times, and we should continue to do so and patch
> any holes that might have crept up here.

I agree with this and have added such stubs in the past.

-- 
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/7] avformat/vivo: Do not use the general expression evaluator for parsing a floating point value

[Michael Niedermayer]

On Mon, Dec 06, 2021 at 10:25:01AM +0100, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2021-12-05 22:19:01)
> > Fixes: Timeout
> > Fixes: 
> > 41564/clusterfuzz-testcase-minimized-ffmpeg_dem_VIVO_fuzzer-6309014024093696
> > 
> > Found-by: continuous fuzzing process 
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavformat/vivo.c | 7 ---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/libavformat/vivo.c b/libavformat/vivo.c
> > index b2904cd25a7..8e819d910b7 100644
> > --- a/libavformat/vivo.c
> > +++ b/libavformat/vivo.c
> > @@ -206,11 +206,12 @@ static int vivo_read_header(AVFormatContext *s)
> >  return AVERROR_INVALIDDATA;
> >  value_used = 1;
> >  } else if (!strcmp(key, "FPS")) {
> > -AVRational tmp;
> > +double d;
> > +if (sscanf(value, "%f", &d) != 1)
> > +return AVERROR_INVALIDDATA;
> >  
> >  value_used = 1;
> > -if (!av_parse_ratio(&tmp, value, 1, AV_LOG_WARNING, s))
> > -fps = av_inv_q(tmp);
> > +fps = av_d2q(1/d, 1);
> >  }
> >  
> >  if (!value_used)
> > -- 
> > 2.17.1
> 
> Won't this be inexact? 

you mean the 1/d or in general ?
the code before used av_expr_parse_and_eval() in av_parse_ratio() so was
using floating point already. The 1/d could be done in AVRational if you
prefer?
The intermediate is inexact, the end result i would expect to be exact
as it should be the "closest" fraction with num and den below 1

we also could do the whole without floating point. The actual fields
all where of the format AB.XY so 2 digits after the decimal dot.
if it was always that then its very easy to parse. But if some
oddball 3E1 or .99 something turns up that could add bugs for little
gain


> You might also skip parsing the field if the
> relevant key has already been seen.

yes, but it didnt do that before, ill add a seperate patch for that

thx

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who are best at talking, realize last or never when they are wrong.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 2/4] avcodec/apedec: Use 64bit in 4/3 calculation in do_apply_filter()

[Michael Niedermayer]

On Mon, Dec 06, 2021 at 11:23:12AM +0100, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2021-12-03 18:19:54)
> > Fixes: Integer overflow
> > Fixes: 
> > 40973/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_APE_fuzzer-6739312704618496
> > 
> > Found-by: continuous fuzzing process 
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavcodec/apedec.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c
> > index 9c723f29977..5c3261cf5cb 100644
> > --- a/libavcodec/apedec.c
> > +++ b/libavcodec/apedec.c
> > @@ -1337,7 +1337,7 @@ static void do_apply_filter(APEContext *ctx, int 
> > version, APEFilter *f,
> >  absres = FFABSU(res);
> >  if (absres)
> >  *f->adaptcoeffs = APESIGN(res) *
> > -  (8 << ((absres > f->avg * 3LL) + (absres 
> > > (f->avg + f->avg / 3;
> > +  (8 << ((absres > f->avg * 3LL) + (absres 
> > > ((int64_t)f->avg + f->avg / 3;
> >  /* equivalent to the following code
> >  if (absres <= f->avg * 4 / 3)
> >  *f->adaptcoeffs = APESIGN(res) * 8;
> > -- 
> > 2.17.1
> 
> Does this not assume a 32bit int?

hmm
you mean avg could overflow 64bit ?
iam not sure that can happen but
we could make avg int32 or uint32 ?

thx

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[FFmpeg-devel] [PATCH 1/2] avdevice/libkvazaar: Increase array size

[Andreas Rheinhardt]

av_image_copy() expects an array of four pointers according to its
declaration; although it currently only touches pointers that
are actually in use (depending upon the pixel format) this might
change at any time (as has already happened for the linesizes
in d7bc52bf456deba0f32d9fe5c288ec441f1ebef5).

This fixes a -Wstringop-overflow= warning with GCC 11.2.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/libkvazaar.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/libavcodec/libkvazaar.c b/libavcodec/libkvazaar.c
index aabe446a28..5f9ab43093 100644
--- a/libavcodec/libkvazaar.c
+++ b/libavcodec/libkvazaar.c
@@ -208,13 +208,19 @@ static int libkvazaar_encode(AVCodecContext *avctx,
 
 // Copy pixels from frame to input_pic.
 {
+uint8_t *dst[4] = {
+input_pic->data[0],
+input_pic->data[1],
+input_pic->data[2],
+NULL,
+};
 int dst_linesizes[4] = {
   frame->width,
   frame->width / 2,
   frame->width / 2,
   0
 };
-av_image_copy(input_pic->data, dst_linesizes,
+av_image_copy(dst, dst_linesizes,
   (const uint8_t **)frame->data, frame->linesize,
   frame->format, frame->width, frame->height);
 }
-- 
2.32.0

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

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

[FFmpeg-devel] [PATCH 2/2] avdevice/libopenh264dec: Increase array sizes, fix stack-buffer overread

[Andreas Rheinhardt]

av_image_copy() expects an array of four pointers and linesizes
according to its declaration; it currently only pointers that are
actually in use (depending upon the pixel format), but this might
change at any time. It has already happened for the linesizes in
d7bc52bf456deba0f32d9fe5c288ec441f1ebef5 and so increasing their
array fixes a stack-buffer overread.

This fixes a -Wstringop-overflow= and -Wstringop-overread warning
from GCC 11.2.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/libopenh264dec.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/libavcodec/libopenh264dec.c b/libavcodec/libopenh264dec.c
index ea70a8e143..7f5e85402a 100644
--- a/libavcodec/libopenh264dec.c
+++ b/libavcodec/libopenh264dec.c
@@ -91,8 +91,8 @@ static int svc_decode_frame(AVCodecContext *avctx, void *data,
 {
 SVCContext *s = avctx->priv_data;
 SBufferInfo info = { 0 };
-uint8_t* ptrs[3];
-int ret, linesize[3];
+uint8_t *ptrs[4] = { NULL };
+int ret, linesize[4];
 AVFrame *avframe = data;
 DECODING_STATE state;
 #if OPENH264_VER_AT_LEAST(1, 7)
@@ -140,6 +140,7 @@ static int svc_decode_frame(AVCodecContext *avctx, void 
*data,
 
 linesize[0] = info.UsrData.sSystemBuffer.iStride[0];
 linesize[1] = linesize[2] = info.UsrData.sSystemBuffer.iStride[1];
+linesize[3] = 0;
 av_image_copy(avframe->data, avframe->linesize, (const uint8_t **) ptrs, 
linesize, avctx->pix_fmt, avctx->width, avctx->height);
 
 avframe->pts = info.uiOutYuvTimeStamp;
-- 
2.32.0

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

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

[FFmpeg-devel] [PATCH] libswresample/swresamplec: Err num(negative-size) was used as a function parameter

[Yu Yang]

If cannot allocate memory, ERROR(ENOMEM) '-12' as a parameter will be 
constantly being returned.
When run resample() firstly, negative size param would cause buffer-overflow 
and SEGV in swri_rematrix(). 
When run swri_rematrix() firstly, resample() would not cause error but Err num 
as a wrong parameter passing.
Err num should be returned immediately. And remove assert to ensure the return 
of the error code.

coredump info:
#0 0x499517 in posix_memalign (/home/r1/ffmpeg/ffmpeg_4.4.1+0x499517)
#1 0x6c1f0b4 in av_malloc 
/home/r1/ffmpeg/ffmpeg-4.4.1/build/src/libavutil/mem.c:86:9
#2 0x6c208fe in av_mallocz 
/home/r1/ffmpeg/ffmpeg-4.4.1/build/src/libavutil/mem.c:239:17
#3 0x6c207ad in av_mallocz_array 
/home/r1/ffmpeg/ffmpeg-4.4.1/build/src/libavutil/mem.c:195:12
#4 0x654b2e5 in swri_realloc_audio 
/home/r1/ffmpeg/ffmpeg-4.4.1/build/src/libswresample/swresample.c:418:14
#5 0x654f9a1 in swr_convert_internal 
/home/r1/ffmpeg/ffmpeg-4.4.1/build/src/libswresample/swresample.c:601:17
#6 0x654d2c0 in swr_convert 
/home/r1/ffmpeg/ffmpeg-4.4.1/build/src/libswresample/swresample.c:766:19
#7 0x186cf56 in flush_frame 
/home/r1/ffmpeg/ffmpeg-4.4.1/build/src/libavfilter/af_aresample.c:251:13
#8 0x186a454 in request_frame 
/home/r1/ffmpeg/ffmpeg-4.4.1/build/src/libavfilter/af_aresample.c:288:20
#9 0x787d9c in ff_request_frame_to_filter 
/home/r1/ffmpeg/ffmpeg-4.4.1/build/src/libavfilter/avfilter.c:459:15
#10 0x7877f1 in forward_status_change 
/home/r1/ffmpeg/ffmpeg-4.4.1/build/src/libavfilter/avfilter.c:1257:19
#11 0x77ed7e in ff_filter_activate_default 
/home/r1/ffmpeg/ffmpeg-4.4.1/build/src/libavfilter/avfilter.c:1288:20
#12 0x77e4e1 in ff_filter_activate 
/home/r1/ffmpeg/ffmpeg-4.4.1/build/src/libavfilter/avfilter.c:1441:11
#13 0x793b3f in ff_filter_graph_run_once 
/home/r1/ffmpeg/ffmpeg-4.4.1/build/src/libavfilter/avfiltergraph.c:1403:12
#14 0x7a7bee in get_frame_internal 
/home/r1/ffmpeg/ffmpeg-4.4.1/build/src/libavfilter/buffersink.c:131:19
#15 0x7a7287 in av_buffersink_get_frame_flags 
/home/r1/ffmpeg/ffmpeg-4.4.1/build/src/libavfilter/buffersink.c:142:12
#16 0x792888 in avfilter_graph_request_oldest 
/home/r1/ffmpeg/ffmpeg-4.4.1/build/src/libavfilter/avfiltergraph.c:1356:17
#17 0x5d07df in transcode_from_filter 
/home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:4639:11
#18 0x59e557 in transcode_step 
/home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:4729:20
#19 0x593970 in transcode 
/home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:4805:15
#20 0x58f7a4 in main 
/home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:5010:9
#21 0x7f6fd2dee0b2 in __libc_start_main 
/build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16

SUMMARY: AddressSanitizer: negative-size-param 
(/home/r1/ffmpeg/ffmpeg_4.4.1+0x497e67) in __asan_memcpy

Reported-by: TOTE Robot 
Signed-off-by: Yu Yang 
---
 libswresample/swresample.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/libswresample/swresample.c b/libswresample/swresample.c
index c03fe5528f..92ab6a9148 100644
--- a/libswresample/swresample.c
+++ b/libswresample/swresample.c
@@ -644,6 +644,8 @@ static int swr_convert_internal(struct SwrContext *s, 
AudioData *out, int out_co
 if(s->resample_first){
 if(postin != midbuf)
 out_count= resample(s, midbuf, out_count, postin, in_count);
+if (out_count < 0)
+return out_count;
 if(midbuf != preout)
 swri_rematrix(s, preout, midbuf, out_count, preout==out);
 }else{
@@ -651,6 +653,8 @@ static int swr_convert_internal(struct SwrContext *s, 
AudioData *out, int out_co
 swri_rematrix(s, midbuf, postin, in_count, midbuf==out);
 if(midbuf != preout)
 out_count= resample(s, preout, out_count, midbuf, in_count);
+if (out_count < 0)
+return out_count;
 }
 
 if(preout != out && out_count){
@@ -769,7 +773,7 @@ int attribute_align_arg swr_convert(struct SwrContext *s,
 if(ret>0 && !s->drop_output)
 s->outpts += ret * (int64_t)s->in_sample_rate;
 
-av_assert2(max_output < 0 || ret < 0 || ret <= max_output);
+av_assert2(max_output < 0 || ret <= max_output);
 
 return ret;
 }else{
-- 
2.33.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 2/2] avdevice/libopenh264dec: Increase array sizes, fix stack-buffer overread

[Linjie Fu]

On Mon, Dec 6, 2021 at 7:37 PM Andreas Rheinhardt <
andreas.rheinha...@outlook.com> wrote:

> av_image_copy() expects an array of four pointers and linesizes
> according to its declaration; it currently only pointers that are
> actually in use (depending upon the pixel format), but this might
> change at any time. It has already happened for the linesizes in
> d7bc52bf456deba0f32d9fe5c288ec441f1ebef5 and so increasing their
> array fixes a stack-buffer overread.
>
> This fixes a -Wstringop-overflow= and -Wstringop-overread warning
> from GCC 11.2.
>
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/libopenh264dec.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/libopenh264dec.c b/libavcodec/libopenh264dec.c
> index ea70a8e143..7f5e85402a 100644
> --- a/libavcodec/libopenh264dec.c
> +++ b/libavcodec/libopenh264dec.c
> @@ -91,8 +91,8 @@ static int svc_decode_frame(AVCodecContext *avctx, void
> *data,
>  {
>  SVCContext *s = avctx->priv_data;
>  SBufferInfo info = { 0 };
> -uint8_t* ptrs[3];
> -int ret, linesize[3];
> +uint8_t *ptrs[4] = { NULL };
> +int ret, linesize[4];
>  AVFrame *avframe = data;
>  DECODING_STATE state;
>  #if OPENH264_VER_AT_LEAST(1, 7)
> @@ -140,6 +140,7 @@ static int svc_decode_frame(AVCodecContext *avctx,
> void *data,
>
>  linesize[0] = info.UsrData.sSystemBuffer.iStride[0];
>  linesize[1] = linesize[2] = info.UsrData.sSystemBuffer.iStride[1];
> +linesize[3] = 0;
>  av_image_copy(avframe->data, avframe->linesize, (const uint8_t **)
> ptrs, linesize, avctx->pix_fmt, avctx->width, avctx->height);
>
>  avframe->pts = info.uiOutYuvTimeStamp;
> --
> 2.32.0
>
 lgtm. (guess the title is referring to  "avcodec/libopenh264dec: xxx" ?)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 2/2] avdevice/libopenh264dec: Increase array sizes, fix stack-buffer overread

[Andreas Rheinhardt]

Linjie Fu:
> On Mon, Dec 6, 2021 at 7:37 PM Andreas Rheinhardt <
> andreas.rheinha...@outlook.com> wrote:
> 
>> av_image_copy() expects an array of four pointers and linesizes
>> according to its declaration; it currently only pointers that are
>> actually in use (depending upon the pixel format), but this might
>> change at any time. It has already happened for the linesizes in
>> d7bc52bf456deba0f32d9fe5c288ec441f1ebef5 and so increasing their
>> array fixes a stack-buffer overread.
>>
>> This fixes a -Wstringop-overflow= and -Wstringop-overread warning
>> from GCC 11.2.
>>
>> Signed-off-by: Andreas Rheinhardt 
>> ---
>>  libavcodec/libopenh264dec.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavcodec/libopenh264dec.c b/libavcodec/libopenh264dec.c
>> index ea70a8e143..7f5e85402a 100644
>> --- a/libavcodec/libopenh264dec.c
>> +++ b/libavcodec/libopenh264dec.c
>> @@ -91,8 +91,8 @@ static int svc_decode_frame(AVCodecContext *avctx, void
>> *data,
>>  {
>>  SVCContext *s = avctx->priv_data;
>>  SBufferInfo info = { 0 };
>> -uint8_t* ptrs[3];
>> -int ret, linesize[3];
>> +uint8_t *ptrs[4] = { NULL };
>> +int ret, linesize[4];
>>  AVFrame *avframe = data;
>>  DECODING_STATE state;
>>  #if OPENH264_VER_AT_LEAST(1, 7)
>> @@ -140,6 +140,7 @@ static int svc_decode_frame(AVCodecContext *avctx,
>> void *data,
>>
>>  linesize[0] = info.UsrData.sSystemBuffer.iStride[0];
>>  linesize[1] = linesize[2] = info.UsrData.sSystemBuffer.iStride[1];
>> +linesize[3] = 0;
>>  av_image_copy(avframe->data, avframe->linesize, (const uint8_t **)
>> ptrs, linesize, avctx->pix_fmt, avctx->width, avctx->height);
>>
>>  avframe->pts = info.uiOutYuvTimeStamp;
>> --
>> 2.32.0
>>
>  lgtm. (guess the title is referring to  "avcodec/libopenh264dec: xxx" ?)
> 

Yes. I reused and adapted the commit message of
9b17273c77ee2868ef34abc49efa70260453235b, but apparently forgot this.
Will fix before committing.

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

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

[FFmpeg-devel] [PATCH] avformat/vivo: Favor setting fps from explicit fractions

[Michael Niedermayer]

Signed-off-by: Michael Niedermayer 
---
 libavformat/vivo.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/libavformat/vivo.c b/libavformat/vivo.c
index 8e819d910b7..6126f62f7ba 100644
--- a/libavformat/vivo.c
+++ b/libavformat/vivo.c
@@ -120,7 +120,7 @@ static int vivo_get_packet_header(AVFormatContext *s)
 static int vivo_read_header(AVFormatContext *s)
 {
 VivoContext *vivo = s->priv_data;
-AVRational fps = { 1, 25};
+AVRational fps = { 0 };
 AVStream *ast, *vst;
 unsigned char *line, *line_end, *key, *value;
 long value_int;
@@ -211,13 +211,16 @@ static int vivo_read_header(AVFormatContext *s)
 return AVERROR_INVALIDDATA;
 
 value_used = 1;
-fps = av_d2q(1/d, 1);
+if (!fps.num && !fps.den)
+fps = av_d2q(1/d, 1);
 }
 
 if (!value_used)
 av_dict_set(&s->metadata, key, value, 0);
 }
 }
+if (!fps.num || !fps.den)
+fps = (AVRational){ 1, 25};
 
 avpriv_set_pts_info(ast, 64, 1, ast->codecpar->sample_rate);
 avpriv_set_pts_info(vst, 64, fps.num, fps.den);
-- 
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 1/4] avformat/mov: Check for EOF in mov_read_glbl()

[Michael Niedermayer]

On Sat, Dec 04, 2021 at 10:32:55PM +0100, Michael Niedermayer wrote:
> Fixes: Infinite loop
> Fixes: 
> 41351/clusterfuzz-testcase-minimized-ffmpeg_dem_MOV_fuzzer-5433895854669824
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavformat/mov.c | 2 ++
>  1 file changed, 2 insertions(+)

will apply

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have often repented speaking, but never of holding my tongue.
-- Xenocrates


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 2/4] avcodec/avpacket: Perform fewer reallocations in repeated av_grow_packet()

[Michael Niedermayer]

On Mon, Dec 06, 2021 at 11:17:17AM +0100, Michael Niedermayer wrote:
> On Mon, Dec 06, 2021 at 11:04:09AM +0100, Anton Khirnov wrote:
> > Quoting Michael Niedermayer (2021-12-04 22:32:56)
> > > Fixes: Timeout
> > > Fixes: 
> > > 41446/clusterfuzz-testcase-minimized-ffmpeg_dem_SAMI_fuzzer-4667644540747776
> > > 
> > > Found-by: continuous fuzzing process 
> > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > Signed-off-by: Michael Niedermayer 
> > > ---
> > >  libavcodec/avpacket.c | 7 ++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
> > > index d8d8fef3b9e..b10498bfc1d 100644
> > > --- a/libavcodec/avpacket.c
> > > +++ b/libavcodec/avpacket.c
> > > @@ -142,7 +142,12 @@ int av_grow_packet(AVPacket *pkt, int grow_by)
> > >  
> > >  if (new_size + data_offset > pkt->buf->size ||
> > >  !av_buffer_is_writable(pkt->buf)) {
> > > -int ret = av_buffer_realloc(&pkt->buf, new_size + 
> > > data_offset);
> > > +int ret;
> > > +
> > > +if (new_size + data_offset < INT_MAX - new_size/16)
> > > +new_size += new_size/16;
> > > +
> > > +ret = av_buffer_realloc(&pkt->buf, new_size + data_offset);
> > 
> > This needs a comment, e.g.
> > // allocate slightly more than requested to avoid excessive
> > // reallocations
> 
> above comment added

will apply

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them. -- Plutarch


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 6/7] tools/target_dec_fuzzer: adjust threshold for gem

[Michael Niedermayer]

On Sun, Dec 05, 2021 at 10:19:06PM +0100, Michael Niedermayer wrote:
> Fixes: Timeout
> Fixes: 
> 42035/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_GEM_fuzzer-5033604191748096
> 
> 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 | 1 +
>  1 file changed, 1 insertion(+)

will apply

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If the United States is serious about tackling the national security threats 
related to an insecure 5G network, it needs to rethink the extent to which it
values corporate profits and government espionage over security.-Bruce Schneier


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 4/4] tools/target_dec_fuzzer: Adjust threshold for HQ_HQA

[Michael Niedermayer]

On Fri, Dec 03, 2021 at 06:19:56PM +0100, Michael Niedermayer wrote:
> Fixes: Timeout
> Fixes: 
> 41120/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HQ_HQA_fuzzer-6327761690558464
> 
> 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 | 1 +
>  1 file changed, 1 insertion(+)

will apply

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Freedom in capitalist society always remains about the same as it was in
ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 7/7] avcodec/gemdec: Move all support checks before before image allocation

[Michael Niedermayer]

On Mon, Dec 06, 2021 at 06:44:00PM +1100, Peter Ross wrote:
> On Sun, Dec 05, 2021 at 10:19:07PM +0100, Michael Niedermayer wrote:
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavcodec/gemdec.c | 32 +---
> >  1 file changed, 21 insertions(+), 11 deletions(-)
> > 
> > diff --git a/libavcodec/gemdec.c b/libavcodec/gemdec.c
> > index eee21a50d4b..fd14b22390c 100644
> > --- a/libavcodec/gemdec.c
> > +++ b/libavcodec/gemdec.c
> > @@ -157,10 +157,22 @@ static int gem_decode_frame(AVCodecContext *avctx,
> >  if (header_size >= 11)
> >  tag = bytestream2_peek_be32(&gb);
> >  
> > -if (tag == AV_RB32("STTT") || tag == AV_RB32("TIMG") || tag == 
> > AV_RB32("XIMG") ||
> > -planes == 1 || planes == 2 || planes == 3 || planes == 4 ||
> > -planes == 8 || planes == 16 || planes == 24) {
> > -} else {
> > +if (tag == AV_RB32("STTT")) {
> > +if (planes != 4) {
> > +avpriv_request_sample(avctx, "STTT planes=%d", planes);
> > +return AVERROR_PATCHWELCOME;
> > +}
> > +} else if (tag == AV_RB32("TIMG")) {
> > +if (planes != 15) {
> > +avpriv_request_sample(avctx, "TIMG planes=%d", planes);
> > +return AVERROR_PATCHWELCOME;
> > +}
> > +} else if (tag == AV_RB32("XIMG")) {
> > +if (planes != 1 && planes != 2 && planes != 4 && planes != 8 && 
> > planes != 16 && planes != 24 && planes != 32) {
> > +avpriv_request_sample(avctx, "XIMG planes=%d", planes);
> > +return AVERROR_PATCHWELCOME;
> > +}
> > +} else if (planes != 1 && planes != 2 && planes != 3 && planes != 4 && 
> > planes != 8 && planes != 16 && planes != 24) {
> >  avpriv_request_sample(avctx, "planes=%d", planes);
> >  return AVERROR_PATCHWELCOME;
> >  }
> > @@ -184,14 +196,12 @@ static int gem_decode_frame(AVCodecContext *avctx,
> >  palette[i] = 0xFF00 | r << 16 | g << 8 | b;
> >  }
> >  } else {
> > -avpriv_request_sample(avctx, "STTT planes=%d", planes);
> > -return AVERROR_PATCHWELCOME;
> > +av_assert0(0);
> >  }
> >  } else if (tag == AV_RB32("TIMG")) {
> >  bytestream2_skip(&gb, 4);
> >  if (planes != 15) {
> > -avpriv_request_sample(avctx, "TIMG planes=%d", planes);
> > -return AVERROR_PATCHWELCOME;
> > +av_assert0(0);
> >  }
> >  } else if (tag == AV_RB32("XIMG")) {
> >  bytestream2_skip(&gb, 6);
> > @@ -215,8 +225,7 @@ static int gem_decode_frame(AVCodecContext *avctx,
> >  row_width = avctx->width * pixel_size;
> >  put_lines = put_lines_bytes;
> >  } else {
> > -avpriv_request_sample(avctx, "XIMG planes=%d", planes);
> > -return AVERROR_PATCHWELCOME;
> > +av_assert0(0);
> >  }
> >  } else if (planes == 1) {
> >  palette[0] = 0x;
> > @@ -244,7 +253,8 @@ static int gem_decode_frame(AVCodecContext *avctx,
> >  planes = 1;
> >  row_width = avctx->width * pixel_size;
> >  put_lines = put_lines_bytes;
> > -}
> > +} else
> > +av_assert0(0);
> >  
> >  ret = av_reallocp_array(&avctx->priv_data, planes, row_width);
> >  if (ret < 0)
> > -- 
> > 2.17.1
> 
> please apply

will apply

thx

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 1/4] avformat/mov: Disallow duplicate smdm

[Michael Niedermayer]

On Fri, Dec 03, 2021 at 06:19:53PM +0100, Michael Niedermayer wrote:
> Fixes: memleak
> Fixes: 
> 39879/clusterfuzz-testcase-minimized-ffmpeg_dem_MOV_fuzzer-5327819907923968
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavformat/mov.c | 3 +++
>  1 file changed, 3 insertions(+)

will apply

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Does the universe only have a finite lifespan? No, its going to go on
forever, its just that you wont like living in it. -- Hiranya Peiri


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH v20 02/20] avutil/frame: Prepare AVFrame for subtitle handling

[Nicolas George]

Hendrik Leppkes (12021-12-06):
> You just keep re-iterating that avfilter internals need it. And I'll
> just keep saying that avfilter internals shall not define public API
> design. I have not heard anything that justifies yet another field
> that has no benefit to the external public API - to the contrary even,
> a user would assume pts is pts, and he would be wrong.

Thank you.

I need to add that libavfilter internals need it only because it was
designed in a very naive way, by duplicating the frame instead of using
dummy frames without content.

This is not the only part of these patch series that are improperly
designed. Having two different filters to overlay subs depending on
whether they are text or bitmap is unacceptable. Not having any utility
filters (to shift or scale the times, to discard a segment, tu
concatenate subtitles along with audio and video, to print diagnostic
information, etc.) is also a big issue — and of course, triplicating the
boilerplate code for these filters is a big no. The way the
overlaytextsubs filter synchronizes its inputs is completely broken; it
works with the obvious test case, of course, but it will fail in any
complicated case.

I have made these comments before. I also have offered to help do things
properly. But instead of welcoming my help, Soft Works has rudely
rejected it. So now I am just looking with annoyance as people waste
their time helping Soft Works polishing patches whose design is wrong
from the core.

What everybody should be telling Soft Works on this subject is just
this: You do not try to redesign a major API of the project when you
have been in the project in just a few months and are still struggling
with our coding conventions and with configuring a mail client.

Especially not something that have been in project for more than a
decade. There is a reason I have not yet implemented subtitles
filtering, or offered it as a Summer of Code project: it is hard and has
a ton of prerequisite.

Now, if I knew I had the support of the core developers here, I could
serenely get on with implementing FATE tests for the negotiation system
and refactor it, which is the necessary first step before extending it
for a third media type.

Regards,

-- 
  Nicolas George


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 2/4] avcodec/apedec: Use 64bit in 4/3 calculation in do_apply_filter()

[Anton Khirnov]

Quoting Michael Niedermayer (2021-12-06 12:01:09)
> On Mon, Dec 06, 2021 at 11:23:12AM +0100, Anton Khirnov wrote:
> > Quoting Michael Niedermayer (2021-12-03 18:19:54)
> > > Fixes: Integer overflow
> > > Fixes: 
> > > 40973/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_APE_fuzzer-6739312704618496
> > > 
> > > Found-by: continuous fuzzing process 
> > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > Signed-off-by: Michael Niedermayer 
> > > ---
> > >  libavcodec/apedec.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c
> > > index 9c723f29977..5c3261cf5cb 100644
> > > --- a/libavcodec/apedec.c
> > > +++ b/libavcodec/apedec.c
> > > @@ -1337,7 +1337,7 @@ static void do_apply_filter(APEContext *ctx, int 
> > > version, APEFilter *f,
> > >  absres = FFABSU(res);
> > >  if (absres)
> > >  *f->adaptcoeffs = APESIGN(res) *
> > > -  (8 << ((absres > f->avg * 3LL) + 
> > > (absres > (f->avg + f->avg / 3;
> > > +  (8 << ((absres > f->avg * 3LL) + 
> > > (absres > ((int64_t)f->avg + f->avg / 3;
> > >  /* equivalent to the following code
> > >  if (absres <= f->avg * 4 / 3)
> > >  *f->adaptcoeffs = APESIGN(res) * 8;
> > > -- 
> > > 2.17.1
> > 
> > Does this not assume a 32bit int?
> 
> hmm
> you mean avg could overflow 64bit ?
> iam not sure that can happen but
> we could make avg int32 or uint32 ?

Seems it cannot, but it's not completely obvious. So changing it to
uint32 might make sense, since it cannot be negative.

-- 
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/7] avformat/vivo: Do not use the general expression evaluator for parsing a floating point value

[Anton Khirnov]

Quoting Michael Niedermayer (2021-12-06 11:35:50)
> On Mon, Dec 06, 2021 at 10:25:01AM +0100, Anton Khirnov wrote:
> > Quoting Michael Niedermayer (2021-12-05 22:19:01)
> > > Fixes: Timeout
> > > Fixes: 
> > > 41564/clusterfuzz-testcase-minimized-ffmpeg_dem_VIVO_fuzzer-6309014024093696
> > > 
> > > Found-by: continuous fuzzing process 
> > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > Signed-off-by: Michael Niedermayer 
> > > ---
> > >  libavformat/vivo.c | 7 ---
> > >  1 file changed, 4 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/libavformat/vivo.c b/libavformat/vivo.c
> > > index b2904cd25a7..8e819d910b7 100644
> > > --- a/libavformat/vivo.c
> > > +++ b/libavformat/vivo.c
> > > @@ -206,11 +206,12 @@ static int vivo_read_header(AVFormatContext *s)
> > >  return AVERROR_INVALIDDATA;
> > >  value_used = 1;
> > >  } else if (!strcmp(key, "FPS")) {
> > > -AVRational tmp;
> > > +double d;
> > > +if (sscanf(value, "%f", &d) != 1)
> > > +return AVERROR_INVALIDDATA;
> > >  
> > >  value_used = 1;
> > > -if (!av_parse_ratio(&tmp, value, 1, AV_LOG_WARNING, 
> > > s))
> > > -fps = av_inv_q(tmp);
> > > +fps = av_d2q(1/d, 1);
> > >  }
> > >  
> > >  if (!value_used)
> > > -- 
> > > 2.17.1
> > 
> > Won't this be inexact? 
> 
> you mean the 1/d or in general ?
> the code before used av_expr_parse_and_eval() in av_parse_ratio() so was
> using floating point already. The 1/d could be done in AVRational if you
> prefer?
> The intermediate is inexact, the end result i would expect to be exact
> as it should be the "closest" fraction with num and den below 1
> 
> we also could do the whole without floating point. The actual fields
> all where of the format AB.XY so 2 digits after the decimal dot.
> if it was always that then its very easy to parse. But if some
> oddball 3E1 or .99 something turns up that could add bugs for little
> gain

I mean involving floats at all. If that's already the case, then I don't
object to the patch (though it would be nice to know if it is in fact
fixed point and parse it as such).

-- 
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/7] avformat/vivo: Do not use the general expression evaluator for parsing a floating point value

[Marvin Scholz]

On 5 Dec 2021, at 22:19, Michael Niedermayer wrote:


Fixes: Timeout
Fixes: 
41564/clusterfuzz-testcase-minimized-ffmpeg_dem_VIVO_fuzzer-6309014024093696


Found-by: continuous fuzzing process 
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg

Signed-off-by: Michael Niedermayer 
---
 libavformat/vivo.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/libavformat/vivo.c b/libavformat/vivo.c
index b2904cd25a7..8e819d910b7 100644
--- a/libavformat/vivo.c
+++ b/libavformat/vivo.c
@@ -206,11 +206,12 @@ static int vivo_read_header(AVFormatContext *s)
 return AVERROR_INVALIDDATA;
 value_used = 1;
 } else if (!strcmp(key, "FPS")) {
-AVRational tmp;
+double d;
+if (sscanf(value, "%f", &d) != 1)
+return AVERROR_INVALIDDATA;



Shouldn't this use av_sscanf, so it's not locale dependent?


 value_used = 1;
-if (!av_parse_ratio(&tmp, value, 1, 
AV_LOG_WARNING, s))

-fps = av_inv_q(tmp);
+fps = av_d2q(1/d, 1);
 }

 if (!value_used)
--
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".

___
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 v18 01/19] avcodec, avutil: Move enum AVSubtitleType

[Anton Khirnov]

Quoting Soft Works (2021-12-03 11:25:08)
> 
> 
> > -Original Message-
> > From: ffmpeg-devel  On Behalf Of Anton
> > Khirnov
> > Sent: Wednesday, December 1, 2021 2:35 PM
> > To: ffmpeg-devel@ffmpeg.org
> > Subject: Re: [FFmpeg-devel] [PATCH v18 01/19] avcodec, avutil: Move enum
> > AVSubtitleType
> > 
> > Quoting Soft Works (2021-11-29 20:47:52)
> > > Signed-off-by: softworkz 
> > > ---
> > >  libavcodec/avcodec.h | 19 +--
> > >  libavutil/subfmt.h   | 58 
> > >  2 files changed, 59 insertions(+), 18 deletions(-)
> > >  create mode 100644 libavutil/subfmt.h
> > 
> > This commit does way more than just moving the enum, it also
> > deprecates existing enum values and adds new ones. That should be
> > mentioned in the commit message and in doc/APIchanges.
> 
> This is something I would need some help with. Which of my commits should 
> bump versions in which libs and where major or minor bump?

If you expect the whole set to be pushed at once, it is enough to have a
separate patch at the end that bumps the versions of all the affected
libraries and adds the necessary APIchanges entries.

> 
> Here's what I _think_ (possibly wrong):
> It's based on the V18 patchset
> 
> 
> 01/19 avcodec,avutil: Move enum AVSubtitleType to avutil, add new and
> 
> - major bump in avcodec because AVSubtitleType is removed
> - major bump in avutil because AVSubtitleType is added

Enums exist only in the headers, they are not exported by the binaries
in the way functions are. So moving an enum declaration from avcodec.h
to lavu does not actually break compatibility as long as avcodec.h
includes the new header (and thus provides the type to any legacy
programs).

So this requires only a minor bump in libavutil for adding a new header
and new enum values. A major bump would be required for changes that
break existing callers, adding new declarations does not break
anyything.

> 
> 02/19 avutil/frame: Prepare AVFrame for subtitle handling
> 
> - major bump in avutil because AVFrame struct is modified
>   (not just adding to the end)

Why not just move your changes to the end?

You seem not to realize that a major bump is a big deal. We typically do
it not more than once per years, and there was one already this spring.
You should avoid API/ABI-breaking changes if possible, and it is very
much possible here.

> 
> 03/19 avcodec/subtitles: Introduce new frame-based subtitle decoding
> 
> - minor bump in avcodec. New APIs are added, existing API remains functional
> 
> 04/19 avfilter/subtitles: Update vf_subtitles to use new decoding api
> (no change)
> 
> 05/19 avcodec,avutil: Move ass helper functions to avutil as avpriv_ 
> 
> - major bump in avcodec and avutil
>   no public APIs are changed but each lib depends on the other to be updated

A minor bump in libavutil. libavcodec must be used with libavutil from
the same commit or newer.

> 
> 06/19 avcodec/subtitles: Migrate subtitle encoders to frame-based API
> (no change)
> 07/19 fftools/play,probe: Adjust for subtitle changes
> (no change)
> 08/19 avfilter/subtitles: Add subtitles.c for subtitle frame allocation
> (no change)
> 
> 09/19 avfilter/avfilter: Handle subtitle frames
> 
> - minor bump in avfilter due to additions to the end of AVFilter struct.

No bump - the changes are in a non-public part of that struct.

-- 
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 v18 01/19] avcodec, avutil: Move enum AVSubtitleType

[Anton Khirnov]

Quoting Soft Works (2021-12-03 10:34:29)
> 
> 
> 
> At which place should FF_API_OLD_SUBTITLES get defined then (set to 1 for 
> now)?

I see you figured that out sucessfully in the new version :)

> > > +enum AVSubtitleType {
> > > +
> > > +/**
> > > + * Subtitle format unknown.
> > > + */
> > > +AV_SUBTITLE_FMT_NONE = -1,
> > > +
> > > +/**
> > > + * Subtitle format unknown.
> > > + */
> > > +AV_SUBTITLE_FMT_UNKNOWN = 0,
> > > +SUBTITLE_NONE = 0,  ///< Deprecated, use AV_SUBTITLE_FMT_NONE
> > instead.
> > 
> > This looks suspicious. Are you sure it's not going to break existing
> > code?
> 
> Yes I am. In the legacy API, SUBTITLE_NONE actually had a meaning like 
> "unspecified", "unknown" or "not set" (obviously - having a value of int 0).
> The ***_NONE values of the regular APIs though (e.g. AV_PIXFMT_NONE) 
> have a rather obscure semantic - sometimes being used as array sentinels, 
> sometimes for tristate sef/unset logic etc.

I don't think it's obscure and don't even see the difference --
AV_{PIX,SAMPLE}_FMT_NONE means exactly unspecified/unknown/not set. It's
the equivalent of the null pointer. And exactly like the null pointer,
it can be used as an array terminator.

> 
> SUBTITLE_NONE was nothing like that. It's the logical equivalent to 
> AV_SUBTITLE_FMT_UNKNOWN, despite the naming confusion.

"Nothing like that"? It sounds like exactly the same thing. Also note
that the doxygen for both in your patch says "subtitle format unknown".

> 
> I should also add that the actual define SUBTITLE_NONE has been used at a 
> single
> place in all ffmpeg code only 
> (https://github.com/FFmpeg/FFmpeg/search?q=SUBTITLE_NONE).

And as far as I can tell, it's used for empty subtitle blocks that do
not get output to the user. We do not need a special format for that.

> 
> The naming of the new constants is chosen to follow the API for audio and 
> video
> (AV_SAMPLE_FMT_, AV_PIXFMT_). Actually I tried to establish equality to the
> other two wherever possible, even when something could have been "simplified",
> but I think that uniformity ("know one, know the other") is a much higher 
> value
> from an API perspective than saving two or three functions and a few 
> parameters 
> here and there.

Coherence with the other format APIs is a worthy goal, but seems to me
you could also achieve that by dropping AV_SUBTITLE_FMT_UNKNOWN
completely. What is it actually needed for?

> 
> 
> > > +/**
> > > + * Text Formatted as per ASS specification, contained
> > AVSubtitleRect.ass.
> > > + */
> > > +AV_SUBTITLE_FMT_ASS = 3,
> > > +SUBTITLE_ASS = 3,   ///< Deprecated, use AV_SUBTITLE_FMT_ASS
> > instead.
> > > +
> > > +AV_SUBTITLE_FMT_NB,
> > 
> > The use of this field should be documented. If it's a part of public
> > API, you are preventing any new types from being added without an
> > API/ABI break.
> 
> I guess you mean this text?
> 
> AV_PIX_FMT_NB ///< number of pixel formats, DO NOT USE THIS if 
> you want to link with shared libav* because the number of formats might 
> differ between versions

Yes -- if you add new values to the enum then the value of
AV_SUBTITLE_FMT_NB would increase, breaking binary compatibility with
any callers that used 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 v20 01/20] avcodec, avutil: Move enum AVSubtitleType to avutil, add new and deprecate old values

[Anton Khirnov]

Quoting Soft Works (2021-12-05 17:23:32)
> +
> +enum AVSubtitleType {
> +
> +/**
> + * Subtitle format unknown.
> + */
> +AV_SUBTITLE_FMT_NONE = -1,
> +
> +/**
> + * Subtitle format unknown.
> + */
> +AV_SUBTITLE_FMT_UNKNOWN = 0,
> +#ifdef FF_API_OLD_SUBTITLES

These should be #if, not #ifdef

-- 
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 v20 02/20] avutil/frame: Prepare AVFrame for subtitle handling

[Anton Khirnov]

Quoting Lynne (2021-12-05 22:30:59)
> 5 Dec 2021, 20:21 by softwo...@hotmail.com:
> > One of the reasons why this subtitle filtering patchset has proceeded
> > rather smoothly so far, causing only a small amount of regressions 
> > that were easy to find and fix, is that I have kept as much as possible
> > from the current AVSubtitle implementation, projecting it to the 
> > new frame-based subtitle implementation, while keeping it mostly 
> > compatible with the current logic.
> > Interestingly, an earlier attempt towards subtitle filtering has
> > gotten stuck right at the point about storing subtitle data in 
> > AVFrame buffers. I decided to focus on functionality rather than
> > academic kind of details. What has been in place and working
> > for such a long time can't be so wrong that it can't be accepted.
> >
> > We also need to consider that it's not my patchset that is deliberately
> > deciding to store data in pointers associate with rects/area and 
> > to store start- and end-times in milliseconds: it's all the decoders
> > and encoders that are doing this. I'm just adapting to the status
> > quo.
> >
> > These two things - changing the timebase of the ms-fields and 
> > changing the location of the buffer pointers - is a gigantic task 
> > that would require to change everything that this patchset covers 
> > and implements - plus even more code that this patchset hasn't even 
> > touched. It would require changes to everything everywhere where
> > subtitles are concerned.
> > It is a task that is risky as hell, will cause plenty of 
> > regressions under  guarantee, require extensive testing and might even 
> > drive this whole endeavor into a dead end. 
> >
> > As I said before, please understand that I'm not ready to enter that
> > path.
> >
> 
> You don't have to break anything to make the start/end times proper
> timestamps. We have timebases for that! If you'd like your timestamps
> to have millisecond precision, you use a millisecond timebase!
> 
> So what if we use ASS internally which rounds timestamps? We have
> bitmap formats too, and those are generally following the video
> timestamps. You absolutely don't want the timestamps being rounded
> in that case, or you risk having your subtitles be a frame late or
> a frame early.
> 
> You could add a separate timebase for the subtitle start/end fields,
> in case the packet's timebase doesn't match the subtitle format's
> native timebase. I won't object to that.
> 
> You also don't have to risk that much by removing the subtitle_pts
> field. Once the start/end times are converted to proper timestamps,
> then the AVFrame->pts field can do whatever it wants to, whether
> it mirrors the demuxer's timestamp, be constant, non-existent or even
> go backwards.
> 
> If you just want to have an empty subtitle rect, you can simply
> zero the subtitle struct's data and buffer fields.
> 
> Calling these issues too academic when the frame API is so very
> straightforward and cut-and-paste is like calling computers
> too academic when you have several thousand large numbers to add.
> 
> I'd really rather have a subtitle API that's usable, maintainable and easy
> to extend, even if it's momentarily broken for certain corner cases
> and weird files.
> We can easily fix broken files one by one. We've got plenty of heuristics,
> and have mostly abstracted and documented them well. We can add more.
> We cannot remove cruft from an API that's designed to be crufty, however,
> because the cruft is the only thing holding it together.

+1

This set is a very significant change that will likely stay with us for
many years, if not decades. So it is of utmost importance that the API
design is clean and well-defined, because changing it later will be
extremely hard.

Keeping all the edge cases working flawlessly is IMO secondary here.
It would be nice of course, but they can always be fixed later. The API
cannot be, at least not without major pain.

-- 
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] IMF demuxer ping

[Pierre-Anthony Lemieux]

On Mon, Dec 6, 2021 at 2:16 AM Anton Khirnov  wrote:
>
> Quoting Lynne (2021-12-05 11:13:17)
> > 5 Dec 2021, 02:33 by p...@sandflow.com:
> >
> > > Hi all,
> > >
> > > Quick ping re: libavformat/imf demuxer patch set.
> > >
> > > All outstanding feedback (thanks!) has been addressed as far as I know.
> > >
> > >  What are the next steps?
> > >
> > > It would be good to make progress while it is fresh in peoples' minds.
> > >
> > > Latest patchset at [1] and tracking PR at [2] (including memory leak 
> > > checking):
> > >
> > > [1] http://ffmpeg.org/pipermail/ffmpeg-devel/2021-November/288173.html
> > > [2] https://github.com/sandflow/ffmpeg-imf/pull/74
> > >
> > > Looking forward to (finally) adding support for IMF demuxing to ffmpeg.
> > >
> >
> > There are still many issues.
> > Put your custom header beneath the standard ffmpeg header. Or
> > remove it altogether if you're allowed to.
> > You're not following our code style at all:
> > All internal functions (except intra-file functions) must have
> > an ff_ prefix. If it's a struct, the prefix is FF.
>
> actually we do not require any prefixing for internal structs, since
> they are not visible in the ABI.

Ok. Let me know if I should revert the struct names in imf.h from
"FFIMF*" to "IMF*", e.g. "FFIMFBaseResource" to "FFIMFBaseResource".

>
> --
> 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 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] IMF demuxer ping

[Lynne]

6 Dec 2021, 16:58 by p...@sandflow.com:

> On Mon, Dec 6, 2021 at 2:16 AM Anton Khirnov  wrote:
>
>>
>> Quoting Lynne (2021-12-05 11:13:17)
>> > 5 Dec 2021, 02:33 by p...@sandflow.com:
>> >
>> > > Hi all,
>> > >
>> > > Quick ping re: libavformat/imf demuxer patch set.
>> > >
>> > > All outstanding feedback (thanks!) has been addressed as far as I know.
>> > >
>> > >  What are the next steps?
>> > >
>> > > It would be good to make progress while it is fresh in peoples' minds.
>> > >
>> > > Latest patchset at [1] and tracking PR at [2] (including memory leak 
>> > > checking):
>> > >
>> > > [1] http://ffmpeg.org/pipermail/ffmpeg-devel/2021-November/288173.html
>> > > [2] https://github.com/sandflow/ffmpeg-imf/pull/74
>> > >
>> > > Looking forward to (finally) adding support for IMF demuxing to ffmpeg.
>> > >
>> >
>> > There are still many issues.
>> > Put your custom header beneath the standard ffmpeg header. Or
>> > remove it altogether if you're allowed to.
>> > You're not following our code style at all:
>> > All internal functions (except intra-file functions) must have
>> > an ff_ prefix. If it's a struct, the prefix is FF.
>>
>> actually we do not require any prefixing for internal structs, since
>> they are not visible in the ABI.
>>
>
> Ok. Let me know if I should revert the struct names in imf.h from
> "FFIMF*" to "IMF*", e.g. "FFIMFBaseResource" to "FFIMFBaseResource".
>

Keep the prefix. We don't require it, but it looks better, it's a convention,
and I think we ought to require it.

> Do you mean `-ignore_editlist`?
> An IMF Composition consists of a standalone playlist (the 
> CompositionPlaylist) that
> references MXF files (the Track Files). It is notpossible to process the 
> Track Files without the
> Composition Playlist,i.e. the edit list.What scenario(s) do you have in mind?

In that case, it's fine, as long as the demuxer actually supports all
of this and all demuxers are expected to perform this.

___
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] IMF demuxer ping

[Pierre-Anthony Lemieux]

On Mon, Dec 6, 2021 at 8:02 AM Lynne  wrote:
>
> 6 Dec 2021, 16:58 by p...@sandflow.com:
>
> > On Mon, Dec 6, 2021 at 2:16 AM Anton Khirnov  wrote:
> >
> >>
> >> Quoting Lynne (2021-12-05 11:13:17)
> >> > 5 Dec 2021, 02:33 by p...@sandflow.com:
> >> >
> >> > > Hi all,
> >> > >
> >> > > Quick ping re: libavformat/imf demuxer patch set.
> >> > >
> >> > > All outstanding feedback (thanks!) has been addressed as far as I know.
> >> > >
> >> > >  What are the next steps?
> >> > >
> >> > > It would be good to make progress while it is fresh in peoples' minds.
> >> > >
> >> > > Latest patchset at [1] and tracking PR at [2] (including memory leak 
> >> > > checking):
> >> > >
> >> > > [1] http://ffmpeg.org/pipermail/ffmpeg-devel/2021-November/288173.html
> >> > > [2] https://github.com/sandflow/ffmpeg-imf/pull/74
> >> > >
> >> > > Looking forward to (finally) adding support for IMF demuxing to ffmpeg.
> >> > >
> >> >
> >> > There are still many issues.
> >> > Put your custom header beneath the standard ffmpeg header. Or
> >> > remove it altogether if you're allowed to.
> >> > You're not following our code style at all:
> >> > All internal functions (except intra-file functions) must have
> >> > an ff_ prefix. If it's a struct, the prefix is FF.
> >>
> >> actually we do not require any prefixing for internal structs, since
> >> they are not visible in the ABI.
> >>
> >
> > Ok. Let me know if I should revert the struct names in imf.h from
> > "FFIMF*" to "IMF*", e.g. "FFIMFBaseResource" to "FFIMFBaseResource".
> >
>
> Keep the prefix. We don't require it, but it looks better, it's a convention,
> and I think we ought to require it.
>
> > Do you mean `-ignore_editlist`?
> > An IMF Composition consists of a standalone playlist (the 
> > CompositionPlaylist) that
> > references MXF files (the Track Files). It is notpossible to process the 
> > Track Files without the
> > Composition Playlist,i.e. the edit list.What scenario(s) do you have in 
> > mind?
>
> In that case, it's fine, as long as the demuxer actually supports all
> of this and all demuxers are expected to perform this.

Yes. 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".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 1/2] avcodec/dvdsub: Don't dump images to disk based on DEBUG define

[Soft Works]

> -Original Message-
> From: ffmpeg-devel  On Behalf Of Anton
> Khirnov
> Sent: Monday, December 6, 2021 9:33 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/dvdsub: Don't dump images to
> disk based on DEBUG define
> 
> Quoting Soft Works (2021-11-29 21:17:32)
> > It's been a regular annoyance.
> > Introduce a debug-only parameter for this.
> >
> > Signed-off-by: softworkz 
> > ---
> >  libavcodec/dvdsubdec.c | 9 +++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> Does anyone actually find that code useful? Apparently it's been there
> since 2005, and our standards for what is acceptable have changed
> somewhat.
> 
> I would just drop it.`

I think that it can be useful at times to dump the individual subtitle
images to disk. My primary point is that this shouldn't happen based
on whether ffmpeg is compiled with DEBUG or not.

Actually that's functionality that can be implemented in a new subtitle 
filter, which would be an ultimate justification to drop this from 
dvdsub and dvbsub.

Maybe we can just merge these two patches as an interim solution?

Thanks,
softworkz

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

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

Re: [FFmpeg-devel] [PATCH v18 01/19] avcodec, avutil: Move enum AVSubtitleType

[Soft Works]

> -Original Message-
> From: ffmpeg-devel  On Behalf Of Anton
> Khirnov
> Sent: Monday, December 6, 2021 4:17 PM
> To: FFmpeg development discussions and patches 
> Subject: Re: [FFmpeg-devel] [PATCH v18 01/19] avcodec, avutil: Move enum
> AVSubtitleType
> 
> Quoting Soft Works (2021-12-03 11:25:08)
> >
> >
> > > -Original Message-
> > > From: ffmpeg-devel  On Behalf Of Anton
> > > Khirnov
> > > Sent: Wednesday, December 1, 2021 2:35 PM
> > > To: ffmpeg-devel@ffmpeg.org
> > > Subject: Re: [FFmpeg-devel] [PATCH v18 01/19] avcodec, avutil: Move enum
> > > AVSubtitleType
> > >
> > > Quoting Soft Works (2021-11-29 20:47:52)
> > > > Signed-off-by: softworkz 
> > > > ---
> > > >  libavcodec/avcodec.h | 19 +--
> > > >  libavutil/subfmt.h   | 58 
> > > >  2 files changed, 59 insertions(+), 18 deletions(-)
> > > >  create mode 100644 libavutil/subfmt.h
> > >
> > > This commit does way more than just moving the enum, it also
> > > deprecates existing enum values and adds new ones. That should be
> > > mentioned in the commit message and in doc/APIchanges.
> >
> > This is something I would need some help with. Which of my commits should
> > bump versions in which libs and where major or minor bump?
> 
> If you expect the whole set to be pushed at once, it is enough to have a
> separate patch at the end that bumps the versions of all the affected
> libraries and adds the necessary APIchanges entries.

I'm confused, previously I was told that patches should be atomic and I 
thought that APIchanges entries should be made in the same patchset.

Should this be a separate commit then, that only does APIchanges entries
and version bumps?

> > Here's what I _think_ (possibly wrong):
> > It's based on the V18 patchset
> >
> >
> > 01/19 avcodec,avutil: Move enum AVSubtitleType to avutil, add new and
> >
> > - major bump in avcodec because AVSubtitleType is removed
> > - major bump in avutil because AVSubtitleType is added
> 
> Enums exist only in the headers, they are not exported by the binaries
> in the way functions are. So moving an enum declaration from avcodec.h
> to lavu does not actually break compatibility as long as avcodec.h
> includes the new header (and thus provides the type to any legacy
> programs).
> 
> So this requires only a minor bump in libavutil for adding a new header
> and new enum values. A major bump would be required for changes that
> break existing callers, adding new declarations does not break
> anyything.

Sure. Will change that.

> >
> > 02/19 avutil/frame: Prepare AVFrame for subtitle handling
> >
> > - major bump in avutil because AVFrame struct is modified
> >   (not just adding to the end)
> 
> Why not just move your changes to the end?
> 
> You seem not to realize that a major bump is a big deal. We typically do
> it not more than once per years, and there was one already this spring.
> You should avoid API/ABI-breaking changes if possible, and it is very
> much possible here.

The position of the subtitle fields doesn't matter a lot. I thought
that the AVMediaType field would be too elementary to put it at the
end. But on the other hand: If that would be the only change causing 
a major bump, then it will surely make sense to add it to the end
(it could still be moved later when another major bump happens).

> > 03/19 avcodec/subtitles: Introduce new frame-based subtitle decoding
> >
> > - minor bump in avcodec. New APIs are added, existing API remains
> functional
> >
> > 04/19 avfilter/subtitles: Update vf_subtitles to use new decoding api
> > (no change)
> >
> > 05/19 avcodec,avutil: Move ass helper functions to avutil as avpriv_
> >
> > - major bump in avcodec and avutil
> >   no public APIs are changed but each lib depends on the other to be
> updated
> 
> A minor bump in libavutil. libavcodec must be used with libavutil from
> the same commit or newer.

OK, will change.

> > 06/19 avcodec/subtitles: Migrate subtitle encoders to frame-based API
> > (no change)
> > 07/19 fftools/play,probe: Adjust for subtitle changes
> > (no change)
> > 08/19 avfilter/subtitles: Add subtitles.c for subtitle frame allocation
> > (no change)
> >
> > 09/19 avfilter/avfilter: Handle subtitle frames
> >
> > - minor bump in avfilter due to additions to the end of AVFilter struct.
> 
> No bump - the changes are in a non-public part of that struct.

Understood.

So I make the changes, combine them into a single additional commit at the
end, right?

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

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

Re: [FFmpeg-devel] [PATCH v20 02/20] avutil/frame: Prepare AVFrame for subtitle handling

[Soft Works]

> -Original Message-
> From: ffmpeg-devel  On Behalf Of Anton
> Khirnov
> Sent: Monday, December 6, 2021 4:52 PM
> To: FFmpeg development discussions and patches 
> Subject: Re: [FFmpeg-devel] [PATCH v20 02/20] avutil/frame: Prepare AVFrame
> for subtitle handling
> 
> Quoting Lynne (2021-12-05 22:30:59)
> > 5 Dec 2021, 20:21 by softwo...@hotmail.com:
> > > One of the reasons why this subtitle filtering patchset has proceeded
> > > rather smoothly so far, causing only a small amount of regressions
> > > that were easy to find and fix, is that I have kept as much as possible
> > > from the current AVSubtitle implementation, projecting it to the
> > > new frame-based subtitle implementation, while keeping it mostly
> > > compatible with the current logic.
> > > Interestingly, an earlier attempt towards subtitle filtering has
> > > gotten stuck right at the point about storing subtitle data in
> > > AVFrame buffers. I decided to focus on functionality rather than
> > > academic kind of details. What has been in place and working
> > > for such a long time can't be so wrong that it can't be accepted.
> > >
> > > We also need to consider that it's not my patchset that is deliberately
> > > deciding to store data in pointers associate with rects/area and
> > > to store start- and end-times in milliseconds: it's all the decoders
> > > and encoders that are doing this. I'm just adapting to the status
> > > quo.
> > >
> > > These two things - changing the timebase of the ms-fields and
> > > changing the location of the buffer pointers - is a gigantic task
> > > that would require to change everything that this patchset covers
> > > and implements - plus even more code that this patchset hasn't even
> > > touched. It would require changes to everything everywhere where
> > > subtitles are concerned.
> > > It is a task that is risky as hell, will cause plenty of
> > > regressions under  guarantee, require extensive testing and might even
> > > drive this whole endeavor into a dead end.
> > >
> > > As I said before, please understand that I'm not ready to enter that
> > > path.
> > >
> >
> > You don't have to break anything to make the start/end times proper
> > timestamps. We have timebases for that! If you'd like your timestamps
> > to have millisecond precision, you use a millisecond timebase!
> >
> > So what if we use ASS internally which rounds timestamps? We have
> > bitmap formats too, and those are generally following the video
> > timestamps. You absolutely don't want the timestamps being rounded
> > in that case, or you risk having your subtitles be a frame late or
> > a frame early.
> >
> > You could add a separate timebase for the subtitle start/end fields,
> > in case the packet's timebase doesn't match the subtitle format's
> > native timebase. I won't object to that.
> >
> > You also don't have to risk that much by removing the subtitle_pts
> > field. Once the start/end times are converted to proper timestamps,
> > then the AVFrame->pts field can do whatever it wants to, whether
> > it mirrors the demuxer's timestamp, be constant, non-existent or even
> > go backwards.
> >
> > If you just want to have an empty subtitle rect, you can simply
> > zero the subtitle struct's data and buffer fields.
> >
> > Calling these issues too academic when the frame API is so very
> > straightforward and cut-and-paste is like calling computers
> > too academic when you have several thousand large numbers to add.
> >
> > I'd really rather have a subtitle API that's usable, maintainable and easy
> > to extend, even if it's momentarily broken for certain corner cases
> > and weird files.
> > We can easily fix broken files one by one. We've got plenty of heuristics,
> > and have mostly abstracted and documented them well. We can add more.
> > We cannot remove cruft from an API that's designed to be crufty, however,
> > because the cruft is the only thing holding it together.
> 
> +1
> 
> This set is a very significant change that will likely stay with us for
> many years, if not decades. So it is of utmost importance that the API
> design is clean and well-defined, because changing it later will be
> extremely hard.
> 
> Keeping all the edge cases working flawlessly is IMO secondary here.
> It would be nice of course, but they can always be fixed later. The API
> cannot be, at least not without major pain.

>From the perspective of that logic, the problem is that almost all cases 
are "edge cases".

Kind regards,
softworkz



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

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

Re: [FFmpeg-devel] [PATCH v7 1/2] avformat/imf: Demuxer

[Lynne]

6 Dec 2021, 04:18 by p...@sandflow.com:

> From: Pierre-Anthony Lemieux 
> +
> +/**
> + * Implements IMP CPL processing
> + *
> + * @author Pierre-Anthony Lemieux
> + * @file
> + * @ingroup lavu_imf
> + */
> +
> +#include "imf.h"
> +#include "libavformat/mxf.h"
> +#include "libavutil/bprint.h"
> +#include "libavutil/error.h"
> +#include 
> +
> +xmlNodePtr ff_xml_get_child_element_by_name(xmlNodePtr parent, const char 
> *name_utf8)
> +{
> +xmlNodePtr cur_element;
> +
> +cur_element = xmlFirstElementChild(parent);
> +while (cur_element) {
> +if (xmlStrcmp(cur_element->name, name_utf8) == 0)
> +return cur_element;
> +cur_element = xmlNextElementSibling(cur_element);
> +}
> +return NULL;
> +}
> +
> +int ff_xml_read_UUID(xmlNodePtr element, uint8_t uuid[16])
> +{
> +xmlChar *element_text = NULL;
> +int scanf_ret;
> +int ret = 0;
> +
> +element_text = xmlNodeListGetString(element->doc, 
> element->xmlChildrenNode, 1);
> +scanf_ret = sscanf(element_text,
> +FF_UUID_FORMAT,
> +&uuid[0],
> +&uuid[1],
> +&uuid[2],
> +&uuid[3],
> +&uuid[4],
> +&uuid[5],
> +&uuid[6],
> +&uuid[7],
> +&uuid[8],
> +&uuid[9],
> +&uuid[10],
> +&uuid[11],
> +&uuid[12],
> +&uuid[13],
> +&uuid[14],
> +&uuid[15]);
> +if (scanf_ret != 16) {
> +av_log(NULL, AV_LOG_ERROR, "Invalid UUID\n");
> +ret = AVERROR_INVALIDDATA;
> +}
> +xmlFree(element_text);
> +
> +return ret;
> +}
>

Please don't NIH-parse UUIDs. They're more complicated
than this. Use libuuid, it's stable, bug-free and universally
available. I submitted a patch for it last month for an
unrelated use, which I decided to drop, but you
can see how it was done there.

Also, please follow the code style. We use snake_case
for function names, which means there are no capital
letters in them.
There are also still one or two one-line statements
left that are wrapped in brackets.

We check every single allocation we make, since malloc()
can fail on Windows. So that means all av_strdup calls,
av_append_path_component calls and so on can
return NULL, and result in a nasty crash.

Do not use strtok. It's not thread-safe at all. Use av_strtok,
which is thread-safe.

ff_imf_cpl_free() has a weird branch that can just be
done with a return if cpl is NULL.

There are places where you call av_free() and then
set the pointer to NULL. We have a convenience
function for this, av_freep().

Please use defined-length integers instead of "unsigned long".
If something needs to be 64-bits, use (u)int64_t. If something
doesn't need to be 64-bits, use uint32_t for unsigned, or plain
int for signed. We always require plain ints to be 32-bits.

Also, instead of printing 64-bit ints via "%lu", use "%"PRIu64"
and "%"PRIi64". MSVC has problems with %lu.
___
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 v20 02/20] avutil/frame: Prepare AVFrame for subtitle handling

[Soft Works]

> -Original Message-
> From: ffmpeg-devel  On Behalf Of Nicolas
> George
> Sent: Monday, December 6, 2021 2:43 PM
> To: FFmpeg development discussions and patches 
> Subject: Re: [FFmpeg-devel] [PATCH v20 02/20] avutil/frame: Prepare AVFrame
> for subtitle handling

[..]

> Regards,
> --
>   Nicolas George

I have stripped those points that don't appear to be worth responding to.

If you have an example for a scenario that isn't working with my patchset
(and I'm sure such cases can be found), then I'll look into it and try to 
address it.

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

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

Re: [FFmpeg-devel] fftools/ffmpeg_optc AVDictionary **opts, If memory allocation fails,

[Andreas Rheinhardt]

Yy:
> 
> 
>> 2021年12月3日 下午8:06，Yy  写道：
>>
>>
>>
>>> 2021年12月3日 下午5:42，Andreas Rheinhardt >> > 写道：
>>>
>>> Yu Yang:
 Opts is assigned by setup_find_stream_info_opts(). Opts may be NULL.
 This situation is compatible in avformat_find_stream_info(). 
 Before av_dict_free(), the necessary checks were ignored.

 // in fftools/ffmpeg_opt.c:1266
 1067 static int open_input_file(OptionsContext *o, const char *filename)
 1068 {
 ...
 1191 AVDictionary **opts = setup_find_stream_info_opts(ic, 
 o->g->codec_opts);
 ...
 1196 ret = avformat_find_stream_info(ic, opts);
 1197 
 1198 for (i = 0; i < orig_nb_streams; i++)
 1199 av_dict_free(&opts[i]);
 ...
 1342 }
 ```
 ```c
 // in libavutil/dict.c:203
 203 void av_dict_free(AVDictionary **pm)
 204 {
 205 AVDictionary *m = *pm;
 ...
 215 }

 coredump backtrace info:
 ==6235==ERROR: AddressSanitizer: SEGV on unknown address 0x 
 (pc 0x06ba9c2f bp 0x7ffc3d5baa30 sp 0x7ffc3d5ba9a0 T0)
 ==6235==The signal is caused by a READ memory access.
 ==6235==Hint: address points to the zero page.
   #0 0x6ba9c2f in av_dict_free 
 /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/libavutil/dict.c:205:23
   #1 0x4ce5ac in open_input_file 
 /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg_opt.c:1199:13
   #2 0x4c9dc0 in open_files 
 /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg_opt.c:3338:15
   #3 0x4c9295 in ffmpeg_parse_options 
 /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg_opt.c:3378:11
   #4 0x58f241 in main 
 /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:4988:11
   #5 0x7fe35197f0b2 in __libc_start_main 
 /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16
   #6 0x42033d in _start (/home/r1/ffmpeg/ffmpeg_4.4.1+0x42033d)

 Reported-by: TOTE Robot 
 Signed-off-by: Yu Yang 
 ---
 fftools/ffmpeg_opt.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

 diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
 index a27263b879..a9fc54d948 100644
 --- a/fftools/ffmpeg_opt.c
 +++ b/fftools/ffmpeg_opt.c
 @@ -1197,10 +1197,11 @@ static int open_input_file(OptionsContext *o, 
 const char *filename)
/* If not enough info to get the stream parameters, we decode the
   first frames to get it. (used in mpeg case for example) */
ret = avformat_find_stream_info(ic, opts);
 -
 -for (i = 0; i < orig_nb_streams; i++)
 -av_dict_free(&opts[i]);
 -av_freep(&opts);
 +if (opts){
 +for (i = 0; i < orig_nb_streams; i++)
 +av_dict_free(&opts[i]);
 +av_freep(&opts);
 +}

if (ret < 0) {
av_log(NULL, AV_LOG_FATAL, "%s: could not find codec 
 parameters\n", filename);

>>>
>>> You should instead check setup_find_stream_info_opts() (either only call
>>> it if orig_nb_streams is > 0 or modify it to return an error code given
>>> that it can currently return NULL even on nonerror).
>> Thanks for your advice, i will try to fix it again.
>>> - Andreas
> Hi, I try to fix this problem lastday and it not work. Today, I refer to your 
> suggestion 
> above to modify. But I think may be you misunderstood this bug. Because of no 
> alloc memory for stream opts, ‘&opts[I]’ free caused crash.  And if 
> orig_nb_streams == 0, 
> crash won’t happen. So I think it is unnecessary to fix it call 
> setup_find_stream_info_opts()
> If orig_nb_streams is > 0. Opts == NULL when  orig_nb_streams == 0 or no 
> alloc memory. 
> I don’t think it is error. Because avformat_find_stream_info() allow opts == 
> NULL. 
> But we need to check if opts == NULL before releasing. May be this fix is 
> right?
> How do you think?   
> Thx.
> -Yu Yang

We don't need to check for opts == NULL before releasing if we actually
checked that this array has been properly allocated in case it needs to
be allocated; in case it could not be allocated, we should not call
avformat_find_stream_info() at all.

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

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

Re: [FFmpeg-devel] [PATCH 4/7] avformat/4xm: Consider max_streams on reallocating tracks array

[Andreas Rheinhardt]

Michael Niedermayer:
> Fixes: OOM
> Fixes: 
> 41595/clusterfuzz-testcase-minimized-ffmpeg_dem_FOURXM_fuzzer-6355979363549184
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavformat/4xm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/4xm.c b/libavformat/4xm.c
> index f918b1fc572..5cbae7053d8 100644
> --- a/libavformat/4xm.c
> +++ b/libavformat/4xm.c
> @@ -137,7 +137,8 @@ static int parse_strk(AVFormatContext *s,
>  return AVERROR_INVALIDDATA;
>  
>  track = AV_RL32(buf + 8);
> -if ((unsigned)track >= UINT_MAX / sizeof(AudioTrack) - 1) {
> +if ((unsigned)track >= UINT_MAX / sizeof(AudioTrack) - 1 ||
> +s->max_streams && track >= s->max_streams) {
>  av_log(s, AV_LOG_ERROR, "current_track too large\n");
>  return AVERROR_INVALIDDATA;
>  }
> 

Why do you treat s->max_streams == 0 as "no limit on the number of
streams"? Neither the documentation nor avformat_new_stream() treat it
as such.
I think a better way to fix this is to stop allocating based upon track
and rather allocate based upon the actual number of tracks seen (so
AudioTrack needs to have a track field, but the stream_index field could
be dropped).
Also notice that this demuxer currently doesn't check that the track ids
encountered are unique. As a result, there can be multiple AVStreams
with the same id, only the last of which will return packets.
Another way to limit allocations of this demuxer is to not read the
whole file header into memory.

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

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

[FFmpeg-devel] [PATCH] avutil/frame: Add av_frame_transfer_side_data() function

[Anton Khirnov]

From: Soft Works 

Signed-off-by: softworkz 
Signed-off-by: Anton Khirnov 
---
 doc/APIchanges  |  4 +++
 libavutil/frame.c   | 63 ++---
 libavutil/frame.h   | 20 ++
 libavutil/version.h |  4 +--
 4 files changed, 63 insertions(+), 28 deletions(-)

- renamed the function to av_frame_transfer_side_data(), since actually
  copying side data is only one of the two possible modes of behavior
  (and not even the default one)
- renamed flags accordingly
- added the AV_FRAME_TRANSFER_SD_FILTER flags as discussed

diff --git a/doc/APIchanges b/doc/APIchanges
index 2914ad6734..79cfea00b8 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -14,6 +14,10 @@ libavutil: 2021-04-27
 
 API changes, most recent first:
 
+2021-12-02 - xx - lavu 57.11.100 - frame.h
+  Add av_frame_transfer_side_data(), AV_FRAME_TRANSFER_SD_COPY, and
+  AV_FRAME_TRANSFER_SD_FILTER.
+
 2021-11-xx - xx - lavfi 8.19.100 - avfilter.h
   Add AVFILTER_FLAG_METADATA_ONLY.
 
diff --git a/libavutil/frame.c b/libavutil/frame.c
index 0912ad9131..0b087cc4c9 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -253,9 +253,40 @@ int av_frame_get_buffer(AVFrame *frame, int align)
 return AVERROR(EINVAL);
 }
 
+int av_frame_transfer_side_data(AVFrame *dst, const AVFrame *src, int flags)
+{
+for (unsigned i = 0; i < src->nb_side_data; i++) {
+const AVFrameSideData *sd_src = src->side_data[i];
+AVFrameSideData *sd_dst;
+if ((flags & AV_FRAME_TRANSFER_SD_FILTER) &&
+sd_src->type == AV_FRAME_DATA_PANSCAN  &&
+(src->width != dst->width || src->height != dst->height))
+continue;
+if (flags & AV_FRAME_TRANSFER_SD_COPY) {
+sd_dst = av_frame_new_side_data(dst, sd_src->type,
+sd_src->size);
+if (!sd_dst) {
+wipe_side_data(dst);
+return AVERROR(ENOMEM);
+}
+memcpy(sd_dst->data, sd_src->data, sd_src->size);
+} else {
+AVBufferRef *ref = av_buffer_ref(sd_src->buf);
+sd_dst = av_frame_new_side_data_from_buf(dst, sd_src->type, ref);
+if (!sd_dst) {
+av_buffer_unref(&ref);
+wipe_side_data(dst);
+return AVERROR(ENOMEM);
+}
+}
+av_dict_copy(&sd_dst->metadata, sd_src->metadata, 0);
+}
+return 0;
+}
+
 static int frame_copy_props(AVFrame *dst, const AVFrame *src, int force_copy)
 {
-int ret, i;
+int ret;
 
 dst->key_frame  = src->key_frame;
 dst->pict_type  = src->pict_type;
@@ -291,31 +322,11 @@ static int frame_copy_props(AVFrame *dst, const AVFrame 
*src, int force_copy)
 
 av_dict_copy(&dst->metadata, src->metadata, 0);
 
-for (i = 0; i < src->nb_side_data; i++) {
-const AVFrameSideData *sd_src = src->side_data[i];
-AVFrameSideData *sd_dst;
-if (   sd_src->type == AV_FRAME_DATA_PANSCAN
-&& (src->width != dst->width || src->height != dst->height))
-continue;
-if (force_copy) {
-sd_dst = av_frame_new_side_data(dst, sd_src->type,
-sd_src->size);
-if (!sd_dst) {
-wipe_side_data(dst);
-return AVERROR(ENOMEM);
-}
-memcpy(sd_dst->data, sd_src->data, sd_src->size);
-} else {
-AVBufferRef *ref = av_buffer_ref(sd_src->buf);
-sd_dst = av_frame_new_side_data_from_buf(dst, sd_src->type, ref);
-if (!sd_dst) {
-av_buffer_unref(&ref);
-wipe_side_data(dst);
-return AVERROR(ENOMEM);
-}
-}
-av_dict_copy(&sd_dst->metadata, sd_src->metadata, 0);
-}
+ret = av_frame_transfer_side_data(dst, src,
+(force_copy ? AV_FRAME_TRANSFER_SD_COPY : 0) |
+AV_FRAME_TRANSFER_SD_FILTER);
+if (ret < 0)
+return ret;
 
 ret = av_buffer_replace(&dst->opaque_ref, src->opaque_ref);
 ret |= av_buffer_replace(&dst->private_ref, src->private_ref);
diff --git a/libavutil/frame.h b/libavutil/frame.h
index 3f295f6b9e..deb399f1da 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -873,6 +873,26 @@ AVFrameSideData *av_frame_get_side_data(const AVFrame 
*frame,
  */
 void av_frame_remove_side_data(AVFrame *frame, enum AVFrameSideDataType type);
 
+/**
+ * Copy side data, rather than creating new references.
+ */
+#define AV_FRAME_TRANSFER_SD_COPY  (1 << 0)
+/**
+ * Filter out side data that does not match dst properties.
+ */
+#define AV_FRAME_TRANSFER_SD_FILTER(1 << 1)
+
+/**
+ * Transfer all side-data from src to dst.
+ *
+ * @param flags a combination of AV_FRAME_TRANSFER_SD_*
+ *
+ * @return >= 0 on success, a negative AVERROR on error.
+ *
+ * @note This function will create new references to side data buf

Re: [FFmpeg-devel] [PATCH] avutil/frame: Add av_frame_transfer_side_data() function

[Soft Works]

> -Original Message-
> From: ffmpeg-devel  On Behalf Of Anton
> Khirnov
> Sent: Monday, December 6, 2021 7:38 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH] avutil/frame: Add
> av_frame_transfer_side_data() function
> 
> From: Soft Works 
> 
> Signed-off-by: softworkz 
> Signed-off-by: Anton Khirnov 
> ---
>  doc/APIchanges  |  4 +++
>  libavutil/frame.c   | 63 ++---
>  libavutil/frame.h   | 20 ++
>  libavutil/version.h |  4 +--
>  4 files changed, 63 insertions(+), 28 deletions(-)
> 
> - renamed the function to av_frame_transfer_side_data(), since actually
>   copying side data is only one of the two possible modes of behavior
>   (and not even the default one)
> - renamed flags accordingly
> - added the AV_FRAME_TRANSFER_SD_FILTER flags as discussed
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 2914ad6734..79cfea00b8 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -14,6 +14,10 @@ libavutil: 2021-04-27
> 
>  API changes, most recent first:
> 
> +2021-12-02 - xx - lavu 57.11.100 - frame.h
> +  Add av_frame_transfer_side_data(), AV_FRAME_TRANSFER_SD_COPY, and
> +  AV_FRAME_TRANSFER_SD_FILTER.
> +
>  2021-11-xx - xx - lavfi 8.19.100 - avfilter.h
>Add AVFILTER_FLAG_METADATA_ONLY.
> 
> diff --git a/libavutil/frame.c b/libavutil/frame.c
> index 0912ad9131..0b087cc4c9 100644
> --- a/libavutil/frame.c
> +++ b/libavutil/frame.c
> @@ -253,9 +253,40 @@ int av_frame_get_buffer(AVFrame *frame, int align)
>  return AVERROR(EINVAL);
>  }
> 
> +int av_frame_transfer_side_data(AVFrame *dst, const AVFrame *src, int flags)
> +{
> +for (unsigned i = 0; i < src->nb_side_data; i++) {
> +const AVFrameSideData *sd_src = src->side_data[i];
> +AVFrameSideData *sd_dst;
> +if ((flags & AV_FRAME_TRANSFER_SD_FILTER) &&
> +sd_src->type == AV_FRAME_DATA_PANSCAN  &&
> +(src->width != dst->width || src->height != dst->height))
> +continue;
> +if (flags & AV_FRAME_TRANSFER_SD_COPY) {
> +sd_dst = av_frame_new_side_data(dst, sd_src->type,
> +sd_src->size);
> +if (!sd_dst) {
> +wipe_side_data(dst);
> +return AVERROR(ENOMEM);
> +}
> +memcpy(sd_dst->data, sd_src->data, sd_src->size);
> +} else {
> +AVBufferRef *ref = av_buffer_ref(sd_src->buf);
> +sd_dst = av_frame_new_side_data_from_buf(dst, sd_src->type,
> ref);
> +if (!sd_dst) {
> +av_buffer_unref(&ref);
> +wipe_side_data(dst);
> +return AVERROR(ENOMEM);
> +}
> +}
> +av_dict_copy(&sd_dst->metadata, sd_src->metadata, 0);
> +}
> +return 0;
> +}
> +
>  static int frame_copy_props(AVFrame *dst, const AVFrame *src, int
> force_copy)
>  {
> -int ret, i;
> +int ret;
> 
>  dst->key_frame  = src->key_frame;
>  dst->pict_type  = src->pict_type;
> @@ -291,31 +322,11 @@ static int frame_copy_props(AVFrame *dst, const AVFrame
> *src, int force_copy)
> 
>  av_dict_copy(&dst->metadata, src->metadata, 0);
> 
> -for (i = 0; i < src->nb_side_data; i++) {
> -const AVFrameSideData *sd_src = src->side_data[i];
> -AVFrameSideData *sd_dst;
> -if (   sd_src->type == AV_FRAME_DATA_PANSCAN
> -&& (src->width != dst->width || src->height != dst->height))
> -continue;
> -if (force_copy) {
> -sd_dst = av_frame_new_side_data(dst, sd_src->type,
> -sd_src->size);
> -if (!sd_dst) {
> -wipe_side_data(dst);
> -return AVERROR(ENOMEM);
> -}
> -memcpy(sd_dst->data, sd_src->data, sd_src->size);
> -} else {
> -AVBufferRef *ref = av_buffer_ref(sd_src->buf);
> -sd_dst = av_frame_new_side_data_from_buf(dst, sd_src->type,
> ref);
> -if (!sd_dst) {
> -av_buffer_unref(&ref);
> -wipe_side_data(dst);
> -return AVERROR(ENOMEM);
> -}
> -}
> -av_dict_copy(&sd_dst->metadata, sd_src->metadata, 0);
> -}
> +ret = av_frame_transfer_side_data(dst, src,
> +(force_copy ? AV_FRAME_TRANSFER_SD_COPY : 0) |
> +AV_FRAME_TRANSFER_SD_FILTER);
> +if (ret < 0)
> +return ret;
> 
>  ret = av_buffer_replace(&dst->opaque_ref, src->opaque_ref);
>  ret |= av_buffer_replace(&dst->private_ref, src->private_ref);
> diff --git a/libavutil/frame.h b/libavutil/frame.h
> index 3f295f6b9e..deb399f1da 100644
> --- a/libavutil/frame.h
> +++ b/libavutil/frame.h
> @@ -873,6 +873,26 @@ AVFrameSideData *av_frame_get_side_data(const AVFrame
> *frame,
>   */
>  void av_frame_remove_side_data(AVFrame *frame, enum AVFrameSideDataT

Re: [FFmpeg-devel] [PATCH] avutil/frame: Add av_frame_transfer_side_data() function

[Lynne]

6 Dec 2021, 19:37 by an...@khirnov.net:

> From: Soft Works 
>
> Signed-off-by: softworkz 
> Signed-off-by: Anton Khirnov 
> ---
>  doc/APIchanges  |  4 +++
>  libavutil/frame.c   | 63 ++---
>  libavutil/frame.h   | 20 ++
>  libavutil/version.h |  4 +--
>  4 files changed, 63 insertions(+), 28 deletions(-)
>
> - renamed the function to av_frame_transfer_side_data(), since actually
>  copying side data is only one of the two possible modes of behavior
>  (and not even the default one)
> - renamed flags accordingly
> - added the AV_FRAME_TRANSFER_SD_FILTER flags as discussed
>
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 2914ad6734..79cfea00b8 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -14,6 +14,10 @@ libavutil: 2021-04-27
>  
>  API changes, most recent first:
>  
> +2021-12-02 - xx - lavu 57.11.100 - frame.h
> +  Add av_frame_transfer_side_data(), AV_FRAME_TRANSFER_SD_COPY, and
> +  AV_FRAME_TRANSFER_SD_FILTER.
> +
>  2021-11-xx - xx - lavfi 8.19.100 - avfilter.h
>  Add AVFILTER_FLAG_METADATA_ONLY.
>  
> diff --git a/libavutil/frame.c b/libavutil/frame.c
> index 0912ad9131..0b087cc4c9 100644
> --- a/libavutil/frame.c
> +++ b/libavutil/frame.c
> @@ -253,9 +253,40 @@ int av_frame_get_buffer(AVFrame *frame, int align)
>  return AVERROR(EINVAL);
>  }
>  
> +int av_frame_transfer_side_data(AVFrame *dst, const AVFrame *src, int flags)
> +{
> +for (unsigned i = 0; i < src->nb_side_data; i++) {
> +const AVFrameSideData *sd_src = src->side_data[i];
> +AVFrameSideData *sd_dst;
> +if ((flags & AV_FRAME_TRANSFER_SD_FILTER) &&
> +sd_src->type == AV_FRAME_DATA_PANSCAN  &&
> +(src->width != dst->width || src->height != dst->height))
> +continue;
> +if (flags & AV_FRAME_TRANSFER_SD_COPY) {
> +sd_dst = av_frame_new_side_data(dst, sd_src->type,
> +sd_src->size);
> +if (!sd_dst) {
> +wipe_side_data(dst);
> +return AVERROR(ENOMEM);
> +}
> +memcpy(sd_dst->data, sd_src->data, sd_src->size);
> +} else {
> +AVBufferRef *ref = av_buffer_ref(sd_src->buf);
> +sd_dst = av_frame_new_side_data_from_buf(dst, sd_src->type, ref);
> +if (!sd_dst) {
> +av_buffer_unref(&ref);
> +wipe_side_data(dst);
> +return AVERROR(ENOMEM);
> +}
> +}
> +av_dict_copy(&sd_dst->metadata, sd_src->metadata, 0);
> +}
> +return 0;
> +}
> +
>  static int frame_copy_props(AVFrame *dst, const AVFrame *src, int force_copy)
>  {
> -int ret, i;
> +int ret;
>  
>  dst->key_frame  = src->key_frame;
>  dst->pict_type  = src->pict_type;
> @@ -291,31 +322,11 @@ static int frame_copy_props(AVFrame *dst, const AVFrame 
> *src, int force_copy)
>  
>  av_dict_copy(&dst->metadata, src->metadata, 0);
>  
> -for (i = 0; i < src->nb_side_data; i++) {
> -const AVFrameSideData *sd_src = src->side_data[i];
> -AVFrameSideData *sd_dst;
> -if (   sd_src->type == AV_FRAME_DATA_PANSCAN
> -&& (src->width != dst->width || src->height != dst->height))
> -continue;
> -if (force_copy) {
> -sd_dst = av_frame_new_side_data(dst, sd_src->type,
> -sd_src->size);
> -if (!sd_dst) {
> -wipe_side_data(dst);
> -return AVERROR(ENOMEM);
> -}
> -memcpy(sd_dst->data, sd_src->data, sd_src->size);
> -} else {
> -AVBufferRef *ref = av_buffer_ref(sd_src->buf);
> -sd_dst = av_frame_new_side_data_from_buf(dst, sd_src->type, ref);
> -if (!sd_dst) {
> -av_buffer_unref(&ref);
> -wipe_side_data(dst);
> -return AVERROR(ENOMEM);
> -}
> -}
> -av_dict_copy(&sd_dst->metadata, sd_src->metadata, 0);
> -}
> +ret = av_frame_transfer_side_data(dst, src,
> +(force_copy ? AV_FRAME_TRANSFER_SD_COPY : 0) |
> +AV_FRAME_TRANSFER_SD_FILTER);
> +if (ret < 0)
> +return ret;
>  
>  ret = av_buffer_replace(&dst->opaque_ref, src->opaque_ref);
>  ret |= av_buffer_replace(&dst->private_ref, src->private_ref);
> diff --git a/libavutil/frame.h b/libavutil/frame.h
> index 3f295f6b9e..deb399f1da 100644
> --- a/libavutil/frame.h
> +++ b/libavutil/frame.h
> @@ -873,6 +873,26 @@ AVFrameSideData *av_frame_get_side_data(const AVFrame 
> *frame,
>  */
>  void av_frame_remove_side_data(AVFrame *frame, enum AVFrameSideDataType 
> type);
>  
> +/**
> + * Copy side data, rather than creating new references.
> + */
> +#define AV_FRAME_TRANSFER_SD_COPY  (1 << 0)
> +/**
> + * Filter out side data that does not match dst properties.
> + */
> +#define AV_FRAM

Re: [FFmpeg-devel] [PATCH v7 1/2] avformat/imf: Demuxer

[Andreas Rheinhardt]

p...@sandflow.com:
> From: Pierre-Anthony Lemieux 
> 
> Signed-off-by: Pierre-Anthony Lemieux 
> ---
> 
> Notes:
> The IMF demuxer accepts as input an IMF CPL. The assets referenced by the 
> CPL can be
> contained in multiple deliveries, each defined by an ASSETMAP file:
> 
> ffmpeg -assetmaps ,,... -i  CPL>
> 
> If -assetmaps is not specified, FFMPEG looks for a file called 
> ASSETMAP.xml in the same directory as the CPL.
> 
> EXAMPLE:
> ffmpeg -i 
> http://ffmpeg-imf-samples-public.s3-website-us-west-1.amazonaws.com/countdown/CPL_f5095caa-f204-4e1c-8a84-7af48c7ae16b.xml
>  out.mp4
> 
> The Interoperable Master Format (IMF) is a file-based media format for the
> delivery and storage of professional audio-visual masters.
> An IMF Composition consists of an XML playlist (the Composition Playlist)
> and a collection of MXF files (the Track Files). The Composition Playlist 
> (CPL)
> assembles the Track Files onto a timeline, which consists of multiple 
> tracks.
> The location of the Track Files referenced by the Composition Playlist is 
> stored
> in one or more XML documents called Asset Maps. More details at 
> https://www.imfug.com/explainer.
> The IMF standard was first introduced in 2013 and is managed by the SMPTE.
> 
> CHANGE NOTES:
> 
> - reduced line width
> - use ff_ and FF prefixes for non-local functions and structures
> - modified copyright header
> - fixed rational initialization
> - removed extraneous call to xmlCleanupParser()
> - fix if/for single line braces
> - replace av_realloc_f with av_fast_realloc when allocating CPL Resources 
> and
>   ASSETMAP assets
> 
>  MAINTAINERS  |   1 +
>  configure|   3 +-
>  doc/demuxers.texi|  19 +
>  libavformat/Makefile |   1 +
>  libavformat/allformats.c |   1 +
>  libavformat/imf.h| 206 +
>  libavformat/imf_cpl.c| 769 +
>  libavformat/imfdec.c | 907 +++
>  8 files changed, 1906 insertions(+), 1 deletion(-)
>  create mode 100644 libavformat/imf.h
>  create mode 100644 libavformat/imf_cpl.c
>  create mode 100644 libavformat/imfdec.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index dcac46003e..7a6972fe1a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -433,6 +433,7 @@ Muxers/Demuxers:
>idroqdec.cMike Melanson
>iff.c Jaikrishnan Menon
>img2*.c   Michael Niedermayer
> +  imf*.cMarc-Antoine Arnaud, Pierre-Anthony 
> Lemieux, Valentin Noël
>ipmovie.c Mike Melanson
>ircam*Paul B Mahol
>iss.c Stefan Gehrer
> diff --git a/configure b/configure
> index a98a18abaa..022f17767b 100755
> --- a/configure
> +++ b/configure
> @@ -298,7 +298,7 @@ External library support:
>--enable-libxvid enable Xvid encoding via xvidcore,
> native MPEG-4/Xvid encoder exists [no]
>--enable-libxml2 enable XML parsing using the C library libxml2, 
> needed
> -   for dash demuxing support [no]
> +   for dash and imf demuxing support [no]
>--enable-libzimg enable z.lib, needed for zscale filter [no]
>--enable-libzmq  enable message passing via libzmq [no]
>--enable-libzvbi enable teletext support via libzvbi [no]
> @@ -3400,6 +3400,7 @@ hls_muxer_select="mpegts_muxer"
>  hls_muxer_suggest="gcrypt openssl"
>  image2_alias_pix_demuxer_select="image2_demuxer"
>  image2_brender_pix_demuxer_select="image2_demuxer"
> +imf_demuxer_deps="libxml2"
>  ipod_muxer_select="mov_muxer"
>  ismv_muxer_select="mov_muxer"
>  ivf_muxer_select="av1_metadata_bsf vp9_superframe_bsf"
> diff --git a/doc/demuxers.texi b/doc/demuxers.texi
> index cab8a7072c..614f2e754a 100644
> --- a/doc/demuxers.texi
> +++ b/doc/demuxers.texi
> @@ -267,6 +267,12 @@ which streams to actually receive.
>  Each stream mirrors the @code{id} and @code{bandwidth} properties from the
>  @code{} as metadata keys named "id" and "variant_bitrate" 
> respectively.
>  
> +@section imf
> +
> +Interoperable Master Format demuxer.
> +
> +This demuxer presents audio and video streams found in an IMF Composition.
> +
>  @section flv, live_flv, kux
>  
>  Adobe Flash Video Format demuxer.
> @@ -770,6 +776,19 @@ MJPEG stream. Turning this option on by setting it to 1 
> will result in a stricte
>  of the boundary value.
>  @end table
>  
> +@section mxf
> +
> +MXF demuxer.
> +
> +@table @option
> +
> +@item -eia608_extract @var{bool}
> +Extract EIA-608 captions from SMPTE 436M track
> +
> +@item -skip_audio_reordering @var{bool}
> +This option will disable the audio reordering based on Multi-Channel Audio 
> (MCA) labelling (SMPT

Re: [FFmpeg-devel] [PATCH 1/2] fftools/ffmpeg_opt: Improve checks for truncation/alloc error

[Andreas Rheinhardt]

Andreas Rheinhardt:
> Do this by switching from the dynamic buffer API to the AVBPrint API;
> the former has no defined way to check for errors.
> This also avoids allocating an AVIOContext.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  fftools/ffmpeg_opt.c | 22 ++
>  1 file changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> index 6c2eb53290..78b5574a3d 100644
> --- a/fftools/ffmpeg_opt.c
> +++ b/fftools/ffmpeg_opt.c
> @@ -34,6 +34,7 @@
>  #include "libavutil/avassert.h"
>  #include "libavutil/avstring.h"
>  #include "libavutil/avutil.h"
> +#include "libavutil/bprint.h"
>  #include "libavutil/channel_layout.h"
>  #include "libavutil/intreadwrite.h"
>  #include "libavutil/fifo.h"
> @@ -1649,29 +1650,26 @@ static void parse_matrix_coeffs(uint16_t *dest, const 
> char *str)
>  }
>  
>  /* read file contents into a string */
> -static uint8_t *read_file(const char *filename)
> +static char *read_file(const char *filename)
>  {
>  AVIOContext *pb  = NULL;
> -AVIOContext *dyn_buf = NULL;
>  int ret = avio_open(&pb, filename, AVIO_FLAG_READ);
> -uint8_t buf[1024], *str;
> +AVBPrint bprint;
> +char *str;
>  
>  if (ret < 0) {
>  av_log(NULL, AV_LOG_ERROR, "Error opening file %s.\n", filename);
>  return NULL;
>  }
>  
> -ret = avio_open_dyn_buf(&dyn_buf);
> +av_bprint_init(&bprint, 0, AV_BPRINT_SIZE_UNLIMITED);
> +ret = avio_read_to_bprint(pb, &bprint, SIZE_MAX);
> +avio_closep(&pb);
>  if (ret < 0) {
> -avio_closep(&pb);
> +av_bprint_finalize(&bprint, NULL);
>  return NULL;
>  }
> -while ((ret = avio_read(pb, buf, sizeof(buf))) > 0)
> -avio_write(dyn_buf, buf, ret);
> -avio_w8(dyn_buf, 0);
> -avio_closep(&pb);
> -
> -ret = avio_close_dyn_buf(dyn_buf, &str);
> +ret = av_bprint_finalize(&bprint, &str);
>  if (ret < 0)
>  return NULL;
>  return str;
> @@ -3279,7 +3277,7 @@ static int opt_filter_complex(void *optctx, const char 
> *opt, const char *arg)
>  
>  static int opt_filter_complex_script(void *optctx, const char *opt, const 
> char *arg)
>  {
> -uint8_t *graph_desc = read_file(arg);
> +char *graph_desc = read_file(arg);
>  if (!graph_desc)
>  return AVERROR(EINVAL);
>  
> 
Will apply this patchset tomorrow unless there are objections.

- Andreas

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

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

Re: [FFmpeg-devel] [PATCH 4/7] avformat/4xm: Consider max_streams on reallocating tracks array

[Michael Niedermayer]

On Mon, Dec 06, 2021 at 07:01:24PM +0100, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > Fixes: OOM
> > Fixes: 
> > 41595/clusterfuzz-testcase-minimized-ffmpeg_dem_FOURXM_fuzzer-6355979363549184
> > 
> > Found-by: continuous fuzzing process 
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavformat/4xm.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libavformat/4xm.c b/libavformat/4xm.c
> > index f918b1fc572..5cbae7053d8 100644
> > --- a/libavformat/4xm.c
> > +++ b/libavformat/4xm.c
> > @@ -137,7 +137,8 @@ static int parse_strk(AVFormatContext *s,
> >  return AVERROR_INVALIDDATA;
> >  
> >  track = AV_RL32(buf + 8);
> > -if ((unsigned)track >= UINT_MAX / sizeof(AudioTrack) - 1) {
> > +if ((unsigned)track >= UINT_MAX / sizeof(AudioTrack) - 1 ||
> > +s->max_streams && track >= s->max_streams) {
> >  av_log(s, AV_LOG_ERROR, "current_track too large\n");
> >  return AVERROR_INVALIDDATA;
> >  }
> > 
> 
> Why do you treat s->max_streams == 0 as "no limit on the number of
> streams"? Neither the documentation nor avformat_new_stream() treat it
> as such.

0 was used as no limit in other cases, i did not check nor remembered that
AVFormatContext.max_streams doesnt have 0 as a documented exception
i will remove the 0 check


> I think a better way to fix this is to stop allocating based upon track
> and rather allocate based upon the actual number of tracks seen (so
> AudioTrack needs to have a track field, but the stream_index field could
> be dropped).

> Also notice that this demuxer currently doesn't check that the track ids

will add that


> encountered are unique. As a result, there can be multiple AVStreams
> with the same id, only the last of which will return packets.

> Another way to limit allocations of this demuxer is to not read the
> whole file header into memory.

Most of these files have 1 stream and an id of 0. I have found 1 file 
which has 7 streams, with id 0,1,2,3,4,5,6 in that order IIRC

so it feels a bit overkillish to remap this around, but i surely see
your argument behind this

thx

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[FFmpeg-devel] python filter

[Alex]

Is it possible to implement python backend as ffmpeg video filter, like 
https://openvinotoolkit.github.io/dlstreamer_gst/ ?
___
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] python filter

[Steven Liu]

Alex <3.1...@ukr.net> 于2021年12月7日周二 上午9:12写道：
>
> Is it possible to implement python backend as ffmpeg video filter, like 
> https://openvinotoolkit.github.io/dlstreamer_gst/ ?
Do you mean add a Python parser into libavfilter looks like a GLSL parser?
> ___
> 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] fftools/ffmpeg_optc AVDictionary **opts, If memory allocation fails,

[Yy]

> 2021年12月7日 上午1:48，Andreas Rheinhardt  写道：
> 
> Yy:
>> 
>> 
>>> 2021年12月3日 下午8:06，Yy  写道：
>>> 
>>> 
>>> 
 2021年12月3日 下午5:42，Andreas Rheinhardt >>> > 写道：
 
 Yu Yang:
> Opts is assigned by setup_find_stream_info_opts(). Opts may be NULL.
> This situation is compatible in avformat_find_stream_info(). 
> Before av_dict_free(), the necessary checks were ignored.
> 
> // in fftools/ffmpeg_opt.c:1266
> 1067 static int open_input_file(OptionsContext *o, const char *filename)
> 1068 {
>...
> 1191 AVDictionary **opts = setup_find_stream_info_opts(ic, 
> o->g->codec_opts);
>...
> 1196 ret = avformat_find_stream_info(ic, opts);
> 1197 
> 1198 for (i = 0; i < orig_nb_streams; i++)
> 1199 av_dict_free(&opts[i]);
>...
> 1342 }
> ```
> ```c
> // in libavutil/dict.c:203
> 203 void av_dict_free(AVDictionary **pm)
> 204 {
> 205 AVDictionary *m = *pm;
>...
> 215 }
> 
> coredump backtrace info:
> ==6235==ERROR: AddressSanitizer: SEGV on unknown address 0x 
> (pc 0x06ba9c2f bp 0x7ffc3d5baa30 sp 0x7ffc3d5ba9a0 T0)
> ==6235==The signal is caused by a READ memory access.
> ==6235==Hint: address points to the zero page.
>  #0 0x6ba9c2f in av_dict_free 
> /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/libavutil/dict.c:205:23
>  #1 0x4ce5ac in open_input_file 
> /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg_opt.c:1199:13
>  #2 0x4c9dc0 in open_files 
> /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg_opt.c:3338:15
>  #3 0x4c9295 in ffmpeg_parse_options 
> /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg_opt.c:3378:11
>  #4 0x58f241 in main 
> /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:4988:11
>  #5 0x7fe35197f0b2 in __libc_start_main 
> /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16
>  #6 0x42033d in _start (/home/r1/ffmpeg/ffmpeg_4.4.1+0x42033d)
> 
> Reported-by: TOTE Robot 
> Signed-off-by: Yu Yang 
> ---
> fftools/ffmpeg_opt.c | 9 +
> 1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> index a27263b879..a9fc54d948 100644
> --- a/fftools/ffmpeg_opt.c
> +++ b/fftools/ffmpeg_opt.c
> @@ -1197,10 +1197,11 @@ static int open_input_file(OptionsContext *o, 
> const char *filename)
>   /* If not enough info to get the stream parameters, we decode the
>  first frames to get it. (used in mpeg case for example) */
>   ret = avformat_find_stream_info(ic, opts);
> -
> -for (i = 0; i < orig_nb_streams; i++)
> -av_dict_free(&opts[i]);
> -av_freep(&opts);
> +if (opts){
> +for (i = 0; i < orig_nb_streams; i++)
> +av_dict_free(&opts[i]);
> +av_freep(&opts);
> +}
> 
>   if (ret < 0) {
>   av_log(NULL, AV_LOG_FATAL, "%s: could not find codec 
> parameters\n", filename);
> 
 
 You should instead check setup_find_stream_info_opts() (either only call
 it if orig_nb_streams is > 0 or modify it to return an error code given
 that it can currently return NULL even on nonerror).
>>> Thanks for your advice, i will try to fix it again.
 - Andreas
>> Hi, I try to fix this problem lastday and it not work. Today, I refer to 
>> your suggestion 
>> above to modify. But I think may be you misunderstood this bug. Because of 
>> no alloc memory for stream opts, ‘&opts[I]’ free caused crash.  And if 
>> orig_nb_streams == 0, 
>> crash won’t happen. So I think it is unnecessary to fix it call 
>> setup_find_stream_info_opts()
>> If orig_nb_streams is > 0. Opts == NULL when  orig_nb_streams == 0 or no 
>> alloc memory. 
>> I don’t think it is error. Because avformat_find_stream_info() allow opts == 
>> NULL. 
>> But we need to check if opts == NULL before releasing. May be this fix is 
>> right?
>> How do you think?   
>> Thx.
>> -Yu Yang
> 
> We don't need to check for opts == NULL before releasing if we actually
> checked that this array has been properly allocated in case it needs to
> be allocated; in case it could not be allocated, we should not call
> avformat_find_stream_info() at all.
I got it now. You mean it is redundant to check opts twice. It should 
return error when it could not be allocated at first time.  I will resubmit a 
new patch today. 
Thanks for your answer.  : )  
> 
> - 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

[FFmpeg-devel] [PATCHv2] fate: add audio tests for Silicon Graphics Movie format

[Peter Ross]

Signed-off-by: Peter Ross 
---
Modified to use codec copy instead of aresample
Samples attached to: 
https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2021-December/288976.html

 tests/fate/audio.mak  | 12 
 tests/ref/fate/mv-mono16bit   |  8 
 tests/ref/fate/mv-mono8bit|  8 
 tests/ref/fate/mv-stereo16bit |  8 
 tests/ref/fate/mv-stereo8bit  |  8 
 5 files changed, 44 insertions(+)
 create mode 100644 tests/ref/fate/mv-mono16bit
 create mode 100644 tests/ref/fate/mv-mono8bit
 create mode 100644 tests/ref/fate/mv-stereo16bit
 create mode 100644 tests/ref/fate/mv-stereo8bit

diff --git a/tests/fate/audio.mak b/tests/fate/audio.mak
index fd9905ca0a..1aa5e80bb9 100644
--- a/tests/fate/audio.mak
+++ b/tests/fate/audio.mak
@@ -38,6 +38,18 @@ fate-imc: CMD = pcm -i $(TARGET_SAMPLES)/imc/imc.avi
 fate-imc: CMP = oneoff
 fate-imc: REF = $(SAMPLES)/imc/imc-201706.pcm
 
+FATE_SAMPLES_AUDIO-$(call DEMDEC, MV, PCM_S8) += fate-mv-mono8bit
+fate-mv-mono8bit: CMD = framecrc -i 
$(TARGET_SAMPLES)/mv/mono8bit-minimal.movie -vn -acodec copy
+
+FATE_SAMPLES_AUDIO-$(call DEMDEC, MV, PCM_S16BE) += fate-mv-mono16bit
+fate-mv-mono16bit: CMD = framecrc -i 
$(TARGET_SAMPLES)/mv/mono16bit-minimal.movie -vn -acodec copy
+
+FATE_SAMPLES_AUDIO-$(call DEMDEC, MV, PCM_S8) += fate-mv-stereo8bit
+fate-mv-stereo8bit: CMD = framecrc -i 
$(TARGET_SAMPLES)/mv/stereo8bit-minimal.movie -vn -acodec copy
+
+FATE_SAMPLES_AUDIO-$(call DEMDEC, MV, PCM_S16BE) += fate-mv-stereo16bit
+fate-mv-stereo16bit: CMD = framecrc -i 
$(TARGET_SAMPLES)/mv/stereo16bit-minimal.movie -vn -acodec copy
+
 FATE_SAMPLES_AUDIO-$(call DEMDEC, FLV, NELLYMOSER) += fate-nellymoser
 fate-nellymoser: CMD = pcm -i $(TARGET_SAMPLES)/nellymoser/nellymoser.flv
 fate-nellymoser: CMP = oneoff
diff --git a/tests/ref/fate/mv-mono16bit b/tests/ref/fate/mv-mono16bit
new file mode 100644
index 00..a5c46fd3f2
--- /dev/null
+++ b/tests/ref/fate/mv-mono16bit
@@ -0,0 +1,8 @@
+#tb 0: 1/44100
+#media_type 0: audio
+#codec_id 0: pcm_s16be
+#sample_rate 0: 44100
+#channel_layout 0: 4
+#channel_layout_name 0: mono
+0,  0,  0, 4410, 8820, 0xc48232c3
+0,   4410,   4410, 4410, 8820, 0x47fd4d37
diff --git a/tests/ref/fate/mv-mono8bit b/tests/ref/fate/mv-mono8bit
new file mode 100644
index 00..1fb9a5
--- /dev/null
+++ b/tests/ref/fate/mv-mono8bit
@@ -0,0 +1,8 @@
+#tb 0: 1/44100
+#media_type 0: audio
+#codec_id 0: pcm_s8
+#sample_rate 0: 44100
+#channel_layout 0: 4
+#channel_layout_name 0: mono
+0,  0,  0, 4410, 4410, 0x47df392f
+0,   4410,   4410, 4410, 4410, 0x2ed6ab09
diff --git a/tests/ref/fate/mv-stereo16bit b/tests/ref/fate/mv-stereo16bit
new file mode 100644
index 00..3984acf931
--- /dev/null
+++ b/tests/ref/fate/mv-stereo16bit
@@ -0,0 +1,8 @@
+#tb 0: 1/44100
+#media_type 0: audio
+#codec_id 0: pcm_s16be
+#sample_rate 0: 44100
+#channel_layout 0: 3
+#channel_layout_name 0: stereo
+0,  0,  0, 4410,17640, 0x32a4faf5
+0,   4410,   4410, 4410,17640, 0x0617c559
diff --git a/tests/ref/fate/mv-stereo8bit b/tests/ref/fate/mv-stereo8bit
new file mode 100644
index 00..026a9a23cc
--- /dev/null
+++ b/tests/ref/fate/mv-stereo8bit
@@ -0,0 +1,8 @@
+#tb 0: 1/44100
+#media_type 0: audio
+#codec_id 0: pcm_s8
+#sample_rate 0: 44100
+#channel_layout 0: 3
+#channel_layout_name 0: stereo
+0,  0,  0, 4410, 8820, 0x39fa3ee3
+0,   4410,   4410, 4410, 8820, 0xb3885495
-- 
2.33.0

-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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