Re: [FFmpeg-devel] [PATCH 2/2] avfilter/af_astats: Remove fraction part of integer metadata entries
On 08.04.2021 11:34, Nicolas George wrote: Anton Khirnov (12021-04-08): Does this mean that there are no stability guarantees for metadata exported by filters? We can have stability for the components that are good enough to be stable, and no stability yet for components that need enhancing. Indeed I should at least increment the minor (or micro?) version for libavfilter. And for the metadata changes my goal was to make some obvious things like "lavfi.astats.Bit_depth=24.00" less weird. I didn't dare to touch other things. As the astats filter metadata is not tested by FATE my guess was that stability is not yet a high priority. Regards, Tobias ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH V7 4/6] lavu: add side data AV_FRAME_DATA_BOUNDING_BOXES
Apr 9, 2021, 06:12 by yejun@intel.com:
>
>
>> -Original Message-
>> From: ffmpeg-devel On Behalf Of Lynne
>> Sent: 2021年4月9日 0:57
>> To: FFmpeg development discussions and patches
>> Subject: Re: [FFmpeg-devel] [PATCH V7 4/6] lavu: add side data
>> AV_FRAME_DATA_BOUNDING_BOXES
>>
>
> First of all, thanks for the quick replies, I see, all the
> discussions/comments are to
> make this patch better, thank you.
>
>> >> >
>> >> >> >> > +
>> >> >> >> > +typedef struct AVBoundingBoxHeader {
>> >> >> >> > +/**
>> >> >> >> > + * Information about how the bounding box is generated.
>> >> >> >> > + * for example, the DNN model name.
>> >> >> >> > + */
>> >> >> >> > +char source[128];
>> >> >> >> > +
>> >> >> >> > +/**
>> >> >> >> > + * The size of frame when it is detected.
>> >> >> >> > + */
>> >> >> >> > +int frame_width;
>> >> >> >> > +int frame_height;
>> >> >> >> >
>> >> >> >>
>> >> >> >> Why? This side data is attached to AVFrames only, where we
>> >> >> >> already have width and height.
>> >> >> >>
>> >> >> >
>> >> >> > The detection result will be used by other filters, for example,
>> >> >> > dnn_classify (see https://github.com/guoyejun/ffmpeg/tree/classify).
>> >> >> >
>> >> >> > The filter dnn_detect detects all the objects (cat, dog, person ...)
>> >> >> > in a
>> >> >> > frame, while dnn_classify classifies one detected object (for
>> >> >> > example,
>> >> person)
>> >> >> > for its attribute (for example, emotion, etc.)
>> >> >> >
>> >> >> > The filter dnn_classify have to check if the frame size is changed
>> >> >> > after
>> >> >> > it is detected, to handle the below filter chain:
>> >> >> > dnn_detect -> scale -> dnn_classify
>> >> >> >
>> >> >>
>> >> >> This doesn't look good. Why is dnn_classify needing to know
>> >> >> the original frame size at all?
>> >> >>
>> >> >
>> >> > For example, the original size of the frame is 100*100, and dnn_detect
>> >> > detects a face at place (10, 10) -> (30, 40), such data will be saved in
>> >> > AVBoundingBox.top/left/right/bottom.
>> >> >
>> >> > Then, the frame is scaled into 50*50.
>> >> >
>> >> > Then, dnn_classify is used to analyze the emotion of the face, it needs
>> >> > to
>> >> > know the frame size (100*100) when it is detected, otherwise, it does
>> >> > not
>> >> > work with just (10,10), (30,40) and 50*50.
>> >> >
>> >>
>> >> Why can't the scale filter also rescale this side data as well?
>> >>
>> >
>> > I'm afraid that we could not make sure all such filters (including filters
>> > in the
>> > future) to do the rescale. And in the previous discussion, I got to know
>> > that
>> > 'many other existing side-data types are invalidated by scaling'.
>> >
>> > So, we need frame_width and frame_height here.
>> >
>>
>> No, you don't. You just need to make sure filters which change resolution
>> or do cropping also change the side data parameters.
>> It's called maintainership. As-is, this won't even work with cropping,
>> only with basic aspect ratio preserving scaling.
>> For the lack of a better term, this is a hack.
>>
>
> As discussed in previous email, for the frame size change case, dnn_classify
> (and other filters which use the detection result, for example drawbox) can
> just output a warning message to tell user what happens, and don't do the
> classification, otherwise, it will give a wrong/weird result which makes the
> user confused.
>
>>
>> I would accept just specifying that if the frame dimensions are
>> altered in any way, the side-data is no longer valid and it's up
>> to users to figure that out by out of bound coordinates.
>> This is what we currently do with video_enc_params.
>>
>
> frame_width/frame_height is not perfect (for the cases such as: scale down
> + crop + scale up to the same size), but it provides more info than the
> checking
> of 'out of bound coordinates'. There are many other possible issues when the
> coordinates are within the frame.
>
> If we think we'd better not let user get more info from the warning message,
> I'm ok to remove them.
>
> I'll remove them if there's another comment supporting the removal, and
> there's no objection.
>
We definitely shouldn't include variables in public API structs
that only serve to print a warning if they don't match.
Especially one that's fragile and flawed like this one.
Users should know that scaling or altering a frame could break
this side data, and filters could detect obvious out of bounds
results and report them.
In the meantime, the main scaling and cropping filters could
receive separate patches to preserve metadata at a later data.
This is how the avframe cropping field work - they're just metadata,
and cropping/scaling filters update those.
>> >> >> >> > diff --git a/libavutil/frame.h b/libavutil/frame.h
>> >> >> >> > index a5ed91b20a..41e22de02a 100644
>> >> >> >> > --- a/libavutil/frame.h
>> >> >> >> > +++ b/libavutil/frame.h
>> >> >> >> > @@ -198,6 +198,13 @@ enum AVFrameSideDataType {
>> >> >
[FFmpeg-devel] [PATCH] avfilter/vf_overlay_cuda: check ctx->hwctx->internal null pointer
From: Steven Liu
because it will cannot get ctx->hwctx->internal value when
get ctx->hwctx->internal->cuda_dl value. so should check if internal is
null.
before patch:
liuqi@chinaffmpeg:~/multimedia/build$ ./ffmpeg_g -hwaccel_device 0 -hwaccel
cuda -pix_fmt nv12 -s 1920x1080 -i output.yuv -hwaccel_device 0 -hwaccel cuda
-pix_fmt nv12 -s 800x480 -i output_small.yuv -filter_complex
"[0:v]hwupload_cuda[a];[1:v]hwupload_cuda[b];[a][b]overlay_cuda=x=0:y=0[o];[o]hwdownload[o1]"
-map "[o1]" -pix_fmt nv12 -y -t 1 xxx.yuv
ffmpeg version N-101879-g13bf797ced Copyright (c) 2000-2021 the FFmpeg
developers
built with gcc 9 (Ubuntu 9.3.0-17ubuntu1~20.04)
configuration: --cc='ccache cc' --enable-libx264 --enable-libx265
--enable-libopus --enable-libwebp --enable-libvpx --enable-ffnvcodec
--enable-vaapi --enable-opengl --enable-nonfree --enable-gpl
--enable-libfdk-aac --enable-libfreetype --enable-cuda --enable-libnpp
--enable-cuda-nvcc --enable-cuda-llvm
libavutil 56. 72.100 / 56. 72.100
libavcodec 58.135.100 / 58.135.100
libavformat58. 78.100 / 58. 78.100
libavdevice58. 14.100 / 58. 14.100
libavfilter 7.111.100 / 7.111.100
libswscale 5. 10.100 / 5. 10.100
libswresample 3. 10.100 / 3. 10.100
libpostproc55. 10.100 / 55. 10.100
[rawvideo @ 0x5584b7a9e4c0] Estimating duration from bitrate, this may be
inaccurate
Input #0, rawvideo, from 'output.yuv':
Duration: 00:01:54.64, start: 0.00, bitrate: 622080 kb/s
Stream #0:0: Video: rawvideo (NV12 / 0x3231564E), nv12, 1920x1080, 622080
kb/s, 25 tbr, 25 tbn, 25 tbc
[rawvideo @ 0x5584b7aa30c0] Estimating duration from bitrate, this may be
inaccurate
Input #1, rawvideo, from 'output_small.yuv':
Duration: 00:04:07.16, start: 0.00, bitrate: 115200 kb/s
Stream #1:0: Video: rawvideo (NV12 / 0x3231564E), nv12, 800x480, 115200 kb/s,
25 tbr, 25 tbn, 25 tbc
Stream mapping:
Stream #0:0 (rawvideo) -> hwupload_cuda
Stream #1:0 (rawvideo) -> hwupload_cuda
hwdownload -> Stream #0:0 (rawvideo)
Press [q] to stop, [?] for help
Output #0, rawvideo, to 'xxx.yuv':
Metadata:
encoder : Lavf58.78.100
Stream #0:0: Video: rawvideo (NV12 / 0x3231564E), nv12(progressive),
1920x1080, q=2-31, 622080 kb/s, 25 fps, 25 tbn (default)
Metadata:
encoder : Lavc58.135.100 rawvideo
frame= 25 fps=0.0 q=-0.0 Lsize= 75938kB time=00:00:01.00
bitrate=622080.0kbits/s speed=6.11x
video:75938kB audio:0kB subtitle:0kB other streams:0kB global headers:0kB
muxing overhead: 0.00%
Segmentation fault (core dumped)
This patch can fix the Segmentation fault problem.
Signed-off-by: Steven Liu
---
libavfilter/vf_overlay_cuda.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavfilter/vf_overlay_cuda.c b/libavfilter/vf_overlay_cuda.c
index f6ee43e929..4d8617475f 100644
--- a/libavfilter/vf_overlay_cuda.c
+++ b/libavfilter/vf_overlay_cuda.c
@@ -249,7 +249,7 @@ static av_cold void overlay_cuda_uninit(AVFilterContext
*avctx)
ff_framesync_uninit(&ctx->fs);
-if (ctx->hwctx && ctx->cu_module) {
+if (ctx->hwctx && ctx->cu_module && ctx->hwctx->internal) {
CUcontext dummy;
CudaFunctions *cu = ctx->hwctx->internal->cuda_dl;
CHECK_CU(cu->cuCtxPushCurrent(ctx->cu_ctx));
--
2.25.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] [GSoC 2021] about proposal
Thanks! I will pay attention to it next time! ___ 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/proresenc_kostya: Remove harmful check
Andreas Rheinhardt:
> The ProRes encoder allocates huge worst-case buffers just to be safe;
> and for huge resolutions (8k in this case) these can be so big that the
> number of bits does no longer fit into a (signed 32-bit) int; this means
> that one must no longer use the parts of the PutBits API that deal with
> bit counters. Yet proresenc_kostya did it, namely for a check about
> whether we are already beyond the end. Yet this check is unnecessary
> nowadays, because the PutBits API comes with automatic checks (with
> a log message and a av_assert2() in put_bits() and an av_assert0() in
> flush_put_bits()), so this is unnecessary. So simply remove the check.
>
> Fixes ticket #9173.
>
> Signed-off-by: Andreas Rheinhardt
> ---
> I actually pondered removing said check when I wrote
> f9d1528fc900dac4975ce785dd95004daeacec39 (which stopped using
> intermediate bitcounts).
>
> libavcodec/proresenc_kostya.c | 5 -
> 1 file changed, 5 deletions(-)
>
> diff --git a/libavcodec/proresenc_kostya.c b/libavcodec/proresenc_kostya.c
> index d8edd12f34..54fc6707a1 100644
> --- a/libavcodec/proresenc_kostya.c
> +++ b/libavcodec/proresenc_kostya.c
> @@ -619,11 +619,6 @@ static int encode_slice(AVCodecContext *avctx, const
> AVFrame *pic,
> flush_put_bits(pb);
> sizes[i] = put_bytes_output(pb) - total_size;
> total_size = put_bytes_output(pb);
> -if (put_bits_left(pb) < 0) {
> -av_log(avctx, AV_LOG_ERROR,
> - "Underestimated required buffer size.\n");
> -return AVERROR_BUG;
> -}
> }
> return total_size;
> }
>
Will apply tomorrow unless there are objections.
- Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/9] avcodec/mpegvideo_enc: Make AMV encoder usable without MJPEG encoder
Andreas Rheinhardt:
> Up until now the relevant checks all checked for the existence of the
> MJPEG encoder only.
>
> Signed-off-by: Andreas Rheinhardt
> ---
> libavcodec/mpegvideo_enc.c | 21 +
> 1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
> index 79c4071bad..7c9d1bd894 100644
> --- a/libavcodec/mpegvideo_enc.c
> +++ b/libavcodec/mpegvideo_enc.c
> @@ -789,17 +789,17 @@ FF_ENABLE_DEPRECATION_WARNINGS
> avctx->delay = s->low_delay ? 0 : (s->max_b_frames + 1);
> s->rtp_mode = 1;
> break;
> +#if CONFIG_MJPEG_ENCODER || CONFIG_AMV_ENCODER
> case AV_CODEC_ID_MJPEG:
> case AV_CODEC_ID_AMV:
> s->out_format = FMT_MJPEG;
> s->intra_only = 1; /* force intra only for jpeg */
> -if (!CONFIG_MJPEG_ENCODER)
> -return AVERROR_ENCODER_NOT_FOUND;
> if ((ret = ff_mjpeg_encode_init(s)) < 0)
> return ret;
> avctx->delay = 0;
> s->low_delay = 1;
> break;
> +#endif
> case AV_CODEC_ID_SPEEDHQ:
> s->out_format = FMT_SPEEDHQ;
> s->intra_only = 1; /* force intra only for SHQ */
> @@ -1097,7 +1097,7 @@ av_cold int ff_mpv_encode_end(AVCodecContext *avctx)
> ff_rate_control_uninit(s);
>
> ff_mpv_common_end(s);
> -if (CONFIG_MJPEG_ENCODER &&
> +if ((CONFIG_MJPEG_ENCODER || CONFIG_AMV_ENCODER) &&
> s->out_format == FMT_MJPEG)
> ff_mjpeg_encode_close(s);
>
> @@ -1926,7 +1926,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
>
> frame_end(s);
>
> -if (CONFIG_MJPEG_ENCODER && s->out_format == FMT_MJPEG)
> + if ((CONFIG_MJPEG_ENCODER || CONFIG_AMV_ENCODER) && s->out_format ==
> FMT_MJPEG)
> ff_mjpeg_encode_picture_trailer(&s->pb, s->header_bits);
>
> if (avctx->rc_buffer_size) {
> @@ -2596,11 +2596,12 @@ static av_always_inline void
> encode_mb_internal(MpegEncContext *s,
> if (CONFIG_H263_ENCODER)
> ff_h263_encode_mb(s, s->block, motion_x, motion_y);
> break;
> +#if CONFIG_MJPEG_ENCODER || CONFIG_AMV_ENCODER
> case AV_CODEC_ID_MJPEG:
> case AV_CODEC_ID_AMV:
> -if (CONFIG_MJPEG_ENCODER)
> -ff_mjpeg_encode_mb(s, s->block);
> +ff_mjpeg_encode_mb(s, s->block);
> break;
> +#endif
> case AV_CODEC_ID_SPEEDHQ:
> if (CONFIG_SPEEDHQ_ENCODER)
> ff_speedhq_encode_mb(s, s->block);
> @@ -2853,7 +2854,8 @@ static void write_slice_end(MpegEncContext *s){
> }
>
> ff_mpeg4_stuffing(&s->pb);
> -}else if(CONFIG_MJPEG_ENCODER && s->out_format == FMT_MJPEG){
> +} else if ((CONFIG_MJPEG_ENCODER || CONFIG_AMV_ENCODER) &&
> + s->out_format == FMT_MJPEG) {
> ff_mjpeg_encode_stuffing(s);
> } else if (CONFIG_SPEEDHQ_ENCODER && s->out_format == FMT_SPEEDHQ) {
> ff_speedhq_end_slice(s);
> @@ -3921,11 +3923,14 @@ static int encode_picture(MpegEncContext *s, int
> picture_number)
> s->mb_x = s->mb_y = 0;
> s->last_bits= put_bits_count(&s->pb);
> switch(s->out_format) {
> +#if CONFIG_MJPEG_ENCODER || CONFIG_AMV_ENCODER
> case FMT_MJPEG:
> -if (CONFIG_MJPEG_ENCODER && s->huffman != HUFFMAN_TABLE_OPTIMAL)
> +/* s->huffman == HUFFMAN_TABLE_OPTIMAL can only be true for MJPEG. */
> +if (!CONFIG_MJPEG_ENCODER || s->huffman != HUFFMAN_TABLE_OPTIMAL)
> ff_mjpeg_encode_picture_header(s->avctx, &s->pb,
> &s->intra_scantable,
> s->pred, s->intra_matrix,
> s->chroma_intra_matrix);
> break;
> +#endif
> case FMT_SPEEDHQ:
> if (CONFIG_SPEEDHQ_ENCODER)
> ff_speedhq_encode_picture_header(s);
>
Will apply the rest of this patchset tomorrow unless there are objections.
- Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avfilter/vf_overlay_cuda: check ctx->hwctx->internal null pointer
On 09.04.2021 12:34, Steven Liu wrote:
-if (ctx->hwctx && ctx->cu_module) {
+if (ctx->hwctx && ctx->cu_module && ctx->hwctx->internal) {
CUcontext dummy;
CudaFunctions *cu = ctx->hwctx->internal->cuda_dl;
CHECK_CU(cu->cuCtxPushCurrent(ctx->cu_ctx));
How can this happen at this point?
Did something else prematurely free the hwctx?
This makes me think that the filter is missing a buffer ref somewhere to
prevent things from getting freed too early.
smime.p7s
Description: S/MIME Cryptographic 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 V7 4/6] lavu: add side data AV_FRAME_DATA_BOUNDING_BOXES
> -Original Message-
> From: ffmpeg-devel On Behalf Of
> Lynne
> Sent: 2021年4月9日 18:03
> To: FFmpeg development discussions and patches
>
> Subject: Re: [FFmpeg-devel] [PATCH V7 4/6] lavu: add side data
> AV_FRAME_DATA_BOUNDING_BOXES
>
> Apr 9, 2021, 06:12 by yejun@intel.com:
>
> >
> >
> >> -Original Message-
> >> From: ffmpeg-devel On Behalf Of
> Lynne
> >> Sent: 2021年4月9日 0:57
> >> To: FFmpeg development discussions and patches
>
> >> Subject: Re: [FFmpeg-devel] [PATCH V7 4/6] lavu: add side data
> >> AV_FRAME_DATA_BOUNDING_BOXES
> >>
> >
> > First of all, thanks for the quick replies, I see, all the
> discussions/comments are to
> > make this patch better, thank you.
> >
> >> >> >
> >> >> >> >> > +
> >> >> >> >> > +typedef struct AVBoundingBoxHeader {
> >> >> >> >> > +/**
> >> >> >> >> > + * Information about how the bounding box is
> generated.
> >> >> >> >> > + * for example, the DNN model name.
> >> >> >> >> > + */
> >> >> >> >> > +char source[128];
> >> >> >> >> > +
> >> >> >> >> > +/**
> >> >> >> >> > + * The size of frame when it is detected.
> >> >> >> >> > + */
> >> >> >> >> > +int frame_width;
> >> >> >> >> > +int frame_height;
> >> >> >> >> >
> >> >> >> >>
> >> >> >> >> Why? This side data is attached to AVFrames only, where we
> >> >> >> >> already have width and height.
> >> >> >> >>
> >> >> >> >
> >> >> >> > The detection result will be used by other filters, for example,
> >> >> >> > dnn_classify (see
> https://github.com/guoyejun/ffmpeg/tree/classify).
> >> >> >> >
> >> >> >> > The filter dnn_detect detects all the objects (cat, dog, person
> >> >> >> > ...)
> in a
> >> >> >> > frame, while dnn_classify classifies one detected object (for
> example,
> >> >> person)
> >> >> >> > for its attribute (for example, emotion, etc.)
> >> >> >> >
> >> >> >> > The filter dnn_classify have to check if the frame size is changed
> after
> >> >> >> > it is detected, to handle the below filter chain:
> >> >> >> > dnn_detect -> scale -> dnn_classify
> >> >> >> >
> >> >> >>
> >> >> >> This doesn't look good. Why is dnn_classify needing to know
> >> >> >> the original frame size at all?
> >> >> >>
> >> >> >
> >> >> > For example, the original size of the frame is 100*100, and
> dnn_detect
> >> >> > detects a face at place (10, 10) -> (30, 40), such data will be saved
> >> >> > in
> >> >> > AVBoundingBox.top/left/right/bottom.
> >> >> >
> >> >> > Then, the frame is scaled into 50*50.
> >> >> >
> >> >> > Then, dnn_classify is used to analyze the emotion of the face, it
> needs to
> >> >> > know the frame size (100*100) when it is detected, otherwise, it
> does not
> >> >> > work with just (10,10), (30,40) and 50*50.
> >> >> >
> >> >>
> >> >> Why can't the scale filter also rescale this side data as well?
> >> >>
> >> >
> >> > I'm afraid that we could not make sure all such filters (including
> >> > filters
> in the
> >> > future) to do the rescale. And in the previous discussion, I got to
> know that
> >> > 'many other existing side-data types are invalidated by scaling'.
> >> >
> >> > So, we need frame_width and frame_height here.
> >> >
> >>
> >> No, you don't. You just need to make sure filters which change
> resolution
> >> or do cropping also change the side data parameters.
> >> It's called maintainership. As-is, this won't even work with cropping,
> >> only with basic aspect ratio preserving scaling.
> >> For the lack of a better term, this is a hack.
> >>
> >
> > As discussed in previous email, for the frame size change case,
> dnn_classify
> > (and other filters which use the detection result, for example drawbox)
> can
> > just output a warning message to tell user what happens, and don't do
> the
> > classification, otherwise, it will give a wrong/weird result which makes
> the
> > user confused.
> >
> >>
> >> I would accept just specifying that if the frame dimensions are
> >> altered in any way, the side-data is no longer valid and it's up
> >> to users to figure that out by out of bound coordinates.
> >> This is what we currently do with video_enc_params.
> >>
> >
> > frame_width/frame_height is not perfect (for the cases such as: scale
> down
> > + crop + scale up to the same size), but it provides more info than the
> checking
> > of 'out of bound coordinates'. There are many other possible issues
> when the
> > coordinates are within the frame.
> >
> > If we think we'd better not let user get more info from the warning
> message,
> > I'm ok to remove them.
> >
> > I'll remove them if there's another comment supporting the removal, and
> > there's no objection.
> >
>
> We definitely shouldn't include variables in public API structs
> that only serve to print a warning if they don't match.
Not just 'print a warning', it also impacts the behavior of dnn_classify.
> Especially one that's fragile and flawed like this one.
> Users should know that scaling or altering a frame could break
> this side data, and filters could det
[FFmpeg-devel] [PATCH] avformat: Add parityfec and ulpfec protocol
Parityfec (RFC 2733) and ulpfec (RFC 5109) generic FEC encoding for RTP streams.
Signed-off-by: Camille Gonnet
---
Changelog | 1 +
doc/general_contents.texi | 1 +
doc/protocols.texi| 106 +
libavformat/Makefile | 1 +
libavformat/fecrtp.c | 884 ++
libavformat/protocols.c | 1 +
libavformat/rtpproto.c| 2 +-
7 files changed, 995 insertions(+), 1 deletion(-)
create mode 100644 libavformat/fecrtp.c
diff --git a/Changelog b/Changelog
index a96e350e09..e83ec2c60d 100644
--- a/Changelog
+++ b/Changelog
@@ -83,6 +83,7 @@ version :
- msad video filter
- gophers protocol
- RIST protocol via librist
+- FEC RTP (RFC5109+RFC2733) protocol (encoding only)
version 4.3:
diff --git a/doc/general_contents.texi b/doc/general_contents.texi
index 33ece6e884..2b70dfb2ad 100644
--- a/doc/general_contents.texi
+++ b/doc/general_contents.texi
@@ -1368,6 +1368,7 @@ performance on systems without hardware floating point
support).
@multitable @columnfractions .4 .1
@item Name @tab Support
@item AMQP @tab E
+@item FECRTP @tab X
@item file @tab X
@item FTP @tab X
@item Gopher @tab X
diff --git a/doc/protocols.texi b/doc/protocols.texi
index d3f6cbefcf..403e7b415d 100644
--- a/doc/protocols.texi
+++ b/doc/protocols.texi
@@ -243,6 +243,112 @@ For example, to convert a GIF file given inline with
@command{ffmpeg}:
ffmpeg -i
"data:image/gif;base64,R0lGODdhCAAIAMIE//8AAP//AP///ywACAAIAAADF0gEDLojDgdGiJdJqUX02iB4E8Q9jUMkADs="
smiley.png
@end example
+@section fecrtp
+
+FEC for RTP Protocol. Supports ULP FEC (RFC 5109) and Parity FEC (RFC 2733)
+
+The FEC is a generic parity-check forward error correction mechanism
+for RTP Streams.
+
+This protocol must be used in conjunction with the @code{rtp} protocol.
+
+The required syntax is:
+@example
+-fec fecrtp=@var{option}=@var{val}... rtp://@var{hostname}:@var{port}
+@end example
+
+This protocol accepts the following options:
+@table @option
+
+@item payload_type=@var{n}
+Specify RTP payload type for FEC packets (0-127).
+Default value is 98.
+
+@item port=@var{n}
+Specify RTP port for FEC packets (0-127).
+Default value is (RTP port + 2).
+
+@item host=@var{destination}
+Specify RTP destination for FEC packets.
+If not provided, use same as RTP.
+
+@item kind=@var{kind}
+Set the kind of FEC to use. Default is @code{ulpfec}
+Accepts the following options:
+@table @samp
+@item parityfec
+parityfec (RFC 2733)
+@item ulpfec
+ulpfec (RFC 5109)
+@end table
+
+@item length=@var{n}
+Length of recovery, or 0 for full. (0-1472)
+For @code{kind=ulpfec} only.
+Default value is 0 (full).
+
+@item algorithm=@var{algorithm}
+Set the covered packets algorithm. Default is @code{simple}
+Accepts the following options:
+@table @samp
+@item simple
+Simple usage using @code{span},@code{every} and @code{delay},
+@item rotate
+Combine @code{span-1} packets in the last @code{span} packets in every way.@*
+With span=4 and packets a,b,c,d, will produce (a,b,c) (a,b,d) (a,c,d) (b,c,d).
+@item 1d
+Put the packets in a matrix and compute columns FEC (similar to ProMPEG).
+Use @code{col} and @code{row}.
+@end table
+
+@item span=@var{n}
+The number of RTP packets covered by each FEC packet.
+For @code{algorithm=simple} and @code{algorithm=rotate}.
+Default value is 2.
+
+@item every=@var{n}
+How often we add a FEC packet after a RTP packet (1-20).
+This must be a divider of span.
+For @code{algorithm=simple}.
+Default value is 2.
+
+@item delay=@var{n}
+How many RTP packet we should delay the sending of our FEC packet
+when it is ready (0-20)
+For @code{algorithm=simple}.
+Default value is 0.
+
+@item col=@var{n}
+Number of columns in the matrix.
+For @code{algorithm=1d}.
+Default value is 4.
+
+@item row=@var{n}
+Number of rows in the matrix.
+For @code{algorithm=1d}.
+Default value is 4.
+
+@end table
+
+Example usage:
+@itemize @bullet
+@item
+For 1/4 recovery, 25% overhead
+@example
+-fec fecrtp=span=4:every=4 rtp://@var{hostname}:@var{port}
+@end example
+@item
+For 1/10 recovery, 10% overhead in ulpfec
+@example
+-fec fecrtp=kind=ulpfec,span=10:every=10 rtp://@var{hostname}:@var{port}
+@end example
+@item
+For 1d fec with 4 rows and 4 columns
+@example
+-fec fecrtp=algorithm=1d:col=4:row=4 rtp://@var{hostname}:@var{port}
+@end example
+@end itemize
+
@section file
File access protocol.
diff --git a/libavformat/Makefile b/libavformat/Makefile
index 0f340f74a0..f393ce14ca 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -618,6 +618,7 @@ OBJS-$(CONFIG_CRYPTO_PROTOCOL) += crypto.o
OBJS-$(CONFIG_DATA_PROTOCOL) += data_uri.o
OBJS-$(CONFIG_FFRTMPCRYPT_PROTOCOL) += rtmpcrypt.o rtmpdigest.o rtmpdh.o
OBJS-$(CONFIG_FFRTMPHTTP_PROTOCOL) += rtmphttp.o
+OBJS-$(CONFIG_FECRTP_PROTOCOL) += fecrtp.o
OBJS-$(CONFIG_FILE_PROTOCOL) += file.o
OBJS-$(CONFIG_FTP_PROTOCOL)
Re: [FFmpeg-devel] [PATCH V7 4/6] lavu: add side data AV_FRAME_DATA_BOUNDING_BOXES
Em sex., 9 de abr. de 2021 às 01:13, Guo, Yejun escreveu:
>
>
>
> > -Original Message-
> > From: ffmpeg-devel On Behalf Of Lynne
> > Sent: 2021年4月9日 0:57
> > To: FFmpeg development discussions and patches
> > Subject: Re: [FFmpeg-devel] [PATCH V7 4/6] lavu: add side data
> > AV_FRAME_DATA_BOUNDING_BOXES
> >
>
> First of all, thanks for the quick replies, I see, all the
> discussions/comments are to
> make this patch better, thank you.
>
> > >> >
> > >> >> >> > +
> > >> >> >> > +typedef struct AVBoundingBoxHeader {
> > >> >> >> > +/**
> > >> >> >> > + * Information about how the bounding box is generated.
> > >> >> >> > + * for example, the DNN model name.
> > >> >> >> > + */
> > >> >> >> > +char source[128];
> > >> >> >> > +
> > >> >> >> > +/**
> > >> >> >> > + * The size of frame when it is detected.
> > >> >> >> > + */
> > >> >> >> > +int frame_width;
> > >> >> >> > +int frame_height;
> > >> >> >> >
> > >> >> >>
> > >> >> >> Why? This side data is attached to AVFrames only, where we
> > >> >> >> already have width and height.
> > >> >> >>
> > >> >> >
> > >> >> > The detection result will be used by other filters, for example,
> > >> >> > dnn_classify (see https://github.com/guoyejun/ffmpeg/tree/classify).
> > >> >> >
> > >> >> > The filter dnn_detect detects all the objects (cat, dog, person
> > >> >> > ...) in a
> > >> >> > frame, while dnn_classify classifies one detected object (for
> > >> >> > example,
> > >> person)
> > >> >> > for its attribute (for example, emotion, etc.)
> > >> >> >
> > >> >> > The filter dnn_classify have to check if the frame size is changed
> > >> >> > after
> > >> >> > it is detected, to handle the below filter chain:
> > >> >> > dnn_detect -> scale -> dnn_classify
> > >> >> >
> > >> >>
> > >> >> This doesn't look good. Why is dnn_classify needing to know
> > >> >> the original frame size at all?
> > >> >>
> > >> >
> > >> > For example, the original size of the frame is 100*100, and dnn_detect
> > >> > detects a face at place (10, 10) -> (30, 40), such data will be saved
> > >> > in
> > >> > AVBoundingBox.top/left/right/bottom.
> > >> >
> > >> > Then, the frame is scaled into 50*50.
> > >> >
> > >> > Then, dnn_classify is used to analyze the emotion of the face, it
> > >> > needs to
> > >> > know the frame size (100*100) when it is detected, otherwise, it does
> > >> > not
> > >> > work with just (10,10), (30,40) and 50*50.
> > >> >
> > >>
> > >> Why can't the scale filter also rescale this side data as well?
> > >>
> > >
> > > I'm afraid that we could not make sure all such filters (including
> > > filters in the
> > > future) to do the rescale. And in the previous discussion, I got to know
> > > that
> > > 'many other existing side-data types are invalidated by scaling'.
> > >
> > > So, we need frame_width and frame_height here.
> > >
> >
> > No, you don't. You just need to make sure filters which change resolution
> > or do cropping also change the side data parameters.
> > It's called maintainership. As-is, this won't even work with cropping,
> > only with basic aspect ratio preserving scaling.
> > For the lack of a better term, this is a hack.
>
> As discussed in previous email, for the frame size change case, dnn_classify
> (and other filters which use the detection result, for example drawbox) can
> just output a warning message to tell user what happens, and don't do the
> classification, otherwise, it will give a wrong/weird result which makes the
> user confused.
>
> >
> > I would accept just specifying that if the frame dimensions are
> > altered in any way, the side-data is no longer valid and it's up
> > to users to figure that out by out of bound coordinates.
> > This is what we currently do with video_enc_params.
>
> frame_width/frame_height is not perfect (for the cases such as: scale down
> + crop + scale up to the same size), but it provides more info than the
> checking
> of 'out of bound coordinates'. There are many other possible issues when the
> coordinates are within the frame.
>
> If we think we'd better not let user get more info from the warning message,
> I'm ok to remove them.
>
> I'll remove them if there's another comment supporting the removal, and
> there's no objection.
>
> >
> >
> > >> >> >> > diff --git a/libavutil/frame.h b/libavutil/frame.h
> > >> >> >> > index a5ed91b20a..41e22de02a 100644
> > >> >> >> > --- a/libavutil/frame.h
> > >> >> >> > +++ b/libavutil/frame.h
> > >> >> >> > @@ -198,6 +198,13 @@ enum AVFrameSideDataType {
> > >> >> >> > * Must be present for every frame which should have film grain
> > >> applied.
> > >> >> >> > */
> > >> >> >> > AV_FRAME_DATA_FILM_GRAIN_PARAMS,
> > >> >> >> > +
> > >> >> >> > +/**
> > >> >> >> > + * Bounding boxes for object detection and classification,
> > >> >> >> > the
> > >> data is
> > >> >> a
> > >> >> >> AVBoundingBoxHeader
> > >> >> >> > + * followed with an array of AVBoudingBox, and
> > >> >> >> AVBoundingBoxHe
Re: [FFmpeg-devel] [PATCH V7 4/6] lavu: add side data AV_FRAME_DATA_BOUNDING_BOXES
> -Original Message-
> From: ffmpeg-devel On Behalf Of
> Guo, Yejun
> Sent: 2021年4月9日 20:57
> To: FFmpeg development discussions and patches
>
> Subject: Re: [FFmpeg-devel] [PATCH V7 4/6] lavu: add side data
> AV_FRAME_DATA_BOUNDING_BOXES
>
>
>
> > -Original Message-
> > From: ffmpeg-devel On Behalf Of
> > Lynne
> > Sent: 2021年4月9日 18:03
> > To: FFmpeg development discussions and patches
> >
> > Subject: Re: [FFmpeg-devel] [PATCH V7 4/6] lavu: add side data
> > AV_FRAME_DATA_BOUNDING_BOXES
> >
> > Apr 9, 2021, 06:12 by yejun@intel.com:
> >
> > >
> > >
> > >> -Original Message-
> > >> From: ffmpeg-devel On Behalf
> Of
> > Lynne
> > >> Sent: 2021年4月9日 0:57
> > >> To: FFmpeg development discussions and patches
> >
> > >> Subject: Re: [FFmpeg-devel] [PATCH V7 4/6] lavu: add side data
> > >> AV_FRAME_DATA_BOUNDING_BOXES
> > >>
> > >
> > > First of all, thanks for the quick replies, I see, all the
> > discussions/comments are to
> > > make this patch better, thank you.
> > >
> > >> >> >
> > >> >> >> >> > +
> > >> >> >> >> > +typedef struct AVBoundingBoxHeader {
> > >> >> >> >> > +/**
> > >> >> >> >> > + * Information about how the bounding box is
> > generated.
> > >> >> >> >> > + * for example, the DNN model name.
> > >> >> >> >> > + */
> > >> >> >> >> > +char source[128];
> > >> >> >> >> > +
> > >> >> >> >> > +/**
> > >> >> >> >> > + * The size of frame when it is detected.
> > >> >> >> >> > + */
> > >> >> >> >> > +int frame_width;
> > >> >> >> >> > +int frame_height;
> > >> >> >> >> >
> > >> >> >> >>
> > >> >> >> >> Why? This side data is attached to AVFrames only, where we
> > >> >> >> >> already have width and height.
> > >> >> >> >>
> > >> >> >> >
> > >> >> >> > The detection result will be used by other filters, for example,
> > >> >> >> > dnn_classify (see
> > https://github.com/guoyejun/ffmpeg/tree/classify).
> > >> >> >> >
> > >> >> >> > The filter dnn_detect detects all the objects (cat, dog,
> person ...)
> > in a
> > >> >> >> > frame, while dnn_classify classifies one detected object (for
> > example,
> > >> >> person)
> > >> >> >> > for its attribute (for example, emotion, etc.)
> > >> >> >> >
> > >> >> >> > The filter dnn_classify have to check if the frame size is
> changed
> > after
> > >> >> >> > it is detected, to handle the below filter chain:
> > >> >> >> > dnn_detect -> scale -> dnn_classify
> > >> >> >> >
> > >> >> >>
> > >> >> >> This doesn't look good. Why is dnn_classify needing to know
> > >> >> >> the original frame size at all?
> > >> >> >>
> > >> >> >
> > >> >> > For example, the original size of the frame is 100*100, and
> > dnn_detect
> > >> >> > detects a face at place (10, 10) -> (30, 40), such data will be
> > >> >> > saved
> in
> > >> >> > AVBoundingBox.top/left/right/bottom.
> > >> >> >
> > >> >> > Then, the frame is scaled into 50*50.
> > >> >> >
> > >> >> > Then, dnn_classify is used to analyze the emotion of the face, it
> > needs to
> > >> >> > know the frame size (100*100) when it is detected, otherwise, it
> > does not
> > >> >> > work with just (10,10), (30,40) and 50*50.
> > >> >> >
> > >> >>
> > >> >> Why can't the scale filter also rescale this side data as well?
> > >> >>
> > >> >
> > >> > I'm afraid that we could not make sure all such filters (including
> filters
> > in the
> > >> > future) to do the rescale. And in the previous discussion, I got to
> > know that
> > >> > 'many other existing side-data types are invalidated by scaling'.
> > >> >
> > >> > So, we need frame_width and frame_height here.
> > >> >
> > >>
> > >> No, you don't. You just need to make sure filters which change
> > resolution
> > >> or do cropping also change the side data parameters.
> > >> It's called maintainership. As-is, this won't even work with cropping,
> > >> only with basic aspect ratio preserving scaling.
> > >> For the lack of a better term, this is a hack.
> > >>
> > >
> > > As discussed in previous email, for the frame size change case,
> > dnn_classify
> > > (and other filters which use the detection result, for example drawbox)
> > can
> > > just output a warning message to tell user what happens, and don't do
> > the
> > > classification, otherwise, it will give a wrong/weird result which makes
> > the
> > > user confused.
> > >
> > >>
> > >> I would accept just specifying that if the frame dimensions are
> > >> altered in any way, the side-data is no longer valid and it's up
> > >> to users to figure that out by out of bound coordinates.
> > >> This is what we currently do with video_enc_params.
> > >>
> > >
> > > frame_width/frame_height is not perfect (for the cases such as: scale
> > down
> > > + crop + scale up to the same size), but it provides more info than the
> > checking
> > > of 'out of bound coordinates'. There are many other possible issues
> > when the
> > > coordinates are within the frame.
> > >
> > > If we think we'd better not let user get more info from the warning
> > messa
Re: [FFmpeg-devel] [PATCH] libavformat: IEC61937_EAC3 decoding support
Hi,
How do I ask someone to review the patch? I am concerned if I submitted the
PR correctly.
Best regards,
Denis Shulyaka
вт, 16 мар. 2021 г. в 20:59, Denis Shulyaka :
> This patch adds support to decode IEC61937_EAC3 (aka dolby digital plus).
>
> Signed-off-by: Denis Shulyaka
> ---
> libavformat/spdifdec.c | 47 ++
> 1 file changed, 38 insertions(+), 9 deletions(-)
>
> diff --git a/libavformat/spdifdec.c b/libavformat/spdifdec.c
> index 1808fa9d65..58841e7775 100644
> --- a/libavformat/spdifdec.c
> +++ b/libavformat/spdifdec.c
> @@ -93,6 +93,10 @@ static int spdif_get_offset_and_codec(AVFormatContext
> *s,
> *offset = 8192;
> *codec = AV_CODEC_ID_DTS;
> break;
> +case IEC61937_EAC3:
> +*offset = AC3_FRAME_SIZE << 4;
> +*codec = AV_CODEC_ID_EAC3;
> +break;
> default:
> if (s) { /* be silent during a probe */
> avpriv_request_sample(s, "Data type 0x%04x in IEC 61937",
> @@ -103,9 +107,34 @@ static int spdif_get_offset_and_codec(AVFormatContext
> *s,
> return 0;
> }
>
> -/* Largest offset between bursts we currently handle, i.e. AAC with
> - samples = 4096 */
> -#define SPDIF_MAX_OFFSET 16384
> +static int spdif_read_burst_payload_length(AVFormatContext *s,
> + enum IEC61937DataType
> data_type)
> +{
> +AVIOContext *pb = s->pb;
> +int pkt_size_bits, pkt_size_bytes;
> +
> +switch (data_type & 0xff) {
> +case IEC61937_EAC3:
> +pkt_size_bytes = avio_rl16(pb);
> +
> +if (pkt_size_bytes % 2)
> +avpriv_request_sample(s, "Packet not ending at a 16-bit
> boundary");
> +
> +return FFALIGN(pkt_size_bytes, 2);
> +break;
> +default:
> +pkt_size_bits = avio_rl16(pb);
> +
> +if (pkt_size_bits % 16)
> +avpriv_request_sample(s, "Packet not ending at a 16-bit
> boundary");
> +
> +return FFALIGN(pkt_size_bits, 16) >> 3;
> +}
> +return 0;
> +}
> +
> +/* Largest offset between bursts we currently handle, i.e. E-AC-3 */
> +#define SPDIF_MAX_OFFSET 24576
>
> static int spdif_probe(const AVProbeData *p)
> {
> @@ -140,7 +169,7 @@ int ff_spdif_probe(const uint8_t *p_buf, int buf_size,
> enum AVCodecID *codec)
> break;
>
> /* continue probing to find more sync codes */
> -probe_end = FFMIN(buf + SPDIF_MAX_OFFSET, p_buf + buf_size -
> 1);
> +probe_end = FFMIN(buf + SPDIF_MAX_OFFSET + 1, p_buf +
> buf_size - 1);
>
> /* skip directly to the next sync code */
> if (!spdif_get_offset_and_codec(NULL, (buf[2] << 8) | buf[1],
> @@ -176,7 +205,7 @@ int ff_spdif_read_packet(AVFormatContext *s, AVPacket
> *pkt)
> enum IEC61937DataType data_type;
> enum AVCodecID codec_id;
> uint32_t state = 0;
> -int pkt_size_bits, offset, ret;
> +int pkt_size, offset, ret;
>
> while (state != (AV_BSWAP16C(SYNCWORD1) << 16 |
> AV_BSWAP16C(SYNCWORD2))) {
> state = (state << 8) | avio_r8(pb);
> @@ -185,12 +214,12 @@ int ff_spdif_read_packet(AVFormatContext *s,
> AVPacket *pkt)
> }
>
> data_type = avio_rl16(pb);
> -pkt_size_bits = avio_rl16(pb);
> +pkt_size = spdif_read_burst_payload_length(s, data_type);
>
> -if (pkt_size_bits % 16)
> -avpriv_request_sample(s, "Packet not ending at a 16-bit
> boundary");
> +if (!pkt_size)
> +return AVERROR_BUG;
>
> -ret = av_new_packet(pkt, FFALIGN(pkt_size_bits, 16) >> 3);
> +ret = av_new_packet(pkt, pkt_size);
> if (ret)
> return ret;
>
> --
> 2.30.2
>
>
--
Best regards,
Denis Shulyaka
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH V7 4/6] lavu: add side data AV_FRAME_DATA_BOUNDING_BOXES
Apr 9, 2021, 16:35 by bygran...@gmail.com:
> Em sex., 9 de abr. de 2021 às 01:13, Guo, Yejun
> escreveu:
>
>>
>>
>>
>> > -Original Message-
>> > From: ffmpeg-devel On Behalf Of Lynne
>> > Sent: 2021年4月9日 0:57
>> > To: FFmpeg development discussions and patches
>> > Subject: Re: [FFmpeg-devel] [PATCH V7 4/6] lavu: add side data
>> > AV_FRAME_DATA_BOUNDING_BOXES
>> >
>>
>> First of all, thanks for the quick replies, I see, all the
>> discussions/comments are to
>> make this patch better, thank you.
>>
>> > >> >
>> > >> >> >> > +
>> > >> >> >> > +typedef struct AVBoundingBoxHeader {
>> > >> >> >> > +/**
>> > >> >> >> > + * Information about how the bounding box is generated.
>> > >> >> >> > + * for example, the DNN model name.
>> > >> >> >> > + */
>> > >> >> >> > +char source[128];
>> > >> >> >> > +
>> > >> >> >> > +/**
>> > >> >> >> > + * The size of frame when it is detected.
>> > >> >> >> > + */
>> > >> >> >> > +int frame_width;
>> > >> >> >> > +int frame_height;
>> > >> >> >> >
>> > >> >> >>
>> > >> >> >> Why? This side data is attached to AVFrames only, where we
>> > >> >> >> already have width and height.
>> > >> >> >>
>> > >> >> >
>> > >> >> > The detection result will be used by other filters, for example,
>> > >> >> > dnn_classify (see
>> > >> >> > https://github.com/guoyejun/ffmpeg/tree/classify).
>> > >> >> >
>> > >> >> > The filter dnn_detect detects all the objects (cat, dog, person
>> > >> >> > ...) in a
>> > >> >> > frame, while dnn_classify classifies one detected object (for
>> > >> >> > example,
>> > >> person)
>> > >> >> > for its attribute (for example, emotion, etc.)
>> > >> >> >
>> > >> >> > The filter dnn_classify have to check if the frame size is changed
>> > >> >> > after
>> > >> >> > it is detected, to handle the below filter chain:
>> > >> >> > dnn_detect -> scale -> dnn_classify
>> > >> >> >
>> > >> >>
>> > >> >> This doesn't look good. Why is dnn_classify needing to know
>> > >> >> the original frame size at all?
>> > >> >>
>> > >> >
>> > >> > For example, the original size of the frame is 100*100, and dnn_detect
>> > >> > detects a face at place (10, 10) -> (30, 40), such data will be saved
>> > >> > in
>> > >> > AVBoundingBox.top/left/right/bottom.
>> > >> >
>> > >> > Then, the frame is scaled into 50*50.
>> > >> >
>> > >> > Then, dnn_classify is used to analyze the emotion of the face, it
>> > >> > needs to
>> > >> > know the frame size (100*100) when it is detected, otherwise, it does
>> > >> > not
>> > >> > work with just (10,10), (30,40) and 50*50.
>> > >> >
>> > >>
>> > >> Why can't the scale filter also rescale this side data as well?
>> > >>
>> > >
>> > > I'm afraid that we could not make sure all such filters (including
>> > > filters in the
>> > > future) to do the rescale. And in the previous discussion, I got to know
>> > > that
>> > > 'many other existing side-data types are invalidated by scaling'.
>> > >
>> > > So, we need frame_width and frame_height here.
>> > >
>> >
>> > No, you don't. You just need to make sure filters which change resolution
>> > or do cropping also change the side data parameters.
>> > It's called maintainership. As-is, this won't even work with cropping,
>> > only with basic aspect ratio preserving scaling.
>> > For the lack of a better term, this is a hack.
>>
>> As discussed in previous email, for the frame size change case, dnn_classify
>> (and other filters which use the detection result, for example drawbox) can
>> just output a warning message to tell user what happens, and don't do the
>> classification, otherwise, it will give a wrong/weird result which makes the
>> user confused.
>>
>> >
>> > I would accept just specifying that if the frame dimensions are
>> > altered in any way, the side-data is no longer valid and it's up
>> > to users to figure that out by out of bound coordinates.
>> > This is what we currently do with video_enc_params.
>>
>> frame_width/frame_height is not perfect (for the cases such as: scale down
>> + crop + scale up to the same size), but it provides more info than the
>> checking
>> of 'out of bound coordinates'. There are many other possible issues when the
>> coordinates are within the frame.
>>
>> If we think we'd better not let user get more info from the warning message,
>> I'm ok to remove them.
>>
>> I'll remove them if there's another comment supporting the removal, and
>> there's no objection.
>>
>> >
>> >
>> > >> >> >> > diff --git a/libavutil/frame.h b/libavutil/frame.h
>> > >> >> >> > index a5ed91b20a..41e22de02a 100644
>> > >> >> >> > --- a/libavutil/frame.h
>> > >> >> >> > +++ b/libavutil/frame.h
>> > >> >> >> > @@ -198,6 +198,13 @@ enum AVFrameSideDataType {
>> > >> >> >> > * Must be present for every frame which should have film grain
>> > >> applied.
>> > >> >> >> > */
>> > >> >> >> > AV_FRAME_DATA_FILM_GRAIN_PARAMS,
>> > >> >> >> > +
>> > >> >> >> > +/**
>> > >> >> >> > + * Bounding boxes for object detection and c
Re: [FFmpeg-devel] [PATCH V7 4/6] lavu: add side data AV_FRAME_DATA_BOUNDING_BOXES
Apr 9, 2021, 17:10 by yejun@intel.com:
>
>
>> -Original Message-
>> From: ffmpeg-devel On Behalf Of
>> Guo, Yejun
>> Sent: 2021年4月9日 20:57
>> To: FFmpeg development discussions and patches
>>
>> Subject: Re: [FFmpeg-devel] [PATCH V7 4/6] lavu: add side data
>> AV_FRAME_DATA_BOUNDING_BOXES
>>
>>
>>
>> > -Original Message-
>> > From: ffmpeg-devel On Behalf Of
>> > Lynne
>> > Sent: 2021年4月9日 18:03
>> > To: FFmpeg development discussions and patches
>> >
>> > Subject: Re: [FFmpeg-devel] [PATCH V7 4/6] lavu: add side data
>> > AV_FRAME_DATA_BOUNDING_BOXES
>> >
>> > Apr 9, 2021, 06:12 by yejun@intel.com:
>> >
>> > >
>> > >
>> > >> -Original Message-
>> > >> From: ffmpeg-devel On Behalf
>> Of
>> > Lynne
>> > >> Sent: 2021年4月9日 0:57
>> > >> To: FFmpeg development discussions and patches
>> >
>> > >> Subject: Re: [FFmpeg-devel] [PATCH V7 4/6] lavu: add side data
>> > >> AV_FRAME_DATA_BOUNDING_BOXES
>> > >>
>> > >
>> > > First of all, thanks for the quick replies, I see, all the
>> > discussions/comments are to
>> > > make this patch better, thank you.
>> > >
>> > >> >> >
>> > >> >> >> >> > +
>> > >> >> >> >> > +typedef struct AVBoundingBoxHeader {
>> > >> >> >> >> > +/**
>> > >> >> >> >> > + * Information about how the bounding box is
>> > generated.
>> > >> >> >> >> > + * for example, the DNN model name.
>> > >> >> >> >> > + */
>> > >> >> >> >> > +char source[128];
>> > >> >> >> >> > +
>> > >> >> >> >> > +/**
>> > >> >> >> >> > + * The size of frame when it is detected.
>> > >> >> >> >> > + */
>> > >> >> >> >> > +int frame_width;
>> > >> >> >> >> > +int frame_height;
>> > >> >> >> >> >
>> > >> >> >> >>
>> > >> >> >> >> Why? This side data is attached to AVFrames only, where we
>> > >> >> >> >> already have width and height.
>> > >> >> >> >>
>> > >> >> >> >
>> > >> >> >> > The detection result will be used by other filters, for example,
>> > >> >> >> > dnn_classify (see
>> > https://github.com/guoyejun/ffmpeg/tree/classify).
>> > >> >> >> >
>> > >> >> >> > The filter dnn_detect detects all the objects (cat, dog,
>> person ...)
>> > in a
>> > >> >> >> > frame, while dnn_classify classifies one detected object (for
>> > example,
>> > >> >> person)
>> > >> >> >> > for its attribute (for example, emotion, etc.)
>> > >> >> >> >
>> > >> >> >> > The filter dnn_classify have to check if the frame size is
>> changed
>> > after
>> > >> >> >> > it is detected, to handle the below filter chain:
>> > >> >> >> > dnn_detect -> scale -> dnn_classify
>> > >> >> >> >
>> > >> >> >>
>> > >> >> >> This doesn't look good. Why is dnn_classify needing to know
>> > >> >> >> the original frame size at all?
>> > >> >> >>
>> > >> >> >
>> > >> >> > For example, the original size of the frame is 100*100, and
>> > dnn_detect
>> > >> >> > detects a face at place (10, 10) -> (30, 40), such data will be
>> > >> >> > saved
>> in
>> > >> >> > AVBoundingBox.top/left/right/bottom.
>> > >> >> >
>> > >> >> > Then, the frame is scaled into 50*50.
>> > >> >> >
>> > >> >> > Then, dnn_classify is used to analyze the emotion of the face, it
>> > needs to
>> > >> >> > know the frame size (100*100) when it is detected, otherwise, it
>> > does not
>> > >> >> > work with just (10,10), (30,40) and 50*50.
>> > >> >> >
>> > >> >>
>> > >> >> Why can't the scale filter also rescale this side data as well?
>> > >> >>
>> > >> >
>> > >> > I'm afraid that we could not make sure all such filters (including
>> filters
>> > in the
>> > >> > future) to do the rescale. And in the previous discussion, I got to
>> > know that
>> > >> > 'many other existing side-data types are invalidated by scaling'.
>> > >> >
>> > >> > So, we need frame_width and frame_height here.
>> > >> >
>> > >>
>> > >> No, you don't. You just need to make sure filters which change
>> > resolution
>> > >> or do cropping also change the side data parameters.
>> > >> It's called maintainership. As-is, this won't even work with cropping,
>> > >> only with basic aspect ratio preserving scaling.
>> > >> For the lack of a better term, this is a hack.
>> > >>
>> > >
>> > > As discussed in previous email, for the frame size change case,
>> > dnn_classify
>> > > (and other filters which use the detection result, for example drawbox)
>> > can
>> > > just output a warning message to tell user what happens, and don't do
>> > the
>> > > classification, otherwise, it will give a wrong/weird result which makes
>> > the
>> > > user confused.
>> > >
>> > >>
>> > >> I would accept just specifying that if the frame dimensions are
>> > >> altered in any way, the side-data is no longer valid and it's up
>> > >> to users to figure that out by out of bound coordinates.
>> > >> This is what we currently do with video_enc_params.
>> > >>
>> > >
>> > > frame_width/frame_height is not perfect (for the cases such as: scale
>> > down
>> > > + crop + scale up to the same size), but it provides more info than the
>> > checking
>> > > of 'out of bound coordina
Re: [FFmpeg-devel] [PATCH] libavformat: IEC61937_EAC3 decoding support
On 4/9/2021 11:41 AM, Denis Shulyaka wrote: Hi, How do I ask someone to review the patch? I am concerned if I submitted the PR correctly. Best regards, Denis Shulyaka For some reason your patch referenced another unrelated email, and so mail clients that display threaded discussions list it as a reply to it, which makes it harder to notice. Also, your mail client mangled your patch, breaking long lines. That way it can't be applied by git. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH V7 4/6] lavu: add side data AV_FRAME_DATA_BOUNDING_BOXES
Em sex., 9 de abr. de 2021 às 12:15, Lynne escreveu:
>
> Apr 9, 2021, 16:35 by bygran...@gmail.com:
>
> > Em sex., 9 de abr. de 2021 às 01:13, Guo, Yejun
> > escreveu:
> >
> >>
> >>
> >>
> >> > -Original Message-
> >> > From: ffmpeg-devel On Behalf Of Lynne
> >> > Sent: 2021年4月9日 0:57
> >> > To: FFmpeg development discussions and patches
> >> > Subject: Re: [FFmpeg-devel] [PATCH V7 4/6] lavu: add side data
> >> > AV_FRAME_DATA_BOUNDING_BOXES
> >> >
> >>
> >> First of all, thanks for the quick replies, I see, all the
> >> discussions/comments are to
> >> make this patch better, thank you.
> >>
> >> > >> >
> >> > >> >> >> > +
> >> > >> >> >> > +typedef struct AVBoundingBoxHeader {
> >> > >> >> >> > +/**
> >> > >> >> >> > + * Information about how the bounding box is generated.
> >> > >> >> >> > + * for example, the DNN model name.
> >> > >> >> >> > + */
> >> > >> >> >> > +char source[128];
> >> > >> >> >> > +
> >> > >> >> >> > +/**
> >> > >> >> >> > + * The size of frame when it is detected.
> >> > >> >> >> > + */
> >> > >> >> >> > +int frame_width;
> >> > >> >> >> > +int frame_height;
> >> > >> >> >> >
> >> > >> >> >>
> >> > >> >> >> Why? This side data is attached to AVFrames only, where we
> >> > >> >> >> already have width and height.
> >> > >> >> >>
> >> > >> >> >
> >> > >> >> > The detection result will be used by other filters, for example,
> >> > >> >> > dnn_classify (see
> >> > >> >> > https://github.com/guoyejun/ffmpeg/tree/classify).
> >> > >> >> >
> >> > >> >> > The filter dnn_detect detects all the objects (cat, dog, person
> >> > >> >> > ...) in a
> >> > >> >> > frame, while dnn_classify classifies one detected object (for
> >> > >> >> > example,
> >> > >> person)
> >> > >> >> > for its attribute (for example, emotion, etc.)
> >> > >> >> >
> >> > >> >> > The filter dnn_classify have to check if the frame size is
> >> > >> >> > changed after
> >> > >> >> > it is detected, to handle the below filter chain:
> >> > >> >> > dnn_detect -> scale -> dnn_classify
> >> > >> >> >
> >> > >> >>
> >> > >> >> This doesn't look good. Why is dnn_classify needing to know
> >> > >> >> the original frame size at all?
> >> > >> >>
> >> > >> >
> >> > >> > For example, the original size of the frame is 100*100, and
> >> > >> > dnn_detect
> >> > >> > detects a face at place (10, 10) -> (30, 40), such data will be
> >> > >> > saved in
> >> > >> > AVBoundingBox.top/left/right/bottom.
> >> > >> >
> >> > >> > Then, the frame is scaled into 50*50.
> >> > >> >
> >> > >> > Then, dnn_classify is used to analyze the emotion of the face, it
> >> > >> > needs to
> >> > >> > know the frame size (100*100) when it is detected, otherwise, it
> >> > >> > does not
> >> > >> > work with just (10,10), (30,40) and 50*50.
> >> > >> >
> >> > >>
> >> > >> Why can't the scale filter also rescale this side data as well?
> >> > >>
> >> > >
> >> > > I'm afraid that we could not make sure all such filters (including
> >> > > filters in the
> >> > > future) to do the rescale. And in the previous discussion, I got to
> >> > > know that
> >> > > 'many other existing side-data types are invalidated by scaling'.
> >> > >
> >> > > So, we need frame_width and frame_height here.
> >> > >
> >> >
> >> > No, you don't. You just need to make sure filters which change resolution
> >> > or do cropping also change the side data parameters.
> >> > It's called maintainership. As-is, this won't even work with cropping,
> >> > only with basic aspect ratio preserving scaling.
> >> > For the lack of a better term, this is a hack.
> >>
> >> As discussed in previous email, for the frame size change case,
> >> dnn_classify
> >> (and other filters which use the detection result, for example drawbox) can
> >> just output a warning message to tell user what happens, and don't do the
> >> classification, otherwise, it will give a wrong/weird result which makes
> >> the
> >> user confused.
> >>
> >> >
> >> > I would accept just specifying that if the frame dimensions are
> >> > altered in any way, the side-data is no longer valid and it's up
> >> > to users to figure that out by out of bound coordinates.
> >> > This is what we currently do with video_enc_params.
> >>
> >> frame_width/frame_height is not perfect (for the cases such as: scale down
> >> + crop + scale up to the same size), but it provides more info than the
> >> checking
> >> of 'out of bound coordinates'. There are many other possible issues when
> >> the
> >> coordinates are within the frame.
> >>
> >> If we think we'd better not let user get more info from the warning
> >> message,
> >> I'm ok to remove them.
> >>
> >> I'll remove them if there's another comment supporting the removal, and
> >> there's no objection.
> >>
> >> >
> >> >
> >> > >> >> >> > diff --git a/libavutil/frame.h b/libavutil/frame.h
> >> > >> >> >> > index a5ed91b20a..41e22de02a 100644
> >> > >> >> >> > --- a/libavutil/frame.h
> >> > >> >> >> > +++ b/libavutil/frame.h
>
Re: [FFmpeg-devel] [PATCH V7 4/6] lavu: add side data AV_FRAME_DATA_BOUNDING_BOXES
Apr 9, 2021, 20:08 by bygran...@gmail.com:
> Em sex., 9 de abr. de 2021 às 12:15, Lynne escreveu:
>
>>
>> Apr 9, 2021, 16:35 by bygran...@gmail.com:
>>
>> > Em sex., 9 de abr. de 2021 às 01:13, Guo, Yejun
>> > escreveu:
>> >
>> >>
>> >>
>> >>
>> >> > -Original Message-
>> >> > From: ffmpeg-devel On Behalf Of Lynne
>> >> > Sent: 2021年4月9日 0:57
>> >> > To: FFmpeg development discussions and patches
>> >> > Subject: Re: [FFmpeg-devel] [PATCH V7 4/6] lavu: add side data
>> >> > AV_FRAME_DATA_BOUNDING_BOXES
>> >> >
>> >>
>> >> First of all, thanks for the quick replies, I see, all the
>> >> discussions/comments are to
>> >> make this patch better, thank you.
>> >>
>> >> > >> >
>> >> > >> >> >> > +
>> >> > >> >> >> > +typedef struct AVBoundingBoxHeader {
>> >> > >> >> >> > +/**
>> >> > >> >> >> > + * Information about how the bounding box is generated.
>> >> > >> >> >> > + * for example, the DNN model name.
>> >> > >> >> >> > + */
>> >> > >> >> >> > +char source[128];
>> >> > >> >> >> > +
>> >> > >> >> >> > +/**
>> >> > >> >> >> > + * The size of frame when it is detected.
>> >> > >> >> >> > + */
>> >> > >> >> >> > +int frame_width;
>> >> > >> >> >> > +int frame_height;
>> >> > >> >> >> >
>> >> > >> >> >>
>> >> > >> >> >> Why? This side data is attached to AVFrames only, where we
>> >> > >> >> >> already have width and height.
>> >> > >> >> >>
>> >> > >> >> >
>> >> > >> >> > The detection result will be used by other filters, for example,
>> >> > >> >> > dnn_classify (see
>> >> > >> >> > https://github.com/guoyejun/ffmpeg/tree/classify).
>> >> > >> >> >
>> >> > >> >> > The filter dnn_detect detects all the objects (cat, dog, person
>> >> > >> >> > ...) in a
>> >> > >> >> > frame, while dnn_classify classifies one detected object (for
>> >> > >> >> > example,
>> >> > >> person)
>> >> > >> >> > for its attribute (for example, emotion, etc.)
>> >> > >> >> >
>> >> > >> >> > The filter dnn_classify have to check if the frame size is
>> >> > >> >> > changed after
>> >> > >> >> > it is detected, to handle the below filter chain:
>> >> > >> >> > dnn_detect -> scale -> dnn_classify
>> >> > >> >> >
>> >> > >> >>
>> >> > >> >> This doesn't look good. Why is dnn_classify needing to know
>> >> > >> >> the original frame size at all?
>> >> > >> >>
>> >> > >> >
>> >> > >> > For example, the original size of the frame is 100*100, and
>> >> > >> > dnn_detect
>> >> > >> > detects a face at place (10, 10) -> (30, 40), such data will be
>> >> > >> > saved in
>> >> > >> > AVBoundingBox.top/left/right/bottom.
>> >> > >> >
>> >> > >> > Then, the frame is scaled into 50*50.
>> >> > >> >
>> >> > >> > Then, dnn_classify is used to analyze the emotion of the face, it
>> >> > >> > needs to
>> >> > >> > know the frame size (100*100) when it is detected, otherwise, it
>> >> > >> > does not
>> >> > >> > work with just (10,10), (30,40) and 50*50.
>> >> > >> >
>> >> > >>
>> >> > >> Why can't the scale filter also rescale this side data as well?
>> >> > >>
>> >> > >
>> >> > > I'm afraid that we could not make sure all such filters (including
>> >> > > filters in the
>> >> > > future) to do the rescale. And in the previous discussion, I got to
>> >> > > know that
>> >> > > 'many other existing side-data types are invalidated by scaling'.
>> >> > >
>> >> > > So, we need frame_width and frame_height here.
>> >> > >
>> >> >
>> >> > No, you don't. You just need to make sure filters which change
>> >> > resolution
>> >> > or do cropping also change the side data parameters.
>> >> > It's called maintainership. As-is, this won't even work with cropping,
>> >> > only with basic aspect ratio preserving scaling.
>> >> > For the lack of a better term, this is a hack.
>> >>
>> >> As discussed in previous email, for the frame size change case,
>> >> dnn_classify
>> >> (and other filters which use the detection result, for example drawbox)
>> >> can
>> >> just output a warning message to tell user what happens, and don't do the
>> >> classification, otherwise, it will give a wrong/weird result which makes
>> >> the
>> >> user confused.
>> >>
>> >> >
>> >> > I would accept just specifying that if the frame dimensions are
>> >> > altered in any way, the side-data is no longer valid and it's up
>> >> > to users to figure that out by out of bound coordinates.
>> >> > This is what we currently do with video_enc_params.
>> >>
>> >> frame_width/frame_height is not perfect (for the cases such as: scale down
>> >> + crop + scale up to the same size), but it provides more info than the
>> >> checking
>> >> of 'out of bound coordinates'. There are many other possible issues when
>> >> the
>> >> coordinates are within the frame.
>> >>
>> >> If we think we'd better not let user get more info from the warning
>> >> message,
>> >> I'm ok to remove them.
>> >>
>> >> I'll remove them if there's another comment supporting the removal, and
>> >> there's no objection.
>> >>
>> >> >
>> >> >
>> >> > >> >> >> >
Re: [FFmpeg-devel] [PATCH 1/2] avformat/matroskaenc: Remove remnant of inline-timing subtitle packets
Andreas Rheinhardt:
> Signed-off-by: Andreas Rheinhardt
> ---
> libavformat/matroskaenc.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index 609a588f78..a4ddf1fa6a 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -2187,7 +2187,7 @@ static int mkv_write_vtt_blocks(AVFormatContext *s,
> AVIOContext *pb, const AVPac
> put_ebml_uint(pb, MATROSKA_ID_BLOCKDURATION, pkt->duration);
> end_ebml_master(pb, blockgroup);
>
> -return pkt->duration;
> +return 0;
> }
>
> static int mkv_end_cluster(AVFormatContext *s)
> @@ -2358,9 +2358,9 @@ static int mkv_write_packet_internal(AVFormatContext
> *s, const AVPacket *pkt)
> }
> } else {
> if (par->codec_id == AV_CODEC_ID_WEBVTT) {
> -duration = mkv_write_vtt_blocks(s, pb, pkt);
> -if (duration < 0)
> -return duration;
> +ret = mkv_write_vtt_blocks(s, pb, pkt);
> +if (ret < 0)
> +return ret;
> } else {
> ebml_master blockgroup = start_ebml_master(pb,
> MATROSKA_ID_BLOCKGROUP,
>
> mkv_blockgroup_size(pkt->size,
>
Will apply this patchset tomorrow unless there are objections.
- Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] avformat/matroskaenc: Fix leak when writing attachment without filename
Signed-off-by: Andreas Rheinhardt
---
Will apply this soon and backport.
libavformat/matroskaenc.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 609a588f78..7e108f2db9 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -1768,6 +1768,7 @@ static int mkv_write_attachments(AVFormatContext *s)
put_ebml_string(dyn_cp, MATROSKA_ID_FILEDESC, t->value);
if (!(t = av_dict_get(st->metadata, "filename", NULL, 0))) {
av_log(s, AV_LOG_ERROR, "Attachment stream %d has no filename
tag.\n", i);
+ffio_free_dyn_buf(&dyn_cp);
return AVERROR(EINVAL);
}
put_ebml_string(dyn_cp, MATROSKA_ID_FILENAME, t->value);
--
2.27.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC PATCH v2] avformat/dashdec: Improve implementation
zsugabubus: > Signed-off-by: zsugabubus > --- > Should compile fine now. > > libavformat/dashdec.c | 4614 > +++--- > libavformat/internal.h | 27 + > libavformat/utils.c| 47 + > 3 files changed, 2757 insertions(+), 1931 deletions(-) > > /** > @@ -927,6 +941,19 @@ int ff_unlock_avformat(void); > */ > void ff_format_set_url(AVFormatContext *s, char *url); > > +/* > + * Set parent for subdemuxer opened s context. Also inherit some basic > + * properties. > + */ > +void ff_format_set_parent(AVFormatContext *s, AVFormatContext *parent); > + > +/** > + * Get the AVFormatContext that this context originates from. This context > + * usually the one that was explicitly created by the user so it is useful > for > + * callbacks that would like to refer that specific context. > + */ > +AVFormatContext *ff_format_get_origin(AVFormatContext *s); > + You have added declarations for these two functions, yet not defined them. > void avpriv_register_devices(const AVOutputFormat * const o[], const > AVInputFormat * const i[]); > > #endif /* AVFORMAT_INTERNAL_H */ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC PATCH] avformat/dashdec: Improve implementation
On Sun, Mar 28, 2021 at 09:50:32PM +0100, Derek Buitenhuis wrote: > On 28/03/2021 21:05, Carl Eugen Hoyos wrote: > > The patch clearly mentions that it is a "rework". > > If the maintainer likes it, it would be nice if it can be simplified but > > this may not be possible. > > Of course it's /possible/. People have been splitting rewrites/refactors into > these since forever; FFmpeg's history is full of them. It's great in theory but have you seen this code in reality? I felt forcing new features into the old code base would require more (physical) pain (at least for me) than rewriting everything from almost ground up. And funnily, the circle already closed, since if the old code would be written in an easily extensible manner, I would not be required to send a patch now, since everything is so easy to implement and nicely fitted that all thing is there since the beginnings. No? The approach I chose to trim the code to my needs and get rid of chaos, required rewriting the parser and almost all stuff in some way or another, so you would see a thousand line change anyway. -- zsugabubus ___ 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/msp2dec: Check available space in RLE decoder
On 4/7/2021 11:59 AM, Michael Niedermayer wrote:
On Wed, Apr 07, 2021 at 12:42:50AM +0200, Andreas Rheinhardt wrote:
Michael Niedermayer:
Fixes: out of array read
Fixes:
32968/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_MSP2_fuzzer-5315296027082752
Found-by: continuous fuzzing process
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer
---
libavcodec/msp2dec.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/libavcodec/msp2dec.c b/libavcodec/msp2dec.c
index cc548d218a..87057fb5e2 100644
--- a/libavcodec/msp2dec.c
+++ b/libavcodec/msp2dec.c
@@ -68,9 +68,10 @@ static int msp2_decode_frame(AVCodecContext *avctx,
bytestream2_init(&gb, buf, pkt_size);
x = 0;
-while (bytestream2_get_bytes_left(&gb) && x < width) {
+while (bytestream2_get_bytes_left(&gb) > 0 && x < width) {
This decoder uses the checked bytestream2 API, so != 0 and > 0 should be
equivalent for bytestream2_get_bytes_left(&gb).
I changed it to "> 0" because it felt clearer&more robust
i will drop that as its not needed for the bugfix
int size = bytestream2_get_byte(&gb);
if (size) {
+size = FFMIN(size, bytestream2_get_bytes_left(&gb));
memcpy(p->data[0] + y * p->linesize[0] + x, gb.buffer,
FFMIN(size, width - x));
width can include seven bytes of the packet's padding, but it stays
within the padding, so I wonder where the out of array read comes from.
The only fishy thing in this decoder I see is that 2 * avctx->height
might overflow.
size is a value read from the bytestream, theres no guarntee that
the amount that byte calls for is not beyond the end of the bytestream
and its not checked by memcpy
bytestream2_get_buffer() would check it and it can maybe be used instead
If you knew about it, why didn't you use it? This code here basically
duplicates the three lines in bytestream2_get_buffer().
thx
[...]
___
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 1/2] libavformat/mpegts: Extract arib_caption descriptor data into codecpar->extradata
Changes since v1:
If desc_len < 3, break the switch statement instead of return
AVERROR_INVALIDDATA
to make it more robust.
For arib_caption, data_component_descriptor should contains at least 3 bytes:
data_component_id: uint16, additional_arib_caption_info: uint8.
The recognization of ARIB STD-B24 caption has been introduced in commit a03885b,
which is used as closed caption in Japanese / Brazilian Digital Television.
But whenever copying arib_caption into mpegts output, arib_caption inside the
outputted stream cannot be recgonized as a arib_caption subtitle track once
again,
which is caused by the missing of descriptors in the PMT table.
These patches are intended to fix broken stream copying of arib_caption
subtitle.
This patch extracts necessary fields into codecpar->extradata for future
remuxing,
which describe the stream_identifier, the profile and other additional
information.
Signed-off-by: zheng qian
---
libavformat/mpegts.c | 25 +
1 file changed, 25 insertions(+)
diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
index 6e0d9d7496..fd7ea1f504 100644
--- a/libavformat/mpegts.c
+++ b/libavformat/mpegts.c
@@ -2107,6 +2107,9 @@ int ff_parse_mpeg2_descriptor(AVFormatContext *fc,
AVStream *st, int stream_type
// STD-B24, fascicle 3, chapter 4 defines private_stream_1
// for captions
if (stream_type == STREAM_TYPE_PRIVATE_DATA) {
+if (desc_len < 3)
+break;
+
// This structure is defined in STD-B10, part 1, listing 5.4 and
// part 2, 6.2.20).
// Listing of data_component_ids is in STD-B10, part 2, Annex J.
@@ -2145,6 +2148,28 @@ int ff_parse_mpeg2_descriptor(AVFormatContext *fc,
AVStream *st, int stream_type
st->codecpar->codec_id = AV_CODEC_ID_ARIB_CAPTION;
st->codecpar->profile= picked_profile;
st->internal->request_probe= 0;
+
+// Store stream_identifier and payload of data_component_descriptor
+// (data_component_id & additional_arib_caption_info) as extradata.
+// data_component_descriptor is defined in ARIB STD-B10, part 2,
6.2.20
+// These will be useful for remuxing arib_caption into mpegts
output.
+if (!st->codecpar->extradata) {
+st->codecpar->extradata = av_mallocz(4 +
AV_INPUT_BUFFER_PADDING_SIZE);
+if (!st->codecpar->extradata)
+return AVERROR(ENOMEM);
+
+st->codecpar->extradata_size = 4;
+
+// stream_identifier (component_tag of
stream_identifier_descriptor)
+st->codecpar->extradata[0] = (uint8_t)(st->stream_identifier -
1);
+
+// payload of data_component_descriptor structure
+// data_component_id
+st->codecpar->extradata[1] = (uint8_t)((data_component_id &
0xF0) >> 8);
+st->codecpar->extradata[2] = (uint8_t)(data_component_id &
0x0F);
+// additional_arib_caption_info, defined in ARIB STD-B24,
fascicle 1, Part 3, 9.6.1
+st->codecpar->extradata[3] = get8(pp, desc_end);
+}
}
break;
case 0xb0: /* DOVI video stream descriptor */
--
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 v2 2/2] libavformat/mpegtsenc: Write necessary descriptors into PMT for arib_caption
Stream copy for arib_caption into mpegts is not working correctly for now.
This patch utilizes the data stored in codecpar->extradata to write necessary
stream_identifier_descriptor and data_component_descriptor into PMT table
for arib_caption remuxing.
This patch fixes the copying of arib_caption. After this patch, arib_caption
inside the outputted (remuxed) mpegts stream can be recognized correctly
as an arib_caption subtitle stream.
Signed-off-by: zheng qian
---
libavformat/mpegtsenc.c | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
index a357f3a6aa..e26a5d3b51 100644
--- a/libavformat/mpegtsenc.c
+++ b/libavformat/mpegtsenc.c
@@ -357,6 +357,7 @@ static int get_dvb_stream_type(AVFormatContext *s, AVStream
*st)
break;
case AV_CODEC_ID_DVB_SUBTITLE:
case AV_CODEC_ID_DVB_TELETEXT:
+case AV_CODEC_ID_ARIB_CAPTION:
stream_type = STREAM_TYPE_PRIVATE_DATA;
break;
case AV_CODEC_ID_SMPTE_KLV:
@@ -714,8 +715,21 @@ static int mpegts_write_pmt(AVFormatContext *s,
MpegTSService *service)
}
*len_ptr = q - len_ptr - 1;
+} else if (codec_id == AV_CODEC_ID_ARIB_CAPTION) {
+if (st->codecpar->extradata && st->codecpar->extradata_size >=
4) {
+// stream_identifier_descriptor
+*q++ = 0x52; // descriptor_tag
+*q++ = 1; // descriptor_length
+*q++ = st->codecpar->extradata[0]; // component_tag:
stream_identifier
+
+// data_component_descriptor
+*q++ = 0xFD; // descriptor_tag: ARIB data coding type
descriptor
+*q++ = 3; // descriptor_length
+memcpy(q, &st->codecpar->extradata[1], 3); //
data_component_id & additional_arib_caption_info
+q += 3;
+}
}
-break;
+break;
case AVMEDIA_TYPE_VIDEO:
if (stream_type == STREAM_TYPE_VIDEO_DIRAC) {
put_registration_descriptor(&q, MKTAG('d', 'r', 'a', 'c'));
--
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".
Re: [FFmpeg-devel] [PATCH v3] lavfi/qsvvpp: support async depth
Hi Fei W, On Wed, Mar 31, 2021 at 10:14 AM Wang, Fei W wrote: > > On Wed, 2021-03-31 at 10:07 +0800, Fei Wang wrote: > > Async depth will allow qsv filter cache few frames, and avoid force > > switch and end filter task frame by frame. This change will improve > > performance for some multi-task case, for example 1:N transcode( > > decode + vpp + encode) with all QSV plugins. > > > > Performance data test on my Coffee Lake Desktop(i7-8700K) by using > > the following 1:8 transcode test case improvement: > > 1. Fps improved from 55 to 130. > > 2. Render/Video usage improved from ~61%/~38% to ~100%/~70%.(Data get > > from intel_gpu_top) > > > > test CMD: > > ffmpeg -v verbose -init_hw_device qsv=hw:/dev/dri/renderD128 > > -filter_hw_device \ > > hw -hwaccel qsv -hwaccel_output_format qsv -c:v h264_qsv -i > > 1920x1080.264 \ > > -vf 'vpp_qsv=w=1280:h=720:async_depth=4' -c:v h264_qsv -r:v 30 > > -preset 7 -g 33 -refs 2 -bf 3 -q 24 -f null - \ > > -vf 'vpp_qsv=w=1280:h=720:async_depth=4' -c:v h264_qsv -r:v 30 > > -preset 7 -g 33 -refs 2 -bf 3 -q 24 -f null - \ > > -vf 'vpp_qsv=w=1280:h=720:async_depth=4' -c:v h264_qsv -r:v 30 > > -preset 7 -g 33 -refs 2 -bf 3 -q 24 -f null - \ > > -vf 'vpp_qsv=w=1280:h=720:async_depth=4' -c:v h264_qsv -r:v 30 > > -preset 7 -g 33 -refs 2 -bf 3 -q 24 -f null - \ > > -vf 'vpp_qsv=w=1280:h=720:async_depth=4' -c:v h264_qsv -r:v 30 > > -preset 7 -g 33 -refs 2 -bf 3 -q 24 -f null - \ > > -vf 'vpp_qsv=w=1280:h=720:async_depth=4' -c:v h264_qsv -r:v 30 > > -preset 7 -g 33 -refs 2 -bf 3 -q 24 -f null - \ > > -vf 'vpp_qsv=w=1280:h=720:async_depth=4' -c:v h264_qsv -r:v 30 > > -preset 7 -g 33 -refs 2 -bf 3 -q 24 -f null - > > > > Signed-off-by: Fei Wang > > --- > > Change: > > 1. Add test data in commit message. > > 2. Rmove some duplicate code. Appreciate the detailed data. Verified locally the performance improves in 1:N downscale cases as your description. Also do some experiments for 1:N upscale (1080p->3840x2160) , 1:1 and N:1 cases, the bottleneck seems to be somewhere else hence the performance remains identical for vpp async depth. But this could be another story. Patch functionally LGTM, thx. - linjie ___ 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] lavc/qsv: apply AVCodecContext AVOption -threads to QSV
Hi Haihao, On Thu, Apr 8, 2021 at 3:10 PM Haihao Xiang wrote: > > By default the SDK creates a thread for each CPU when creating a mfx > session for decoding / encoding, which results in CPU overhead on a > multi CPU system. Actually creating 2 threads is a better choice for > most cases in practice. > > This patch allows user to specify the number of threads created for a > mfx session via option -threads. If the number is not specified, 2 > threads will be created by default. > > Note the SDK requires at least 2 threads to avoid dead locks[1] > > [1]https://github.com/Intel-Media-SDK/MediaSDK/blob/master/_studio/mfx_lib/scheduler/linux/src/mfx_scheduler_core_ischeduler.cpp#L90-L93 > --- Optional choice for users to specify the thread number looks reasonable to me, and decreasing the CPU overhead makes sense for HW encoding pipeline. Also curious about what's the tradeoff of decreasing the thread number to 2. Would the performance or something else drop? - linjie ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/4] doc/protocols: update options
On Sun, 15. Nov 13:20, Andriy Gelman wrote:
> From: Andriy Gelman
>
> Define listen_timeout. Set user-agent and timeout to deprecated.
>
> Signed-off-by: Andriy Gelman
> ---
> doc/protocols.texi | 13 +++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/doc/protocols.texi b/doc/protocols.texi
> index b4efa14509..fd866f57a5 100644
> --- a/doc/protocols.texi
> +++ b/doc/protocols.texi
> @@ -1102,11 +1102,16 @@ Set minimum local UDP port. Default value is 5000.
> Set maximum local UDP port. Default value is 65000.
>
> @item timeout
> -Set maximum timeout (in seconds) to wait for incoming connections.
> +This option is deprecated. Use @option{listen_timeout} instead. Set maximum
> timeout (in seconds) to wait for incoming connections.
>
> A value of -1 means infinite (default). This option implies the
> @option{rtsp_flags} set to @samp{listen}.
>
> +@item listen_timeout
> +Set maximum timeout (in seconds) to establish an initial connection. Setting
> +@samp{listen_timeout} > 0 sets @option{rtsp_flags} to @samp{listen}. Default
> is -1
> +which means an infinite timeout when @samp{listen} mode is set.
> +
> @item reorder_queue_size
> Set number of packets to buffer for handling of reordered packets.
>
> @@ -1114,7 +1119,11 @@ Set number of packets to buffer for handling of
> reordered packets.
> Set socket TCP I/O timeout in microseconds.
>
> @item user-agent
> -Override User-Agent header. If not specified, it defaults to the
> +This option is deprecated. Use @option{user_agent} instead. Override
> User-Agent header. If not specified, it defaults to the
> +libavformat identifier string.
> +
> +@item user_agent
> +User-Agent header. If not specified, it defaults to the
> libavformat identifier string.
> @end table
Because user-agent will be removed soon. Is it even worth updating the
documentation for user-agent and timeout?
or should I resend with only the listen_timeout added?
--
Andriy
___
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".
