Re: [FFmpeg-devel] [PATCH] lavc/vaapi_encode_h264: add support for maxframesize

2019-04-22 Thread myp...@gmail.com
On Tue, Apr 23, 2019 at 11:29 AM Linjie Fu  wrote:
>
> Add support for max frame size:
> - max_frame_size (bytes) to indicate the allowed max frame size.
> - pass_num to indicate number of passes.
> - delta_qp to indicate adjust qp value.
>
> Currently only AVC encoder can support this settings in multiple pass case.
> If the frame size exceeds, the encoder will do more pak passes to adjust the
> QP value to control the frame size.
>
> Set Default num_passes to 4 (1~4), set delta_qp[4] = {1, 1, 1, 1}, use
> new_qp for encoder if frame size exceeds the limitation:
> new_qp = base_qp + delta_qp[0] + delta_qp[1] + ...
>
> ffmpeg -hwaccel vaapi -vaapi_device /dev/dri/renderD128 -f rawvideo \
> -v verbose -s:v 352x288 -i ./input.yuv -vf format=nv12,hwupload \
> -c:v h264_vaapi -profile:v main -g 30 -bf 3 -max_frame_size 4 \
> -pass_num 2 -delta_qp 2 -vframes 100 -y ./max_frame_size.h264
>
Some question list as follow:

1. Can I change delta_qp per pass, e,g, 4 pass 1, 2, 3, 4, use
delta_qp[4] like: 1, 2, 4, 8?

2. So let's think about the limiting case, if we setting
max_frame_size = 1 for 1080P, what's the action for this driver?

3. Maybe we can hide the pass_num and delta_qp, only export the
max_frame_size for this case?  I don't think export the driver QP
adjustment detail  logic to user space is good idea, user will
confused about to how to set/adjust pass_num/delta_qp per pass.

4. Missing docs

5. What's the relationship about other bit rate control like VBR or MBBRC ?

6. Only 264 encoder support this feature? What platform have you tested?

> Signed-off-by: Linjie Fu 
> ---
>  libavcodec/vaapi_encode.c  | 46 ++
>  libavcodec/vaapi_encode.h  | 11 
>  libavcodec/vaapi_encode_h264.c | 15 +++
>  3 files changed, 72 insertions(+)
>
> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
> index 2dda451882..762c42ef13 100644
> --- a/libavcodec/vaapi_encode.c
> +++ b/libavcodec/vaapi_encode.c
> @@ -236,6 +236,14 @@ static int vaapi_encode_issue(AVCodecContext *avctx,
>  goto fail;
>  }
>
> +if (ctx->max_frame_size) {
> +err = vaapi_encode_make_param_buffer(avctx, pic, 
> VAEncMiscParameterBufferType,
> +(char*) >mfs_params.misc,
> +sizeof(ctx->mfs_params));
> +if (err < 0)
> +goto fail;
> +}
> +
>  if (pic->type == PICTURE_TYPE_IDR) {
>  if (ctx->va_packed_headers & VA_ENC_PACKED_HEADER_SEQUENCE &&
>  ctx->codec->write_sequence_header) {
> @@ -1630,6 +1638,38 @@ rc_mode_found:
>  return 0;
>  }
>
> +static av_cold int vaapi_encode_init_max_frame_size(AVCodecContext *avctx)
> +{
> +VAAPIEncodeContext *ctx = avctx->priv_data;
> +
> +uint32_t max_frame_size = ctx->max_frame_size;
> +uint8_t num_passes = ctx->pass_num;
> +uint8_t *delta_qp = av_mallocz_array(num_passes, sizeof(uint8_t));
> +int err = 0;
> +int i = 0;
> +
> +if (!delta_qp) {
> +err = AVERROR(ENOMEM);
> +return err;
> +}
> +for (i = 0; i  +delta_qp[i] = ctx->delta_qp;
> +}
> +
> +
> +ctx->mfs_params.misc.type = VAEncMiscParameterTypeMultiPassFrameSize;
> +ctx->mfs_params.mfs.type = VAEncMiscParameterTypeMultiPassFrameSize;
> +ctx->mfs_params.mfs.max_frame_size = max_frame_size;
> +ctx->mfs_params.mfs.num_passes = num_passes;
> +ctx->mfs_params.mfs.delta_qp = delta_qp;
> +
> +av_log(avctx, AV_LOG_VERBOSE, "Max Frame Size: %d bytes, "
> +  "num_passes: %d, delta_qp = %d.\n",
> +ctx->max_frame_size, num_passes, 
> ctx->delta_qp);
> +
> +return 0;
> +}
> +
>  static av_cold int vaapi_encode_init_gop_structure(AVCodecContext *avctx)
>  {
>  VAAPIEncodeContext *ctx = avctx->priv_data;
> @@ -2095,6 +2135,12 @@ av_cold int ff_vaapi_encode_init(AVCodecContext *avctx)
>  goto fail;
>  }
>
> +if (ctx->max_frame_size) {
> +err = vaapi_encode_init_max_frame_size(avctx);
> +if (err < 0)
> +goto fail;
> +}
> +
>  vas = vaCreateConfig(ctx->hwctx->display,
>   ctx->va_profile, ctx->va_entrypoint,
>   ctx->config_attributes, ctx->nb_config_attributes,
> diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h
> index 44a8db566e..557476d226 100644
> --- a/libavcodec/vaapi_encode.h
> +++ b/libavcodec/vaapi_encode.h
> @@ -176,6 +176,13 @@ typedef struct VAAPIEncodeContext {
>  // Desired B frame reference depth.
>  int desired_b_depth;
>
> +// Max Frame Size
> +int max_frame_size;
> +// Number Of Passes
> +int pass_num;
> +// Delta_qp For Each Pass
> +int delta_qp;
> +
>  // Explicitly set RC mode (otherwise attempt to pick from
>  

[FFmpeg-devel] [PATCH] lavc/vaapi_encode_h264: add support for maxframesize

2019-04-22 Thread Linjie Fu
Add support for max frame size:
- max_frame_size (bytes) to indicate the allowed max frame size.
- pass_num to indicate number of passes.
- delta_qp to indicate adjust qp value.

Currently only AVC encoder can support this settings in multiple pass case.
If the frame size exceeds, the encoder will do more pak passes to adjust the
QP value to control the frame size.

Set Default num_passes to 4 (1~4), set delta_qp[4] = {1, 1, 1, 1}, use
new_qp for encoder if frame size exceeds the limitation:
new_qp = base_qp + delta_qp[0] + delta_qp[1] + ...

ffmpeg -hwaccel vaapi -vaapi_device /dev/dri/renderD128 -f rawvideo \
-v verbose -s:v 352x288 -i ./input.yuv -vf format=nv12,hwupload \
-c:v h264_vaapi -profile:v main -g 30 -bf 3 -max_frame_size 4 \
-pass_num 2 -delta_qp 2 -vframes 100 -y ./max_frame_size.h264

Signed-off-by: Linjie Fu 
---
 libavcodec/vaapi_encode.c  | 46 ++
 libavcodec/vaapi_encode.h  | 11 
 libavcodec/vaapi_encode_h264.c | 15 +++
 3 files changed, 72 insertions(+)

diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index 2dda451882..762c42ef13 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -236,6 +236,14 @@ static int vaapi_encode_issue(AVCodecContext *avctx,
 goto fail;
 }
 
+if (ctx->max_frame_size) {
+err = vaapi_encode_make_param_buffer(avctx, pic, 
VAEncMiscParameterBufferType,
+(char*) >mfs_params.misc,
+sizeof(ctx->mfs_params));
+if (err < 0)
+goto fail;
+}
+
 if (pic->type == PICTURE_TYPE_IDR) {
 if (ctx->va_packed_headers & VA_ENC_PACKED_HEADER_SEQUENCE &&
 ctx->codec->write_sequence_header) {
@@ -1630,6 +1638,38 @@ rc_mode_found:
 return 0;
 }
 
+static av_cold int vaapi_encode_init_max_frame_size(AVCodecContext *avctx)
+{
+VAAPIEncodeContext *ctx = avctx->priv_data;
+
+uint32_t max_frame_size = ctx->max_frame_size;
+uint8_t num_passes = ctx->pass_num;
+uint8_t *delta_qp = av_mallocz_array(num_passes, sizeof(uint8_t));
+int err = 0;
+int i = 0;
+
+if (!delta_qp) {
+err = AVERROR(ENOMEM);
+return err;
+}
+for (i = 0; i delta_qp;
+}
+
+
+ctx->mfs_params.misc.type = VAEncMiscParameterTypeMultiPassFrameSize;
+ctx->mfs_params.mfs.type = VAEncMiscParameterTypeMultiPassFrameSize;
+ctx->mfs_params.mfs.max_frame_size = max_frame_size;
+ctx->mfs_params.mfs.num_passes = num_passes;
+ctx->mfs_params.mfs.delta_qp = delta_qp;
+
+av_log(avctx, AV_LOG_VERBOSE, "Max Frame Size: %d bytes, "
+  "num_passes: %d, delta_qp = %d.\n",
+ctx->max_frame_size, num_passes, 
ctx->delta_qp);
+
+return 0;
+}
+
 static av_cold int vaapi_encode_init_gop_structure(AVCodecContext *avctx)
 {
 VAAPIEncodeContext *ctx = avctx->priv_data;
@@ -2095,6 +2135,12 @@ av_cold int ff_vaapi_encode_init(AVCodecContext *avctx)
 goto fail;
 }
 
+if (ctx->max_frame_size) {
+err = vaapi_encode_init_max_frame_size(avctx);
+if (err < 0)
+goto fail;
+}
+
 vas = vaCreateConfig(ctx->hwctx->display,
  ctx->va_profile, ctx->va_entrypoint,
  ctx->config_attributes, ctx->nb_config_attributes,
diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h
index 44a8db566e..557476d226 100644
--- a/libavcodec/vaapi_encode.h
+++ b/libavcodec/vaapi_encode.h
@@ -176,6 +176,13 @@ typedef struct VAAPIEncodeContext {
 // Desired B frame reference depth.
 int desired_b_depth;
 
+// Max Frame Size
+int max_frame_size;
+// Number Of Passes
+int pass_num;
+// Delta_qp For Each Pass
+int delta_qp;
+
 // Explicitly set RC mode (otherwise attempt to pick from
 // available modes).
 int explicit_rc_mode;
@@ -268,6 +275,10 @@ typedef struct VAAPIEncodeContext {
 } quality_params;
 #endif
 
+struct {
+VAEncMiscParameterBuffer misc;
+VAEncMiscParameterBufferMultiPassFrameSize mfs;
+} __attribute__((packed)) mfs_params;
 // Per-sequence parameter structure (VAEncSequenceParameterBuffer*).
 void   *codec_sequence_params;
 
diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c
index 4cf99d7c78..4d55dc2bac 100644
--- a/libavcodec/vaapi_encode_h264.c
+++ b/libavcodec/vaapi_encode_h264.c
@@ -72,6 +72,9 @@ typedef struct VAAPIEncodeH264Context {
 int sei;
 int profile;
 int level;
+int max_frame_size;
+int pass_num;
+int delta_qp;
 
 // Derived settings.
 int mb_width;
@@ -1233,6 +1236,12 @@ static av_cold int vaapi_encode_h264_init(AVCodecContext 
*avctx)
 if (priv->qp > 0)
 ctx->explicit_qp = 

[FFmpeg-devel] [PATCH V3 2/2] configure: replace 'pr' with printf since busybox does not support pr

2019-04-22 Thread Guo, Yejun
It is part of change from https://trac.ffmpeg.org/ticket/5680 provided
by Kylie McClain  at Wed, 29 Jun 2016 16:37:20 -0400.

That change contains two parts, in function log_file and in function 
print_in_columns.

The second part is not good, so I have send out a new patch for 
print_in_columns.

As for the change in the first part, it is good. Just to make the whole thing 
completed,
i send out this patch to copy exactally the change in function log_file.

contributor: Kylie McClain 
Signed-off-by: Guo, Yejun 
---
 configure | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index f8032aa..50cb31f 100755
--- a/configure
+++ b/configure
@@ -503,7 +503,11 @@ log(){
 
 log_file(){
 log BEGIN $1
-pr -n -t $1 >> $logfile
+i=1
+while read line;do
+printf '%5s   %s\n' "${i}" "${line}"
+i=$(($i+1))
+done < $1 >> $logfile
 log END $1
 }
 
-- 
2.7.4

___
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 V3 1/2] configure: sort decoder/encoder/filter/... names in alphabet order

2019-04-22 Thread Guo, Yejun
take decoder names an example, with the default page length, shell command
'pr' needs two pages for all the decoder names. The names are firstly printed
in the first page, then in the second page. So, as a whole, the names are
sorted neither in column order nor in row order. It's a little confused.

One method is to calculate the proper page length, so all the names are printed
in one page by 'pr -l', and so strictly in alphabet order, column by column.

Another method is to use command printf instead of pr, because buybox doesn't
have pr. This patch refines print_in_columns to print the names with printf
in alphabet order, very similar with 'pr -l', except the case when the last
column is not fully filled with names.

The interface of print_in_columns changed, the input needs to be sorted first,
and then pass into print_in_columns as function parameters. It gives the
flexibility the input parameters can be considered as an array and can be 
indexed.

contributor: Alexander Strasser 
Signed-off-by: Guo, Yejun 
---
 configure | 32 +++-
 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/configure b/configure
index 3b11ffe..f8032aa 100755
--- a/configure
+++ b/configure
@@ -3832,14 +3832,28 @@ die_unknown(){
 }
 
 print_in_columns() {
-cols=$(expr $ncols / 24)
-cat | tr ' ' '\n' | sort | pr -r "-$cols" -w $ncols -t
+col_width=24
+cols=$(expr $ncols / $col_width)
+rows=$(expr $(expr $# + $cols - 1) / $cols)
+for row in $(seq $rows); do
+index=$row
+line=""
+fmt=""
+for col in $(seq $cols); do
+if [ $index -le $# ]; then
+eval line='"$line "${'$index'}'
+fmt="$fmt%-${col_width}s"
+fi
+index=$(expr $index + $rows)
+done
+printf "$fmt\n" $line
+done | sed 's/ *$//'
 }
 
 show_list() {
 suffix=_$1
 shift
-echo $* | sed s/$suffix//g | print_in_columns
+print_in_columns $(echo $* | sed s/$suffix//g | tr ' ' '\n' | sort)
 exit 0
 }
 
@@ -7121,32 +7135,32 @@ test -n "$random_seed" &&
 echo
 
 echo "External libraries:"
-print_enabled '' $EXTERNAL_LIBRARY_LIST $EXTERNAL_AUTODETECT_LIBRARY_LIST | 
print_in_columns
+print_in_columns $(print_enabled '' $EXTERNAL_LIBRARY_LIST 
$EXTERNAL_AUTODETECT_LIBRARY_LIST | tr ' ' '\n' | sort)
 echo
 
 echo "External libraries providing hardware acceleration:"
-print_enabled '' $HWACCEL_LIBRARY_LIST $HWACCEL_AUTODETECT_LIBRARY_LIST | 
print_in_columns
+print_in_columns $(print_enabled '' $HWACCEL_LIBRARY_LIST 
$HWACCEL_AUTODETECT_LIBRARY_LIST | tr ' ' '\n' | sort)
 echo
 
 echo "Libraries:"
-print_enabled '' $LIBRARY_LIST | print_in_columns
+print_in_columns $(print_enabled '' $LIBRARY_LIST | tr ' ' '\n' | sort)
 echo
 
 echo "Programs:"
-print_enabled '' $PROGRAM_LIST | print_in_columns
+print_in_columns $(print_enabled '' $PROGRAM_LIST | tr ' ' '\n' | sort)
 echo
 
 for type in decoder encoder hwaccel parser demuxer muxer protocol filter bsf 
indev outdev; do
 echo "Enabled ${type}s:"
 eval list=\$$(toupper $type)_LIST
-print_enabled '_*' $list | print_in_columns
+print_in_columns $(print_enabled '_*' $list | tr ' ' '\n' | sort)
 echo
 done
 
 if test -n "$ignore_tests"; then
 ignore_tests=$(echo $ignore_tests | tr ',' ' ')
 echo "Ignored FATE tests:"
-echo $ignore_tests | print_in_columns
+print_in_columns $(echo $ignore_tests | tr ' ' '\n' | sort)
 echo
 fi
 
-- 
2.7.4

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

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

Re: [FFmpeg-devel] [PATCH V2 1/2] configure: sort decoder/encoder/filter/... names in alphabet order

2019-04-22 Thread Guo, Yejun


> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of
> Alexander Strasser
> Sent: Tuesday, April 23, 2019 8:40 AM
> To: FFmpeg development discussions and patches 
> Subject: Re: [FFmpeg-devel] [PATCH V2 1/2] configure: sort
> decoder/encoder/filter/... names in alphabet order
> 
> Hi!
> 
> On 2019-04-15 21:23 +0800, Guo, Yejun wrote:
> > take decoder names an example, with the default page length, shell
> command
> > 'pr' needs two pages for all the decoder names. The names are firstly 
> > printed
> > in the first page, then in the second page. So, as a whole, the names are
> > sorted neither in column order nor in row order. It's a little confused.
> >
> > One method is to calculate the proper page length, so all the names are
> printed
> > in one page by 'pr -l', and so strictly in alphabet order, column by column.
> >
> > Another method is to use command printf instead of pr, because buybox
> doesn't
> > have pr. This patch refines print_in_columns to print the names with printf
> > in alphabet order, very similar with 'pr -l', except the case when the last
> > column is not fully filled with names.
> 
> Looks promising. Though this kind of change is basically
> a bit difficult.
> 
> There is some risk that it won't work as expected on all
> currently supported shells or perform differently.
> 
> For the performance as long as it is not grave, it should
> not be a problem. Actually this implementation looks faster
> here.
> 
> If it won't work as expected the risk is kind of limited,
> as long as it doesn't modify global state that matters
> and it does not abort the configure script, "only" the
> informative / diagnostic output will be flawed, but the
> build itself will be fine.
> 
> 
> > Signed-off-by: Guo, Yejun 
> > ---
> >  configure | 34 +-
> >  1 file changed, 25 insertions(+), 9 deletions(-)
> >
> > diff --git a/configure b/configure
> > index c2580b3..45a9126 100755
> > --- a/configure
> > +++ b/configure
> > @@ -3830,14 +3830,30 @@ die_unknown(){
> >  }
> >
> >  print_in_columns() {
> > -cols=$(expr $ncols / 24)
> > -cat | tr ' ' '\n' | sort | pr -r "-$cols" -w $ncols -t
> > +width=24
> > +cols=$(expr $ncols / $width)
> > +rows=$(expr $(expr $# + $cols - 1) / $cols)
> 
> > +eval format="%-${width}s"
> 
> Why do you use eval here?

I did many experiments, eval is for an old try and forgot to remove it for the 
new change. Will remove it.

> 
> 
> > +for row in $(seq 1 $rows); do
> 
> I think the 1 argument to seq isn't needed here and below.

yes, you are right. Will remove it.

> 
> 
> > +index=$row
> > +line=""
> > +lfmt=""
> > +for col in $(seq 1 $cols); do
> > +if [ $index -le $# ]; then
> > +eval item=\$${index}
> 
> This will not work on all shells when trying to expand
> the 10th parameter and following.
> 
> Excerpt of the relevant POSIX section:
> 
> When a positional parameter with more than one digit is specified, the
> application shall enclose the digits in braces (see Parameter Expansion).

thanks for pointing this, I did not try all shells.

will replace it like your following patch did:
eval line='$line" "${'$index'}'

> 
> 
> > +line=$line" "$item
> > +lfmt=$lfmt$format
> > +fi
> > +index=$(expr $index + $rows)
> > +done
> > +printf "$lfmt\n" $line
> > +done
> 
> The function will pad the rightmost column up to the column
> width. It is usually not a visual problem in itself, but it
> could lead to additional line breaks. Especially considering
> that the typical values are significantly less than 24 chars.
> 

yes, there will be additional spaces to fill the 24 chars for each name. Will 
remove with sed as your patch did.

> 
> >  }
> >
> >  show_list() {
> >  suffix=_$1
> >  shift
> > -echo $* | sed s/$suffix//g | print_in_columns
> > +print_in_columns $(echo $* | sed s/$suffix//g | tr ' ' '\n' | sort)
> 
> You are changing the interface of the print_in_columns:
> 1. passing input as paramters
> 2. making the sort external
> 
> Point 1 makes it a bit harder to read; preprocessing + sorting
> pipelines are soaked into a command expansion that gets splitted
> into the individual parameters.
> 
> Point 2 makes it more flexible, so it shouldn't be a problem.
> If we decide to implement it this way, it might be better to
> mention that change in the commit message explicitly.

will add in the commit message.

> 
> [...]
> 
> I have experimented locally, inspired by your patch, I came
> up with this version:
> 
> print_in_columns() {
> col_width=24
> cols=$(expr $ncols / $col_width)
> rows=$(expr $(expr $# + $cols - 1) / $cols)
> 
> for idx in $(seq $rows); do
> slice=
> fmt=
> for col in $(seq $cols); do
> if test $idx -le $#; then
> eval 

Re: [FFmpeg-devel] [PATCH V2] lavfi/frei0r: Fixes the compilation warnings

2019-04-22 Thread myp...@gmail.com
On Sun, Apr 21, 2019 at 9:03 PM Paul B Mahol  wrote:
>
> On 4/21/19, Jun Zhao  wrote:
> > From: Jun Zhao 
> >
> > Fixes the compilation warnings
> >
> > Signed-off-by: Jun Zhao 
> > ---
> >  libavfilter/vf_frei0r.c |2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/libavfilter/vf_frei0r.c b/libavfilter/vf_frei0r.c
> > index c775ed1..165fbd7 100644
> > --- a/libavfilter/vf_frei0r.c
> > +++ b/libavfilter/vf_frei0r.c
> > @@ -127,7 +127,7 @@ static int set_param(AVFilterContext *ctx,
> > f0r_param_info_t info, int index, cha
> >  break;
> >
> >  case F0R_PARAM_STRING:
> > -val.str = param;
> > +val.str = (f0r_param_string *)param;
> >  break;
> >  }
> >
> > --
> > 1.7.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".
>
> lgtm
Pushed, Thanks the review and comment.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH V2] lavf/oggparsevorbis: Fix change the case of metadata keys issue

2019-04-22 Thread myp...@gmail.com
On Mon, Apr 22, 2019 at 10:43 PM Derek Buitenhuis
 wrote:
>
> On 22/04/2019 13:19, myp...@gmail.com wrote:
> > Ping?
> >
>
> LGTM.
>
> - Derek
Pushed, Thanks
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH V2 1/2] configure: sort decoder/encoder/filter/... names in alphabet order

2019-04-22 Thread Alexander Strasser
Hi!

On 2019-04-15 21:23 +0800, Guo, Yejun wrote:
> take decoder names an example, with the default page length, shell command
> 'pr' needs two pages for all the decoder names. The names are firstly printed
> in the first page, then in the second page. So, as a whole, the names are
> sorted neither in column order nor in row order. It's a little confused.
>
> One method is to calculate the proper page length, so all the names are 
> printed
> in one page by 'pr -l', and so strictly in alphabet order, column by column.
>
> Another method is to use command printf instead of pr, because buybox doesn't
> have pr. This patch refines print_in_columns to print the names with printf
> in alphabet order, very similar with 'pr -l', except the case when the last
> column is not fully filled with names.

Looks promising. Though this kind of change is basically
a bit difficult.

There is some risk that it won't work as expected on all
currently supported shells or perform differently.

For the performance as long as it is not grave, it should
not be a problem. Actually this implementation looks faster
here.

If it won't work as expected the risk is kind of limited,
as long as it doesn't modify global state that matters
and it does not abort the configure script, "only" the
informative / diagnostic output will be flawed, but the
build itself will be fine.


> Signed-off-by: Guo, Yejun 
> ---
>  configure | 34 +-
>  1 file changed, 25 insertions(+), 9 deletions(-)
>
> diff --git a/configure b/configure
> index c2580b3..45a9126 100755
> --- a/configure
> +++ b/configure
> @@ -3830,14 +3830,30 @@ die_unknown(){
>  }
>
>  print_in_columns() {
> -cols=$(expr $ncols / 24)
> -cat | tr ' ' '\n' | sort | pr -r "-$cols" -w $ncols -t
> +width=24
> +cols=$(expr $ncols / $width)
> +rows=$(expr $(expr $# + $cols - 1) / $cols)

> +eval format="%-${width}s"

Why do you use eval here?


> +for row in $(seq 1 $rows); do

I think the 1 argument to seq isn't needed here and below.


> +index=$row
> +line=""
> +lfmt=""
> +for col in $(seq 1 $cols); do
> +if [ $index -le $# ]; then
> +eval item=\$${index}

This will not work on all shells when trying to expand
the 10th parameter and following.

Excerpt of the relevant POSIX section:

When a positional parameter with more than one digit is specified, the 
application shall enclose the digits in braces (see Parameter Expansion).


> +line=$line" "$item
> +lfmt=$lfmt$format
> +fi
> +index=$(expr $index + $rows)
> +done
> +printf "$lfmt\n" $line
> +done

The function will pad the rightmost column up to the column
width. It is usually not a visual problem in itself, but it
could lead to additional line breaks. Especially considering
that the typical values are significantly less than 24 chars.


>  }
>
>  show_list() {
>  suffix=_$1
>  shift
> -echo $* | sed s/$suffix//g | print_in_columns
> +print_in_columns $(echo $* | sed s/$suffix//g | tr ' ' '\n' | sort)

You are changing the interface of the print_in_columns:
1. passing input as paramters
2. making the sort external

Point 1 makes it a bit harder to read; preprocessing + sorting
pipelines are soaked into a command expansion that gets splitted
into the individual parameters.

Point 2 makes it more flexible, so it shouldn't be a problem.
If we decide to implement it this way, it might be better to
mention that change in the commit message explicitly.

[...]

I have experimented locally, inspired by your patch, I came
up with this version:

print_in_columns() {
col_width=24
cols=$(expr $ncols / $col_width)
rows=$(expr $(expr $# + $cols - 1) / $cols)

for idx in $(seq $rows); do
slice=
fmt=
for col in $(seq $cols); do
if test $idx -le $#; then
eval slice='"$slice "${'$idx'}'
fmt="${fmt}%-${col_width}s"
fi
idx=$(expr $idx + $rows)
done
printf "$fmt\n" $slice
done | sed 's/ *$//'
}

Tested with bash, dash and mksh.


  Alexander

P.S.
Below follows the diff for a random configure command I tested here,
compared to the current implementation.

It's equivalent to the output of your implementation, minus the
trailing spaces after the last column. Speed is nearly the same.

diff --git a/configure-z.log b/configure-b.log
index e9a1834f4a..ca0bafdeef 100644
--- a/configure-z.log
+++ b/configure-b.log
@@ -60,157 +60,157 @@ Programs:
 ffmpeg  ffplay  ffprobe

 Enabled decoders:
-aacatrac1  eatqi
-aac_fixed  atrac3  eightbps
-aac_latm   atrac3aleightsvx_exp
-aasc   atrac3p eightsvx_fib
-ac3atrac3pal   escape124
-ac3_fixed

Re: [FFmpeg-devel] [PATCH] avfilter/af_astats: count number of NaNs/Infs/denormals for floating-point audio too

2019-04-22 Thread Alexander Strasser
Hi Paul,

just three small comments from me...

On 2019-04-22 11:51 +0200, Paul B Mahol wrote:
> Signed-off-by: Paul B Mahol 
> ---
>  doc/filters.texi|  6 +++
>  libavfilter/af_astats.c | 86 ++---
>  2 files changed, 86 insertions(+), 6 deletions(-)

I was a bit surprised when I first saw the number of lines it
takes to add this feature. OTOH this is not a problem of this
patch, because it is mostly caused by the structure of the
code that was in place before.

Changing the structure doesn't seem worth it yet. If
there will be an addition that is applicable to all the
individual stats, it should IMHO be reconsidered.


> diff --git a/doc/filters.texi b/doc/filters.texi
> index cfff9b1f4d..945c557e8f 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -2141,6 +2141,9 @@ Bit_depth
>  Dynamic_range
>  Zero_crossings
>  Zero_crossings_rate

> +Number_of_NaNs
> +Number_of_Infs

> +Number_of_denormals

Nit: Maybe Number_of_NaNs and Number_of_Infs should not be
capitalized like that, e.g. just use Number_of_nans and
Number_of_infs . Anyway if there's a reason for choosing
this names, it should be OK as it is. Just thinking it is
harder to type these from memory if the spelling is a bit
arbitrary.


>  and for Overall:
>  DC_offset
> @@ -2158,6 +2161,9 @@ Flat_factor
>  Peak_count
>  Bit_depth
>  Number_of_samples
> +Number_of_NaNs
> +Number_of_Infs
> +Number_of_denormals
>
>  For example full key look like this @code{lavfi.astats.1.DC_offset} or
>  this @code{lavfi.astats.Overall.Peak_count}.
> diff --git a/libavfilter/af_astats.c b/libavfilter/af_astats.c
> index 92368793c2..25c700b344 100644
> --- a/libavfilter/af_astats.c
> +++ b/libavfilter/af_astats.c
> @@ -20,6 +20,7 @@
>   */
>
>  #include 
> +#include 
>
>  #include "libavutil/opt.h"
>  #include "audio.h"
> @@ -48,6 +49,9 @@
>  #define MEASURE_ZERO_CROSSINGS  (1 << 16)
>  #define MEASURE_ZERO_CROSSINGS_RATE (1 << 17)
>  #define MEASURE_NUMBER_OF_SAMPLES   (1 << 18)
> +#define MEASURE_NUMBER_OF_NANS  (1 << 19)
> +#define MEASURE_NUMBER_OF_INFS  (1 << 20)
> +#define MEASURE_NUMBER_OF_DENORMALS (1 << 21)
>
>  #define MEASURE_MINMAXPEAK  (MEASURE_MIN_LEVEL | 
> MEASURE_MAX_LEVEL | MEASURE_PEAK_LEVEL)
>
> @@ -68,6 +72,9 @@ typedef struct ChannelStats {
>  uint64_t min_count, max_count;
>  uint64_t zero_runs;
>  uint64_t nb_samples;
> +uint64_t nb_nans;
> +uint64_t nb_infs;
> +uint64_t nb_denormals;
>  } ChannelStats;
>
>  typedef struct AudioStatsContext {
> @@ -83,6 +90,8 @@ typedef struct AudioStatsContext {
>  int maxbitdepth;
>  int measure_perchannel;
>  int measure_overall;
> +int is_float;
> +int is_double;
>  } AudioStatsContext;
>
>  #define OFFSET(x) offsetof(AudioStatsContext, x)
> @@ -114,6 +123,9 @@ static const AVOption astats_options[] = {
>{ "Zero_crossings", "", 0, AV_OPT_TYPE_CONST, 
> {.i64=MEASURE_ZERO_CROSSINGS  }, 0, 0, FLAGS, "measure" },
>{ "Zero_crossings_rate"   , "", 0, AV_OPT_TYPE_CONST, 
> {.i64=MEASURE_ZERO_CROSSINGS_RATE }, 0, 0, FLAGS, "measure" },
>{ "Number_of_samples" , "", 0, AV_OPT_TYPE_CONST, 
> {.i64=MEASURE_NUMBER_OF_SAMPLES   }, 0, 0, FLAGS, "measure" },
> +  { "Number_of_NaNs", "", 0, AV_OPT_TYPE_CONST, 
> {.i64=MEASURE_NUMBER_OF_NANS  }, 0, 0, FLAGS, "measure" },
> +  { "Number_of_Infs", "", 0, AV_OPT_TYPE_CONST, 
> {.i64=MEASURE_NUMBER_OF_INFS  }, 0, 0, FLAGS, "measure" },
> +  { "Number_of_denormals"   , "", 0, AV_OPT_TYPE_CONST, 
> {.i64=MEASURE_NUMBER_OF_DENORMALS }, 0, 0, FLAGS, "measure" },
>  { "measure_overall", "only measure_perchannel these overall statistics", 
> OFFSET(measure_overall), AV_OPT_TYPE_FLAGS, {.i64=MEASURE_ALL}, 0, UINT_MAX, 
> FLAGS, "measure" },
>  { NULL }
>  };
> @@ -181,6 +193,9 @@ static void reset_stats(AudioStatsContext *s)
>  p->max_count = 0;
>  p->zero_runs = 0;
>  p->nb_samples = 0;
> +p->nb_nans = 0;
> +p->nb_infs = 0;
> +p->nb_denormals = 0;
>  }
>  }
>
> @@ -196,6 +211,11 @@ static int config_output(AVFilterLink *outlink)
>  s->tc_samples = 5 * s->time_constant * outlink->sample_rate + .5;
>  s->nb_frames = 0;
>  s->maxbitdepth = av_get_bytes_per_sample(outlink->format) * 8;
> +s->is_double = outlink->format == AV_SAMPLE_FMT_DBL  ||
> +   outlink->format == AV_SAMPLE_FMT_DBLP;
> +
> +s->is_float = outlink->format == AV_SAMPLE_FMT_FLT  ||
> +  outlink->format == AV_SAMPLE_FMT_FLTP;
>
>  reset_stats(s);
>
> @@ -280,6 +300,24 @@ static inline void update_stat(AudioStatsContext *s, 
> ChannelStats *p, double d,
>  p->nb_samples++;
>  }
>
> +static inline void update_float_stat(AudioStatsContext *s, ChannelStats *p, 
> float d)

I guess "float d" is rather uncommon, but it's not unseen in the
FFmpeg code base.


> +{
> +   

Re: [FFmpeg-devel] [PATCH 14/15] avformat/matroskaenc: Improve log messages for blocks

2019-04-22 Thread Andreas Rheinhardt
Michael Niedermayer:
> On Sun, Apr 21, 2019 at 11:04:00PM +, Andreas Rheinhardt wrote:
>> Michael Niedermayer:
>>> On Sat, Apr 20, 2019 at 01:41:09AM +0200, Andreas Rheinhardt wrote:
 Up until now, a block's relative offset has been reported as the offset
 in the log messages output when writing blocks; given that it is
 impossible to know the real offset from the beginning of the file at
 this point due to the fact that it is not yet known how many bytes will
 be used for the containing cluster's length field both the relative
 offset in the cluster as well as the offset of the containing cluster
 will be reported from now on.

 Also, the log message for writing vtt blocks has been brought in line
 with the message for normal blocks.

 Signed-off-by: Andreas Rheinhardt 
 ---
  libavformat/matroskaenc.c | 15 ---
  1 file changed, 8 insertions(+), 7 deletions(-)

 diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
 index 441315e2d5..fc0e95440e 100644
 --- a/libavformat/matroskaenc.c
 +++ b/libavformat/matroskaenc.c
 @@ -2124,10 +2124,10 @@ static void mkv_write_block(AVFormatContext *s, 
 AVIOContext *pb,
  
  ts += mkv->tracks[pkt->stream_index].ts_offset;
  
 -av_log(s, AV_LOG_DEBUG, "Writing block at offset %" PRIu64 ", size 
 %d, "
 -   "pts %" PRId64 ", dts %" PRId64 ", duration %" PRId64 ", 
 keyframe %d\n",
 -   avio_tell(pb), pkt->size, pkt->pts, pkt->dts, pkt->duration,
 -   keyframe != 0);
 +av_log(s, AV_LOG_DEBUG, "Writing block at relative offset %" PRId64 " 
 in "
 +   "cluster at offset %" PRId64 "; size %d, pts %" PRId64 ", dts 
 %" PRId64
 +   ", duration %" PRId64 ", keyframe %d\n", avio_tell(pb), 
 mkv->cluster_pos,
 +   pkt->size, pkt->pts, pkt->dts, pkt->duration, keyframe != 0);
  if (par->codec_id == AV_CODEC_ID_H264 && par->extradata_size > 0 &&
  (AV_RB24(par->extradata) == 1 || AV_RB32(par->extradata) == 1))
  ff_avc_parse_nal_units_buf(pkt->data, , );
>>>
 @@ -2231,9 +2231,10 @@ static int mkv_write_vtt_blocks(AVFormatContext *s, 
 AVIOContext *pb, AVPacket *p
  
  size = id_size + 1 + settings_size + 1 + pkt->size;
  
 -av_log(s, AV_LOG_DEBUG, "Writing block at offset %" PRIu64 ", size 
 %d, "
 -   "pts %" PRId64 ", dts %" PRId64 ", duration %" PRId64 ", flags 
 %d\n",
 -   avio_tell(pb), size, pkt->pts, pkt->dts, pkt->duration, flags);
 +av_log(s, AV_LOG_DEBUG, "Writing block at relative offset %" PRId64 " 
 in "
 +   "cluster at offset %" PRId64 "; size %d, pts %" PRId64 ", dts 
 %" PRId64
 +   ", duration %" PRId64 ", keyframe %d\n", avio_tell(pb), 
 mkv->cluster_pos,
 +   size, pkt->pts, pkt->dts, pkt->duration, 1);
>>>
>>> It does feel a bit odd to pass a litteral 1 as argument to %d to print a 1.
>>> why is that not a "1" or ommited ?
>>> also these 2 look a bit duplicated
>>> and its a unreadable spagethi, iam sure this can be made more readable with
>>> some line break changes
>>
>> The duplication is intentional: It means that there needs to be only
>> one string literal in the actual binary. Given that the first
> 
> hmm, ok
> it is still duplicated in the source though and this can mutate by being
> edited by someone not knowing it should stay in sync.
> so it may be better to put that string in s static const or #define
> or the printing code itself could be in its own function
> 
Actually, I intend to integrate the writing of WebVTT subtitles into
the main write_block function. This will probably be done once I find
the time to implement support for the Matroska way of WebVTT subtitles
(currently only the WebM way is supported). This uses BlockAdditions
and so it would be natural to do this inside write_block as it already
contains code for writing block additions.
But for the time being, a comment will suffice. (The last change to
this part of the code happened four years ago, so I don't think that
someone will inadvertently unsync them any time soon.)

- Andreas

PS: The Matroska muxer currently doesn't write the MaxBlockAdditionID
element at all, but this is needed for tracks with BlockAdditions,
i.e. our muxer currently creates spec-incompliant files if it uses
BlockAdditions. But that's something for another day, too.
___
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: fix inputs initialization in mpegts muxer with filters

2019-04-22 Thread Michael Niedermayer
On Mon, Apr 22, 2019 at 11:42:57AM +, Andreas Håkon wrote:
> 
> ‐‐‐ Original Message ‐‐‐
> On Friday, 19 de April de 2019 17:08, Michael Niedermayer 
>  wrote:
> 
> > On Fri, Apr 19, 2019 at 08:23:35AM +, Andreas Håkon via ffmpeg-devel 
> > wrote:
> >
> > > From 936740731c17a9757aa093bdb35d9772df1e64a8 Mon Sep 17 00:00:00 2001
> > > From: Andreas Hakon andreas.ha...@protonmail.com
> > > Date: Fri, 19 Apr 2019 09:17:32 +0100
> > > Subject: [PATCH] libavformat: input init mpegts with filters v2
> > > This patch solves the initialization of the inputs when using filters
> > > (a graph filter) with the mpegts muxer.
> > > This bug seems to be generated by a simple forgetting to copy. The same 
> > > code is
> > > repeated two times, but only in one case the variable inputs_done is 
> > > initialized.
> >
> > > Testcase:
> > > $ ffmpeg -f mpegts -i mpeg.ts \
> > > -filter_complex "[i:100]fps=fps=25[out]" \
> > > -map "[out]" -c:v libx264 -f mpegts out.ts
> >
> > i see no difference in the output (same md5)
> > with what input file does this show a difference?
> 
> 
> Hi Michael,
> 
> Let me to explain the problem inside the code. Here the relevant part of the
> file "/fftools/ffmpeg.c" (line #4606):
> 
> -8<--
> if (ost->filter && ost->filter->graph->graph) {
> if (!ost->initialized) {
> char error[1024] = {0};
> ret = init_output_stream(ost, error, sizeof(error));
> if (ret < 0) {
> av_log(NULL, AV_LOG_ERROR, "Error initializing output stream 
> %d:%d -- %s\n",
>ost->file_index, ost->index, error);
> exit_program(1);
> }
> }
> if ((ret = transcode_from_filter(ost->filter->graph, )) < 0)
> return ret;
> if (!ist)
> return 0;
> } else if (ost->filter) {
> int i;
> for (i = 0; i < ost->filter->graph->nb_inputs; i++) {
> InputFilter *ifilter = ost->filter->graph->inputs[i];
> if (!ifilter->ist->got_output && 
> !input_files[ifilter->ist->file_index]->eof_reached) {
> ist = ifilter->ist;
> break;
> }
> }
> if (!ist) {
> ost->inputs_done = 1;
> return 0;
> }
> } else {
> av_assert0(ost->source_index >= 0);
> ist = input_streams[ost->source_index];
> }
> -8<--
> 
> Please, note the first block of the "if (ost->filter && 
> ost->filter->graph->graph)".
> This block normally returns with "if (!ist) return 0;".
> 
> But the second block (the one "else if (ost->filter)") returns with
> "if (!ist) {ost->inputs_done = 1; return 0;"
> 
> What the second block likes to protect is when the input stream is completed.
> However, the first block doesn't do it!
> 
> One difference is that the first block uses the transcode_from_filter() 
> function.
> But that not makes difference, as the second block iterates over the inputs, 
> and
> the function transcode_from_filter() does it too.
> 
> Futhermore, the "ost->inputs_done" is used only in the function 
> "choose_output()"
> (at line #3882 of /fftools/ffmpeg.c):
> 
> -8<--
> static OutputStream *choose_output(void)
> {
> int i;
> int64_t opts_min = INT64_MAX;
> OutputStream *ost_min = NULL;
> 
> for (i = 0; i < nb_output_streams; i++) {
> OutputStream *ost = output_streams[i];
> int64_t opts = ost->st->cur_dts == AV_NOPTS_VALUE ? INT64_MIN :
>av_rescale_q(ost->st->cur_dts, ost->st->time_base,
> AV_TIME_BASE_Q);
> if (ost->st->cur_dts == AV_NOPTS_VALUE)
> av_log(NULL, AV_LOG_DEBUG,
> "cur_dts is invalid st:%d (%d) [init:%d i_done:%d finish:%d] 
> (this is harmless if it occurs once at the start per stream)\n",
> ost->st->index, ost->st->id, ost->initialized, 
> ost->inputs_done, ost->finished);
> 
> if (!ost->initialized && !ost->inputs_done)
> return ost;
> 
> if (!ost->finished && opts < opts_min) {
> opts_min = opts;
> ost_min  = ost->unavailable ? NULL : ost;
> }
> }
> return ost_min;
> }
> 8<--
> 
> So, the case with troubles is when the output stream is selected because
> is not initilized and no inputs are completed. And without this patch
> it's possible that inputs can be completed but not marked as done.
> 
> I assume that the error is a simple incorrect "copy" of the original
> code, as the second block sets the "ost->inputs_done = 1;" but not the
> first one. And the difference between both blocks is that the first
> is when a filter graph is running.


> 
> Please, revise the code! I hope you understand it when you look at my
> descriptions.

I would prefer to have some 

Re: [FFmpeg-devel] [PATCH] libavformat: fix copyts and muxrate in mpegts muxer

2019-04-22 Thread Michael Niedermayer
On Fri, Apr 19, 2019 at 08:47:34AM +, Andreas Håkon via ffmpeg-devel wrote:
> ‐‐‐ Original Message ‐‐‐
> On Thursday, 18 de April de 2019 11:01, Andreas Håkon via ffmpeg-devel 
>  wrote:
> 
> > Hi,
> >
> > This patch resolves one very specific use case:
> >
> > -   When you use the mpegts muxer;
> > -   And use the global parameter “-copyts”;
> > -   And use the parameter “-muxrate” for the mpegts muxer;
> > -   And use too the parameter “-mpegts_copyts”.
> >
> > The problem is created because the member “first_pcr” of the 
> > MpegTSWrite struct isn’t initialized with a correct timestamp (so when 
> > copying the timestamps the initial value is 0). And in this case an 
> > infinite loop is created because the code never writes PES packets when the 
> > “mux_rate” isn’t VBR (equals to 1). See the block that creates the loop 
> > here (note the "continue" command at end):
> > 
> > https://github.com/FFmpeg/FFmpeg/blob/a0559fcd81f42f446c93357a943699f9d44eeb79/libavformat/mpegtsenc.c#L1211
> >
> > So, this patch fixes the problem initializing the “first_pcr” with the 
> > first DTS value that comes when using the incoming timestamps.
> >
> > Regards.
> > A.H.
> >
> 
> Hi,
> 
> Here a new version of the patch.
> 
> The "fist_pcr" value is now derived from DTS in a more consistent way.
> 
> Regards,
> A.H.
> 
> 
> ---
> 

> From c59569ca9426fef455edabfa648cb2ff678c1640 Mon Sep 17 00:00:00 2001
> From: Andreas Hakon 
> Date: Fri, 19 Apr 2019 09:32:33 +0100
> Subject: [PATCH] libavformat: fix copyts and muxrate in mpegts muxer v2
> 
> When using "-copyts" and "-muxrate" with the mpegts muxer the muxing fails
> because the member "first_pcr" of the MpegTSWrite struct isn't initialized
> with a correct timestamp.
> 
> The behaviour of the error is an infinite loop created in the function
> mpegts_write_pes() because the code never writes PES packets.
> 
> This patch resolves the problem initializing the "first_pcr" with a value
> derived from the first DTS value seen.
> 
> Signed-off-by: Andreas Hakon 
> ---
>  libavformat/mpegtsenc.c |   10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
> index fc0ea22..858b0d7 100644
> --- a/libavformat/mpegtsenc.c
> +++ b/libavformat/mpegtsenc.c
> @@ -971,6 +971,9 @@ static int mpegts_init(AVFormatContext *s)
>  
>  if (ts->copyts < 1)
>  ts->first_pcr = av_rescale(s->max_delay, PCR_TIME_BASE, 
> AV_TIME_BASE);
> +else
> +ts->first_pcr = AV_NOPTS_VALUE;
> +
>  } else {
>  /* Arbitrary values, PAT/PMT will also be written on video key 
> frames */
>  ts->sdt_packet_period = 200;
> @@ -1186,12 +1189,16 @@ static void mpegts_write_pes(AVFormatContext *s, 
> AVStream *st,
>  int64_t pcr = -1; /* avoid warning */
>  int64_t delay = av_rescale(s->max_delay, 9, AV_TIME_BASE);
>  int force_pat = st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO && key && 
> !ts_st->prev_payload_key;
> +int last_payload_size = 0;
>  
>  av_assert0(ts_st->payload != buf || st->codecpar->codec_type != 
> AVMEDIA_TYPE_VIDEO);
>  if (ts->flags & MPEGTS_FLAG_PAT_PMT_AT_FRAMES && 
> st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) {
>  force_pat = 1;
>  }
>  
> +if (ts->mux_rate > 1 && dts != AV_NOPTS_VALUE && ts->first_pcr == 
> AV_NOPTS_VALUE)
> +ts->first_pcr = (dts * 300) - av_rescale(s->max_delay, 
> PCR_TIME_BASE, AV_TIME_BASE);
> +

if this doesnt execute  first_pcr could remain AV_NOPTS_VALUE, this looks
a bit fragile to me as there is code using first_pcr with implicit assumtation
that it is not AV_NOPTS_VALUE in get_pcr().
is something preventing this combination ?


>  is_start = 1;
>  while (payload_size > 0) {
>  retransmit_si_info(s, force_pat, dts);

> @@ -1209,12 +1216,13 @@ static void mpegts_write_pes(AVFormatContext *s, 
> AVStream *st,
>  }
>  
>  if (ts->mux_rate > 1 && dts != AV_NOPTS_VALUE &&
> -(dts - get_pcr(ts, s->pb) / 300) > delay) {
> +last_payload_size != payload_size && (dts - get_pcr(ts, s->pb) / 
> 300) > delay) {
>  /* pcr insert gets priority over null packet insert */
>  if (write_pcr)
>  mpegts_insert_pcr_only(s, st);
>  else
>  mpegts_insert_null_packet(s);
> +last_payload_size = payload_size;

why is the check on payload_size needed ? (it is not for the testcase)
the commit message also seems not to explain this part

thanks

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

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


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

To unsubscribe, visit link above, or email

Re: [FFmpeg-devel] [PATCH 14/15] avformat/matroskaenc: Improve log messages for blocks

2019-04-22 Thread Michael Niedermayer
On Sun, Apr 21, 2019 at 11:04:00PM +, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > On Sat, Apr 20, 2019 at 01:41:09AM +0200, Andreas Rheinhardt wrote:
> >> Up until now, a block's relative offset has been reported as the offset
> >> in the log messages output when writing blocks; given that it is
> >> impossible to know the real offset from the beginning of the file at
> >> this point due to the fact that it is not yet known how many bytes will
> >> be used for the containing cluster's length field both the relative
> >> offset in the cluster as well as the offset of the containing cluster
> >> will be reported from now on.
> >>
> >> Also, the log message for writing vtt blocks has been brought in line
> >> with the message for normal blocks.
> >>
> >> Signed-off-by: Andreas Rheinhardt 
> >> ---
> >>  libavformat/matroskaenc.c | 15 ---
> >>  1 file changed, 8 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> >> index 441315e2d5..fc0e95440e 100644
> >> --- a/libavformat/matroskaenc.c
> >> +++ b/libavformat/matroskaenc.c
> >> @@ -2124,10 +2124,10 @@ static void mkv_write_block(AVFormatContext *s, 
> >> AVIOContext *pb,
> >>  
> >>  ts += mkv->tracks[pkt->stream_index].ts_offset;
> >>  
> >> -av_log(s, AV_LOG_DEBUG, "Writing block at offset %" PRIu64 ", size 
> >> %d, "
> >> -   "pts %" PRId64 ", dts %" PRId64 ", duration %" PRId64 ", 
> >> keyframe %d\n",
> >> -   avio_tell(pb), pkt->size, pkt->pts, pkt->dts, pkt->duration,
> >> -   keyframe != 0);
> >> +av_log(s, AV_LOG_DEBUG, "Writing block at relative offset %" PRId64 " 
> >> in "
> >> +   "cluster at offset %" PRId64 "; size %d, pts %" PRId64 ", dts 
> >> %" PRId64
> >> +   ", duration %" PRId64 ", keyframe %d\n", avio_tell(pb), 
> >> mkv->cluster_pos,
> >> +   pkt->size, pkt->pts, pkt->dts, pkt->duration, keyframe != 0);
> >>  if (par->codec_id == AV_CODEC_ID_H264 && par->extradata_size > 0 &&
> >>  (AV_RB24(par->extradata) == 1 || AV_RB32(par->extradata) == 1))
> >>  ff_avc_parse_nal_units_buf(pkt->data, , );
> > 
> >> @@ -2231,9 +2231,10 @@ static int mkv_write_vtt_blocks(AVFormatContext *s, 
> >> AVIOContext *pb, AVPacket *p
> >>  
> >>  size = id_size + 1 + settings_size + 1 + pkt->size;
> >>  
> >> -av_log(s, AV_LOG_DEBUG, "Writing block at offset %" PRIu64 ", size 
> >> %d, "
> >> -   "pts %" PRId64 ", dts %" PRId64 ", duration %" PRId64 ", flags 
> >> %d\n",
> >> -   avio_tell(pb), size, pkt->pts, pkt->dts, pkt->duration, flags);
> >> +av_log(s, AV_LOG_DEBUG, "Writing block at relative offset %" PRId64 " 
> >> in "
> >> +   "cluster at offset %" PRId64 "; size %d, pts %" PRId64 ", dts 
> >> %" PRId64
> >> +   ", duration %" PRId64 ", keyframe %d\n", avio_tell(pb), 
> >> mkv->cluster_pos,
> >> +   size, pkt->pts, pkt->dts, pkt->duration, 1);
> > 
> > It does feel a bit odd to pass a litteral 1 as argument to %d to print a 1.
> > why is that not a "1" or ommited ?
> > also these 2 look a bit duplicated
> > and its a unreadable spagethi, iam sure this can be made more readable with
> > some line break changes
> 
> The duplication is intentional: It means that there needs to be only
> one string literal in the actual binary. Given that the first

hmm, ok
it is still duplicated in the source though and this can mutate by being
edited by someone not knowing it should stay in sync.
so it may be better to put that string in s static const or #define
or the printing code itself could be in its own function

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Awnsering whenever a program halts or runs forever is
On a turing machine, in general impossible (turings halting problem).
On any real computer, always possible as a real computer has a finite number
of states N, and will either halt in less than N cycles or never halt.


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/ccaption_dec: Add a blank like at the end to avoid rollup reading from outside

2019-04-22 Thread Michael Niedermayer
On Mon, Apr 22, 2019 at 09:49:41AM +0200, Alexander Strasser wrote:
> Hi Michael!
> 
> On 2019-04-20 18:11 +0200, Michael Niedermayer wrote:
> > Fixes: index 20 out of bounds for type 'const char *[4][128]'
> 
> Somehow I don't understand this diagnostic message. It says "index
> 20 out of bounds for [4][128]".
> 
> You're change looks like an off-by-one fix.
> 
> Sorry; I'm surely missing something as I didn't look up the code,
> just thought I might as well ask...

yes, the out of array read causes a value that is out of range to be
used and later cause the quoted 2nd out of array read

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

The educated differ from the uneducated as much as the living from the
dead. -- 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".

[FFmpeg-devel] [PATCH] avcodec/zmbv: optimize motion compensation with memcpy()

2019-04-22 Thread Michael Niedermayer
Fixes: Timeout (16 sec - 7 sec)
Fixes: 
14237/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ZMBV_fuzzer-5693453897302016

Found-by: continuous fuzzing process 
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer 
---
 libavcodec/zmbv.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/libavcodec/zmbv.c b/libavcodec/zmbv.c
index 898b62d065..99e735cfd9 100644
--- a/libavcodec/zmbv.c
+++ b/libavcodec/zmbv.c
@@ -121,6 +121,8 @@ static int zmbv_decode_xor_8(ZmbvContext *c)
 for (j = 0; j < bh2; j++) {
 if (my + j < 0 || my + j >= c->height) {
 memset(out, 0, bw2);
+} else if (mx >= 0 && mx + bw2 <= c->width){
+memcpy(out, tprev, sizeof(*out) * bw2);
 } else {
 for (i = 0; i < bw2; i++) {
 if (mx + i < 0 || mx + i >= c->width)
@@ -193,6 +195,8 @@ static int zmbv_decode_xor_16(ZmbvContext *c)
 for (j = 0; j < bh2; j++) {
 if (my + j < 0 || my + j >= c->height) {
 memset(out, 0, bw2 * 2);
+} else if (mx >= 0 && mx + bw2 <= c->width){
+memcpy(out, tprev, sizeof(*out) * bw2);
 } else {
 for (i = 0; i < bw2; i++) {
 if (mx + i < 0 || mx + i >= c->width)
@@ -270,6 +274,8 @@ static int zmbv_decode_xor_24(ZmbvContext *c)
 for (j = 0; j < bh2; j++) {
 if (my + j < 0 || my + j >= c->height) {
 memset(out, 0, bw2 * 3);
+} else if (mx >= 0 && mx + bw2 <= c->width){
+memcpy(out, tprev, 3 * bw2);
 } else {
 for (i = 0; i < bw2; i++){
 if (mx + i < 0 || mx + i >= c->width) {
@@ -351,6 +357,8 @@ static int zmbv_decode_xor_32(ZmbvContext *c)
 for (j = 0; j < bh2; j++) {
 if (my + j < 0 || my + j >= c->height) {
 memset(out, 0, bw2 * 4);
+} else if (mx >= 0 && mx + bw2 <= c->width){
+memcpy(out, tprev, sizeof(*out) * bw2);
 } else {
 for (i = 0; i < bw2; i++){
 if (mx + i < 0 || mx + i >= c->width)
-- 
2.21.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] avdevice/alsa: fix indefinite stop on closing PCM capture

2019-04-22 Thread Nicolas George
Lou Logan (12019-04-19):
> Attached patch resolves the issue mentioned within.

> >From 10800493523b9274e7cc8784b65cc183a94b7281 Mon Sep 17 00:00:00 2001
> From: Takayuki 'January June' Suwa 
> Date: Thu, 18 Apr 2019 10:56:40 +0900
> Subject: [PATCH] avdevice/alsa: fix indefinite stop on closing PCM capture
> 
> Sorry, I forgot to take recording into account...
> 
> Fixes:  https://bugs.archlinux.org/task/58619
> 
> Found-by: Elias (Bleuzen) https://bugs.archlinux.org/user/26956
> ---
>  libavdevice/alsa.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)

Sorry, missed it. LGTM.

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] avdevice/alsa: fix indefinite stop on closing PCM capture

2019-04-22 Thread Lou Logan
On Fri, 19 Apr 2019 10:54:26 -0800
Lou Logan  wrote:

> From 10800493523b9274e7cc8784b65cc183a94b7281 Mon Sep 17 00:00:00 2001
> From: Takayuki 'January June' Suwa 
> Date: Thu, 18 Apr 2019 10:56:40 +0900
> Subject: [PATCH] avdevice/alsa: fix indefinite stop on closing PCM capture
> 
> Sorry, I forgot to take recording into account...
> 
> Fixes:  https://bugs.archlinux.org/task/58619
> 
> Found-by: Elias (Bleuzen) https://bugs.archlinux.org/user/26956
> ---
>  libavdevice/alsa.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/libavdevice/alsa.c b/libavdevice/alsa.c
> index 1b21beb6d5..117b2ea144 100644
> --- a/libavdevice/alsa.c
> +++ b/libavdevice/alsa.c
> @@ -300,8 +300,10 @@ av_cold int ff_alsa_close(AVFormatContext *s1)
>  {
>  AlsaData *s = s1->priv_data;
>  
> -snd_pcm_nonblock(s->h, 0);
> -snd_pcm_drain(s->h);
> +if (snd_pcm_stream(s->h) == SND_PCM_STREAM_PLAYBACK) {
> +snd_pcm_nonblock(s->h, 0);
> +snd_pcm_drain(s->h);
> +}
>  av_freep(>reorder_buf);
>  if (CONFIG_ALSA_INDEV)
>  ff_timefilter_destroy(s->timefilter);

Will push after 24 hours.
___
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: limit nb_frames_for_fps to INT_MAX

2019-04-22 Thread Dan Sanders
It's this or add overflow detection in mov_read_header().
---
 libavformat/mov.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index d5ce077e63..247a65ed11 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -2940,7 +2940,7 @@ static int mov_read_stts(MOVContext *c,
AVIOContext *pb, MOVAtom atom)

 if (duration > 0 &&
 duration <= INT64_MAX - sc->duration_for_fps &&
-total_sample_count <= INT64_MAX - sc->nb_frames_for_fps
+total_sample_count <= INT_MAX - sc->nb_frames_for_fps
 ) {
 sc->duration_for_fps  += duration;
 sc->nb_frames_for_fps += total_sample_count;
@@ -4897,7 +4897,7 @@ static int mov_read_trun(MOVContext *c,
AVIOContext *pb, MOVAtom atom)
 sc->data_size += sample_size;

 if (sample_duration <= INT64_MAX - sc->duration_for_fps &&
-1 <= INT64_MAX - sc->nb_frames_for_fps
+1 <= INT_MAX - sc->nb_frames_for_fps
 ) {
 sc->duration_for_fps += sample_duration;
 sc->nb_frames_for_fps ++;
-- 
2.21.0.593.g511ec345e18-goog
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[FFmpeg-devel] [PATCH] cuviddec: improved way of finding out if a frame is interlaced or progressive

2019-04-22 Thread Sergey Svechnikov
There are 2 types of problems when using adaptive deinterlace with cuvid:

1. Sometimes, in the middle of transcoding, cuvid outputs frames with visible 
horizontal lines (as though weave deinterlace method was chosen);
2. Occasionally, on scene changes, cuvid outputs a wrong frame, which should 
have been shown several seconds before (as if the frame was assigned some wrong 
PTS value).

The reason is that sometimes CUVIDPARSERDISPINFO has property progressive_frame 
equal to 1 with interlaced videos.
In order to fix the problem we should check if the video is interlaced or 
progressive in the beginning of a video sequence (cuvid_handle_video_sequence).
And then we just use this information instead of the property progressive_frame 
in CUVIDPARSERDISPINFO (which is unreliable).

More info, samples and reproduction steps are here 
https://github.com/Svechnikov/ffmpeg-cuda-deinterlace-problems
---
 libavcodec/cuviddec.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/libavcodec/cuviddec.c b/libavcodec/cuviddec.c
index 2aecb45..671fc8c 100644
--- a/libavcodec/cuviddec.c
+++ b/libavcodec/cuviddec.c
@@ -77,6 +77,7 @@ typedef struct CuvidContext
 int deint_mode;
 int deint_mode_current;
 int64_t prev_pts;
+unsigned char progressive_sequence;
 
 int internal_error;
 int decoder_flushing;
@@ -216,6 +217,8 @@ static int CUDAAPI cuvid_handle_video_sequence(void 
*opaque, CUVIDEOFORMAT* form
   ? cudaVideoDeinterlaceMode_Weave
   : ctx->deint_mode;
 
+ctx->progressive_sequence = format->progressive_sequence;
+
 if (!format->progressive_sequence && ctx->deint_mode_current == 
cudaVideoDeinterlaceMode_Weave)
 avctx->flags |= AV_CODEC_FLAG_INTERLACED_DCT;
 else
@@ -509,6 +512,8 @@ static int cuvid_output_frame(AVCodecContext *avctx, 
AVFrame *frame)
 
 av_fifo_generic_read(ctx->frame_queue, _frame, 
sizeof(CuvidParsedFrame), NULL);
 
+parsed_frame.dispinfo.progressive_frame = ctx->progressive_sequence;
+
 memset(, 0, sizeof(params));
 params.progressive_frame = parsed_frame.dispinfo.progressive_frame;
 params.second_field = parsed_frame.second_field;
-- 
2.7.4

___
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/cbs_h2645: add macros to read and write fields with no custom range of values

2019-04-22 Thread James Almer
On 4/16/2019 11:56 PM, James Almer wrote:
> Signed-off-by: James Almer 
> ---
> Better macro names welcome. I used the same convention as in cbs_av1.
> 
> fate-cbs passes, but i'm sure a bunch of these are not tested by it,
> so help double checking i didn't screw up is welcome.
> 
>  libavcodec/cbs_h2645.c|  10 +-
>  libavcodec/cbs_h264_syntax_template.c |  60 ++--
>  libavcodec/cbs_h265_syntax_template.c | 126 +++---
>  3 files changed, 90 insertions(+), 106 deletions(-)

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

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

[FFmpeg-devel] [PATCHv2 3/5] avformat/mxfdec: guess wrapping of tracks by other tracks with the same body sid

2019-04-22 Thread Marton Balint
This affects the following samples:

samples/ffmpeg-bugs/roundup/issue1775/av_seek_frame_failure.mxf
samples/ffmpeg-bugs/trac/ticket1957/16ch.mxf
samples/ffmpeg-bugs/trac/ticket5016/r0.mxf
samples/ffmpeg-bugs/trac/ticket5016/r1.mxf
samples/ffmpeg-bugs/trac/ticket5316/hq.MXF
samples/ffmpeg-bugs/trac/ticket5316/hqx.MXF

Some AVPacket->pos values are changed because for frame wrapped tracks we point
to the KLV offset and not the data.

Signed-off-by: Marton Balint 
---
 libavformat/mxfdec.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index 2c44852062..034025bcaa 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -2553,6 +2553,24 @@ static int mxf_parse_structural_metadata(MXFContext *mxf)
 }
 }
 
+for (int i = 0; i < mxf->fc->nb_streams; i++) {
+MXFTrack *track1 = mxf->fc->streams[i]->priv_data;
+if (track1 && track1->body_sid) {
+for (int j = i + 1; j < mxf->fc->nb_streams; j++) {
+MXFTrack *track2 = mxf->fc->streams[j]->priv_data;
+if (track2 && track1->body_sid == track2->body_sid && 
track1->wrapping != track2->wrapping) {
+if (track1->wrapping == UnknownWrapped)
+track1->wrapping = track2->wrapping;
+else if (track2->wrapping == UnknownWrapped)
+track2->wrapping = track1->wrapping;
+else
+av_log(mxf->fc, AV_LOG_ERROR, "stream %d and stream %d 
have the same BodySID (%d) "
+  "with different 
wrapping\n", i, j, track1->body_sid);
+}
+}
+}
+}
+
 ret = 0;
 fail_and_free:
 return ret;
-- 
2.16.4

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

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

Re: [FFmpeg-devel] [PATCH] avformat/mpegenc - reject unsupported audio streams

2019-04-22 Thread Gyan



On 22-04-2019 04:30 PM, Gyan wrote:



On 22-04-2019 01:15 PM, Gyan wrote:



On 22-04-2019 12:30 PM, Carl Eugen Hoyos wrote:



Am 20.04.2019 um 11:31 schrieb Gyan :


Old patch that was never applied. Rebased.

Please return patch_welcome for mlp and truehd.

Will do.
Wasn’t there another comment (not by me): “Why can’t .codec_tag be 
used?”


There's no codec_tag member set. This patch is just to disallow an 
invalid muxing to proceed.


Revised.


Pushed as 6829c3cbe480501725250c62cd1b920a75a44ec7

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

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

Re: [FFmpeg-devel] [PATCH V2] lavf/oggparsevorbis: Fix change the case of metadata keys issue

2019-04-22 Thread Derek Buitenhuis
On 22/04/2019 13:19, myp...@gmail.com wrote:
> Ping?
> 

LGTM.

- Derek
___
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] libavcodec: QSV protect GPB code with CO3 define

2019-04-22 Thread myp...@gmail.com
On Mon, Apr 22, 2019 at 8:42 PM Andreas Håkon
 wrote:
>
>
> ‐‐‐ Original Message ‐‐‐
> On Monday, 22 de April de 2019 14:17, myp...@gmail.com  
> wrote:
>
> > On Mon, Apr 22, 2019 at 7:38 PM Li, Zhong zhong...@intel.com wrote:
> >
> > > > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> > > > Of Andreas Håkon via ffmpeg-devel
> > > > Sent: Thursday, April 18, 2019 6:11 PM
> > > > To: FFmpeg development discussions and patches
> > > > ffmpeg-devel@ffmpeg.org
> > > > Cc: Andreas Håkon andreas.ha...@protonmail.com
> > > > Subject: [FFmpeg-devel] [PATCH] libavcodec: QSV protect GPB code with
> > > > CO3 define
> > > > Hi,
> > > > In response to this ticket I provide the first part of the patch:
> > > > https://trac.ffmpeg.org/ticket/7839
> > > > This QSV_HAVE_GPB code needs to be protected by QSV_HAVE_CO3.
> > > > Regards.
> > > > A.H.
> > >
> > > Why it is must? It is impossible that QSV_HAVE_GPB is enabled but 
> > > QSV_HAVE_CO3 is not enabled.
> > > QSV_HAVE_GPB is enabled by MSDK API v1.18, but QSV_HAVE_CO3 is API V1.11.
> >
> > In the ticket, Andreas Håkon pointed out that it is "only as a
> > temporary solution", so I think it's a workaround.
>
> Hi mypopy,
>
> My patch only resolves one part of the problem. It's not a temporary
> solution, but a forgot in the original code.
>
> I feel Mr. Zhong needs to fix the "mpeg2_qsv" encoder. But this simple
> patch can help with the "temporary solution" of disable QVBR in the code.
> In any case, this patch needs to be merged.
>
> Regards.
> A.H.

Thanks the details and clarification, I've got the point for this patch.
___
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] libavcodec: QSV protect GPB code with CO3 define

2019-04-22 Thread Andreas Håkon

‐‐‐ Original Message ‐‐‐
On Monday, 22 de April de 2019 14:17, myp...@gmail.com  wrote:

> On Mon, Apr 22, 2019 at 7:38 PM Li, Zhong zhong...@intel.com wrote:
>
> > > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> > > Of Andreas Håkon via ffmpeg-devel
> > > Sent: Thursday, April 18, 2019 6:11 PM
> > > To: FFmpeg development discussions and patches
> > > ffmpeg-devel@ffmpeg.org
> > > Cc: Andreas Håkon andreas.ha...@protonmail.com
> > > Subject: [FFmpeg-devel] [PATCH] libavcodec: QSV protect GPB code with
> > > CO3 define
> > > Hi,
> > > In response to this ticket I provide the first part of the patch:
> > > https://trac.ffmpeg.org/ticket/7839
> > > This QSV_HAVE_GPB code needs to be protected by QSV_HAVE_CO3.
> > > Regards.
> > > A.H.
> >
> > Why it is must? It is impossible that QSV_HAVE_GPB is enabled but 
> > QSV_HAVE_CO3 is not enabled.
> > QSV_HAVE_GPB is enabled by MSDK API v1.18, but QSV_HAVE_CO3 is API V1.11.
>
> In the ticket, Andreas Håkon pointed out that it is "only as a
> temporary solution", so I think it's a workaround.

Hi mypopy,

My patch only resolves one part of the problem. It's not a temporary
solution, but a forgot in the original code.

I feel Mr. Zhong needs to fix the "mpeg2_qsv" encoder. But this simple
patch can help with the "temporary solution" of disable QVBR in the code.
In any case, this patch needs to be merged.

Regards.
A.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] [PATCH] libavcodec: QSV protect GPB code with CO3 define

2019-04-22 Thread myp...@gmail.com
On Mon, Apr 22, 2019 at 7:38 PM Li, Zhong  wrote:
>
> > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> > Of Andreas Håkon via ffmpeg-devel
> > Sent: Thursday, April 18, 2019 6:11 PM
> > To: FFmpeg development discussions and patches
> > 
> > Cc: Andreas Håkon 
> > Subject: [FFmpeg-devel] [PATCH] libavcodec: QSV protect GPB code with
> > CO3 define
> >
> > Hi,
> >
> > In response to this ticket I provide the first part of the patch:
> > https://trac.ffmpeg.org/ticket/7839
> >
> > This QSV_HAVE_GPB code needs to be protected by QSV_HAVE_CO3.
> >
> > Regards.
> > A.H.
>
> Why it is must? It is impossible that QSV_HAVE_GPB is enabled but 
> QSV_HAVE_CO3 is not enabled.
> QSV_HAVE_GPB is enabled by MSDK API v1.18, but QSV_HAVE_CO3 is API V1.11.

In the ticket,  Andreas Håkon  pointed out that it is "only as a
temporary solution", so I think it's a workaround.
___
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] libavcodec: QSV protect GPB code with CO3 define

2019-04-22 Thread Andreas Håkon

‐‐‐ Original Message ‐‐‐
On Monday, 22 de April de 2019 13:33, Li, Zhong  wrote:

> > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> > Of Andreas Håkon via ffmpeg-devel
> > Sent: Thursday, April 18, 2019 6:11 PM
> > To: FFmpeg development discussions and patches
> > ffmpeg-devel@ffmpeg.org
> > Cc: Andreas Håkon andreas.ha...@protonmail.com
> > Subject: [FFmpeg-devel] [PATCH] libavcodec: QSV protect GPB code with
> > CO3 define
> > Hi,
> > In response to this ticket I provide the first part of the patch:
> > https://trac.ffmpeg.org/ticket/7839
> > This QSV_HAVE_GPB code needs to be protected by QSV_HAVE_CO3.
> > Regards.
> > A.H.
>
> Why it is must? It is impossible that QSV_HAVE_GPB is enabled but 
> QSV_HAVE_CO3 is not enabled.
> QSV_HAVE_GPB is enabled by MSDK API v1.18, but QSV_HAVE_CO3 is API V1.11.
>

Hi Li,

> Why it is must?

Let me to explain why:

- The "#if QSV_HAVE_GPB" only appears two times inside "/libavcodec/qsvenc.c":
  1. https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/qsvenc.c#L756
  2. https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/qsvenc.c#L270

In the first occurrence (line #756) the code is protected by one
"#if QSV_HAVE_CO3". This is necessary because if QSV_HAVE_CO3 is not enabled
then the code fails when using "extco3.GPB". The original author
(which I think was you) included the test with sound common sense.

In the second occurrence (line #270) where my patch is applied, the code isn't
protected. The reason is because this code is changed **after**. And the
developer forgot to protect it.

So my patch is a simple fixing.

> It is impossible that QSV_HAVE_GPB is enabled but QSV_HAVE_CO3 is not enabled.
> QSV_HAVE_GPB is enabled by MSDK API v1.18, but QSV_HAVE_CO3 is API V1.11.

This isn't true in all cases. As described in 
"https://trac.ffmpeg.org/ticket/7839;
one current bug breaks the "mpeg2_qsv" encoder. One solution is to MANUALLY
disable the QVBR. This can be done with a simple "#define QSV_HAVE_QVBR 0". Even
if you compile with a recent version of the SDK > v1.11.

But the problem is then that this implies too to disable "QSV_HAVE_CO3". And
doing this the code doesn't compile as this patch isn't applied.

So, the best solution is to merge this patch. It enables then the option to
disable manually what you like, and the code compiles clean.

Please, accept the patch.
Thank you!
A.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] [PATCH V2] lavf/oggparsevorbis: Fix change the case of metadata keys issue

2019-04-22 Thread myp...@gmail.com
On Wed, Apr 17, 2019 at 12:44 PM Jun Zhao  wrote:
>
> From: Jun Zhao 
>
> The spec in https://xiph.org/vorbis/doc/v-comment.html states that
> the metadata keys are case-insensitive, so don't change the case
> and update the fate test case.
>
> Fix #7784
>
> Signed-off-by: Jun Zhao 
> ---
>  libavformat/oggparsevorbis.c |9 -
>  tests/ref/fate/limited_input_seek|2 +-
>  tests/ref/fate/limited_input_seek-copyts |2 +-
>  3 files changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/libavformat/oggparsevorbis.c b/libavformat/oggparsevorbis.c
> index bcfd246..43f05f9 100644
> --- a/libavformat/oggparsevorbis.c
> +++ b/libavformat/oggparsevorbis.c
> @@ -44,7 +44,7 @@ static int ogm_chapter(AVFormatContext *as, uint8_t *key, 
> uint8_t *val)
>  int i, cnum, h, m, s, ms, keylen = strlen(key);
>  AVChapter *chapter = NULL;
>
> -if (keylen < 9 || sscanf(key, "CHAPTER%03d", ) != 1)
> +if (keylen < 9 || av_strncasecmp(key, "CHAPTER", 7) || sscanf(key+7, 
> "%03d", ) != 1)
>  return 0;
>
>  if (keylen <= 10) {
> @@ -55,7 +55,7 @@ static int ogm_chapter(AVFormatContext *as, uint8_t *key, 
> uint8_t *val)
> ms + 1000 * (s + 60 * (m + 60 * h)),
> AV_NOPTS_VALUE, NULL);
>  av_free(val);
> -} else if (!strcmp(key + keylen - 4, "NAME")) {
> +} else if (!av_strcasecmp(key + keylen - 4, "NAME")) {
>  for (i = 0; i < as->nb_chapters; i++)
>  if (as->chapters[i]->id == cnum) {
>  chapter = as->chapters[i];
> @@ -91,7 +91,7 @@ int ff_vorbis_comment(AVFormatContext *as, AVDictionary **m,
>  const uint8_t *p   = buf;
>  const uint8_t *end = buf + size;
>  int updates= 0;
> -unsigned n, j;
> +unsigned n;
>  int s;
>
>  /* must have vendor_length and user_comment_list_length */
> @@ -139,8 +139,7 @@ int ff_vorbis_comment(AVFormatContext *as, AVDictionary 
> **m,
>  return AVERROR(ENOMEM);
>  }
>
> -for (j = 0; j < tl; j++)
> -tt[j] = av_toupper(t[j]);
> +memcpy(tt, t, tl);
>  tt[tl] = 0;
>
>  memcpy(ct, v, vl);
> diff --git a/tests/ref/fate/limited_input_seek 
> b/tests/ref/fate/limited_input_seek
> index e0c4bf1..3269dce 100644
> --- a/tests/ref/fate/limited_input_seek
> +++ b/tests/ref/fate/limited_input_seek
> @@ -1 +1 @@
> -20a1bb9a1cfb23c1fe86f14e6065cd95
> +ae878bdaec23f36a63d142165fe57f49
> diff --git a/tests/ref/fate/limited_input_seek-copyts 
> b/tests/ref/fate/limited_input_seek-copyts
> index 92790a8..6eeaef8 100644
> --- a/tests/ref/fate/limited_input_seek-copyts
> +++ b/tests/ref/fate/limited_input_seek-copyts
> @@ -1 +1 @@
> -ec3604b1954ed80de364b8ef491771ce
> +ffe8a674bdf38e4f650f91963debc654
> --
> 1.7.1
>

Ping?
___
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: fix copyts and muxrate in mpegts muxer

2019-04-22 Thread Andreas Håkon

‐‐‐ Original Message ‐‐‐
On Friday, 19 de April de 2019 10:47, Andreas Håkon via ffmpeg-devel 
 wrote:

> ‐‐‐ Original Message ‐‐‐
> On Thursday, 18 de April de 2019 11:01, Andreas Håkon via ffmpeg-devel 
> ffmpeg-devel@ffmpeg.org wrote:
>
> > Hi,
> > This patch resolves one very specific use case:
> >
> > -   When you use the mpegts muxer;
> >
> > -   And use the global parameter “-copyts”;
> >
> > -   And use the parameter “-muxrate” for the mpegts muxer;
> >
> > -   And use too the parameter “-mpegts_copyts”.
> > The problem is created because the member “first_pcr” of the 
> > MpegTSWrite struct isn’t initialized with a correct timestamp (so when 
> > copying the timestamps the initial value is 0). And in this case an 
> > infinite loop is created because the code never writes PES packets when the 
> > “mux_rate” isn’t VBR (equals to 1). See the block that creates the loop 
> > here (note the "continue" command at end):
> > 
> > https://github.com/FFmpeg/FFmpeg/blob/a0559fcd81f42f446c93357a943699f9d44eeb79/libavformat/mpegtsenc.c#L1211
> > So, this patch fixes the problem initializing the “first_pcr” with the 
> > first DTS value that comes when using the incoming timestamps.
> > Regards.
> > A.H.
> >
>
> Hi,
>
> Here a new version of the patch.
>
> The "fist_pcr" value is now derived from DTS in a more consistent way.
>
> Regards,
> A.H.
>

Hi Michael Niedermayer,

Please review this patch. It resolves a **serious** bug when using
"copyts" and "muxrate" with the mpegts muxer.

Testcase:
 $ ffmpeg -loglevel debug \
  -copyts -f mpegts -i 01c56b0dc1.ts \
  -filter_complex "[i:0xfb]fps=fps=25[out]" \
  -map "[out]" -c:v libx264 \
  -f mpegts -muxrate 10M -mpegts_copyts 1 out.ts

File "01c56b0dc1.ts" is "http://samples.ffmpeg.org/ts/01c56b0dc1.ts;.

Without this patch the ffmpeg enters in a INFINITE LOOP !!!
With the patch all is processed as excepted.

Please, apply it.
Regards.
A.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] [PATCH] libavformat: fix inputs initialization in mpegts muxer with filters

2019-04-22 Thread Andreas Håkon

‐‐‐ Original Message ‐‐‐
On Friday, 19 de April de 2019 17:08, Michael Niedermayer 
 wrote:

> On Fri, Apr 19, 2019 at 08:23:35AM +, Andreas Håkon via ffmpeg-devel 
> wrote:
>
> > From 936740731c17a9757aa093bdb35d9772df1e64a8 Mon Sep 17 00:00:00 2001
> > From: Andreas Hakon andreas.ha...@protonmail.com
> > Date: Fri, 19 Apr 2019 09:17:32 +0100
> > Subject: [PATCH] libavformat: input init mpegts with filters v2
> > This patch solves the initialization of the inputs when using filters
> > (a graph filter) with the mpegts muxer.
> > This bug seems to be generated by a simple forgetting to copy. The same 
> > code is
> > repeated two times, but only in one case the variable inputs_done is 
> > initialized.
>
> > Testcase:
> > $ ffmpeg -f mpegts -i mpeg.ts \
> > -filter_complex "[i:100]fps=fps=25[out]" \
> > -map "[out]" -c:v libx264 -f mpegts out.ts
>
> i see no difference in the output (same md5)
> with what input file does this show a difference?


Hi Michael,

Let me to explain the problem inside the code. Here the relevant part of the
file "/fftools/ffmpeg.c" (line #4606):

-8<--
if (ost->filter && ost->filter->graph->graph) {
if (!ost->initialized) {
char error[1024] = {0};
ret = init_output_stream(ost, error, sizeof(error));
if (ret < 0) {
av_log(NULL, AV_LOG_ERROR, "Error initializing output stream 
%d:%d -- %s\n",
   ost->file_index, ost->index, error);
exit_program(1);
}
}
if ((ret = transcode_from_filter(ost->filter->graph, )) < 0)
return ret;
if (!ist)
return 0;
} else if (ost->filter) {
int i;
for (i = 0; i < ost->filter->graph->nb_inputs; i++) {
InputFilter *ifilter = ost->filter->graph->inputs[i];
if (!ifilter->ist->got_output && 
!input_files[ifilter->ist->file_index]->eof_reached) {
ist = ifilter->ist;
break;
}
}
if (!ist) {
ost->inputs_done = 1;
return 0;
}
} else {
av_assert0(ost->source_index >= 0);
ist = input_streams[ost->source_index];
}
-8<--

Please, note the first block of the "if (ost->filter && 
ost->filter->graph->graph)".
This block normally returns with "if (!ist) return 0;".

But the second block (the one "else if (ost->filter)") returns with
"if (!ist) {ost->inputs_done = 1; return 0;"

What the second block likes to protect is when the input stream is completed.
However, the first block doesn't do it!

One difference is that the first block uses the transcode_from_filter() 
function.
But that not makes difference, as the second block iterates over the inputs, and
the function transcode_from_filter() does it too.

Futhermore, the "ost->inputs_done" is used only in the function 
"choose_output()"
(at line #3882 of /fftools/ffmpeg.c):

-8<--
static OutputStream *choose_output(void)
{
int i;
int64_t opts_min = INT64_MAX;
OutputStream *ost_min = NULL;

for (i = 0; i < nb_output_streams; i++) {
OutputStream *ost = output_streams[i];
int64_t opts = ost->st->cur_dts == AV_NOPTS_VALUE ? INT64_MIN :
   av_rescale_q(ost->st->cur_dts, ost->st->time_base,
AV_TIME_BASE_Q);
if (ost->st->cur_dts == AV_NOPTS_VALUE)
av_log(NULL, AV_LOG_DEBUG,
"cur_dts is invalid st:%d (%d) [init:%d i_done:%d finish:%d] 
(this is harmless if it occurs once at the start per stream)\n",
ost->st->index, ost->st->id, ost->initialized, 
ost->inputs_done, ost->finished);

if (!ost->initialized && !ost->inputs_done)
return ost;

if (!ost->finished && opts < opts_min) {
opts_min = opts;
ost_min  = ost->unavailable ? NULL : ost;
}
}
return ost_min;
}
8<--

So, the case with troubles is when the output stream is selected because
is not initilized and no inputs are completed. And without this patch
it's possible that inputs can be completed but not marked as done.

I assume that the error is a simple incorrect "copy" of the original
code, as the second block sets the "ost->inputs_done = 1;" but not the
first one. And the difference between both blocks is that the first
is when a filter graph is running.

Please, revise the code! I hope you understand it when you look at my
descriptions.

Regards,
A.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] [PATCH] libavcodec: QSV protect GPB code with CO3 define

2019-04-22 Thread Li, Zhong
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Andreas Håkon via ffmpeg-devel
> Sent: Thursday, April 18, 2019 6:11 PM
> To: FFmpeg development discussions and patches
> 
> Cc: Andreas Håkon 
> Subject: [FFmpeg-devel] [PATCH] libavcodec: QSV protect GPB code with
> CO3 define
> 
> Hi,
> 
> In response to this ticket I provide the first part of the patch:
> https://trac.ffmpeg.org/ticket/7839
> 
> This QSV_HAVE_GPB code needs to be protected by QSV_HAVE_CO3.
> 
> Regards.
> A.H.

Why it is must? It is impossible that QSV_HAVE_GPB is enabled but QSV_HAVE_CO3 
is not enabled.
QSV_HAVE_GPB is enabled by MSDK API v1.18, but QSV_HAVE_CO3 is API V1.11.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] avformat/mpegenc - reject unsupported audio streams

2019-04-22 Thread Gyan



On 22-04-2019 01:15 PM, Gyan wrote:



On 22-04-2019 12:30 PM, Carl Eugen Hoyos wrote:



Am 20.04.2019 um 11:31 schrieb Gyan :


Old patch that was never applied. Rebased.

Please return patch_welcome for mlp and truehd.

Will do.
Wasn’t there another comment (not by me): “Why can’t .codec_tag be 
used?”


There's no codec_tag member set. This patch is just to disallow an 
invalid muxing to proceed.


Revised.

Gyan
From 608bf79e2aed04cd34dca733c778b7e630f31a46 Mon Sep 17 00:00:00 2001
From: Gyan Doshi 
Date: Tue, 20 Feb 2018 20:42:21 +0530
Subject: [PATCH v2] avformat/mpegenc - reject unsupported audio streams

Only MP1, MP2, MP3, 16-bit PCM_DVD, PCM S16BE,
AC3 and DTS audio codecs are supported by the muxer.
---
 libavformat/mpegenc.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/libavformat/mpegenc.c b/libavformat/mpegenc.c
index 1389288b7f..43ebc46e0e 100644
--- a/libavformat/mpegenc.c
+++ b/libavformat/mpegenc.c
@@ -407,6 +407,16 @@ static av_cold int mpeg_mux_init(AVFormatContext *ctx)
 stream->lpcm_header[2] = 0x80;
 stream->id = lpcm_id++;
 stream->lpcm_align = st->codecpar->channels * 
st->codecpar->bits_per_coded_sample / 8;
+} else if (st->codecpar->codec_id == AV_CODEC_ID_MLP ||
+   st->codecpar->codec_id == AV_CODEC_ID_TRUEHD) {
+   av_log(ctx, AV_LOG_ERROR, "Support for muxing audio 
codec %s not implemented.\n",
+  avcodec_get_name(st->codecpar->codec_id));
+   return AVERROR_PATCHWELCOME;
+} else if (st->codecpar->codec_id != AV_CODEC_ID_MP1 &&
+   st->codecpar->codec_id != AV_CODEC_ID_MP2 &&
+   st->codecpar->codec_id != AV_CODEC_ID_MP3) {
+   av_log(ctx, AV_LOG_ERROR, "Unsupported audio codec. 
Must be one of mp1, mp2, mp3, 16-bit pcm_dvd, pcm_s16be, ac3 or dts.\n");
+   goto fail;
 } else {
 stream->id = mpa_id++;
 }
-- 
2.21.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 2/3] avfilter: add audio upsample filter

2019-04-22 Thread Paul B Mahol
On 4/22/19, Nicolas George  wrote:
> Paul B Mahol (12019-04-21):
>> https://dspguru.com/dsp/faqs/multirate/resampling/
>>
>> Resampling involves interpolation.
>> If I do resampling with aresample and resampling with factor 2 from
>> 44100 to 88200
>> I can see there is still some spectrum data in highest frequencies.
>
> Resampling MAY involve interpolation, to enhance the subjective quality
> of the output.

It is not MAY, it MUST involve interpolation in almost any scenario.

>
>> If you haven't noticed, this is just upsampling/downsampling without
>> lowpass.
>> And that is not resampling in any way.
>
> No matter how many times you repeat it, saying that changing the sample
> rate is not resampling will not make it true.

By your flawed reasoning asetrate filter is doing resamping too.

>
>> That is just your opinion now, you need to provide technical terms
>> to support your statements.
>
> I have given technical considerations: we have automatic sample rate
> negotiation. These filter would not fit with it.

If you haven't tried them, they fit well with sample rate negotiation.
Filters change sample rate metadata by integer factor up or down.

>
> What have you provided besides your opinion and links to generic
> considerations that do not substantiate your views?

I have provided actual facts, and not opinions unlike you.

>
>> Another option to aresample filter would just confuse users.
>
> Less so than yet another pair of filters that do almost exactly the same
> thing as others.

No filter in libavfilter does what those filters do.

And there is reason why SoX have those as separate filters and not some
strange combination with their generic resampling effect.
___
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] avfilter/af_astats: count number of NaNs/Infs/denormals for floating-point audio too

2019-04-22 Thread Paul B Mahol
Signed-off-by: Paul B Mahol 
---
 doc/filters.texi|  6 +++
 libavfilter/af_astats.c | 86 ++---
 2 files changed, 86 insertions(+), 6 deletions(-)

diff --git a/doc/filters.texi b/doc/filters.texi
index cfff9b1f4d..945c557e8f 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -2141,6 +2141,9 @@ Bit_depth
 Dynamic_range
 Zero_crossings
 Zero_crossings_rate
+Number_of_NaNs
+Number_of_Infs
+Number_of_denormals
 
 and for Overall:
 DC_offset
@@ -2158,6 +2161,9 @@ Flat_factor
 Peak_count
 Bit_depth
 Number_of_samples
+Number_of_NaNs
+Number_of_Infs
+Number_of_denormals
 
 For example full key look like this @code{lavfi.astats.1.DC_offset} or
 this @code{lavfi.astats.Overall.Peak_count}.
diff --git a/libavfilter/af_astats.c b/libavfilter/af_astats.c
index 92368793c2..25c700b344 100644
--- a/libavfilter/af_astats.c
+++ b/libavfilter/af_astats.c
@@ -20,6 +20,7 @@
  */
 
 #include 
+#include 
 
 #include "libavutil/opt.h"
 #include "audio.h"
@@ -48,6 +49,9 @@
 #define MEASURE_ZERO_CROSSINGS  (1 << 16)
 #define MEASURE_ZERO_CROSSINGS_RATE (1 << 17)
 #define MEASURE_NUMBER_OF_SAMPLES   (1 << 18)
+#define MEASURE_NUMBER_OF_NANS  (1 << 19)
+#define MEASURE_NUMBER_OF_INFS  (1 << 20)
+#define MEASURE_NUMBER_OF_DENORMALS (1 << 21)
 
 #define MEASURE_MINMAXPEAK  (MEASURE_MIN_LEVEL | MEASURE_MAX_LEVEL 
| MEASURE_PEAK_LEVEL)
 
@@ -68,6 +72,9 @@ typedef struct ChannelStats {
 uint64_t min_count, max_count;
 uint64_t zero_runs;
 uint64_t nb_samples;
+uint64_t nb_nans;
+uint64_t nb_infs;
+uint64_t nb_denormals;
 } ChannelStats;
 
 typedef struct AudioStatsContext {
@@ -83,6 +90,8 @@ typedef struct AudioStatsContext {
 int maxbitdepth;
 int measure_perchannel;
 int measure_overall;
+int is_float;
+int is_double;
 } AudioStatsContext;
 
 #define OFFSET(x) offsetof(AudioStatsContext, x)
@@ -114,6 +123,9 @@ static const AVOption astats_options[] = {
   { "Zero_crossings", "", 0, AV_OPT_TYPE_CONST, 
{.i64=MEASURE_ZERO_CROSSINGS  }, 0, 0, FLAGS, "measure" },
   { "Zero_crossings_rate"   , "", 0, AV_OPT_TYPE_CONST, 
{.i64=MEASURE_ZERO_CROSSINGS_RATE }, 0, 0, FLAGS, "measure" },
   { "Number_of_samples" , "", 0, AV_OPT_TYPE_CONST, 
{.i64=MEASURE_NUMBER_OF_SAMPLES   }, 0, 0, FLAGS, "measure" },
+  { "Number_of_NaNs", "", 0, AV_OPT_TYPE_CONST, 
{.i64=MEASURE_NUMBER_OF_NANS  }, 0, 0, FLAGS, "measure" },
+  { "Number_of_Infs", "", 0, AV_OPT_TYPE_CONST, 
{.i64=MEASURE_NUMBER_OF_INFS  }, 0, 0, FLAGS, "measure" },
+  { "Number_of_denormals"   , "", 0, AV_OPT_TYPE_CONST, 
{.i64=MEASURE_NUMBER_OF_DENORMALS }, 0, 0, FLAGS, "measure" },
 { "measure_overall", "only measure_perchannel these overall statistics", 
OFFSET(measure_overall), AV_OPT_TYPE_FLAGS, {.i64=MEASURE_ALL}, 0, UINT_MAX, 
FLAGS, "measure" },
 { NULL }
 };
@@ -181,6 +193,9 @@ static void reset_stats(AudioStatsContext *s)
 p->max_count = 0;
 p->zero_runs = 0;
 p->nb_samples = 0;
+p->nb_nans = 0;
+p->nb_infs = 0;
+p->nb_denormals = 0;
 }
 }
 
@@ -196,6 +211,11 @@ static int config_output(AVFilterLink *outlink)
 s->tc_samples = 5 * s->time_constant * outlink->sample_rate + .5;
 s->nb_frames = 0;
 s->maxbitdepth = av_get_bytes_per_sample(outlink->format) * 8;
+s->is_double = outlink->format == AV_SAMPLE_FMT_DBL  ||
+   outlink->format == AV_SAMPLE_FMT_DBLP;
+
+s->is_float = outlink->format == AV_SAMPLE_FMT_FLT  ||
+  outlink->format == AV_SAMPLE_FMT_FLTP;
 
 reset_stats(s);
 
@@ -280,6 +300,24 @@ static inline void update_stat(AudioStatsContext *s, 
ChannelStats *p, double d,
 p->nb_samples++;
 }
 
+static inline void update_float_stat(AudioStatsContext *s, ChannelStats *p, 
float d)
+{
+int type = fpclassify(d);
+
+p->nb_nans  += type == FP_NAN;
+p->nb_infs  += type == FP_INFINITE;
+p->nb_denormals += type == FP_SUBNORMAL;
+}
+
+static inline void update_double_stat(AudioStatsContext *s, ChannelStats *p, 
double d)
+{
+int type = fpclassify(d);
+
+p->nb_nans  += type == FP_NAN;
+p->nb_infs  += type == FP_INFINITE;
+p->nb_denormals += type == FP_SUBNORMAL;
+}
+
 static void set_meta(AVDictionary **metadata, int chan, const char *key,
  const char *fmt, double val)
 {
@@ -299,6 +337,7 @@ static void set_meta(AVDictionary **metadata, int chan, 
const char *key,
 static void set_metadata(AudioStatsContext *s, AVDictionary **metadata)
 {
 uint64_t mask = 0, imask = 0x, min_count = 0, max_count = 
0, nb_samples = 0;
+uint64_t nb_nans = 0, nb_infs = 0, nb_denormals = 0;
 double min_runs = 0, max_runs = 0,
min = DBL_MAX, max = DBL_MIN, min_diff = DBL_MAX, max_diff = 0,
nmin = DBL_MAX, nmax = DBL_MIN,
@@ -337,6 +376,9 @@ static void 

Re: [FFmpeg-devel] [PATCH v10 1/2] lavc/svt_hevc: add libsvt hevc encoder wrapper

2019-04-22 Thread Sun, Jing A
-Original Message-
From: Sun, Jing A 
Sent: Monday, April 1, 2019 10:38 AM
To: ffmpeg-devel@ffmpeg.org
Cc: Sun, Jing A ; Huang, Zhengxu 
; Tmar, Hassene ; Jun Zhao 

Subject: [PATCH v10 1/2] lavc/svt_hevc: add libsvt hevc encoder wrapper

Signed-off-by: Zhengxu Huang 
Signed-off-by: Hassene Tmar 
Signed-off-by: Jun Zhao 
Signed-off-by: Jing Sun 
---
 configure|   4 +
 libavcodec/Makefile  |   1 +
 libavcodec/allcodecs.c   |   1 +
 libavcodec/libsvt_hevc.c | 482 +++
 4 files changed, 488 insertions(+)
 create mode 100644 libavcodec/libsvt_hevc.c

diff --git a/configure b/configure
index 938ff10..2aabac4 100755
--- a/configure
+++ b/configure
@@ -264,6 +264,7 @@ External library support:
   --enable-libspeexenable Speex de/encoding via libspeex [no]
   --enable-libsrt  enable Haivision SRT protocol via libsrt [no]
   --enable-libssh  enable SFTP protocol via libssh [no]
+  --enable-libsvthevc  enable HEVC encoding via svt [no]
   --enable-libtensorflow   enable TensorFlow as a DNN module backend
for DNN based filters like sr [no]
   --enable-libtesseractenable Tesseract, needed for ocr filter [no]
@@ -1784,6 +1785,7 @@ EXTERNAL_LIBRARY_LIST="
 libspeex
 libsrt
 libssh
+libsvthevc
 libtensorflow
 libtesseract
 libtheora
@@ -3173,6 +3175,7 @@ libshine_encoder_select="audio_frame_queue"
 libspeex_decoder_deps="libspeex"
 libspeex_encoder_deps="libspeex"
 libspeex_encoder_select="audio_frame_queue"
+libsvt_hevc_encoder_deps="libsvthevc"
 libtheora_encoder_deps="libtheora"
 libtwolame_encoder_deps="libtwolame"
 libvo_amrwbenc_encoder_deps="libvo_amrwbenc"
@@ -6209,6 +6212,7 @@ enabled libsoxr   && require libsoxr soxr.h 
soxr_create -lsoxr
 enabled libssh&& require_pkg_config libssh libssh libssh/sftp.h 
sftp_init
 enabled libspeex  && require_pkg_config libspeex speex speex/speex.h 
speex_decoder_init
 enabled libsrt&& require_pkg_config libsrt "srt >= 1.3.0" 
srt/srt.h srt_socket
+enabled libsvthevc&& require_pkg_config libsvthevc SvtHevcEnc EbApi.h 
EbInitHandle
 enabled libtensorflow && require libtensorflow tensorflow/c/c_api.h 
TF_Version -ltensorflow
 enabled libtesseract  && require_pkg_config libtesseract tesseract 
tesseract/capi.h TessBaseAPICreate
 enabled libtheora && require libtheora theora/theoraenc.h th_info_init 
-ltheoraenc -ltheoradec -logg
diff --git a/libavcodec/Makefile b/libavcodec/Makefile index 15c43a8..c93e545 
100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -987,6 +987,7 @@ OBJS-$(CONFIG_LIBOPUS_ENCODER)+= libopusenc.o 
libopus.o \
 OBJS-$(CONFIG_LIBSHINE_ENCODER)   += libshine.o
 OBJS-$(CONFIG_LIBSPEEX_DECODER)   += libspeexdec.o
 OBJS-$(CONFIG_LIBSPEEX_ENCODER)   += libspeexenc.o
+OBJS-$(CONFIG_LIBSVT_HEVC_ENCODER)+= libsvt_hevc.o
 OBJS-$(CONFIG_LIBTHEORA_ENCODER)  += libtheoraenc.o
 OBJS-$(CONFIG_LIBTWOLAME_ENCODER) += libtwolame.o
 OBJS-$(CONFIG_LIBVO_AMRWBENC_ENCODER) += libvo-amrwbenc.o
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c index 
b26aeca..e93f66f 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -703,6 +703,7 @@ extern AVCodec ff_librsvg_decoder;  extern AVCodec 
ff_libshine_encoder;  extern AVCodec ff_libspeex_encoder;  extern AVCodec 
ff_libspeex_decoder;
+extern AVCodec ff_libsvt_hevc_encoder;
 extern AVCodec ff_libtheora_encoder;
 extern AVCodec ff_libtwolame_encoder;
 extern AVCodec ff_libvo_amrwbenc_encoder; diff --git 
a/libavcodec/libsvt_hevc.c b/libavcodec/libsvt_hevc.c new file mode 100644 
index 000..2ab5323
--- /dev/null
+++ b/libavcodec/libsvt_hevc.c
@@ -0,0 +1,482 @@
+/*
+* Scalable Video Technology for HEVC encoder library plugin
+*
+* Copyright (c) 2018 Intel Corporation
+*
+* 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 this program; if not, write to the Free Software
+* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 
+02110-1301 USA */
+
+#include "EbErrorCodes.h"
+#include "EbTime.h"
+#include "EbApi.h"
+
+#include "libavutil/common.h"
+#include "libavutil/frame.h"
+#include "libavutil/opt.h"
+
+#include "internal.h"
+#include "avcodec.h"
+
+typedef enum eos_status {
+EOS_NOT_REACHED = 0,
+EOS_REACHED,
+

Re: [FFmpeg-devel] [PATCH] avformat/mpegenc - reject unsupported audio streams

2019-04-22 Thread Gyan



On 22-04-2019 12:30 PM, Carl Eugen Hoyos wrote:



Am 20.04.2019 um 11:31 schrieb Gyan :


Old patch that was never applied. Rebased.

Please return patch_welcome for mlp and truehd.

Will do.

Wasn’t there another comment (not by me): “Why can’t .codec_tag be used?”


There's no codec_tag member set. This patch is just to disallow an 
invalid muxing to proceed.


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

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

Re: [FFmpeg-devel] [PATCH] avcodec/ccaption_dec: Add a blank like at the end to avoid rollup reading from outside

2019-04-22 Thread Alexander Strasser
Hi Michael!

On 2019-04-20 18:11 +0200, Michael Niedermayer wrote:
> Fixes: index 20 out of bounds for type 'const char *[4][128]'

Somehow I don't understand this diagnostic message. It says "index
20 out of bounds for [4][128]".

You're change looks like an off-by-one fix.

Sorry; I'm surely missing something as I didn't look up the code,
just thought I might as well ask...


  Alexander

> Fixes: 
> 14367/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CCAPTION_fuzzer-5718819672162304
>
> 14367/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CCAPTION_fuzzer-5718819672162304
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/ccaption_dec.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/libavcodec/ccaption_dec.c b/libavcodec/ccaption_dec.c
> index 09ceb1b3bf..bf3563a0bc 100644
> --- a/libavcodec/ccaption_dec.c
> +++ b/libavcodec/ccaption_dec.c
> @@ -212,10 +212,10 @@ static const unsigned char pac2_attribs[32][3] = // 
> Color, font, ident
>
>  struct Screen {
>  /* +1 is used to compensate null character of string */
> -uint8_t characters[SCREEN_ROWS][SCREEN_COLUMNS+1];
> -uint8_t charsets[SCREEN_ROWS][SCREEN_COLUMNS+1];
> -uint8_t colors[SCREEN_ROWS][SCREEN_COLUMNS+1];
> -uint8_t fonts[SCREEN_ROWS][SCREEN_COLUMNS+1];
> +uint8_t characters[SCREEN_ROWS+1][SCREEN_COLUMNS+1];
> +uint8_t charsets[SCREEN_ROWS+1][SCREEN_COLUMNS+1];
> +uint8_t colors[SCREEN_ROWS+1][SCREEN_COLUMNS+1];
> +uint8_t fonts[SCREEN_ROWS+1][SCREEN_COLUMNS+1];
>  /*
>   * Bitmask of used rows; if a bit is not set, the
>   * corresponding row is not used.
> --
> 2.21.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] avformat/mpegenc - reject unsupported audio streams

2019-04-22 Thread Carl Eugen Hoyos


> Am 20.04.2019 um 11:31 schrieb Gyan :
> 
> 
> Old patch that was never applied. Rebased.

Please return patch_welcome for mlp and truehd.

Wasn’t there another comment (not by me): “Why can’t .codec_tag be used?”

Carl Eugen
___
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/3] avfilter: add audio upsample filter

2019-04-22 Thread Nicolas George
Paul B Mahol (12019-04-21):
> https://dspguru.com/dsp/faqs/multirate/resampling/
> 
> Resampling involves interpolation.
> If I do resampling with aresample and resampling with factor 2 from
> 44100 to 88200
> I can see there is still some spectrum data in highest frequencies.

Resampling MAY involve interpolation, to enhance the subjective quality
of the output.

> If you haven't noticed, this is just upsampling/downsampling without lowpass.
> And that is not resampling in any way.

No matter how many times you repeat it, saying that changing the sample
rate is not resampling will not make it true.

> That is just your opinion now, you need to provide technical terms
> to support your statements.

I have given technical considerations: we have automatic sample rate
negotiation. These filter would not fit with it.

What have you provided besides your opinion and links to generic
considerations that do not substantiate your views?

> Another option to aresample filter would just confuse users.

Less so than yet another pair of filters that do almost exactly the same
thing as others.

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] avformat/dashenc: Fix a bug with writing "final" manifest

2019-04-22 Thread Jeyapal, Karthick

On 4/17/19 11:28 AM, Karthick J wrote:
> This bug was introduced in the commit 951561b64ee6c11f01daedd9dcf73276cc1e765b
> ---
>  libavformat/dashenc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
> index 5f1333e436..b88d4b3496 100644
> --- a/libavformat/dashenc.c
> +++ b/libavformat/dashenc.c
> @@ -1631,7 +1631,7 @@ static int dash_flush(AVFormatContext *s, int final, 
> int stream)
>  }
>  }
>  if (ret >= 0) {
> -if (c->has_video) {
> +if (c->has_video && !final) {
>  c->nr_of_streams_flushed++;
>  if (c->nr_of_streams_flushed != c->nr_of_streams_to_flush)
>  return ret;

Pushed.

Regards,
Karthick 

___
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".