Re: [FFmpeg-devel] [PATCH 1/2] avformat/utils: Change compute_chapters_end() from O(n²) to O(n log n)

2020-12-04 Thread Anton Khirnov
Quoting Michael Niedermayer (2020-11-22 16:37:32)
> Fixes: Timeout (49sec -> 9sec)
> Fixes: 
> 27427/clusterfuzz-testcase-minimized-ffmpeg_dem_FFMETADATA_fuzzer-5140589838073856
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---

Not objecting to the patch itself, but just wondering: do we actually
want to compute chapter ends when the file doesn't store them?
I can imagine users might want to distinguish whether the end time is
explicitly specified or not.

-- 
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/2] lavc/qsv: use avpriv_ instead of ff_ as prefix for some functions

2020-12-04 Thread Anton Khirnov
Quoting Haihao Xiang (2020-11-23 09:16:13)
> ff_qsv_print_iopattern, ff_qsv_print_error and ff_qsv_print_warning can be
> used outside of lavc. In addition, ff_qsv_map_error is used in
> libavcodec/qsv.c only, so remove ff_ from ff_qsv_map_error and make it
> static.
> 
> Signed-off-by: Haihao Xiang 
> ---

New avpriv functions are strongly discouraged. Dependencies between
libavcodec and libavfilter are also not good.

Those should either be inline in a private header, or exported as proper
public API from libavutil.

-- 
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/2] lavc/qsv: use avpriv_ instead of ff_ as prefix for some functions

2020-12-04 Thread Nicolas George
Anton Khirnov (12020-12-04):
> Those should either be inline in a private header, or exported as proper
> public API from libavutil.

What a waste of energy.

And you still haven't given a single practical benefit of keeping the
libraries separate that could not be achieved more efficiently
otherwise.

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 1/4] avcodec/cdgraphics: Check frame before clearing

2020-12-04 Thread Anton Khirnov
Quoting Michael Niedermayer (2020-12-04 01:07:04)
> Fixes: null pointer dereference
> Fixes: 
> 27730/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CDGRAPHICS_fuzzer-6212402236096512
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 

Looks ok.

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

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

Re: [FFmpeg-devel] [PATCH 02/45] avcodec/a64multienc: Mark encoders as init-threadsafe

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:06)
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/a64multienc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

LGTM

-- 
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 03/45] avcodec/cdtoons: Mark decoder as init-threadsafe

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:07)
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/cdtoons.c | 1 +
>  1 file changed, 1 insertion(+)
> 

LGTM

-- 
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 04/45] avcodec/adpcm: Mark decoders as init-threadsafe

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:08)
> They don't modify any global state
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/adpcm.c | 1 +
>  1 file changed, 1 insertion(+)

Looks ok

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

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

Re: [FFmpeg-devel] [PATCH 05/45] avcodec/adpcmenc: Mark encoders as init-threadsafe

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:09)
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/adpcmenc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Looks ok

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

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

Re: [FFmpeg-devel] [PATCH 06/45] avcodec/pcm: Make encoders init-threadsafe

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:10)
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/pcm.c | 24 +++-
>  1 file changed, 15 insertions(+), 9 deletions(-)
> 

Looks ok

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

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

Re: [FFmpeg-devel] [PATCH 07/45] avcodec/pcm: Mark decoders as init-threadsafe

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:11)
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/pcm.c | 1 +
>  1 file changed, 1 insertion(+)

Looks ok

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

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

Re: [FFmpeg-devel] [PATCH 08/45] avcodec/loco: Mark decoder as init-threadsafe

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:12)
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/loco.c | 1 +
>  1 file changed, 1 insertion(+)

Looks ok

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

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

Re: [FFmpeg-devel] [PATCH 09/45] avcodec/cljrdec: Mark decoder as init-threadsafe

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:13)
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/cljrdec.c | 1 +
>  1 file changed, 1 insertion(+)

Obviously ok

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

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

Re: [FFmpeg-devel] [PATCH 10/45] avcodec/yop: Mark decoder as init-threadsafe

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:14)
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/yop.c | 1 +
>  1 file changed, 1 insertion(+)

Most likely ok

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

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

Re: [FFmpeg-devel] [PATCH 11/45] avcodec/y41pdec: Mark decoder as init-threadsafe

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:15)
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/y41pdec.c | 1 +
>  1 file changed, 1 insertion(+)
> 

Clearly ok

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

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

Re: [FFmpeg-devel] [PATCH 12/45] avcodec/y41penc: Mark encoder as init-threadsafe

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:16)
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/y41penc.c | 1 +
>  1 file changed, 1 insertion(+)
> 

Very ok

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

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

Re: [FFmpeg-devel] [PATCH 13/45] avcodec/yuv4dec: Mark decoder as init-threadsafe

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:17)
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/yuv4dec.c | 1 +
>  1 file changed, 1 insertion(+)

Much ok

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

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

Re: [FFmpeg-devel] [PATCH 14/45] avcodec/xan: Cleanup generically on init failure

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:18)
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/xan.c | 9 +++--
>  1 file changed, 3 insertions(+), 6 deletions(-)

Looks good

-- 
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 15/45] avcodec/xan: Mark decoder as init-threadsafe

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:19)
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/xan.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Looks even better.

-- 
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 16/45] avcodec/xfacedec: Mark decoder as init-threadsafe

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:20)
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/xfacedec.c | 1 +
>  1 file changed, 1 insertion(+)
> 

Ok

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

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

Re: [FFmpeg-devel] [PATCH 17/45] avcodec/xl: Mark decoder as init-threadsafe

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:21)
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/xl.c | 1 +
>  1 file changed, 1 insertion(+)

Approval

-- 
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 18/45] avcodec/xsubdec: Mark decoder as init-threadsafe

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:22)
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/xsubdec.c | 2 ++
>  1 file changed, 2 insertions(+)
> 

Good

-- 
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 19/45] avcodec/xsubenc: Mark decoder as init-threadsafe

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:23)
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/xsubenc.c | 2 ++
>  1 file changed, 2 insertions(+)

Extremely good

-- 
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 20/45] avcodec/xxan: Cleanup generically on init failure

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:24)
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/xxan.c | 9 +++--
>  1 file changed, 3 insertions(+), 6 deletions(-)

Assent

-- 
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 21/45] avcodec/xxan: Mark decoder as init-threadsafe

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:25)
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/xxan.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Doubleplusgood

-- 
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 22/45] avcodec/ws-snd1: Mark decoder as init-threadsafe

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:26)
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/ws-snd1.c | 1 +
>  1 file changed, 1 insertion(+)
> 

Clearly ok

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

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

Re: [FFmpeg-devel] [PATCH 23/45] avcodec/ulti: Mark decoder as init-threadsafe

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:27)
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/ulti.c | 1 +
>  1 file changed, 1 insertion(+)

Acceptable

-- 
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 24/45] avcodec/tmv: Mark decoder as init-threadsafe

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:28)
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/tmv.c | 1 +
>  1 file changed, 1 insertion(+)

Brilliant

-- 
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 25/45] avcodec/sgidec: Mark decoder as init-threadsafe

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:29)
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/sgidec.c | 1 +
>  1 file changed, 1 insertion(+)

Sure

-- 
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 26/45] avcodec/sgienc: Mark encoder as init-threadsafe

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:30)
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/sgienc.c | 1 +
>  1 file changed, 1 insertion(+)
> 

Good and just

-- 
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 27/45] avcodec/sgienc: Combine av_log() statements

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:31)
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/sgienc.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 

Makes sense

-- 
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 28/45] avcodec/sgirledec: Mark decoder as init-threadsafe

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:32)
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/sgirledec.c | 1 +
>  1 file changed, 1 insertion(+)
> 

Obviously ok

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

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

Re: [FFmpeg-devel] [PATCH 29/45] avcodec/shorten: Mark decoder as init-threadsafe

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:33)
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/shorten.c | 1 +
>  1 file changed, 1 insertion(+)

Ok

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

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

Re: [FFmpeg-devel] [PATCH 30/45] avcodec/sipr: Mark decoder as init-threadsafe

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:34)
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/sipr.c | 1 +
>  1 file changed, 1 insertion(+)

Looks safe indeed

-- 
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 31/45] avcodec/smc: Mark decoder as init-threadsafe

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:35)
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/smc.c | 1 +
>  1 file changed, 1 insertion(+)

Visibly ok

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

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

Re: [FFmpeg-devel] [PATCH 35/45] avcodec/mjpegdec: Fix memleak upon init failure

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:39)
> This affected all decoders that used ff_mjpeg_decode_init() as init
> function; and it also affected decoders that open jpeg decoders via
> ff_codec_open2_recursive() as well as MxPEG.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/jpeglsdec.c | 2 +-
>  libavcodec/mjpegbdec.c | 2 +-
>  libavcodec/mjpegdec.c  | 4 ++--
>  libavcodec/sp5xdec.c   | 4 ++--
>  4 files changed, 6 insertions(+), 6 deletions(-)

Looks good.

-- 
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 36/45] avcodec/mxpegdec: Fix memleaks upon init failure

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:40)
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/mjpegdec.c | 2 ++
>  libavcodec/mxpegdec.c | 6 ++
>  2 files changed, 4 insertions(+), 4 deletions(-)

This use of mjpeg by other decoders is an ugly mess, but the patch looks
ok

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

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

Re: [FFmpeg-devel] [PATCH 37/45] avcodec: Fix invalid uses of ff_codec_open2_recursive()

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:41)
> Normally no two codecs with FF_CODEC_CAP_INIT_THREADSAFE unset
> can be initialized at the same time: a mutex in avcodec_open2()
> ensures this. This implies that one cannot simply open a codec
> with a non-threadsafe init-function from the init function of
> a codec whose own init function is not threadsafe either as the child
> codec couldn't acquire the lock.
> 
> ff_codec_open2_recursive() exists to get around this limitation:
> If the init function of the child codec to be initialized is not
> thread-safe, the mutex is unlocked, the child is initialized and
> the mutex is locked again. This of course has as a prerequisite that
> the parent AVCodecContext actually holds the lock, i.e. that the
> parent codec's init function is not thread-safe. If it is, then one
> can (and has to) just use avcodec_open2() directly (if the child's
> init function is not thread-safe, then avcodec_open2() will have to
> acquire the mutex itself (and potentially wait for it), so that it is
> perfectly fine for an otherwise thread-safe init function to open
> a codec with a potentially non-thread-safe init function via
> avcodec_open2()).
> 
> Yet several of the users of ff_codec_open2_recursive() have the
> FF_CODEC_CAP_INIT_THREADSAFE flag set; this only worked because
> all the child codecs' init functions were thread-safe themselves
> so that ff_codec_open2_recursive() didn't touch the mutex at all.
> But of course the real solution to this is to directly use
> avcodec_open2().
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/avrndec.c | 2 +-
>  libavcodec/imm5.c| 4 ++--
>  libavcodec/tdsc.c| 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)

ok

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

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

Re: [FFmpeg-devel] [PATCH 38/45] avcodec/cri: Make decoder init-threadsafe

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:42)
> The only thing that stands in the way of adding the
> FF_CODEC_CAP_INIT_THREADSAFE flag to the Cintel RAW decoder is its usage
> of ff_codec_open2_recursive(): This function requires its caller to hold
> the lock for the mutex that guards initialization of AVCodecContexts
> whose codecs have a non-threadsafe init function and only callers whose
> codec does not have the FF_CODEC_CAP_INIT_THREADSAFE flag set hold said
> lock (the others don't need to care about said lock). But one can set
> the flag if one switches to avcodec_open2() at the same time.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/cri.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Looks ok

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

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

Re: [FFmpeg-devel] [PATCH 43/45] avcodec/faxcompr: Make ff_ccitt_unpack_init() thread-safe

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:47)
> This will allow to make the TIFF decoder's init function thread-safe.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/faxcompr.c | 13 -
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 

Looks ok

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

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

Re: [FFmpeg-devel] [PATCH 44/45] avcodec/tiff: Make decoder init-threadsafe

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:48)
> The only thing that stands in the way of adding the
> FF_CODEC_CAP_INIT_THREADSAFE flag to the TIFF decoder is its usage
> of ff_codec_open2_recursive(): This function requires its caller to hold
> the lock for the mutex that guards initialization of AVCodecContexts
> whose codecs have a non-threadsafe init function and only callers whose
> codec does not have the FF_CODEC_CAP_INIT_THREADSAFE flag set hold said
> lock (the others don't need to care about said lock). But one can set
> the flag if one switches to avcodec_open2() at the same time.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/tiff.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Ok

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

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

Re: [FFmpeg-devel] [PATCH 45/45] avcodec/utils: Remove ff_codec_open2_recursive()

2020-12-04 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-11-27 02:02:49)
> This function existed to enable codecs with non-threadsafe init functions
> to initialize other codecs despite the fact that normally no two codecs
> with non-threadsafe init functions can be initialized at the same time
> (there is a mutex guarding this). Yet there are no users of this
> function any more as all users have been made thread-safe (switching
> away from ff_codec_open2_recursive() was required for this as said
> function requires the caller to hold the lock to the mutex guarding the
> initializations and this is only true for codecs with the
> FF_CODEC_CAP_INIT_THREADSAFE flag unset); so remove it.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/internal.h |  7 ---
>  libavcodec/utils.c| 12 
>  2 files changed, 19 deletions(-)

Awesome!

-- 
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] Moves yuv2yuvX_sse3 to yasm, unrolls main loop and other small optimizations for ~20% speedup.

2020-12-04 Thread Anton Khirnov
Quoting Alan Kelly (2020-11-19 09:41:56)
> ---
>  All of Henrik's suggestions have been implemented. Additionally,
>  m3 and m6 are permuted in avx2 before storing to ensure bit by bit
>  identical results in avx2.
>  libswscale/x86/Makefile |   1 +
>  libswscale/x86/swscale.c|  75 +++
>  libswscale/x86/yuv2yuvX.asm | 118 
>  3 files changed, 129 insertions(+), 65 deletions(-)
>  create mode 100644 libswscale/x86/yuv2yuvX.asm

Is this function tested by FATE?
I did some brief testing and apparently it gets called during
fate-filter-shuffleplanes-dup-luma, but the results do not change even
if I comment out the whole function.

Also, it seems like you are adding an AVX2 version of the function, but
I don't see it being used.

-- 
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/2] lavc/qsv: use avpriv_ instead of ff_ as prefix for some functions

2020-12-04 Thread Xiang, Haihao
On Fri, 2020-12-04 at 11:24 +0100, Anton Khirnov wrote:
> Quoting Haihao Xiang (2020-11-23 09:16:13)
> > ff_qsv_print_iopattern, ff_qsv_print_error and ff_qsv_print_warning can be
> > used outside of lavc. In addition, ff_qsv_map_error is used in
> > libavcodec/qsv.c only, so remove ff_ from ff_qsv_map_error and make it
> > static.
> > 
> > Signed-off-by: Haihao Xiang 
> > ---
> 
> New avpriv functions are strongly discouraged. Dependencies between
> libavcodec and libavfilter are also not good.
> 
> Those should either be inline in a private header, or exported as proper
> public API from libavutil.
> 

Thanks for the comment, I will update the patch.

Regards
Haihao

___
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] lavc/qsv: use avpriv_ instead of ff_ as prefix for some functions

2020-12-04 Thread James Almer

On 12/4/2020 7:24 AM, Anton Khirnov wrote:

Quoting Haihao Xiang (2020-11-23 09:16:13)

ff_qsv_print_iopattern, ff_qsv_print_error and ff_qsv_print_warning can be
used outside of lavc. In addition, ff_qsv_map_error is used in
libavcodec/qsv.c only, so remove ff_ from ff_qsv_map_error and make it
static.

Signed-off-by: Haihao Xiang 
---


New avpriv functions are strongly discouraged. Dependencies between
libavcodec and libavfilter are also not good.

Those should either be inline in a private header, or exported as proper
public API from libavutil.


Public API, or even avpriv_ symbols that depend on external 
libraries/hardware are not a good idea and should be avoided. And these 
are all "print error", "print warning" kind of helpers that most likely 
have no use outside of libav*.


All of them look like can be inlined just fine, too, so he should do 
that instead.

___
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] avfilter/vf_hwupload_cuda: add transparent pix_fmts

2020-12-04 Thread Timo Rothenpieler

On 03.12.2020 16:56, Daniel Raniz Raneland wrote:

Signed-off-by: Daniel Raniz Raneland 
---
  libavfilter/vf_hwupload_cuda.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/libavfilter/vf_hwupload_cuda.c b/libavfilter/vf_hwupload_cuda.c
index 33906f2515..83b7babeaa 100644
--- a/libavfilter/vf_hwupload_cuda.c
+++ b/libavfilter/vf_hwupload_cuda.c
@@ -60,6 +60,7 @@ static int cudaupload_query_formats(AVFilterContext *ctx)
  AV_PIX_FMT_NV12, AV_PIX_FMT_YUV420P, AV_PIX_FMT_YUV444P,
  AV_PIX_FMT_P010, AV_PIX_FMT_P016, AV_PIX_FMT_YUV444P16,
  AV_PIX_FMT_0RGB32, AV_PIX_FMT_0BGR32,
+AV_PIX_FMT_YUVA420P, AV_PIX_FMT_YUVA444P,
  #if CONFIG_VULKAN
  AV_PIX_FMT_VULKAN,
  #endif



AV_PIX_FMT_YUVA444P is not supported by the CUDA hwcontext.
AV_PIX_FMT_YUVA420P however is.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[FFmpeg-devel] [PATCH 0/3] Initial implementation of TTML encoding/muxing

2020-12-04 Thread Jan Ekström
I've intentionally kept this initial version simple (no styling etc) to focus
on the basics. As this goes through review, additional features can be added
(I had initial PoC for styling implemented some time around previous VDD), and
there is another patch set in my queue which would then add support for muxing
TTML into MP4.

The basic issue with this specific patch set is that implementing subtitle
encoding/muxing as it is implemented with all other subtitle formats now
mis-matches the behavior of how the TTML demuxing in various formats currently
works in FFmpeg:

* The demuxers return AVPackets with full documents (as most containers'
  packets just contain a full document).
* Subtitle encoding returns single paragraph's content based on the
  AVSubtitle rectangles fed to it.

Thus codec copy remuxing from, say, MXF would not work as the demuxer pushes
out full documents, and the TTML muxer expects paragraphs' contents.

There are multiple ways of handling this, such as:
* Some sort of check based on ?!?!
** adding flags into extradata
** string comparison of packet's contents  against "https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[FFmpeg-devel] [PATCH 3/3] TTML encoder and muxer

2020-12-04 Thread Jan Ekström
From: Jan Ekström 

Enables encoding of other subtitle formats into TTML and writing
them out as such documents.

Signed-off-by: Jan Ekström 
---
 Changelog  |   1 +
 doc/general_contents.texi  |   1 +
 libavcodec/Makefile|   1 +
 libavcodec/allcodecs.c |   1 +
 libavcodec/ttmlenc.c   | 154 +
 libavcodec/version.h   |   2 +-
 libavformat/Makefile   |   1 +
 libavformat/allformats.c   |   1 +
 libavformat/ttmlenc.c  | 123 +
 libavformat/version.h  |   2 +-
 tests/fate/subtitles.mak   |   3 +
 tests/ref/fate/sub-ttmlenc | 122 +
 12 files changed, 410 insertions(+), 2 deletions(-)
 create mode 100644 libavcodec/ttmlenc.c
 create mode 100644 libavformat/ttmlenc.c
 create mode 100644 tests/ref/fate/sub-ttmlenc

diff --git a/Changelog b/Changelog
index ebb1727875..71476eb366 100644
--- a/Changelog
+++ b/Changelog
@@ -48,6 +48,7 @@ version :
 - speechnorm filter
 - SpeedHQ encoder
 - asupercut filter
+- TTML subtitle encoder and muxer
 
 
 version 4.3:
diff --git a/doc/general_contents.texi b/doc/general_contents.texi
index 1be6f9b683..dca183e9ca 100644
--- a/doc/general_contents.texi
+++ b/doc/general_contents.texi
@@ -1332,6 +1332,7 @@ performance on systems without hardware floating point 
support).
 @item SubViewer v1 @tab   @tab X @tab   @tab X
 @item SubViewer@tab   @tab X @tab   @tab X
 @item TED Talks captions @tab @tab X @tab   @tab X
+@item TTML @tab X @tab   @tab X @tab
 @item VobSub (IDX+SUB) @tab   @tab X @tab   @tab X
 @item VPlayer  @tab   @tab X @tab   @tab X
 @item WebVTT   @tab X @tab X @tab X @tab X
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index a6435c9e85..9d2b62a263 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -665,6 +665,7 @@ OBJS-$(CONFIG_TSCC_DECODER)+= tscc.o msrledec.o
 OBJS-$(CONFIG_TSCC2_DECODER)   += tscc2.o
 OBJS-$(CONFIG_TTA_DECODER) += tta.o ttadata.o ttadsp.o
 OBJS-$(CONFIG_TTA_ENCODER) += ttaenc.o ttaencdsp.o ttadata.o
+OBJS-$(CONFIG_TTML_ENCODER)+= ttmlenc.o ass_split.o
 OBJS-$(CONFIG_TWINVQ_DECODER)  += twinvqdec.o twinvq.o
 OBJS-$(CONFIG_TXD_DECODER) += txd.o
 OBJS-$(CONFIG_ULTI_DECODER)+= ulti.o
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index 774d5670bf..b12538905b 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -685,6 +685,7 @@ extern AVCodec ff_subviewer_decoder;
 extern AVCodec ff_subviewer1_decoder;
 extern AVCodec ff_text_encoder;
 extern AVCodec ff_text_decoder;
+extern AVCodec ff_ttml_encoder;
 extern AVCodec ff_vplayer_decoder;
 extern AVCodec ff_webvtt_encoder;
 extern AVCodec ff_webvtt_decoder;
diff --git a/libavcodec/ttmlenc.c b/libavcodec/ttmlenc.c
new file mode 100644
index 00..7eb89e73f4
--- /dev/null
+++ b/libavcodec/ttmlenc.c
@@ -0,0 +1,154 @@
+/*
+ * TTML subtitle encoder
+ * Copyright (c) 2020 24i
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/**
+ * @file
+ * TTML subtitle encoder
+ * @see https://www.w3.org/TR/ttml1/
+ * @see https://www.w3.org/TR/ttml2/
+ * @see https://www.w3.org/TR/ttml-imsc/rec
+ */
+
+#include "avcodec.h"
+#include "libavutil/avstring.h"
+#include "libavutil/bprint.h"
+#include "ass_split.h"
+#include "ass.h"
+
+typedef struct {
+AVCodecContext *avctx;
+ASSSplitContext *ass_ctx;
+AVBPrint buffer;
+} TTMLContext;
+
+static void ttml_text_cb(void *priv, const char *text, int len)
+{
+TTMLContext *s = priv;
+AVBPrint cur_line = { 0 };
+AVBPrint *buffer = &s->buffer;
+
+av_bprint_init(&cur_line, len, AV_BPRINT_SIZE_UNLIMITED);
+
+av_bprint_append_data(&cur_line, text, len);
+if (!av_bprint_is_complete(&cur_line)) {
+av_log(s->avctx, AV_LOG_ERROR,
+   "Failed to move the current subtitle dialog to AVBPrint!\n");
+av_bprint_finalize(&cur_line, NULL);
+return;
+}
+
+
+av_bprint_escape(buffer, cur_line.str, NULL, AV_ESCAPE_MODE_XML, 0);
+
+av_bprint_finalize(&cur_line, NULL);
+}
+
+static void ttml_new_line_cb(void *priv, int forced)
+{
+TTMLContext *s = priv;
+
+av_bprintf(&s->buffer, "");

Re: [FFmpeg-devel] [PATCH 3/3] TTML encoder and muxer

2020-12-04 Thread Paul B Mahol
AWESOME!!

On Fri, Dec 4, 2020 at 3:55 PM Jan Ekström  wrote:

> From: Jan Ekström 
>
> Enables encoding of other subtitle formats into TTML and writing
> them out as such documents.
>
> Signed-off-by: Jan Ekström 
> ---
>  Changelog  |   1 +
>  doc/general_contents.texi  |   1 +
>  libavcodec/Makefile|   1 +
>  libavcodec/allcodecs.c |   1 +
>  libavcodec/ttmlenc.c   | 154 +
>  libavcodec/version.h   |   2 +-
>  libavformat/Makefile   |   1 +
>  libavformat/allformats.c   |   1 +
>  libavformat/ttmlenc.c  | 123 +
>  libavformat/version.h  |   2 +-
>  tests/fate/subtitles.mak   |   3 +
>  tests/ref/fate/sub-ttmlenc | 122 +
>  12 files changed, 410 insertions(+), 2 deletions(-)
>  create mode 100644 libavcodec/ttmlenc.c
>  create mode 100644 libavformat/ttmlenc.c
>  create mode 100644 tests/ref/fate/sub-ttmlenc
>
> diff --git a/Changelog b/Changelog
> index ebb1727875..71476eb366 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -48,6 +48,7 @@ version :
>  - speechnorm filter
>  - SpeedHQ encoder
>  - asupercut filter
> +- TTML subtitle encoder and muxer
>
>
>  version 4.3:
> diff --git a/doc/general_contents.texi b/doc/general_contents.texi
> index 1be6f9b683..dca183e9ca 100644
> --- a/doc/general_contents.texi
> +++ b/doc/general_contents.texi
> @@ -1332,6 +1332,7 @@ performance on systems without hardware floating
> point support).
>  @item SubViewer v1 @tab   @tab X @tab   @tab X
>  @item SubViewer@tab   @tab X @tab   @tab X
>  @item TED Talks captions @tab @tab X @tab   @tab X
> +@item TTML @tab X @tab   @tab X @tab
>  @item VobSub (IDX+SUB) @tab   @tab X @tab   @tab X
>  @item VPlayer  @tab   @tab X @tab   @tab X
>  @item WebVTT   @tab X @tab X @tab X @tab X
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index a6435c9e85..9d2b62a263 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -665,6 +665,7 @@ OBJS-$(CONFIG_TSCC_DECODER)+= tscc.o
> msrledec.o
>  OBJS-$(CONFIG_TSCC2_DECODER)   += tscc2.o
>  OBJS-$(CONFIG_TTA_DECODER) += tta.o ttadata.o ttadsp.o
>  OBJS-$(CONFIG_TTA_ENCODER) += ttaenc.o ttaencdsp.o ttadata.o
> +OBJS-$(CONFIG_TTML_ENCODER)+= ttmlenc.o ass_split.o
>  OBJS-$(CONFIG_TWINVQ_DECODER)  += twinvqdec.o twinvq.o
>  OBJS-$(CONFIG_TXD_DECODER) += txd.o
>  OBJS-$(CONFIG_ULTI_DECODER)+= ulti.o
> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> index 774d5670bf..b12538905b 100644
> --- a/libavcodec/allcodecs.c
> +++ b/libavcodec/allcodecs.c
> @@ -685,6 +685,7 @@ extern AVCodec ff_subviewer_decoder;
>  extern AVCodec ff_subviewer1_decoder;
>  extern AVCodec ff_text_encoder;
>  extern AVCodec ff_text_decoder;
> +extern AVCodec ff_ttml_encoder;
>  extern AVCodec ff_vplayer_decoder;
>  extern AVCodec ff_webvtt_encoder;
>  extern AVCodec ff_webvtt_decoder;
> diff --git a/libavcodec/ttmlenc.c b/libavcodec/ttmlenc.c
> new file mode 100644
> index 00..7eb89e73f4
> --- /dev/null
> +++ b/libavcodec/ttmlenc.c
> @@ -0,0 +1,154 @@
> +/*
> + * TTML subtitle encoder
> + * Copyright (c) 2020 24i
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> 02110-1301 USA
> + */
> +
> +/**
> + * @file
> + * TTML subtitle encoder
> + * @see https://www.w3.org/TR/ttml1/
> + * @see https://www.w3.org/TR/ttml2/
> + * @see https://www.w3.org/TR/ttml-imsc/rec
> + */
> +
> +#include "avcodec.h"
> +#include "libavutil/avstring.h"
> +#include "libavutil/bprint.h"
> +#include "ass_split.h"
> +#include "ass.h"
> +
> +typedef struct {
> +AVCodecContext *avctx;
> +ASSSplitContext *ass_ctx;
> +AVBPrint buffer;
> +} TTMLContext;
> +
> +static void ttml_text_cb(void *priv, const char *text, int len)
> +{
> +TTMLContext *s = priv;
> +AVBPrint cur_line = { 0 };
> +AVBPrint *buffer = &s->buffer;
> +
> +av_bprint_init(&cur_line, len, AV_BPRINT_SIZE_UNLIMITED);
> +
> +av_bprint_append_data(&cur_line, text, len);
> +if (!av_bprint_is_complete(&cur_line)) {
> +av_log(s->avctx, AV_LOG_ERROR,
> +   "Failed to move the current subtitle dialog to

[FFmpeg-devel] [PATCH 2/3] ffprobe: switch to av_bprint_escape for XML escaping

2020-12-04 Thread Jan Ekström
From: Jan Ekström 

Signed-off-by: Jan Ekström 
---
 fftools/ffprobe.c | 29 -
 1 file changed, 8 insertions(+), 21 deletions(-)

diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
index 86bd23d36d..a127a3f9bb 100644
--- a/fftools/ffprobe.c
+++ b/fftools/ffprobe.c
@@ -1671,24 +1671,6 @@ static av_cold int xml_init(WriterContext *wctx)
 return 0;
 }
 
-static const char *xml_escape_str(AVBPrint *dst, const char *src, void 
*log_ctx)
-{
-const char *p;
-
-for (p = src; *p; p++) {
-switch (*p) {
-case '&' : av_bprintf(dst, "%s", "&");  break;
-case '<' : av_bprintf(dst, "%s", "<");   break;
-case '>' : av_bprintf(dst, "%s", ">");   break;
-case '"' : av_bprintf(dst, "%s", """); break;
-case '\'': av_bprintf(dst, "%s", "'"); break;
-default: av_bprint_chars(dst, *p, 1);
-}
-}
-
-return dst->str;
-}
-
 #define XML_INDENT() printf("%*c", xml->indent_level * 4, ' ')
 
 static void xml_print_section_header(WriterContext *wctx)
@@ -1760,14 +1742,19 @@ static void xml_print_str(WriterContext *wctx, const 
char *key, const char *valu
 
 if (section->flags & SECTION_FLAG_HAS_VARIABLE_FIELDS) {
 XML_INDENT();
+av_bprint_escape(&buf, key, NULL, AV_ESCAPE_MODE_XML, 0);
 printf("<%s key=\"%s\"",
-   section->element_name, xml_escape_str(&buf, key, wctx));
+   section->element_name, buf.str);
 av_bprint_clear(&buf);
-printf(" value=\"%s\"/>\n", xml_escape_str(&buf, value, wctx));
+
+av_bprint_escape(&buf, value, NULL, AV_ESCAPE_MODE_XML, 0);
+printf(" value=\"%s\"/>\n", buf.str);
 } else {
 if (wctx->nb_item[wctx->level])
 printf(" ");
-printf("%s=\"%s\"", key, xml_escape_str(&buf, value, wctx));
+
+av_bprint_escape(&buf, value, NULL, AV_ESCAPE_MODE_XML, 0);
+printf("%s=\"%s\"", key, buf.str);
 }
 
 av_bprint_finalize(&buf, NULL);
-- 
2.29.2

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

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

[FFmpeg-devel] [PATCH 1/3] avutil/{avstring, bprint}: add XML escaping from ffprobe to avutil

2020-12-04 Thread Jan Ekström
From: Stefano Sabatini 

---
 libavutil/avstring.h |  1 +
 libavutil/bprint.c   | 14 ++
 tools/ffescape.c |  1 +
 3 files changed, 16 insertions(+)

diff --git a/libavutil/avstring.h b/libavutil/avstring.h
index ee225585b3..79bb920a70 100644
--- a/libavutil/avstring.h
+++ b/libavutil/avstring.h
@@ -324,6 +324,7 @@ enum AVEscapeMode {
 AV_ESCAPE_MODE_AUTO,  ///< Use auto-selected escaping mode.
 AV_ESCAPE_MODE_BACKSLASH, ///< Use backslash escaping.
 AV_ESCAPE_MODE_QUOTE, ///< Use single-quote escaping.
+AV_ESCAPE_MODE_XML,   ///< Use XML non-markup character data escaping.
 };
 
 /**
diff --git a/libavutil/bprint.c b/libavutil/bprint.c
index 2f059c5ba6..d825b61b14 100644
--- a/libavutil/bprint.c
+++ b/libavutil/bprint.c
@@ -283,6 +283,20 @@ void av_bprint_escape(AVBPrint *dstbuf, const char *src, 
const char *special_cha
 av_bprint_chars(dstbuf, '\'', 1);
 break;
 
+case AV_ESCAPE_MODE_XML:
+/* escape XML non-markup character data as per 2.4 */
+for (; *src; src++) {
+switch (*src) {
+case '&' : av_bprintf(dstbuf, "%s", "&");  break;
+case '<' : av_bprintf(dstbuf, "%s", "<");   break;
+case '>' : av_bprintf(dstbuf, "%s", ">");   break;
+case '"' : av_bprintf(dstbuf, "%s", """); break;
+case '\'': av_bprintf(dstbuf, "%s", "'"); break;
+default: av_bprint_chars(dstbuf, *src, 1);
+}
+}
+break;
+
 /* case AV_ESCAPE_MODE_BACKSLASH or unknown mode */
 default:
 /* \-escape characters */
diff --git a/tools/ffescape.c b/tools/ffescape.c
index 0530d28c6d..8537235d5e 100644
--- a/tools/ffescape.c
+++ b/tools/ffescape.c
@@ -104,6 +104,7 @@ int main(int argc, char **argv)
 if  (!strcmp(optarg, "auto"))  escape_mode = 
AV_ESCAPE_MODE_AUTO;
 else if (!strcmp(optarg, "backslash")) escape_mode = 
AV_ESCAPE_MODE_BACKSLASH;
 else if (!strcmp(optarg, "quote")) escape_mode = 
AV_ESCAPE_MODE_QUOTE;
+else if (!strcmp(optarg, "xml"))   escape_mode = 
AV_ESCAPE_MODE_XML;
 else {
 av_log(NULL, AV_LOG_ERROR,
"Invalid value '%s' for option -m, "
-- 
2.29.2

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

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

[FFmpeg-devel] [PATCH] avcodec/decode: port last_pkt_props to AVFifoBuffer

2020-12-04 Thread James Almer
Signed-off-by: James Almer 
---
The idea of making avpriv_packet_list_* public was not liked, and it was
suggested to just use AVFifoBuffer instead.

After this, the avpriv_packet_list_* functions can be made local to
libavformat again.

 libavcodec/decode.c   | 41 +
 libavcodec/internal.h |  4 ++--
 libavcodec/utils.c| 11 ++-
 3 files changed, 37 insertions(+), 19 deletions(-)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 5a1849f944..0550637029 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -145,22 +145,40 @@ fail2:
 
 #define IS_EMPTY(pkt) (!(pkt)->data)
 
+static int copy_packet_props(void *_src, void *_dst, int size)
+{
+AVPacket *src = _src, *dst = _dst;
+int ret;
+
+av_init_packet(dst);
+ret = av_packet_copy_props(dst, src);
+if (ret < 0)
+return ret;
+
+dst->size = src->size; // HACK: Needed for ff_decode_frame_props().
+dst->data = (void*)1;  // HACK: Needed for IS_EMPTY().
+
+return sizeof(*dst);
+}
+
 static int extract_packet_props(AVCodecInternal *avci, AVPacket *pkt)
 {
 int ret = 0;
 
-ret = avpriv_packet_list_put(&avci->pkt_props, &avci->pkt_props_tail, pkt,
- av_packet_copy_props, 0);
+if (av_fifo_space(avci->pkt_props) < sizeof(*pkt)) {
+ret = av_fifo_grow(avci->pkt_props, sizeof(*pkt));
+if (ret < 0)
+return ret;
+}
+
+ret = av_fifo_generic_write(avci->pkt_props, pkt, sizeof(*pkt), 
copy_packet_props);
 if (ret < 0)
 return ret;
-avci->pkt_props_tail->pkt.size = pkt->size; // HACK: Needed for 
ff_decode_frame_props().
-avci->pkt_props_tail->pkt.data = (void*)1;  // HACK: Needed for IS_EMPTY().
 
+av_assert0(ret == sizeof(*pkt));
 if (IS_EMPTY(avci->last_pkt_props)) {
-ret = avpriv_packet_list_get(&avci->pkt_props,
- &avci->pkt_props_tail,
- avci->last_pkt_props);
-av_assert0(ret != AVERROR(EAGAIN));
+ret = av_fifo_generic_read(avci->pkt_props, avci->last_pkt_props,
+   sizeof(*avci->last_pkt_props), NULL);
 }
 return ret;
 }
@@ -1721,10 +1739,9 @@ int ff_decode_frame_props(AVCodecContext *avctx, AVFrame 
*frame)
 { AV_PKT_DATA_S12M_TIMECODE,  AV_FRAME_DATA_S12M_TIMECODE 
},
 };
 
-if (IS_EMPTY(pkt))
-avpriv_packet_list_get(&avctx->internal->pkt_props,
-   &avctx->internal->pkt_props_tail,
-   pkt);
+if (IS_EMPTY(pkt) && av_fifo_size(avctx->internal->pkt_props) >= 
sizeof(*pkt))
+av_fifo_generic_read(avctx->internal->pkt_props,
+ pkt, sizeof(*pkt), NULL);
 
 if (pkt) {
 frame->pts = pkt->pts;
diff --git a/libavcodec/internal.h b/libavcodec/internal.h
index 17defb9b50..8a51bca2df 100644
--- a/libavcodec/internal.h
+++ b/libavcodec/internal.h
@@ -28,6 +28,7 @@
 
 #include "libavutil/buffer.h"
 #include "libavutil/channel_layout.h"
+#include "libavutil/fifo.h"
 #include "libavutil/mathematics.h"
 #include "libavutil/pixfmt.h"
 #include "avcodec.h"
@@ -145,8 +146,7 @@ typedef struct AVCodecInternal {
  * for decoding.
  */
 AVPacket *last_pkt_props;
-AVPacketList *pkt_props;
-AVPacketList *pkt_props_tail;
+AVFifoBuffer *pkt_props;
 
 /**
  * temporary buffer used for encoders to store their bitstream
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 45295dd3ce..c81cc972dc 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -593,10 +593,12 @@ int attribute_align_arg avcodec_open2(AVCodecContext 
*avctx, const AVCodec *code
 avci->es.in_frame = av_frame_alloc();
 avci->ds.in_pkt = av_packet_alloc();
 avci->last_pkt_props = av_packet_alloc();
+avci->pkt_props = av_fifo_alloc(sizeof(AVPacket));
 if (!avci->compat_decode_frame || !avci->compat_encode_packet ||
 !avci->buffer_frame || !avci->buffer_pkt  ||
 !avci->es.in_frame  || !avci->ds.in_pkt   ||
-!avci->to_free  || !avci->last_pkt_props) {
+!avci->to_free  || !avci->last_pkt_props  ||
+!avci->pkt_props) {
 ret = AVERROR(ENOMEM);
 goto free_and_end;
 }
@@ -1076,6 +1078,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
 av_packet_free(&avci->compat_encode_packet);
 av_packet_free(&avci->buffer_pkt);
 av_packet_free(&avci->last_pkt_props);
+av_fifo_freep(&avci->pkt_props);
 
 av_packet_free(&avci->ds.in_pkt);
 av_frame_free(&avci->es.in_frame);
@@ -1116,8 +1119,7 @@ void avcodec_flush_buffers(AVCodecContext *avctx)
 av_packet_unref(avci->buffer_pkt);
 
 av_packet_unref(avci->last_pkt_props);
-avpriv_packet_list_free(&avci->pkt_props,
-&avci->pkt_props_tail);
+av_fifo_reset(avci->pkt_props);
 
 av_frame_unref(avci->es.in_frame);
 a

Re: [FFmpeg-devel] [PATCH 4/7] lavfi/vf_spp: convert to the video_enc_params API

2020-12-04 Thread Anton Khirnov
ping

Michael, do you still object to this set? If not, I am going to push 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 4/5] avformat/mpc8: Check remaining space in mpc8_parse_seektable()

2020-12-04 Thread Michael Niedermayer
On Fri, Oct 30, 2020 at 10:52:05PM +0100, Michael Niedermayer wrote:
> Fixes: Fixes infinite loop
> Fixes: 
> 26704/clusterfuzz-testcase-minimized-ffmpeg_dem_MPC8_fuzzer-6327056939614208
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavformat/mpc8.c | 4 
>  1 file changed, 4 insertions(+)

will apply

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

Frequently ignored answer#1 FFmpeg bugs should be sent to our bugtracker. User
questions about the command line tools should be sent to the ffmpeg-user ML.
And questions about how to use libav* should be sent to the libav-user ML.


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 3/5] avformat/wavdec: Check for EOF in cues reading

2020-12-04 Thread Michael Niedermayer
On Mon, Nov 02, 2020 at 01:21:26AM +0100, Michael Niedermayer wrote:
> Fixes: Timeout (>20sec -> 1ms)
> Fixes: 
> 26793/clusterfuzz-testcase-minimized-ffmpeg_dem_WAV_fuzzer-5674966852567040
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavformat/wavdec.c | 3 +++
>  1 file changed, 3 insertions(+)

will apply

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

"You are 36 times more likely to die in a bathtub than at the hands of a
terrorist. Also, you are 2.5 times more likely to become a president and
2 times more likely to become an astronaut, than to die in a terrorist
attack." -- Thoughty2



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/5] avformat/vqf: Check len for COMM chunks

2020-12-04 Thread Michael Niedermayer
On Mon, Nov 02, 2020 at 01:21:24AM +0100, Michael Niedermayer wrote:
> Fixes: Infinite loop
> Fixes: 
> 26696/clusterfuzz-testcase-minimized-ffmpeg_dem_VQF_fuzzer-5648269168082944
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavformat/vqf.c | 3 +++
>  1 file changed, 3 insertions(+)

will apply

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

If you think the mosad wants you dead since a long time then you are either
wrong or dead since a long time.


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 3/6] avformat/mov: Use av_sat_add64() in mov_read_sidx()

2020-12-04 Thread Michael Niedermayer
On Sat, Oct 31, 2020 at 11:46:30PM +0100, Michael Niedermayer wrote:
> This avoids a potential integer overflow, no testcase is known
> 
> Signed-off-by: Michael Niedermayer 
> ---
>  libavformat/mov.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

will apply

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

Rewriting code that is poorly written but fully understood is good.
Rewriting code that one doesnt understand is a sign that one is less smart
then the original author, trying to rewrite it will not make it better.


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/6] avformat/mov: Avoid overflow in end computation in mov_read_custom()

2020-12-04 Thread Michael Niedermayer
On Sat, Oct 31, 2020 at 11:46:29PM +0100, Michael Niedermayer wrote:
> Fixes: signed integer overflow: 18 + 9223372036854775799 cannot be 
> represented in type 'long'
> Fixes: 
> 26731/clusterfuzz-testcase-minimized-ffmpeg_dem_MOV_fuzzer-5696846019952640
> 
> 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, 1 insertion(+), 1 deletion(-)

will apply

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

Those who would give up essential Liberty, to purchase a little
temporary Safety, deserve neither Liberty nor Safety -- Benjamin Franklin


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/3] avformat/id3v2: Sanity check tlen before alloc and uncompress

2020-12-04 Thread Michael Niedermayer
On Thu, Dec 03, 2020 at 10:11:37AM +0100, Michael Niedermayer wrote:
> Fixes: Timeout (>20sec -> 65ms)
> Fixes: 
> 26896/clusterfuzz-testcase-minimized-ffmpeg_dem_DAUD_fuzzer-5691024049176576
> Fixes: 
> 27627/clusterfuzz-testcase-minimized-ffmpeg_dem_AEA_fuzzer-4907019324358656
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavformat/id3v2.c | 3 +++
>  1 file changed, 3 insertions(+)

will apply

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

What does censorship reveal? It reveals fear. -- Julian Assange


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] avcodec/cdgraphics: Check frame before clearing

2020-12-04 Thread Michael Niedermayer
On Fri, Dec 04, 2020 at 11:27:46AM +0100, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2020-12-04 01:07:04)
> > Fixes: null pointer dereference
> > Fixes: 
> > 27730/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CDGRAPHICS_fuzzer-6212402236096512
> > 
> > Found-by: continuous fuzzing process 
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer 
> 
> Looks ok.

will apply

thx

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

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


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

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

Re: [FFmpeg-devel] [PATCH 3/7] lavfi/vf_pp: convert to the video_enc_params API

2020-12-04 Thread Michael Niedermayer
On Fri, Oct 02, 2020 at 08:03:27PM +0200, Anton Khirnov wrote:
> ---
>  libavfilter/Makefile|  2 +-
>  libavfilter/qp_table.c  | 66 +
>  libavfilter/qp_table.h  | 32 ++
>  libavfilter/vf_pp.c | 19 ---
>  tests/fate/filter-video.mak |  6 ++--

I think the qp_table changes should be in a seperate patch


[...]
> +int ff_qp_table_extract(AVFrame *frame, int8_t **table, int *table_w, int 
> *table_h)
> +{
> +AVFrameSideData *sd;
> +AVVideoEncParams *par;
> +unsigned int mb_h = (frame->height + 15) / 16;
> +unsigned int mb_w = (frame->width + 15) / 16;
> +unsigned int nb_mb = mb_h * mb_w;
> +unsigned int block_idx;
> +
> +*table = NULL;
> +
> +sd = av_frame_get_side_data(frame, AV_FRAME_DATA_VIDEO_ENC_PARAMS);
> +if (!sd)
> +return 0;

> +par = (AVVideoEncParams*)sd->data;
> +if (par->type != AV_VIDEO_ENC_PARAMS_MPEG2 ||
> +(par->nb_blocks != 0 && par->nb_blocks != nb_mb))
> +return 0;

This should be an error of some kind (patchwelcome or something else indicating 
lack of support)


> +
> +*table = av_malloc(nb_mb);
> +if (!*table)
> +return AVERROR(ENOMEM);
> +if (table_w)
> +*table_w = mb_w;
> +if (table_h)
> +*table_h = mb_h;
> +
> +if (par->nb_blocks == 0) {
> +memset(*table, par->qp, nb_mb);
> +return 0;
> +}
> +
> +for (block_idx = 0; block_idx < nb_mb; block_idx++) {
> +AVVideoBlockParams *b = av_video_enc_params_block(par, block_idx);
> +(*table)[block_idx] = par->qp + b->delta_qp;
> +}
> +
> +return 0;
> +}
> +
> diff --git a/libavfilter/qp_table.h b/libavfilter/qp_table.h
> new file mode 100644
> index 00..5a9ec889b6
> --- /dev/null
> +++ b/libavfilter/qp_table.h
> @@ -0,0 +1,32 @@
> +/*
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with FFmpeg; if not, write to the Free Software Foundation, Inc.,
> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + */
> +
> +#ifndef AVFILTER_QP_TABLE_H
> +#define AVFILTER_QP_TABLE_H
> +
> +#include 
> +
> +#include "libavutil/frame.h"
> +

> +/**
> + * Extract a "classic" libpostproc-style QP table from AVVideoEncParams side
> + * data.
> + */

This description is inadequate i think.
Few except me and you will understand what that is
I think the description should be sufficient to use the function without having
to read its implementation

thx

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

The worst form of inequality is to try to make unequal things equal.
-- Aristotle


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

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

Re: [FFmpeg-devel] [PATCH] avcodec/decode: port last_pkt_props to AVFifoBuffer

2020-12-04 Thread Andreas Rheinhardt
James Almer:
> Signed-off-by: James Almer 
> ---
> The idea of making avpriv_packet_list_* public was not liked, and it was
> suggested to just use AVFifoBuffer instead.
> 
> After this, the avpriv_packet_list_* functions can be made local to
> libavformat again.
> 
>  libavcodec/decode.c   | 41 +
>  libavcodec/internal.h |  4 ++--
>  libavcodec/utils.c| 11 ++-
>  3 files changed, 37 insertions(+), 19 deletions(-)
> 
> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> index 5a1849f944..0550637029 100644
> --- a/libavcodec/decode.c
> +++ b/libavcodec/decode.c
> @@ -145,22 +145,40 @@ fail2:
>  
>  #define IS_EMPTY(pkt) (!(pkt)->data)
>  
> +static int copy_packet_props(void *_src, void *_dst, int size)
> +{
> +AVPacket *src = _src, *dst = _dst;
> +int ret;
> +
> +av_init_packet(dst);
> +ret = av_packet_copy_props(dst, src);
> +if (ret < 0)
> +return ret;> +
> +dst->size = src->size; // HACK: Needed for ff_decode_frame_props().
> +dst->data = (void*)1;  // HACK: Needed for IS_EMPTY().
> +
> +return sizeof(*dst);
> +}

This is not how a write function for a fifo should look like: A fifo may
need to store the beginning of a packet at the end of the fifo and the
end of a packet at the beginning of a fifo; you can check for this by
checking whether size is < sizeof(AVPacket).
(The current implementation seems to actually only allocate multiples of
a certain unit if all av_fifo_grow()/av_fifo_realloc2()/av_fifo_alloc()
calls use only multiples of this unit, but it doesn't seem to be
documented. Should probably be done as this simplifies using fifo arrays.)

And apart from that: The current implementation of
av_fifo_generic_write() does not forward errors from the write function;
and changing this require be an API break.

> +
>  static int extract_packet_props(AVCodecInternal *avci, AVPacket *pkt)
>  {
>  int ret = 0;
>  
> -ret = avpriv_packet_list_put(&avci->pkt_props, &avci->pkt_props_tail, 
> pkt,
> - av_packet_copy_props, 0);
> +if (av_fifo_space(avci->pkt_props) < sizeof(*pkt)) {
> +ret = av_fifo_grow(avci->pkt_props, sizeof(*pkt));
> +if (ret < 0)
> +return ret;
> +}
> +
> +ret = av_fifo_generic_write(avci->pkt_props, pkt, sizeof(*pkt), 
> copy_packet_props);
>  if (ret < 0)
>  return ret;
> -avci->pkt_props_tail->pkt.size = pkt->size; // HACK: Needed for 
> ff_decode_frame_props().
> -avci->pkt_props_tail->pkt.data = (void*)1;  // HACK: Needed for 
> IS_EMPTY().
>  
> +av_assert0(ret == sizeof(*pkt));
>  if (IS_EMPTY(avci->last_pkt_props)) {
> -ret = avpriv_packet_list_get(&avci->pkt_props,
> - &avci->pkt_props_tail,
> - avci->last_pkt_props);
> -av_assert0(ret != AVERROR(EAGAIN));
> +ret = av_fifo_generic_read(avci->pkt_props, avci->last_pkt_props,
> +   sizeof(*avci->last_pkt_props), NULL);
>  }
>  return ret;
>  }
> @@ -1721,10 +1739,9 @@ int ff_decode_frame_props(AVCodecContext *avctx, 
> AVFrame *frame)
>  { AV_PKT_DATA_S12M_TIMECODE,  
> AV_FRAME_DATA_S12M_TIMECODE },
>  };
>  
> -if (IS_EMPTY(pkt))
> -avpriv_packet_list_get(&avctx->internal->pkt_props,
> -   &avctx->internal->pkt_props_tail,
> -   pkt);
> +if (IS_EMPTY(pkt) && av_fifo_size(avctx->internal->pkt_props) >= 
> sizeof(*pkt))
> +av_fifo_generic_read(avctx->internal->pkt_props,
> + pkt, sizeof(*pkt), NULL);
>  
>  if (pkt) {
>  frame->pts = pkt->pts;
> diff --git a/libavcodec/internal.h b/libavcodec/internal.h
> index 17defb9b50..8a51bca2df 100644
> --- a/libavcodec/internal.h
> +++ b/libavcodec/internal.h
> @@ -28,6 +28,7 @@
>  
>  #include "libavutil/buffer.h"
>  #include "libavutil/channel_layout.h"
> +#include "libavutil/fifo.h"
>  #include "libavutil/mathematics.h"
>  #include "libavutil/pixfmt.h"
>  #include "avcodec.h"
> @@ -145,8 +146,7 @@ typedef struct AVCodecInternal {
>   * for decoding.
>   */
>  AVPacket *last_pkt_props;
> -AVPacketList *pkt_props;
> -AVPacketList *pkt_props_tail;
> +AVFifoBuffer *pkt_props;
>  
>  /**
>   * temporary buffer used for encoders to store their bitstream
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index 45295dd3ce..c81cc972dc 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -593,10 +593,12 @@ int attribute_align_arg avcodec_open2(AVCodecContext 
> *avctx, const AVCodec *code
>  avci->es.in_frame = av_frame_alloc();
>  avci->ds.in_pkt = av_packet_alloc();
>  avci->last_pkt_props = av_packet_alloc();
> +avci->pkt_props = av_fifo_alloc(sizeof(AVPacket));
>  if (!avci->compat_decode_frame || !avci->compat_encode_packet ||
>  !avci->bu

Re: [FFmpeg-devel] [PATCH 4/7] lavfi/vf_spp: convert to the video_enc_params API

2020-12-04 Thread Michael Niedermayer
On Fri, Nov 27, 2020 at 03:38:51PM +0100, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2020-10-09 19:17:06)
> > On Fri, Oct 09, 2020 at 08:13:24AM +0200, Anton Khirnov wrote:
> > > Quoting Michael Niedermayer (2020-10-06 00:15:06)
> > > > On Mon, Oct 05, 2020 at 10:26:24AM +0200, Anton Khirnov wrote:
> > > > > Quoting Michael Niedermayer (2020-10-03 20:23:02)
> > > > > > On Fri, Oct 02, 2020 at 08:03:28PM +0200, Anton Khirnov wrote:
> > > > > > > ---
> > > > > > >  libavfilter/Makefile|  2 +-
> > > > > > >  libavfilter/vf_spp.c| 57 
> > > > > > > -
> > > > > > >  libavfilter/vf_spp.h|  3 +-
> > > > > > >  tests/fate/filter-video.mak |  4 +--
> > > > > > >  4 files changed, 29 insertions(+), 37 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> > > > > > > index d20f2937b6..2669d7b84b 100644
> > > > > > > --- a/libavfilter/Makefile
> > > > > > > +++ b/libavfilter/Makefile
> > > > > > > @@ -404,7 +404,7 @@ OBJS-$(CONFIG_SOBEL_FILTER)  
> > > > > > > += vf_convolution.o
> > > > > > >  OBJS-$(CONFIG_SOBEL_OPENCL_FILTER)   += 
> > > > > > > vf_convolution_opencl.o opencl.o \
> > > > > > >  
> > > > > > > opencl/convolution.o
> > > > > > >  OBJS-$(CONFIG_SPLIT_FILTER)  += split.o
> > > > > > > -OBJS-$(CONFIG_SPP_FILTER)+= vf_spp.o
> > > > > > > +OBJS-$(CONFIG_SPP_FILTER)+= vf_spp.o 
> > > > > > > qp_table.o
> > > > > > >  OBJS-$(CONFIG_SR_FILTER) += vf_sr.o
> > > > > > >  OBJS-$(CONFIG_SSIM_FILTER)   += vf_ssim.o 
> > > > > > > framesync.o
> > > > > > >  OBJS-$(CONFIG_STEREO3D_FILTER)   += vf_stereo3d.o
> > > > > > > diff --git a/libavfilter/vf_spp.c b/libavfilter/vf_spp.c
> > > > > > > index 4bcc6429e0..2eb383be03 100644
> > > > > > > --- a/libavfilter/vf_spp.c
> > > > > > > +++ b/libavfilter/vf_spp.c
> > > > > > > @@ -36,6 +36,7 @@
> > > > > > >  #include "libavutil/opt.h"
> > > > > > >  #include "libavutil/pixdesc.h"
> > > > > > >  #include "internal.h"
> > > > > > > +#include "qp_table.h"
> > > > > > >  #include "vf_spp.h"
> > > > > > >  
> > > > > > >  enum mode {
> > > > > > > @@ -289,7 +290,7 @@ static void filter(SPPContext *p, uint8_t 
> > > > > > > *dst, uint8_t *src,
> > > > > > >  } else{
> > > > > > >  const int qps = 3 + is_luma;
> > > > > > >  qp = qp_table[(FFMIN(x, width - 1) >> qps) + 
> > > > > > > (FFMIN(y, height - 1) >> qps) * qp_stride];
> > > > > > > -qp = FFMAX(1, ff_norm_qscale(qp, 
> > > > > > > p->qscale_type));
> > > > > > > +qp = FFMAX(1, ff_norm_qscale(qp, 
> > > > > > > FF_QSCALE_TYPE_MPEG2));
> > > > > > 
> > > > > > wouldnt this break the cases where qscale_type is not 
> > > > > > FF_QSCALE_TYPE_MPEG2 ?
> > > > > 
> > > > > There should be no such cases - in the new API, only TYPE_MPEG2 exists
> > > > > (disregarding newer codecs that were not supported by this filter
> > > > > anyway).
> > > > 
> > > > before the patch the code was intended to convert the quantization step 
> > > > size
> > > > used in the codec to the same scale as the filter used. disregarding if 
> > > > the
> > > > codec was 8x8 dct based. In fact spp should not require the input to be 
> > > > 8x8 dct
> > > > based at all, why should it? It uses the DCT as a means to favor real 
> > > > images
> > > > and remove "noise" that is not part of real images. It should for 
> > > > example also
> > > > work if a image has blocking artifacts that look like hexagons or 
> > > > triangles
> > > > 
> > > > I never tried but H264 with disabled loop filter should benefit from 
> > > > spp in
> > > > terms of subjective quality as long as the filter strength is set 
> > > > appropriately
> > > > That is for streams where the bitrate is low enough to see artifacts
> > > 
> > > Are you saying I should expand this filter to support new codecs? That
> > > does not seem appropriate for a patch that only adapts it to new API.
> > 
> > no
> > but before your patch theres a central place ff_norm_qscale()
> > that has access to both the QP type (codec type) and the QP
> > and can produce a standarized QP
> > So someone who wanted to SPP support some random new codec could
> > do it there and at the same time have 4 other filters for free also
> > support the new codec.
> 
> ff_norm_qscale() is still there and the filter still calls it. But if
> someone wants to extend it to support other codecs, they still need to
> add code to ff_qp_table_extract() to handle
> AVVideoEncParams.type != AV_VIDEO_ENC_PARAMS_MPEG2.
> 
> It does not seem reasonable to me to pretend the code supports something
> that it actually does not.

I see what you mean but as its done by the patch the code is not clean 
afterwards
at all. 
For example the argument is always the same, it asks for i

[FFmpeg-devel] [PATCH] avfilter/af_earwax: fix filter behavior

2020-12-04 Thread Paul B Mahol
Previous filter output was incorrect. New one actually follows
graph in comments described on side of filter taps.

Signed-off-by: Paul B Mahol 
---
 libavfilter/af_earwax.c  | 119 +++
 tests/fate/filter-audio.mak  |   2 +-
 tests/ref/fate/filter-earwax |  40 ++--
 3 files changed, 113 insertions(+), 48 deletions(-)

diff --git a/libavfilter/af_earwax.c b/libavfilter/af_earwax.c
index cdd2b4fc49..951aaccb5d 100644
--- a/libavfilter/af_earwax.c
+++ b/libavfilter/af_earwax.c
@@ -34,9 +34,9 @@
 #include "audio.h"
 #include "formats.h"
 
-#define NUMTAPS 64
+#define NUMTAPS 32
 
-static const int8_t filt[NUMTAPS] = {
+static const int8_t filt[NUMTAPS * 2] = {
 /* 30°  330° */
 4,   -6, /* 32 tap stereo FIR filter. */
 4,  -11, /* One side filters as if the */
@@ -72,7 +72,10 @@ static const int8_t filt[NUMTAPS] = {
 4,0};
 
 typedef struct EarwaxContext {
-int16_t taps[NUMTAPS * 2];
+int16_t filter[2][NUMTAPS];
+int16_t taps[4][NUMTAPS * 2];
+
+AVFrame *frame[2];
 } EarwaxContext;
 
 static int query_formats(AVFilterContext *ctx)
@@ -83,7 +86,7 @@ static int query_formats(AVFilterContext *ctx)
 AVFilterFormats *formats = NULL;
 AVFilterChannelLayouts *layout = NULL;
 
-if ((ret = ff_add_format (&formats, AV_SAMPLE_FMT_S16  
   )) < 0 ||
+if ((ret = ff_add_format (&formats, AV_SAMPLE_FMT_S16P 
   )) < 0 ||
 (ret = ff_set_common_formats (ctx , formats
   )) < 0 ||
 (ret = ff_add_channel_layout (&layout , AV_CH_LAYOUT_STEREO
   )) < 0 ||
 (ret = ff_set_common_channel_layouts (ctx , layout 
   )) < 0 ||
@@ -94,7 +97,8 @@ static int query_formats(AVFilterContext *ctx)
 }
 
 //FIXME: replace with DSPContext.scalarproduct_int16
-static inline int16_t *scalarproduct(const int16_t *in, const int16_t *endin, 
int16_t *out)
+static inline int16_t *scalarproduct(const int16_t *in, const int16_t *endin,
+ const int16_t *filt, int16_t *out)
 {
 int32_t sample;
 int16_t j;
@@ -111,40 +115,99 @@ static inline int16_t *scalarproduct(const int16_t *in, 
const int16_t *endin, in
 return out;
 }
 
-static int filter_frame(AVFilterLink *inlink, AVFrame *insamples)
+static int config_input(AVFilterLink *inlink)
 {
-AVFilterLink *outlink = inlink->dst->outputs[0];
-int16_t *taps, *endin, *in, *out;
-AVFrame *outsamples = ff_get_audio_buffer(outlink, insamples->nb_samples);
-int len;
+EarwaxContext *s = inlink->dst->priv;
 
-if (!outsamples) {
-av_frame_free(&insamples);
-return AVERROR(ENOMEM);
+for (int i = 0; i < NUMTAPS; i++) {
+s->filter[0][i] = filt[i * 2];
+s->filter[1][i] = filt[i * 2 + 1];
 }
-av_frame_copy_props(outsamples, insamples);
 
-taps  = ((EarwaxContext *)inlink->dst->priv)->taps;
-out   = (int16_t *)outsamples->data[0];
-in= (int16_t *)insamples ->data[0];
+return 0;
+}
+
+static void convolve(AVFilterContext *ctx, AVFrame *in,
+ int input_ch, int output_ch,
+ int filter_ch, int tap_ch)
+{
+EarwaxContext *s = ctx->priv;
+int16_t *taps, *endin, *dst, *src;
+int len;
+
+taps  = s->taps[tap_ch];
+dst   = (int16_t *)s->frame[input_ch]->data[output_ch];
+src   = (int16_t *)in->data[input_ch];
 
-len = FFMIN(NUMTAPS, 2*insamples->nb_samples);
+len = FFMIN(NUMTAPS, in->nb_samples);
 // copy part of new input and process with saved input
-memcpy(taps+NUMTAPS, in, len * sizeof(*taps));
-out   = scalarproduct(taps, taps + len, out);
+memcpy(taps+NUMTAPS, src, len * sizeof(*taps));
+dst = scalarproduct(taps, taps + len, s->filter[filter_ch], dst);
 
 // process current input
-if (2*insamples->nb_samples >= NUMTAPS ){
-endin = in + insamples->nb_samples * 2 - NUMTAPS;
-scalarproduct(in, endin, out);
+if (2*in->nb_samples >= NUMTAPS ){
+endin = src + in->nb_samples - NUMTAPS;
+scalarproduct(src, endin, s->filter[filter_ch], dst);
 
 // save part of input for next round
 memcpy(taps, endin, NUMTAPS * sizeof(*taps));
-} else
-memmove(taps, taps + 2*insamples->nb_samples, NUMTAPS * sizeof(*taps));
+} else {
+memmove(taps, taps + in->nb_samples, NUMTAPS * sizeof(*taps));
+}
+}
+
+static void mix(AVFilterContext *ctx, AVFrame *out,
+int output_ch, int f0, int f1, int i0, int i1)
+{
+EarwaxContext *s = ctx->priv;
+const int16_t *srcl = (const int16_t *)s->frame[f0]->data[i0];
+const int16_t *srcr = (const int16_t *)s->frame[f1]->data[i1];
+int16_t *dst = (int16_t *)out->data[output_ch];
+
+for (int n = 0; n < out->nb_samples; n++)
+dst[n] = srcl[n] + srcr[n];
+}
+
+static int filter_frame(AVFilterLink *inlink, AVFrame *i

Re: [FFmpeg-devel] [PATCH] avcodec/decode: port last_pkt_props to AVFifoBuffer

2020-12-04 Thread James Almer

On 12/4/2020 2:08 PM, Andreas Rheinhardt wrote:

James Almer:

Signed-off-by: James Almer 
---
The idea of making avpriv_packet_list_* public was not liked, and it was
suggested to just use AVFifoBuffer instead.

After this, the avpriv_packet_list_* functions can be made local to
libavformat again.

  libavcodec/decode.c   | 41 +
  libavcodec/internal.h |  4 ++--
  libavcodec/utils.c| 11 ++-
  3 files changed, 37 insertions(+), 19 deletions(-)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 5a1849f944..0550637029 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -145,22 +145,40 @@ fail2:
  
  #define IS_EMPTY(pkt) (!(pkt)->data)
  
+static int copy_packet_props(void *_src, void *_dst, int size)

+{
+AVPacket *src = _src, *dst = _dst;
+int ret;
+
+av_init_packet(dst);
+ret = av_packet_copy_props(dst, src);
+if (ret < 0)
+return ret;> +
+dst->size = src->size; // HACK: Needed for ff_decode_frame_props().
+dst->data = (void*)1;  // HACK: Needed for IS_EMPTY().
+
+return sizeof(*dst);
+}


This is not how a write function for a fifo should look like: A fifo may
need to store the beginning of a packet at the end of the fifo and the
end of a packet at the beginning of a fifo; you can check for this by
checking whether size is < sizeof(AVPacket).


I'm already ensuring there's always sizeof(AVPacket) bytes left before 
calling av_fifo_generic_write().



(The current implementation seems to actually only allocate multiples of
a certain unit if all av_fifo_grow()/av_fifo_realloc2()/av_fifo_alloc()
calls use only multiples of this unit, but it doesn't seem to be
documented. Should probably be done as this simplifies using fifo arrays.)

And apart from that: The current implementation of
av_fifo_generic_write() does not forward errors from the write function;
and changing this require be an API break.


Accepting a function that can return < 0 but must not be an error sounds 
like an awful oversight when defining this API...


I guess i can just do the av_packet_copy_props() call in 
extract_packet_props() and not use a custom function at all.





+
  static int extract_packet_props(AVCodecInternal *avci, AVPacket *pkt)
  {
  int ret = 0;
  
-ret = avpriv_packet_list_put(&avci->pkt_props, &avci->pkt_props_tail, pkt,

- av_packet_copy_props, 0);
+if (av_fifo_space(avci->pkt_props) < sizeof(*pkt)) {
+ret = av_fifo_grow(avci->pkt_props, sizeof(*pkt));
+if (ret < 0)
+return ret;
+}
+
+ret = av_fifo_generic_write(avci->pkt_props, pkt, sizeof(*pkt), 
copy_packet_props);
  if (ret < 0)
  return ret;
-avci->pkt_props_tail->pkt.size = pkt->size; // HACK: Needed for 
ff_decode_frame_props().
-avci->pkt_props_tail->pkt.data = (void*)1;  // HACK: Needed for IS_EMPTY().
  
+av_assert0(ret == sizeof(*pkt));

  if (IS_EMPTY(avci->last_pkt_props)) {
-ret = avpriv_packet_list_get(&avci->pkt_props,
- &avci->pkt_props_tail,
- avci->last_pkt_props);
-av_assert0(ret != AVERROR(EAGAIN));
+ret = av_fifo_generic_read(avci->pkt_props, avci->last_pkt_props,
+   sizeof(*avci->last_pkt_props), NULL);
  }
  return ret;
  }
@@ -1721,10 +1739,9 @@ int ff_decode_frame_props(AVCodecContext *avctx, AVFrame 
*frame)
  { AV_PKT_DATA_S12M_TIMECODE,  AV_FRAME_DATA_S12M_TIMECODE 
},
  };
  
-if (IS_EMPTY(pkt))

-avpriv_packet_list_get(&avctx->internal->pkt_props,
-   &avctx->internal->pkt_props_tail,
-   pkt);
+if (IS_EMPTY(pkt) && av_fifo_size(avctx->internal->pkt_props) >= 
sizeof(*pkt))
+av_fifo_generic_read(avctx->internal->pkt_props,
+ pkt, sizeof(*pkt), NULL);
  
  if (pkt) {

  frame->pts = pkt->pts;
diff --git a/libavcodec/internal.h b/libavcodec/internal.h
index 17defb9b50..8a51bca2df 100644
--- a/libavcodec/internal.h
+++ b/libavcodec/internal.h
@@ -28,6 +28,7 @@
  
  #include "libavutil/buffer.h"

  #include "libavutil/channel_layout.h"
+#include "libavutil/fifo.h"
  #include "libavutil/mathematics.h"
  #include "libavutil/pixfmt.h"
  #include "avcodec.h"
@@ -145,8 +146,7 @@ typedef struct AVCodecInternal {
   * for decoding.
   */
  AVPacket *last_pkt_props;
-AVPacketList *pkt_props;
-AVPacketList *pkt_props_tail;
+AVFifoBuffer *pkt_props;
  
  /**

   * temporary buffer used for encoders to store their bitstream
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 45295dd3ce..c81cc972dc 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -593,10 +593,12 @@ int attribute_align_arg avcodec_open2(AVCodecContext 
*avctx, const AVCodec *code
  avci->es.in_frame = av_frame_alloc();
  avci->ds.in_

Re: [FFmpeg-devel] [PATCH] avcodec/decode: port last_pkt_props to AVFifoBuffer

2020-12-04 Thread Andreas Rheinhardt
James Almer:
> On 12/4/2020 2:08 PM, Andreas Rheinhardt wrote:
>> James Almer:
>>> Signed-off-by: James Almer 
>>> ---
>>> The idea of making avpriv_packet_list_* public was not liked, and it was
>>> suggested to just use AVFifoBuffer instead.
>>>
>>> After this, the avpriv_packet_list_* functions can be made local to
>>> libavformat again.
>>>
>>>   libavcodec/decode.c   | 41 +
>>>   libavcodec/internal.h |  4 ++--
>>>   libavcodec/utils.c    | 11 ++-
>>>   3 files changed, 37 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
>>> index 5a1849f944..0550637029 100644
>>> --- a/libavcodec/decode.c
>>> +++ b/libavcodec/decode.c
>>> @@ -145,22 +145,40 @@ fail2:
>>>     #define IS_EMPTY(pkt) (!(pkt)->data)
>>>   +static int copy_packet_props(void *_src, void *_dst, int size)
>>> +{
>>> +    AVPacket *src = _src, *dst = _dst;
>>> +    int ret;
>>> +
>>> +    av_init_packet(dst);
>>> +    ret = av_packet_copy_props(dst, src);
>>> +    if (ret < 0)
>>> +    return ret;> +
>>> +    dst->size = src->size; // HACK: Needed for ff_decode_frame_props().
>>> +    dst->data = (void*)1;  // HACK: Needed for IS_EMPTY().
>>> +
>>> +    return sizeof(*dst);
>>> +}
>>
>> This is not how a write function for a fifo should look like: A fifo may
>> need to store the beginning of a packet at the end of the fifo and the
>> end of a packet at the beginning of a fifo; you can check for this by
>> checking whether size is < sizeof(AVPacket).
> 
> I'm already ensuring there's always sizeof(AVPacket) bytes left before
> calling av_fifo_generic_write().
> 

And? This just means one can write sizeof(AVPacket) bytes to the fifo,
not that there are sizeof(AVPacket) contiguous bytes available at the
current write pointer. The free space might be partially at the end and
partially at the beginning of the fifo buffer.

>> (The current implementation seems to actually only allocate multiples of
>> a certain unit if all av_fifo_grow()/av_fifo_realloc2()/av_fifo_alloc()
>> calls use only multiples of this unit, but it doesn't seem to be
>> documented. Should probably be done as this simplifies using fifo
>> arrays.)
>>
>> And apart from that: The current implementation of
>> av_fifo_generic_write() does not forward errors from the write function;
>> and changing this require be an API break.
> 
> Accepting a function that can return < 0 but must not be an error sounds
> like an awful oversight when defining this API...
> 
> I guess i can just do the av_packet_copy_props() call in
> extract_packet_props() and not use a custom function at all.
> 
>>
>>> +
>>>   static int extract_packet_props(AVCodecInternal *avci, AVPacket *pkt)
>>>   {
>>>   int ret = 0;
>>>   -    ret = avpriv_packet_list_put(&avci->pkt_props,
>>> &avci->pkt_props_tail, pkt,
>>> - av_packet_copy_props, 0);
>>> +    if (av_fifo_space(avci->pkt_props) < sizeof(*pkt)) {
>>> +    ret = av_fifo_grow(avci->pkt_props, sizeof(*pkt));
>>> +    if (ret < 0)
>>> +    return ret;
>>> +    }
>>> +
>>> +    ret = av_fifo_generic_write(avci->pkt_props, pkt, sizeof(*pkt),
>>> copy_packet_props);
>>>   if (ret < 0)
>>>   return ret;
>>> -    avci->pkt_props_tail->pkt.size = pkt->size; // HACK: Needed for
>>> ff_decode_frame_props().
>>> -    avci->pkt_props_tail->pkt.data = (void*)1;  // HACK: Needed for
>>> IS_EMPTY().
>>>   +    av_assert0(ret == sizeof(*pkt));
>>>   if (IS_EMPTY(avci->last_pkt_props)) {
>>> -    ret = avpriv_packet_list_get(&avci->pkt_props,
>>> - &avci->pkt_props_tail,
>>> - avci->last_pkt_props);
>>> -    av_assert0(ret != AVERROR(EAGAIN));
>>> +    ret = av_fifo_generic_read(avci->pkt_props,
>>> avci->last_pkt_props,
>>> +   sizeof(*avci->last_pkt_props),
>>> NULL);
>>>   }
>>>   return ret;
>>>   }
>>> @@ -1721,10 +1739,9 @@ int ff_decode_frame_props(AVCodecContext
>>> *avctx, AVFrame *frame)
>>>   { AV_PKT_DATA_S12M_TIMECODE, 
>>> AV_FRAME_DATA_S12M_TIMECODE },
>>>   };
>>>   -    if (IS_EMPTY(pkt))
>>> -    avpriv_packet_list_get(&avctx->internal->pkt_props,
>>> -   &avctx->internal->pkt_props_tail,
>>> -   pkt);
>>> +    if (IS_EMPTY(pkt) && av_fifo_size(avctx->internal->pkt_props) >=
>>> sizeof(*pkt))
>>> +    av_fifo_generic_read(avctx->internal->pkt_props,
>>> + pkt, sizeof(*pkt), NULL);
>>>     if (pkt) {
>>>   frame->pts = pkt->pts;
>>> diff --git a/libavcodec/internal.h b/libavcodec/internal.h
>>> index 17defb9b50..8a51bca2df 100644
>>> --- a/libavcodec/internal.h
>>> +++ b/libavcodec/internal.h
>>> @@ -28,6 +28,7 @@
>>>     #include "libavutil/buffer.h"
>>>   #include "libavutil/channel_layout.h"
>>> +#include "libavutil/fifo.h"
>>>   #include "libavutil

Re: [FFmpeg-devel] [PATCH] avcodec/decode: port last_pkt_props to AVFifoBuffer

2020-12-04 Thread James Almer

On 12/4/2020 4:08 PM, Andreas Rheinhardt wrote:

James Almer:

On 12/4/2020 2:08 PM, Andreas Rheinhardt wrote:

James Almer:

Signed-off-by: James Almer 
---
The idea of making avpriv_packet_list_* public was not liked, and it was
suggested to just use AVFifoBuffer instead.

After this, the avpriv_packet_list_* functions can be made local to
libavformat again.

   libavcodec/decode.c   | 41 +
   libavcodec/internal.h |  4 ++--
   libavcodec/utils.c    | 11 ++-
   3 files changed, 37 insertions(+), 19 deletions(-)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 5a1849f944..0550637029 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -145,22 +145,40 @@ fail2:
     #define IS_EMPTY(pkt) (!(pkt)->data)
   +static int copy_packet_props(void *_src, void *_dst, int size)
+{
+    AVPacket *src = _src, *dst = _dst;
+    int ret;
+
+    av_init_packet(dst);
+    ret = av_packet_copy_props(dst, src);
+    if (ret < 0)
+    return ret;> +
+    dst->size = src->size; // HACK: Needed for ff_decode_frame_props().
+    dst->data = (void*)1;  // HACK: Needed for IS_EMPTY().
+
+    return sizeof(*dst);
+}


This is not how a write function for a fifo should look like: A fifo may
need to store the beginning of a packet at the end of the fifo and the
end of a packet at the beginning of a fifo; you can check for this by
checking whether size is < sizeof(AVPacket).


I'm already ensuring there's always sizeof(AVPacket) bytes left before
calling av_fifo_generic_write().



And? This just means one can write sizeof(AVPacket) bytes to the fifo,
not that there are sizeof(AVPacket) contiguous bytes available at the
current write pointer. The free space might be partially at the end and
partially at the beginning of the fifo buffer.


It wraps around? This is not obvious from reading the doxy.

In any case, by always incrementing the FIFO by sizeof(AVPacket) bytes 
it shouldn't be an issue since it will always be a multiple of it. But 
as i said, i'll just do it all outside of av_fifo_generic_write() anyway 
since i can't propagate errors.





(The current implementation seems to actually only allocate multiples of
a certain unit if all av_fifo_grow()/av_fifo_realloc2()/av_fifo_alloc()
calls use only multiples of this unit, but it doesn't seem to be
documented. Should probably be done as this simplifies using fifo
arrays.)


Return values for av_fifo_generic_read are also undocumented. Currently, 
it's always 0 no matter what.




And apart from that: The current implementation of
av_fifo_generic_write() does not forward errors from the write function;
and changing this require be an API break.


Accepting a function that can return < 0 but must not be an error sounds
like an awful oversight when defining this API...

I guess i can just do the av_packet_copy_props() call in
extract_packet_props() and not use a custom function at all.




+
   static int extract_packet_props(AVCodecInternal *avci, AVPacket *pkt)
   {
   int ret = 0;
   -    ret = avpriv_packet_list_put(&avci->pkt_props,
&avci->pkt_props_tail, pkt,
- av_packet_copy_props, 0);
+    if (av_fifo_space(avci->pkt_props) < sizeof(*pkt)) {
+    ret = av_fifo_grow(avci->pkt_props, sizeof(*pkt));
+    if (ret < 0)
+    return ret;
+    }
+
+    ret = av_fifo_generic_write(avci->pkt_props, pkt, sizeof(*pkt),
copy_packet_props);
   if (ret < 0)
   return ret;
-    avci->pkt_props_tail->pkt.size = pkt->size; // HACK: Needed for
ff_decode_frame_props().
-    avci->pkt_props_tail->pkt.data = (void*)1;  // HACK: Needed for
IS_EMPTY().
   +    av_assert0(ret == sizeof(*pkt));
   if (IS_EMPTY(avci->last_pkt_props)) {
-    ret = avpriv_packet_list_get(&avci->pkt_props,
- &avci->pkt_props_tail,
- avci->last_pkt_props);
-    av_assert0(ret != AVERROR(EAGAIN));
+    ret = av_fifo_generic_read(avci->pkt_props,
avci->last_pkt_props,
+   sizeof(*avci->last_pkt_props),
NULL);
   }
   return ret;
   }
@@ -1721,10 +1739,9 @@ int ff_decode_frame_props(AVCodecContext
*avctx, AVFrame *frame)
   { AV_PKT_DATA_S12M_TIMECODE,
AV_FRAME_DATA_S12M_TIMECODE },
   };
   -    if (IS_EMPTY(pkt))
-    avpriv_packet_list_get(&avctx->internal->pkt_props,
-   &avctx->internal->pkt_props_tail,
-   pkt);
+    if (IS_EMPTY(pkt) && av_fifo_size(avctx->internal->pkt_props) >=
sizeof(*pkt))
+    av_fifo_generic_read(avctx->internal->pkt_props,
+ pkt, sizeof(*pkt), NULL);
     if (pkt) {
   frame->pts = pkt->pts;
diff --git a/libavcodec/internal.h b/libavcodec/internal.h
index 17defb9b50..8a51bca2df 100644
--- a/libavcodec/internal.h
+++ b/libavcodec/internal.h
@@ -28,6 +28,7 @@
     #include "libavutil/buffer.h"
   #include "libavutil/ch

Re: [FFmpeg-devel] [PATCH] avcodec/decode: port last_pkt_props to AVFifoBuffer

2020-12-04 Thread Andreas Rheinhardt
James Almer:
> On 12/4/2020 4:08 PM, Andreas Rheinhardt wrote:
>> James Almer:
>>> On 12/4/2020 2:08 PM, Andreas Rheinhardt wrote:
 James Almer:
> Signed-off-by: James Almer 
> ---
> The idea of making avpriv_packet_list_* public was not liked, and
> it was
> suggested to just use AVFifoBuffer instead.
>
> After this, the avpriv_packet_list_* functions can be made local to
> libavformat again.
>
>    libavcodec/decode.c   | 41
> +
>    libavcodec/internal.h |  4 ++--
>    libavcodec/utils.c    | 11 ++-
>    3 files changed, 37 insertions(+), 19 deletions(-)
>
> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> index 5a1849f944..0550637029 100644
> --- a/libavcodec/decode.c
> +++ b/libavcodec/decode.c
> @@ -145,22 +145,40 @@ fail2:
>      #define IS_EMPTY(pkt) (!(pkt)->data)
>    +static int copy_packet_props(void *_src, void *_dst, int size)
> +{
> +    AVPacket *src = _src, *dst = _dst;
> +    int ret;
> +
> +    av_init_packet(dst);
> +    ret = av_packet_copy_props(dst, src);
> +    if (ret < 0)
> +    return ret;> +
> +    dst->size = src->size; // HACK: Needed for
> ff_decode_frame_props().
> +    dst->data = (void*)1;  // HACK: Needed for IS_EMPTY().
> +
> +    return sizeof(*dst);
> +}

 This is not how a write function for a fifo should look like: A fifo
 may
 need to store the beginning of a packet at the end of the fifo and the
 end of a packet at the beginning of a fifo; you can check for this by
 checking whether size is < sizeof(AVPacket).
>>>
>>> I'm already ensuring there's always sizeof(AVPacket) bytes left before
>>> calling av_fifo_generic_write().
>>>
>>
>> And? This just means one can write sizeof(AVPacket) bytes to the fifo,
>> not that there are sizeof(AVPacket) contiguous bytes available at the
>> current write pointer. The free space might be partially at the end and
>> partially at the beginning of the fifo buffer.
> 
> It wraps around? This is not obvious from reading the doxy.
> 
> In any case, by always incrementing the FIFO by sizeof(AVPacket) bytes
> it shouldn't be an issue since it will always be a multiple of it. But
> as i said, i'll just do it all outside of av_fifo_generic_write() anyway
> since i can't propagate errors.
> 

James, this is a fifo: It uses a circular buffer. Of course it wraps
around. And this is documented: "a very simple circular buffer FIFO
implementation".

And as I said:

>>
 (The current implementation seems to actually only allocate
 multiples of
 a certain unit if all av_fifo_grow()/av_fifo_realloc2()/av_fifo_alloc()
 calls use only multiples of this unit, but it doesn't seem to be
 documented. Should probably be done as this simplifies using fifo
 arrays.)
>

So, yes, it really seems to work nicely when using it to store arrays;
yet this is undocumented.

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

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

Re: [FFmpeg-devel] [PATCH] avcodec/decode: port last_pkt_props to AVFifoBuffer

2020-12-04 Thread James Almer

On 12/4/2020 5:18 PM, Andreas Rheinhardt wrote:

James Almer:

On 12/4/2020 4:08 PM, Andreas Rheinhardt wrote:

James Almer:

On 12/4/2020 2:08 PM, Andreas Rheinhardt wrote:

James Almer:

Signed-off-by: James Almer 
---
The idea of making avpriv_packet_list_* public was not liked, and
it was
suggested to just use AVFifoBuffer instead.

After this, the avpriv_packet_list_* functions can be made local to
libavformat again.

    libavcodec/decode.c   | 41
+
    libavcodec/internal.h |  4 ++--
    libavcodec/utils.c    | 11 ++-
    3 files changed, 37 insertions(+), 19 deletions(-)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 5a1849f944..0550637029 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -145,22 +145,40 @@ fail2:
      #define IS_EMPTY(pkt) (!(pkt)->data)
    +static int copy_packet_props(void *_src, void *_dst, int size)
+{
+    AVPacket *src = _src, *dst = _dst;
+    int ret;
+
+    av_init_packet(dst);
+    ret = av_packet_copy_props(dst, src);
+    if (ret < 0)
+    return ret;> +
+    dst->size = src->size; // HACK: Needed for
ff_decode_frame_props().
+    dst->data = (void*)1;  // HACK: Needed for IS_EMPTY().
+
+    return sizeof(*dst);
+}


This is not how a write function for a fifo should look like: A fifo
may
need to store the beginning of a packet at the end of the fifo and the
end of a packet at the beginning of a fifo; you can check for this by
checking whether size is < sizeof(AVPacket).


I'm already ensuring there's always sizeof(AVPacket) bytes left before
calling av_fifo_generic_write().



And? This just means one can write sizeof(AVPacket) bytes to the fifo,
not that there are sizeof(AVPacket) contiguous bytes available at the
current write pointer. The free space might be partially at the end and
partially at the beginning of the fifo buffer.


It wraps around? This is not obvious from reading the doxy.

In any case, by always incrementing the FIFO by sizeof(AVPacket) bytes
it shouldn't be an issue since it will always be a multiple of it. But
as i said, i'll just do it all outside of av_fifo_generic_write() anyway
since i can't propagate errors.



James, this is a fifo: It uses a circular buffer. Of course it wraps
around. And this is documented: "a very simple circular buffer FIFO
implementation".


Right, was thinking about it and handling it like a linked list FIFO 
where you always add complete elements at the end than as a byte array 
with read/write index pointers.




And as I said:




(The current implementation seems to actually only allocate
multiples of
a certain unit if all av_fifo_grow()/av_fifo_realloc2()/av_fifo_alloc()
calls use only multiples of this unit, but it doesn't seem to be
documented. Should probably be done as this simplifies using fifo
arrays.)




So, yes, it really seems to work nicely when using it to store arrays;
yet this is undocumented.

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

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



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

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

[FFmpeg-devel] [PATCH v2] avcodec/decode: port last_pkt_props to AVFifoBuffer

2020-12-04 Thread James Almer
Signed-off-by: James Almer 
---
 libavcodec/decode.c   | 31 +++
 libavcodec/internal.h |  4 ++--
 libavcodec/utils.c| 26 +-
 3 files changed, 42 insertions(+), 19 deletions(-)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 5a1849f944..68cdeccd06 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -147,20 +147,28 @@ fail2:
 
 static int extract_packet_props(AVCodecInternal *avci, AVPacket *pkt)
 {
+AVPacket tmp;
 int ret = 0;
 
-ret = avpriv_packet_list_put(&avci->pkt_props, &avci->pkt_props_tail, pkt,
- av_packet_copy_props, 0);
+if (av_fifo_space(avci->pkt_props) < sizeof(*pkt)) {
+ret = av_fifo_grow(avci->pkt_props, sizeof(*pkt));
+if (ret < 0)
+return ret;
+}
+
+av_init_packet(&tmp);
+ret = av_packet_copy_props(&tmp, pkt);
 if (ret < 0)
 return ret;
-avci->pkt_props_tail->pkt.size = pkt->size; // HACK: Needed for 
ff_decode_frame_props().
-avci->pkt_props_tail->pkt.data = (void*)1;  // HACK: Needed for IS_EMPTY().
+tmp.size = pkt->size; // HACK: Needed for ff_decode_frame_props().
+tmp.data = (void*)1;  // HACK: Needed for IS_EMPTY().
+
+ret = av_fifo_generic_write(avci->pkt_props, &tmp, sizeof(tmp), NULL);
+av_assert0(ret == sizeof(tmp));
 
 if (IS_EMPTY(avci->last_pkt_props)) {
-ret = avpriv_packet_list_get(&avci->pkt_props,
- &avci->pkt_props_tail,
- avci->last_pkt_props);
-av_assert0(ret != AVERROR(EAGAIN));
+ret = av_fifo_generic_read(avci->pkt_props, avci->last_pkt_props,
+   sizeof(*avci->last_pkt_props), NULL);
 }
 return ret;
 }
@@ -1721,10 +1729,9 @@ int ff_decode_frame_props(AVCodecContext *avctx, AVFrame 
*frame)
 { AV_PKT_DATA_S12M_TIMECODE,  AV_FRAME_DATA_S12M_TIMECODE 
},
 };
 
-if (IS_EMPTY(pkt))
-avpriv_packet_list_get(&avctx->internal->pkt_props,
-   &avctx->internal->pkt_props_tail,
-   pkt);
+if (IS_EMPTY(pkt) && av_fifo_size(avctx->internal->pkt_props) >= 
sizeof(*pkt))
+av_fifo_generic_read(avctx->internal->pkt_props,
+ pkt, sizeof(*pkt), NULL);
 
 if (pkt) {
 frame->pts = pkt->pts;
diff --git a/libavcodec/internal.h b/libavcodec/internal.h
index 141f3fb88e..19c96d672c 100644
--- a/libavcodec/internal.h
+++ b/libavcodec/internal.h
@@ -28,6 +28,7 @@
 
 #include "libavutil/buffer.h"
 #include "libavutil/channel_layout.h"
+#include "libavutil/fifo.h"
 #include "libavutil/mathematics.h"
 #include "libavutil/pixfmt.h"
 #include "avcodec.h"
@@ -145,8 +146,7 @@ typedef struct AVCodecInternal {
  * for decoding.
  */
 AVPacket *last_pkt_props;
-AVPacketList *pkt_props;
-AVPacketList *pkt_props_tail;
+AVFifoBuffer *pkt_props;
 
 /**
  * temporary buffer used for encoders to store their bitstream
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 0226e36ee7..e156914af8 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -581,10 +581,12 @@ int attribute_align_arg avcodec_open2(AVCodecContext 
*avctx, const AVCodec *code
 avci->es.in_frame = av_frame_alloc();
 avci->ds.in_pkt = av_packet_alloc();
 avci->last_pkt_props = av_packet_alloc();
+avci->pkt_props = av_fifo_alloc(sizeof(*avci->last_pkt_props));
 if (!avci->compat_decode_frame || !avci->compat_encode_packet ||
 !avci->buffer_frame || !avci->buffer_pkt  ||
 !avci->es.in_frame  || !avci->ds.in_pkt   ||
-!avci->to_free  || !avci->last_pkt_props) {
+!avci->to_free  || !avci->last_pkt_props  ||
+!avci->pkt_props) {
 ret = AVERROR(ENOMEM);
 goto free_and_end;
 }
@@ -1064,6 +1066,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
 av_packet_free(&avci->compat_encode_packet);
 av_packet_free(&avci->buffer_pkt);
 av_packet_free(&avci->last_pkt_props);
+av_fifo_freep(&avci->pkt_props);
 
 av_packet_free(&avci->ds.in_pkt);
 av_frame_free(&avci->es.in_frame);
@@ -1104,8 +1107,13 @@ void avcodec_flush_buffers(AVCodecContext *avctx)
 av_packet_unref(avci->buffer_pkt);
 
 av_packet_unref(avci->last_pkt_props);
-avpriv_packet_list_free(&avci->pkt_props,
-&avci->pkt_props_tail);
+while (av_fifo_size(avci->pkt_props) >= sizeof(*avci->last_pkt_props)) {
+av_fifo_generic_read(avci->pkt_props,
+ avci->last_pkt_props, 
sizeof(*avci->last_pkt_props),
+ NULL);
+av_packet_unref(avci->last_pkt_props);
+}
+av_fifo_reset(avci->pkt_props);
 
 av_frame_unref(avci->es.in_frame);
 av_packet_unref(avci->ds.in_pkt);
@@ -1167,9 +1175,17 @@ av_cold int avcodec_close(AVCodecContext *avctx)
 av_

Re: [FFmpeg-devel] [PATCH] avfilter: add asubcut filter

2020-12-04 Thread Paul B Mahol
Will apply shortly.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] libavformat/mov.c: export vendor id as metadata

2020-12-04 Thread Michael Niedermayer
On Thu, Dec 03, 2020 at 12:40:40PM -0800, Thierry Foucu wrote:
> On Wed, Nov 18, 2020 at 12:09 PM Thierry Foucu  wrote:
> 
> > ---
> >  libavformat/mov.c | 8 ++--
> >  tests/ref/fate/hapqa-extract-nosnappy-to-hapalphaonly-mov | 1 +
> >  tests/ref/fate/hapqa-extract-nosnappy-to-hapq-mov | 1 +
> >  tests/ref/fate/mov-zombie | 2 +-
> >  tests/ref/fate/rgb24-mkv  | 4 ++--
> >  5 files changed, 11 insertions(+), 5 deletions(-)
> >
> > diff --git a/libavformat/mov.c b/libavformat/mov.c
> > index 2b90e31170..1ba1a748e8 100644
> > --- a/libavformat/mov.c
> > +++ b/libavformat/mov.c
> > @@ -2095,6 +2095,7 @@ static void mov_parse_stsd_video(MOVContext *c,
> > AVIOContext *pb,
> >  uint8_t codec_name[32] = { 0 };
> >  int64_t stsd_start;
> >  unsigned int len;
> > +uint32_t id = 0;
> >
> >  /* The first 16 bytes of the video sample description are already
> >   * read in ff_mov_read_stsd_entries() */
> > @@ -2102,7 +2103,8 @@ static void mov_parse_stsd_video(MOVContext *c,
> > AVIOContext *pb,
> >
> >  avio_rb16(pb); /* version */
> >  avio_rb16(pb); /* revision level */
> > -avio_rb32(pb); /* vendor */
> > +id = avio_rl32(pb); /* vendor */
> > +av_dict_set(&st->metadata, "vendor_id", av_fourcc2str(id), 0);
> >  avio_rb32(pb); /* temporal quality */
> >  avio_rb32(pb); /* spatial quality */
> >
> > @@ -2150,10 +2152,12 @@ static void mov_parse_stsd_audio(MOVContext *c,
> > AVIOContext *pb,
> >  {
> >  int bits_per_sample, flags;
> >  uint16_t version = avio_rb16(pb);
> > +uint32_t id = 0;
> >  AVDictionaryEntry *compatible_brands = av_dict_get(c->fc->metadata,
> > "compatible_brands", NULL, AV_DICT_MATCH_CASE);
> >
> >  avio_rb16(pb); /* revision level */
> > -avio_rb32(pb); /* vendor */
> > +id = avio_rl32(pb); /* vendor */
> > +av_dict_set(&st->metadata, "vendor_id", av_fourcc2str(id), 0);
> >
> >  st->codecpar->channels  = avio_rb16(pb); /* channel count
> > */
> >  st->codecpar->bits_per_coded_sample = avio_rb16(pb); /* sample size */
> > diff --git a/tests/ref/fate/hapqa-extract-nosnappy-to-hapalphaonly-mov
> > b/tests/ref/fate/hapqa-extract-nosnappy-to-hapalphaonly-mov
> > index 61af08aa23..04a965b12a 100644
> > --- a/tests/ref/fate/hapqa-extract-nosnappy-to-hapalphaonly-mov
> > +++ b/tests/ref/fate/hapqa-extract-nosnappy-to-hapalphaonly-mov
> > @@ -69,5 +69,6 @@ DISPOSITION:attached_pic=0
> >  DISPOSITION:timed_thumbnails=0
> >  TAG:language=eng
> >  TAG:handler_name=Module de gestion video
> > +TAG:vendor_id=FFMP
> >  TAG:encoder=HAPAlpha Only
> >  [/STREAM]
> > diff --git a/tests/ref/fate/hapqa-extract-nosnappy-to-hapq-mov
> > b/tests/ref/fate/hapqa-extract-nosnappy-to-hapq-mov
> > index 1fb31ec7f0..d9e5c94ffb 100644
> > --- a/tests/ref/fate/hapqa-extract-nosnappy-to-hapq-mov
> > +++ b/tests/ref/fate/hapqa-extract-nosnappy-to-hapq-mov
> > @@ -69,5 +69,6 @@ DISPOSITION:attached_pic=0
> >  DISPOSITION:timed_thumbnails=0
> >  TAG:language=eng
> >  TAG:handler_name=Module de gestion video
> > +TAG:vendor_id=FFMP
> >  TAG:encoder=HAPQ
> >  [/STREAM]
> > diff --git a/tests/ref/fate/mov-zombie b/tests/ref/fate/mov-zombie
> > index 1a6625bc8f..c566b14d47 100644
> > --- a/tests/ref/fate/mov-zombie
> > +++ b/tests/ref/fate/mov-zombie
> > @@ -194,5 +194,5 @@
> > frame|media_type=video|stream_index=0|key_frame=0|pkt_pts=188623|pkt_pts_time=2.
> >
> >  
> > packet|codec_type=video|stream_index=0|pts=197632|pts_time=2.195911|dts=191625|dts_time=2.129167|duration=3003|duration_time=0.033367|convergence_duration=N/A|convergence_duration_time=N/A|size=580|pos=101820|flags=__
> >  
> > frame|media_type=video|stream_index=0|key_frame=0|pkt_pts=191626|pkt_pts_time=2.129178|pkt_dts=N/A|pkt_dts_time=N/A|best_effort_timestamp=191626|best_effort_timestamp_time=2.129178|pkt_duration=3003|pkt_duration_time=0.033367|pkt_pos=99180|pkt_size=1666|width=160|height=240|pix_fmt=yuv420p|sample_aspect_ratio=2:1|pict_type=P|coded_picture_number=63|display_picture_number=0|interlaced_frame=0|top_field_first=0|repeat_pict=0|color_range=tv|color_space=smpte170m|color_primaries=smpte170m|color_transfer=bt709|chroma_location=topleftside_data|side_data_type=H.26[45]
> > User Data Unregistered SEI message
> >
> > -stream|index=0|codec_name=h264|profile=77|codec_type=video|codec_time_base=212521/12744000|codec_tag_string=avc1|codec_tag=0x31637661|width=160|height=240|coded_width=160|coded_height=240|closed_captions=0|has_b_frames=1|sample_aspect_ratio=2:1|display_aspect_ratio=4:3|pix_fmt=yuv420p|level=12|color_range=tv|color_space=smpte170m|color_transfer=bt709|color_primaries=smpte170m|chroma_location=topleft|field_order=unknown|timecode=N/A|refs=2|is_avc=true|nal_length_size=4|id=N/A|r_frame_rate=3/1001|avg_frame_rate=6372000/212521|time_base=1/9|start_pts=0|start_time=0.00|duration_ts=2125200|duration=23.61|bit_rate

[FFmpeg-devel] [PATCH 1/3] avcodec/av1dec: Fix leak in case of failure

2020-12-04 Thread Andreas Rheinhardt
A reference to an AV1RawFrameHeader and consequently the
AV1RawFrameHeader itself and everything it has a reference to leak
if the hardware has no AV1 decoding capabilities. It happens e.g.
in the cbs-av1-av1-1-b8-02-allintra FATE-test; it has just been masked
because the return value of ffmpeg (which indicates failure when using
Valgrind or ASAN) is ignored when doing tests of type md5.

Signed-off-by: Andreas Rheinhardt 
---
I switched the av_buffer_ref() and update_context_with_frame_header()
because the latter does not need any cleanup on failure.

Also notice that actual decoding with this patch applied is completely
untested.

 libavcodec/av1dec.c | 20 +++-
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
index 1589b8f0c0..d7b2ac9d46 100644
--- a/libavcodec/av1dec.c
+++ b/libavcodec/av1dec.c
@@ -674,20 +674,20 @@ static int av1_frame_alloc(AVCodecContext *avctx, 
AV1Frame *f)
 AVFrame *frame;
 int ret;
 
-f->header_ref = av_buffer_ref(s->header_ref);
-if (!f->header_ref)
-return AVERROR(ENOMEM);
-
-f->raw_frame_header = s->raw_frame_header;
-
 ret = update_context_with_frame_header(avctx, header);
 if (ret < 0) {
 av_log(avctx, AV_LOG_ERROR, "Failed to update context with frame 
header\n");
 return ret;
 }
 
+f->header_ref = av_buffer_ref(s->header_ref);
+if (!f->header_ref)
+return AVERROR(ENOMEM);
+
+f->raw_frame_header = s->raw_frame_header;
+
 if ((ret = ff_thread_get_buffer(avctx, &f->tf, AV_GET_BUFFER_FLAG_REF)) < 
0)
-return ret;
+goto fail;
 
 frame = f->tf.f;
 frame->key_frame = header->frame_type == AV1_FRAME_KEY;
@@ -710,8 +710,10 @@ static int av1_frame_alloc(AVCodecContext *avctx, AV1Frame 
*f)
 if (hwaccel->frame_priv_data_size) {
 f->hwaccel_priv_buf =
 av_buffer_allocz(hwaccel->frame_priv_data_size);
-if (!f->hwaccel_priv_buf)
+if (!f->hwaccel_priv_buf) {
+ret = AVERROR(ENOMEM);
 goto fail;
+}
 f->hwaccel_picture_private = f->hwaccel_priv_buf->data;
 }
 }
@@ -719,7 +721,7 @@ static int av1_frame_alloc(AVCodecContext *avctx, AV1Frame 
*f)
 
 fail:
 av1_frame_unref(avctx, f);
-return AVERROR(ENOMEM);
+return ret;
 }
 
 static int set_output_frame(AVCodecContext *avctx, AVFrame *frame,
-- 
2.25.1

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

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

[FFmpeg-devel] [PATCH 2/3] fate/mxf: Fix d10-user-comments test

2020-12-04 Thread Andreas Rheinhardt
The mxf_d10 muxer is very picky regarding the input it accepts:
The only video accepted is MPEG-2 with absolutely constant bitrate,
i.e. all packets need to have exactly the same size; and only a few
bitrates are accepted.

The sample file used did not abide by this: Writing the first packet
(a video packet) errors out and afterwards an audio packet from the
muxing queue has been written. That's all besides metadata (which this
test is about). The FFmpeg cli returned an error, but said error has
been ignored by the md5 test.

This commit changes the test to actually send a compliant stream to the
muxer, so that it does not error out; furthermore, the test is changed
to explicitly check the metadata instead of it only being implicitly
included in the md5 checksum. The compliant stream is created by our
encoder at runtime.

Signed-off-by: Andreas Rheinhardt 
---
This test needs to be fixed in preparation for the next patch.

 tests/fate/mxf.mak   |  8 
 tests/ref/fate/mxf-d10-user-comments | 26 +-
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/tests/fate/mxf.mak b/tests/fate/mxf.mak
index ca119fa677..467357ed2d 100644
--- a/tests/fate/mxf.mak
+++ b/tests/fate/mxf.mak
@@ -45,9 +45,8 @@ FATE_MXF_USER_COMMENTS-$(call ENCDEC2, MPEG2VIDEO, PCM_S16LE, 
MXF) += fate-mxf-u
 fate-mxf-user-comments: $(SAMPLES)/mxf/Sony-1.mxf
 fate-mxf-user-comments: CMD = md5 -y -i $(TARGET_SAMPLES)/mxf/Sony-1.mxf 
-c copy -metadata "comment_test=value" -fflags +bitexact -f mxf
 
-FATE_MXF_D10_USER_COMMENTS-$(call ENCDEC2, MPEG2VIDEO, PCM_S16LE, MXF) += 
fate-mxf-d10-user-comments
-fate-mxf-d10-user-comments: $(SAMPLES)/mxf/Sony-1.mxf
-fate-mxf-d10-user-comments: CMD = md5 -y -i 
$(TARGET_SAMPLES)/mxf/Sony-1.mxf -c copy -metadata "comment_test=value" 
-store_user_comments 1 -fflags +bitexact -f mxf_d10
+FATE_MXF_D10_USER_COMMENTS-$(call ALLYES, FILE_PROTOCOL MXF_DEMUXER 
DVVIDEO_DECODER SCALE_FILTER MPEG2VIDEO_ENCODER MXF_D10_MUXER 
EXTRACT_EXTRADATA_BSF MPEGVIDEO_PARSER PIPE_PROTOCOL FRAMECRC_MUXER) += 
fate-mxf-d10-user-comments
+fate-mxf-d10-user-comments: CMD = transcode mxf 
$(TARGET_SAMPLES)/mxf/Avid-5.mxf mxf_d10 "-c:v mpeg2video -b:v 3k 
-minrate:v 3k -maxrate:v 3k -bufsize:v 3k -rc_init_occupancy 3k 
-vf scale=w=1280:h=720 -an -metadata comment_test=value -store_user_comments 1" 
"-c copy -frames:v 5" "" "-show_entries format_tags"
 
 FATE_MXF_OPATOM_USER_COMMENTS-$(call ENCDEC2, MPEG2VIDEO, PCM_S16LE, MXF) += 
fate-mxf-opatom-user-comments
 fate-mxf-opatom-user-comments: $(SAMPLES)/mxf/Sony-1.mxf
@@ -56,7 +55,8 @@ fate-mxf-opatom-user-comments: CMD = md5 -y -i 
$(TARGET_SAMPLES)/mxf/Sony-1.
 FATE_MXF-$(CONFIG_MXF_DEMUXER) += $(FATE_MXF)
 
 FATE_SAMPLES_AVCONV += $(FATE_MXF-yes) $(FATE_MXF_REEL_NAME-yes)
-FATE_SAMPLES_AVCONV += $(FATE_MXF_USER_COMMENTS-yes) 
$(FATE_MXF_D10_USER_COMMENTS-yes) $(FATE_MXF_OPATOM_USER_COMMENTS-yes)
+FATE_SAMPLES_AVCONV += $(FATE_MXF_USER_COMMENTS-yes) 
$(FATE_MXF_OPATOM_USER_COMMENTS-yes)
+FATE_SAMPLES_FFMPEG_FFPROBE += $(FATE_MXF_D10_USER_COMMENTS-yes)
 FATE_SAMPLES_FFPROBE += $(FATE_MXF_PROBE-yes)
 
 fate-mxf: $(FATE_MXF-yes) $(FATE_MXF_PROBE-yes) $(FATE_MXF_REEL_NAME-yes) 
$(FATE_MXF_USER_COMMENTS-yes) $(FATE_MXF_D10_USER_COMMENTS-yes) 
$(FATE_MXF_OPATOM_USER_COMMENTS-yes)
diff --git a/tests/ref/fate/mxf-d10-user-comments 
b/tests/ref/fate/mxf-d10-user-comments
index de4f26c6f8..a2fae7ec01 100644
--- a/tests/ref/fate/mxf-d10-user-comments
+++ b/tests/ref/fate/mxf-d10-user-comments
@@ -1 +1,25 @@
-68f0fa62b6a676894afbbe4c34ebf70b
+eea0dd8376b059d41a3063064d3ac811 *tests/data/fate/mxf-d10-user-comments.mxf_d10
+3782189 tests/data/fate/mxf-d10-user-comments.mxf_d10
+#extradata 0:   34, 0x716b05c4
+#tb 0: 1/25
+#media_type 0: video
+#codec_id 0: mpeg2video
+#dimensions 0: 1280x720
+#sar 0: 3/4
+0, -1,  0,1,   15, 0x0547870d, S=1,   24, 
0x5aa90ad0
+0,  0,  1,1,   15, 0xe80a1612, F=0x0
+0,  1,  2,1,   15, 0xc5c50e2f, F=0x0
+0,  2,  3,1,   15, 0x51e28a04, F=0x0
+0,  3,  4,1,   15, 0x9bbe6feb, F=0x0
+[FORMAT]
+TAG:operational_pattern_ul=060e2b34.04010101.0d010201.01010900
+TAG:uid=adab4424-2f25-4dc7-92ff-29bd000c
+TAG:generation_uid=adab4424-2f25-4dc7-92ff-29bd000c0001
+TAG:company_name=FFmpeg
+TAG:product_name=OP1a Muxer
+TAG:product_version=0.0.0
+TAG:product_uid=adab4424-2f25-4dc7-92ff-29bd000c0002
+TAG:material_package_umid=0x060A2B340101010501010D001300
+TAG:comment_test=value
+TAG:timecode=01:00:00:00
+[/FORMAT]
-- 
2.25.1

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

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

[FFmpeg-devel] [PATCH 3/3] tests/fate-run.sh: Don't overlook errors from md5 tests

2020-12-04 Thread Andreas Rheinhardt
The md5 test up until now ignored errors from ffmpeg (the cli) and just
md5'ed whatever ffmpeg has output; while testing scenarios in which
ffmpeg fails has its merits, errors should not be overlooked by default;
doing so also reduces the effectiveness of sanitizers as errors from
them are ignored. This has happened with a memleak in the AV1 decoder.

Signed-off-by: Andreas Rheinhardt 
---
 tests/fate-run.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/fate-run.sh b/tests/fate-run.sh
index 58d5fdbb60..b69176f7c3 100755
--- a/tests/fate-run.sh
+++ b/tests/fate-run.sh
@@ -158,7 +158,7 @@ md5pipe(){
 md5(){
 encfile="${outdir}/${test}.out"
 cleanfiles="$cleanfiles $encfile"
-ffmpeg -y "$@" $(target_path $encfile)
+ffmpeg -y "$@" $(target_path $encfile) || return
 do_md5sum $encfile | awk '{print $1}'
 }
 
-- 
2.25.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/3] avcodec/av1dec: Fix leak in case of failure

2020-12-04 Thread James Almer

On 12/4/2020 7:57 PM, Andreas Rheinhardt wrote:

A reference to an AV1RawFrameHeader and consequently the
AV1RawFrameHeader itself and everything it has a reference to leak
if the hardware has no AV1 decoding capabilities.


It looks like it can happen if get_buffer() fails even with hardware 
support.



It happens e.g.
in the cbs-av1-av1-1-b8-02-allintra FATE-test; it has just been masked
because the return value of ffmpeg (which indicates failure when using
Valgrind or ASAN) is ignored when doing tests of type md5.

Signed-off-by: Andreas Rheinhardt 
---
I switched the av_buffer_ref() and update_context_with_frame_header()
because the latter does not need any cleanup on failure.

Also notice that actual decoding with this patch applied is completely
untested.

  libavcodec/av1dec.c | 20 +++-
  1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
index 1589b8f0c0..d7b2ac9d46 100644
--- a/libavcodec/av1dec.c
+++ b/libavcodec/av1dec.c
@@ -674,20 +674,20 @@ static int av1_frame_alloc(AVCodecContext *avctx, 
AV1Frame *f)
  AVFrame *frame;
  int ret;
  
-f->header_ref = av_buffer_ref(s->header_ref);

-if (!f->header_ref)
-return AVERROR(ENOMEM);
-
-f->raw_frame_header = s->raw_frame_header;
-
  ret = update_context_with_frame_header(avctx, header);
  if (ret < 0) {
  av_log(avctx, AV_LOG_ERROR, "Failed to update context with frame 
header\n");
  return ret;
  }
  
+f->header_ref = av_buffer_ref(s->header_ref);

+if (!f->header_ref)
+return AVERROR(ENOMEM);
+
+f->raw_frame_header = s->raw_frame_header;
+
  if ((ret = ff_thread_get_buffer(avctx, &f->tf, AV_GET_BUFFER_FLAG_REF)) < 
0)
-return ret;
+goto fail;
  
  frame = f->tf.f;

  frame->key_frame = header->frame_type == AV1_FRAME_KEY;
@@ -710,8 +710,10 @@ static int av1_frame_alloc(AVCodecContext *avctx, AV1Frame 
*f)
  if (hwaccel->frame_priv_data_size) {
  f->hwaccel_priv_buf =
  av_buffer_allocz(hwaccel->frame_priv_data_size);
-if (!f->hwaccel_priv_buf)
+if (!f->hwaccel_priv_buf) {
+ret = AVERROR(ENOMEM);
  goto fail;
+}
  f->hwaccel_picture_private = f->hwaccel_priv_buf->data;
  }
  }
@@ -719,7 +721,7 @@ static int av1_frame_alloc(AVCodecContext *avctx, AV1Frame 
*f)
  
  fail:


Just to be safe, add a ret = 0 above this line.


  av1_frame_unref(avctx, f);
-return AVERROR(ENOMEM);
+return ret;
  }
  
  static int set_output_frame(AVCodecContext *avctx, AVFrame *frame,


LGTM, and while unrelated to this fix, this also reveals that in some 
scenarios, decoding without hardware support reaches this point, when 
it's meant to abort when parsing the sequence header and being unable to 
set up a hardware pixel format in avctx.


Looks like when parsing a second sequence header (Like in the second 
keyframe) where s->pix_fmt is already set to a software format, 
get_pixel_format() is not called, and so decoding proceeds to deal with 
frames.

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

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

Re: [FFmpeg-devel] [PATCH 1/3] avcodec/av1dec: Fix leak in case of failure

2020-12-04 Thread Andreas Rheinhardt
James Almer:
> On 12/4/2020 7:57 PM, Andreas Rheinhardt wrote:
>> A reference to an AV1RawFrameHeader and consequently the
>> AV1RawFrameHeader itself and everything it has a reference to leak
>> if the hardware has no AV1 decoding capabilities.
> 
> It looks like it can happen if get_buffer() fails even with hardware
> support.
> 
>> It happens e.g.
>> in the cbs-av1-av1-1-b8-02-allintra FATE-test; it has just been masked
>> because the return value of ffmpeg (which indicates failure when using
>> Valgrind or ASAN) is ignored when doing tests of type md5.
>>
>> Signed-off-by: Andreas Rheinhardt 
>> ---
>> I switched the av_buffer_ref() and update_context_with_frame_header()
>> because the latter does not need any cleanup on failure.
>>
>> Also notice that actual decoding with this patch applied is completely
>> untested.
>>
>>   libavcodec/av1dec.c | 20 +++-
>>   1 file changed, 11 insertions(+), 9 deletions(-)
>>
>> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
>> index 1589b8f0c0..d7b2ac9d46 100644
>> --- a/libavcodec/av1dec.c
>> +++ b/libavcodec/av1dec.c
>> @@ -674,20 +674,20 @@ static int av1_frame_alloc(AVCodecContext
>> *avctx, AV1Frame *f)
>>   AVFrame *frame;
>>   int ret;
>>   -    f->header_ref = av_buffer_ref(s->header_ref);
>> -    if (!f->header_ref)
>> -    return AVERROR(ENOMEM);
>> -
>> -    f->raw_frame_header = s->raw_frame_header;
>> -
>>   ret = update_context_with_frame_header(avctx, header);
>>   if (ret < 0) {
>>   av_log(avctx, AV_LOG_ERROR, "Failed to update context with
>> frame header\n");
>>   return ret;
>>   }
>>   +    f->header_ref = av_buffer_ref(s->header_ref);
>> +    if (!f->header_ref)
>> +    return AVERROR(ENOMEM);
>> +
>> +    f->raw_frame_header = s->raw_frame_header;
>> +
>>   if ((ret = ff_thread_get_buffer(avctx, &f->tf,
>> AV_GET_BUFFER_FLAG_REF)) < 0)
>> -    return ret;
>> +    goto fail;
>>     frame = f->tf.f;
>>   frame->key_frame = header->frame_type == AV1_FRAME_KEY;
>> @@ -710,8 +710,10 @@ static int av1_frame_alloc(AVCodecContext *avctx,
>> AV1Frame *f)
>>   if (hwaccel->frame_priv_data_size) {
>>   f->hwaccel_priv_buf =
>>   av_buffer_allocz(hwaccel->frame_priv_data_size);
>> -    if (!f->hwaccel_priv_buf)
>> +    if (!f->hwaccel_priv_buf) {
>> +    ret = AVERROR(ENOMEM);
>>   goto fail;
>> +    }
>>   f->hwaccel_picture_private = f->hwaccel_priv_buf->data;
>>   }
>>   }
>> @@ -719,7 +721,7 @@ static int av1_frame_alloc(AVCodecContext *avctx,
>> AV1Frame *f)
>>     fail:
> 
> Just to be safe, add a ret = 0 above this line.
> 
There is a "return 0;" above this line (the non-error path does not
include this av1_frame_unref()), so this makes no sense.

>>   av1_frame_unref(avctx, f);
>> -    return AVERROR(ENOMEM);
>> +    return ret;
>>   }
>>     static int set_output_frame(AVCodecContext *avctx, AVFrame *frame,
> 
> LGTM, and while unrelated to this fix, this also reveals that in some
> scenarios, decoding without hardware support reaches this point, when
> it's meant to abort when parsing the sequence header and being unable to
> set up a hardware pixel format in avctx.
> 
Yeah, I get a screen full of error messages from this decoder.

> Looks like when parsing a second sequence header (Like in the second
> keyframe) where s->pix_fmt is already set to a software format,
> get_pixel_format() is not called, and so decoding proceeds to deal with
> frames.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 1/3] avcodec/av1dec: Fix leak in case of failure

2020-12-04 Thread James Almer

On 12/4/2020 8:19 PM, Andreas Rheinhardt wrote:

James Almer:

On 12/4/2020 7:57 PM, Andreas Rheinhardt wrote:

A reference to an AV1RawFrameHeader and consequently the
AV1RawFrameHeader itself and everything it has a reference to leak
if the hardware has no AV1 decoding capabilities.


It looks like it can happen if get_buffer() fails even with hardware
support.


It happens e.g.
in the cbs-av1-av1-1-b8-02-allintra FATE-test; it has just been masked
because the return value of ffmpeg (which indicates failure when using
Valgrind or ASAN) is ignored when doing tests of type md5.

Signed-off-by: Andreas Rheinhardt 
---
I switched the av_buffer_ref() and update_context_with_frame_header()
because the latter does not need any cleanup on failure.

Also notice that actual decoding with this patch applied is completely
untested.

   libavcodec/av1dec.c | 20 +++-
   1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
index 1589b8f0c0..d7b2ac9d46 100644
--- a/libavcodec/av1dec.c
+++ b/libavcodec/av1dec.c
@@ -674,20 +674,20 @@ static int av1_frame_alloc(AVCodecContext
*avctx, AV1Frame *f)
   AVFrame *frame;
   int ret;
   -    f->header_ref = av_buffer_ref(s->header_ref);
-    if (!f->header_ref)
-    return AVERROR(ENOMEM);
-
-    f->raw_frame_header = s->raw_frame_header;
-
   ret = update_context_with_frame_header(avctx, header);
   if (ret < 0) {
   av_log(avctx, AV_LOG_ERROR, "Failed to update context with
frame header\n");
   return ret;
   }
   +    f->header_ref = av_buffer_ref(s->header_ref);
+    if (!f->header_ref)
+    return AVERROR(ENOMEM);
+
+    f->raw_frame_header = s->raw_frame_header;
+
   if ((ret = ff_thread_get_buffer(avctx, &f->tf,
AV_GET_BUFFER_FLAG_REF)) < 0)
-    return ret;
+    goto fail;
     frame = f->tf.f;
   frame->key_frame = header->frame_type == AV1_FRAME_KEY;
@@ -710,8 +710,10 @@ static int av1_frame_alloc(AVCodecContext *avctx,
AV1Frame *f)
   if (hwaccel->frame_priv_data_size) {
   f->hwaccel_priv_buf =
   av_buffer_allocz(hwaccel->frame_priv_data_size);
-    if (!f->hwaccel_priv_buf)
+    if (!f->hwaccel_priv_buf) {
+    ret = AVERROR(ENOMEM);
   goto fail;
+    }
   f->hwaccel_picture_private = f->hwaccel_priv_buf->data;
   }
   }
@@ -719,7 +721,7 @@ static int av1_frame_alloc(AVCodecContext *avctx,
AV1Frame *f)
     fail:


Just to be safe, add a ret = 0 above this line.


There is a "return 0;" above this line (the non-error path does not
include this av1_frame_unref()), so this makes no sense.


Ah true, missed it. For some reason i assumed it was written the same 
way as some bsfs where it falls through. Nevermind then.





   av1_frame_unref(avctx, f);
-    return AVERROR(ENOMEM);
+    return ret;
   }
     static int set_output_frame(AVCodecContext *avctx, AVFrame *frame,


LGTM, and while unrelated to this fix, this also reveals that in some
scenarios, decoding without hardware support reaches this point, when
it's meant to abort when parsing the sequence header and being unable to
set up a hardware pixel format in avctx.


Yeah, I get a screen full of error messages from this decoder.


You'll get errors no matter what, but the idea is that they should not 
be get_buffer() ones, since they are a lot noisier.





Looks like when parsing a second sequence header (Like in the second
keyframe) where s->pix_fmt is already set to a software format,
get_pixel_format() is not called, and so decoding proceeds to deal with
frames.

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

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



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

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

[FFmpeg-devel] [PATCH] avformat/mxfenc: Discard audio until valid video has been received

2020-12-04 Thread Andreas Rheinhardt
Normally, video packets are muxed before audio packets for mxf (there is
a dedicated interleave function for this); furthermore the first (video)
packet triggers writing the actual header. Yet when the first video packet
fails the checks performed on it, codec_ul (a value set based upon
properties of the bitstream which necessitates actually inspecting
packets) may be NULL; the next packet written (an audio packet) will then
trigger outputting the header and this leads to a segfault because
codec_ul is dereferenced (this is ticket #7993). Therefore this commit
discards audio packets until a valid video packet has been received,
fixing said ticket.

Signed-off-by: Andreas Rheinhardt 
---
This implements what I had originally in mind to fix said ticket and it
is also what Baptiste Coudurier wanted [1]. The main obstacle in fixing
it was fixing the mxf-d10-user-comments test (with this patch, the old
test would output nothing at all).

[1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2020-March/257900.html

 libavformat/mxfenc.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
index d8678c9d25..2b0b7342a7 100644
--- a/libavformat/mxfenc.c
+++ b/libavformat/mxfenc.c
@@ -2843,6 +2843,13 @@ static int mxf_write_packet(AVFormatContext *s, AVPacket 
*pkt)
 MXFIndexEntry ie = {0};
 int err;
 
+if (!mxf->header_written && pkt->stream_index != 0 &&
+s->oformat != &ff_mxf_opatom_muxer) {
+av_log(s, AV_LOG_ERROR, "Received non-video packet before "
+"header has been written\n");
+return AVERROR_INVALIDDATA;
+}
+
 if (!mxf->cbr_index && !mxf->edit_unit_byte_count && 
!(mxf->edit_units_count % EDIT_UNITS_PER_BODY)) {
 if ((err = av_reallocp_array(&mxf->index_entries, mxf->edit_units_count
  + EDIT_UNITS_PER_BODY, 
sizeof(*mxf->index_entries))) < 0) {
-- 
2.25.1

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

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

[FFmpeg-devel] [PATCH] fate: fix fate-filter-hqx on BE arches

2020-12-04 Thread Andriy Gelman
From: Andriy Gelman 

One of the inputs to the fate test has an rgba pixel format which needs
to be converted to rgb32 (argb on BE) for the hqx filter. Because auto
scaling in the fate test is disabled, this needs a separate scale
filter.

Signed-off-by: Andriy Gelman 
---
 tests/fate/filter-video.mak | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/fate/filter-video.mak b/tests/fate/filter-video.mak
index f9059f42f8..a2967de3ae 100644
--- a/tests/fate/filter-video.mak
+++ b/tests/fate/filter-video.mak
@@ -423,9 +423,9 @@ fate-filter-overlay-dvdsub-2397: CMD = framecrc 
-auto_conversion_filters -flags
 
 FATE_FILTER_HQX-$(call ALLYES, IMAGE2_DEMUXER PNG_DECODER HQX_FILTER) = 
fate-filter-hq2x fate-filter-hq3x fate-filter-hq4x
 FATE_FILTER_SAMPLES-yes += $(FATE_FILTER_HQX-yes)
-fate-filter-hq2x: CMD = framecrc -i $(TARGET_SAMPLES)/filter/pixelart%d.png 
-vf scale,hqx=2 -pix_fmt bgra
-fate-filter-hq3x: CMD = framecrc -i $(TARGET_SAMPLES)/filter/pixelart%d.png 
-vf scale,hqx=3 -pix_fmt bgra
-fate-filter-hq4x: CMD = framecrc -i $(TARGET_SAMPLES)/filter/pixelart%d.png 
-vf scale,hqx=4 -pix_fmt bgra
+fate-filter-hq2x: CMD = framecrc -i $(TARGET_SAMPLES)/filter/pixelart%d.png 
-vf scale,format=rgb32,hqx=2,scale,format=bgra
+fate-filter-hq3x: CMD = framecrc -i $(TARGET_SAMPLES)/filter/pixelart%d.png 
-vf scale,format=rgb32,hqx=3,scale,format=bgra
+fate-filter-hq4x: CMD = framecrc -i $(TARGET_SAMPLES)/filter/pixelart%d.png 
-vf scale,format=rgb32,hqx=4,scale,format=bgra
 fate-filter-hqx: $(FATE_FILTER_HQX-yes)
 
 FATE_FILTER_XBR-$(call ALLYES, IMAGE2_DEMUXER PNG_DECODER XBR_FILTER) = 
fate-filter-2xbr fate-filter-3xbr fate-filter-4xbr
-- 
2.28.0

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

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

Re: [FFmpeg-devel] [PATCH] libavformat/mov.c: export vendor id as metadata

2020-12-04 Thread Gyan Doshi



On 05-12-2020 04:00 am, Michael Niedermayer wrote:

On Thu, Dec 03, 2020 at 12:40:40PM -0800, Thierry Foucu wrote:

On Wed, Nov 18, 2020 at 12:09 PM Thierry Foucu  wrote:


---
  libavformat/mov.c | 8 ++--
  tests/ref/fate/hapqa-extract-nosnappy-to-hapalphaonly-mov | 1 +
  tests/ref/fate/hapqa-extract-nosnappy-to-hapq-mov | 1 +
  tests/ref/fate/mov-zombie | 2 +-
  tests/ref/fate/rgb24-mkv  | 4 ++--
  5 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 2b90e31170..1ba1a748e8 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -2095,6 +2095,7 @@ static void mov_parse_stsd_video(MOVContext *c,
AVIOContext *pb,
  uint8_t codec_name[32] = { 0 };
  int64_t stsd_start;
  unsigned int len;
+uint32_t id = 0;

  /* The first 16 bytes of the video sample description are already
   * read in ff_mov_read_stsd_entries() */
@@ -2102,7 +2103,8 @@ static void mov_parse_stsd_video(MOVContext *c,
AVIOContext *pb,

  avio_rb16(pb); /* version */
  avio_rb16(pb); /* revision level */
-avio_rb32(pb); /* vendor */
+id = avio_rl32(pb); /* vendor */
+av_dict_set(&st->metadata, "vendor_id", av_fourcc2str(id), 0);
  avio_rb32(pb); /* temporal quality */
  avio_rb32(pb); /* spatial quality */

@@ -2150,10 +2152,12 @@ static void mov_parse_stsd_audio(MOVContext *c,
AVIOContext *pb,
  {
  int bits_per_sample, flags;
  uint16_t version = avio_rb16(pb);
+uint32_t id = 0;
  AVDictionaryEntry *compatible_brands = av_dict_get(c->fc->metadata,
"compatible_brands", NULL, AV_DICT_MATCH_CASE);

  avio_rb16(pb); /* revision level */
-avio_rb32(pb); /* vendor */
+id = avio_rl32(pb); /* vendor */
+av_dict_set(&st->metadata, "vendor_id", av_fourcc2str(id), 0);

  st->codecpar->channels  = avio_rb16(pb); /* channel count
*/
  st->codecpar->bits_per_coded_sample = avio_rb16(pb); /* sample size */
diff --git a/tests/ref/fate/hapqa-extract-nosnappy-to-hapalphaonly-mov
b/tests/ref/fate/hapqa-extract-nosnappy-to-hapalphaonly-mov
index 61af08aa23..04a965b12a 100644
--- a/tests/ref/fate/hapqa-extract-nosnappy-to-hapalphaonly-mov
+++ b/tests/ref/fate/hapqa-extract-nosnappy-to-hapalphaonly-mov
@@ -69,5 +69,6 @@ DISPOSITION:attached_pic=0
  DISPOSITION:timed_thumbnails=0
  TAG:language=eng
  TAG:handler_name=Module de gestion video
+TAG:vendor_id=FFMP
  TAG:encoder=HAPAlpha Only
  [/STREAM]
diff --git a/tests/ref/fate/hapqa-extract-nosnappy-to-hapq-mov
b/tests/ref/fate/hapqa-extract-nosnappy-to-hapq-mov
index 1fb31ec7f0..d9e5c94ffb 100644
--- a/tests/ref/fate/hapqa-extract-nosnappy-to-hapq-mov
+++ b/tests/ref/fate/hapqa-extract-nosnappy-to-hapq-mov
@@ -69,5 +69,6 @@ DISPOSITION:attached_pic=0
  DISPOSITION:timed_thumbnails=0
  TAG:language=eng
  TAG:handler_name=Module de gestion video
+TAG:vendor_id=FFMP
  TAG:encoder=HAPQ
  [/STREAM]
diff --git a/tests/ref/fate/mov-zombie b/tests/ref/fate/mov-zombie
index 1a6625bc8f..c566b14d47 100644
--- a/tests/ref/fate/mov-zombie
+++ b/tests/ref/fate/mov-zombie
@@ -194,5 +194,5 @@
frame|media_type=video|stream_index=0|key_frame=0|pkt_pts=188623|pkt_pts_time=2.

  
packet|codec_type=video|stream_index=0|pts=197632|pts_time=2.195911|dts=191625|dts_time=2.129167|duration=3003|duration_time=0.033367|convergence_duration=N/A|convergence_duration_time=N/A|size=580|pos=101820|flags=__
  
frame|media_type=video|stream_index=0|key_frame=0|pkt_pts=191626|pkt_pts_time=2.129178|pkt_dts=N/A|pkt_dts_time=N/A|best_effort_timestamp=191626|best_effort_timestamp_time=2.129178|pkt_duration=3003|pkt_duration_time=0.033367|pkt_pos=99180|pkt_size=1666|width=160|height=240|pix_fmt=yuv420p|sample_aspect_ratio=2:1|pict_type=P|coded_picture_number=63|display_picture_number=0|interlaced_frame=0|top_field_first=0|repeat_pict=0|color_range=tv|color_space=smpte170m|color_primaries=smpte170m|color_transfer=bt709|chroma_location=topleftside_data|side_data_type=H.26[45]
User Data Unregistered SEI message

-stream|index=0|codec_name=h264|profile=77|codec_type=video|codec_time_base=212521/12744000|codec_tag_string=avc1|codec_tag=0x31637661|width=160|height=240|coded_width=160|coded_height=240|closed_captions=0|has_b_frames=1|sample_aspect_ratio=2:1|display_aspect_ratio=4:3|pix_fmt=yuv420p|level=12|color_range=tv|color_space=smpte170m|color_transfer=bt709|color_primaries=smpte170m|chroma_location=topleft|field_order=unknown|timecode=N/A|refs=2|is_avc=true|nal_length_size=4|id=N/A|r_frame_rate=3/1001|avg_frame_rate=6372000/212521|time_base=1/9|start_pts=0|start_time=0.00|duration_ts=2125200|duration=23.61|bit_rate=333874|max_bit_rate=N/A|bits_per_raw_sample=8|nb_frames=708|nb_read_frames=65|nb_read_packets=66|disposition:default=1|disposition:dub=0|disposition:original=0|disposition:comment=0|disposition:lyrics=0|disposition:karaoke=0|disposition:forced=0|dispos