[FFmpeg-devel] [PATCH] fate/matroska: Add test for remuxing file with spherical metadata

[Andreas Rheinhardt]

Also, test modifying colorspace properties and the default_mode
passthrough which is used here to create a file that has no default
track at all.

Signed-off-by: Andreas Rheinhardt 
---
 tests/fate/matroska.mak  |  8 +++
 tests/ref/fate/matroska-spherical-mono-remux | 66 
 2 files changed, 74 insertions(+)
 create mode 100644 tests/ref/fate/matroska-spherical-mono-remux

diff --git a/tests/fate/matroska.mak b/tests/fate/matroska.mak
index 1d2921194f..e03579def2 100644
--- a/tests/fate/matroska.mak
+++ b/tests/fate/matroska.mak
@@ -57,6 +57,14 @@ FATE_MATROSKA_FFMPEG_FFPROBE-$(call ALLYES, MATROSKA_DEMUXER 
OGG_DEMUXER  \
+= fate-webm-dash-chapters
 fate-webm-dash-chapters: CMD = transcode ogg 
$(TARGET_SAMPLES)/vorbis/vorbis_chapter_extension_demo.ogg webm "-c copy 
-cluster_time_limit 1500 -dash 1 -dash_track_number 124 -reserve_index_space 
400" "-c copy -t 0.5" "" -show_chapters
 
+# This test the following features of the Matroska muxer: Writing projection
+# stream side-data; not setting any track to default if the user requested it;
+# and modifying and writing colorspace properties.
+FATE_MATROSKA_FFMPEG_FFPROBE-$(call ALLYES, MATROSKA_DEMUXER MATROSKA_MUXER \
+H264_DECODER H264_PARSER) \
+   += fate-matroska-spherical-mono-remux
+fate-matroska-spherical-mono-remux: CMD = transcode matroska 
$(TARGET_SAMPLES)/mkv/spherical.mkv matroska "-map 0 -map 0 -c copy 
-disposition:0 -default+forced -disposition:1 -default -default_mode 
passthrough -color_primaries:1 bt709 -color_trc:1 smpte170m -colorspace:1 
bt2020c -color_range:1 pc"  "-map 0 -c copy -t 0" "" "-show_entries 
stream_side_data_list:stream_disposition=default,forced:stream=color_range,color_space,color_primaries,color_transfer"
+
 FATE_MATROSKA_FFPROBE-$(call ALLYES, MATROSKA_DEMUXER) += 
fate-matroska-spherical-mono
 fate-matroska-spherical-mono: CMD = run ffprobe$(PROGSSUF)$(EXESUF) 
-show_entries stream_side_data_list -select_streams v -v 0 
$(TARGET_SAMPLES)/mkv/spherical.mkv
 
diff --git a/tests/ref/fate/matroska-spherical-mono-remux 
b/tests/ref/fate/matroska-spherical-mono-remux
new file mode 100644
index 00..c72fc692bc
--- /dev/null
+++ b/tests/ref/fate/matroska-spherical-mono-remux
@@ -0,0 +1,66 @@
+1a3be6691ace86663faeea688e6bb95a 
*tests/data/fate/matroska-spherical-mono-remux.matroska
+161562 tests/data/fate/matroska-spherical-mono-remux.matroska
+#extradata 0:   43, 0x2b0e0d7b
+#extradata 1:   43, 0x2b0e0d7b
+#tb 0: 1/1000
+#media_type 0: video
+#codec_id 0: h264
+#dimensions 0: 1920x1080
+#sar 0: 0/1
+#tb 1: 1/1000
+#media_type 1: video
+#codec_id 1: h264
+#dimensions 1: 1920x1080
+#sar 1: 0/1
+0,-80,  0,   40,69118, 0x73cb52f0, S=2,   12, 
0x,   36, 0x2cf8035c
+1,-80,  0,   40,69118, 0x73cb52f0, S=2,   12, 
0x,   36, 0x2cf8035c
+0,-40,160,   40, 1103, 0x082a059f, F=0x0
+1,-40,160,   40, 1103, 0x082a059f, F=0x0
+[STREAM]
+color_range=unknown
+color_space=unknown
+color_transfer=unknown
+color_primaries=unknown
+DISPOSITION:default=0
+DISPOSITION:forced=1
+[SIDE_DATA]
+side_data_type=Stereo 3D
+type=2D
+inverted=0
+[/SIDE_DATA]
+[SIDE_DATA]
+side_data_type=Spherical Mapping
+projection=tiled equirectangular
+bound_left=148
+bound_top=73
+bound_right=147
+bound_bottom=72
+yaw=45
+pitch=30
+roll=15
+[/SIDE_DATA]
+[/STREAM]
+[STREAM]
+color_range=pc
+color_space=bt2020c
+color_transfer=smpte170m
+color_primaries=bt709
+DISPOSITION:default=0
+DISPOSITION:forced=0
+[SIDE_DATA]
+side_data_type=Stereo 3D
+type=2D
+inverted=0
+[/SIDE_DATA]
+[SIDE_DATA]
+side_data_type=Spherical Mapping
+projection=tiled equirectangular
+bound_left=148
+bound_top=73
+bound_right=147
+bound_bottom=72
+yaw=45
+pitch=30
+roll=15
+[/SIDE_DATA]
+[/STREAM]
-- 
2.20.1

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

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

Re: [FFmpeg-devel] [PATCH] [libavutil] Add saturated add/sub operations for int64_t.

[Carl Eugen Hoyos]

Am Fr., 1. Mai 2020 um 01:31 Uhr schrieb James Almer :
>
> On 4/30/2020 8:20 PM, Carl Eugen Hoyos wrote:
> > Am Fr., 1. Mai 2020 um 00:20 Uhr schrieb Dale Curtis 
> > :
> >>
> >> Many places are using their own custom code for handling overflow
> >> around timestamps or other int64_t values. There are enough of these
> >> now that having some common saturated math functions seems sound.
> >
> >> This adds implementations that just use the builtin functions for
> >> recent gcc, clang when available or implements its own version for
> >> older compilers.
> >
> > Should be tested in configure and I believe this should be a separate patch.
>
> Not really, the AV_GCC_VERSION_AT_LEAST helper macro exists precisely
> for this purpose.

The macro exists to avoid separate patches?

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 v2] doc/filters.texi: complete rewrite of fps filter doc, v2.

[Carl Eugen Hoyos]

Am Fr., 1. Mai 2020 um 06:05 Uhr schrieb :
>
> From: Jim DeLaHunt 
>
> Fix unclear wording and spelling mistakes based on review.

> Reduce overall word count by 11%.

This is heavily misleading and definitely not ok.

> Ready for patch review.
>
> No other docs, and no executable code, are changed.
>
> Signed-off-by: Jim DeLaHunt 
> ---
>  doc/filters.texi | 157 +--
>  1 file changed, 138 insertions(+), 19 deletions(-)
>
> diff --git a/doc/filters.texi b/doc/filters.texi
> index d19fd346ae..0d7d15c448 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -11194,25 +11194,30 @@ format=pix_fmts=yuv420p|yuv444p|yuv410p
>  @anchor{fps}
>  @section fps

> -Convert the video to specified constant frame rate by duplicating or dropping
> -frames as necessary.
> +Make a new video from the frames and presentation time stamps (PTSs) of the

While I am not a native speaker, the idea that the fps filter "makes a
new video"
is absurd imo (independently of what I may have written in the past, the
documentation has higher standards than the user mailing list).

> +input. The new video has a specified constant frame rate, and new PTSs. It
> +generally keeps frames from the old video, but might repeat or drop some 
> frames.
> +You can choose the method for rounding from input PTS to output PTS. This
> +affects which frames @var{fps} keeps, repeats, or drops.
>
>  It accepts the following parameters:
>  @table @option
>
>  @item fps
> -The desired output frame rate. The default is @code{25}.
> +The output frame rate, in frames per second. May be an integer, real, or
> +rational number, or an abbreviation. The default is @code{25}.

I seriously wonder who really profits from this change but it may be ok.

>  @item start_time
> -Assume the first PTS should be the given value, in seconds. This allows for
> -padding/trimming at the start of stream. By default, no assumption is made
> -about the first frame's expected PTS, so no padding or trimming is done.
> -For example, this could be set to 0 to pad the beginning with duplicates of
> -the first frame if a video stream starts after the audio stream or to trim 
> any
> -frames with a negative PTS.

> +A time, in seconds from the start of the input stream

I don't think this is correct, somebody has to confirm this before the change
is acceptable.
Since it contradicts the text above, this should be explicitely mentioned in
the commit message (if correct).

> , which @var{fps} converts
> +to an input starting PTS and an output starting PTS. If set,
> +@var{fps} drops input frames which have PTSs less than the input starting
> +PTS. If not set, the input and output starting PTSs are zero, but
> +@var{fps} drops no input frames based on PTS. (See details below.)
>
>  @item round
> -Timestamp (PTS) rounding method.
> +Rounding method to use when calculating output PTSs from input PTSs.
> +If the calculated output PTS is not exactly an integer, then the value
> +determines which neighbouring integer value @var{fps} selects.
>
>  Possible values are:
>  @table @option
> @@ -11225,43 +11230,157 @@ round towards -infinity
>  @item up
>  round towards +infinity
>  @item near
> -round to nearest
> +round to nearest (midpoints round away from 0)
>  @end table
>  The default is @code{near}.
>
>  @item eof_action
> -Action performed when reading the last frame.
> +Action which @var{fps} takes with the final input frame. The input video 
> passes
> +in a final input PTS, which @var{fps} converts to an output PTS limit.

> +@var{fps} drops any input frames with a PTS at or after this limit.

This sounds wrong to me: In which situation are frames dropped?

>  Possible values are:
>  @table @option
>  @item round
> -Use same timestamp rounding method as used for other frames.
> +Use same rounding method as for other frames.

>  @item pass
> -Pass through last frame if input duration has not been reached yet.
> +Round the ending input PTS using @code{up}. This might make @ref{fps} include
> +one last input frame.
>  @end table

This seems to contradict the original description. Is this intended?

> +
>  The default is @code{round}.
>
>  @end table
>
> -Alternatively, the options can be specified as a flat string:
> +Alternatively, the options may be specified as a flat string:
>  @var{fps}[:@var{start_time}[:@var{round}]].
>
> -See also the @ref{setpts} filter.
> +@var{fps} makes an output video with consecutive integer PTSs, and with a
> +time base set to the inverse of the given frame rate. @var{fps} keeps, 
> repeats,
> +or drops input frames, in sequence, to the output video. It does so according

> +to their input PTSs, as converted to seconds (via the input time base),

Why would the fps filter convert the PTS to seconds?
And how is this relevant for users of the filter?

> +then rounded to output PTSs.

In which situation does fps round?

> +@var{fps} sets output PTSs in terms of a timeline which starts at
> +zero. For any output frame, 

Re: [FFmpeg-devel] [PATCH v3 2/2] avcodec/mpeg12enc: Support mpeg2 encoder profile with const options

[lance . lmwang]

On Sat, Apr 04, 2020 at 08:25:31AM +0800, lance.lmw...@gmail.com wrote:
> From: Limin Wang 
> 
> Signed-off-by: Limin Wang 
> ---
>  doc/encoders.texi  |  8 
>  libavcodec/mpeg12enc.c | 22 +-
>  libavcodec/mpegvideo.h |  1 +
>  3 files changed, 26 insertions(+), 5 deletions(-)
> 
> diff --git a/doc/encoders.texi b/doc/encoders.texi
> index e23b6b3..5022b94 100644
> --- a/doc/encoders.texi
> +++ b/doc/encoders.texi
> @@ -2753,6 +2753,14 @@ For maximum compatibility, use @samp{component}.
>  @item a53cc @var{boolean}
>  Import closed captions (which must be ATSC compatible format) into output.
>  Default is 1 (on).
> +@item profile @var{integer}
> +Select the mpeg2 profile to encode, possible values:
> +@table @samp
> +@item 422
> +@item high
> +@item main
> +@item simple
> +@end table
>  @end table
>  
>  @section png
> diff --git a/libavcodec/mpeg12enc.c b/libavcodec/mpeg12enc.c
> index 643ba81..9fe0c8b 100644
> --- a/libavcodec/mpeg12enc.c
> +++ b/libavcodec/mpeg12enc.c
> @@ -156,23 +156,30 @@ static av_cold int encode_init(AVCodecContext *avctx)
>  }
>  }
>  
> -if (avctx->profile == FF_PROFILE_UNKNOWN) {
> +if (s->profile == FF_PROFILE_UNKNOWN)
> +s->profile = avctx->profile;
> +
> +if (s->profile == FF_PROFILE_UNKNOWN) {
>  if (avctx->level != FF_LEVEL_UNKNOWN) {
>  av_log(avctx, AV_LOG_ERROR, "Set profile and level\n");
>  return -1;
>  }
>  /* Main or 4:2:2 */
>  avctx->profile = s->chroma_format == CHROMA_420 ? 
> FF_PROFILE_MPEG2_MAIN : FF_PROFILE_MPEG2_422;
> +s->profile = s->chroma_format == CHROMA_420 ? FF_PROFILE_MPEG2_MAIN 
> : FF_PROFILE_MPEG2_422;
> +} else if (s->profile < FF_PROFILE_MPEG2_422) {
> +av_log(avctx, AV_LOG_ERROR, "Invalid mpeg2 profile set\n");
> +return -1;
>  }
>  
>  if (avctx->level == FF_LEVEL_UNKNOWN) {
> -if (avctx->profile == FF_PROFILE_MPEG2_422) {   /* 4:2:2 */
> +if (s->profile == FF_PROFILE_MPEG2_422) {   /* 4:2:2 */
>  if (avctx->width <= 720 && avctx->height <= 608)
>  avctx->level = 5;   /* Main */
>  else
>  avctx->level = 2;   /* High */
>  } else {
> -if (avctx->profile != FF_PROFILE_MPEG2_HIGH && s->chroma_format 
> != CHROMA_420) {
> +if (s->profile != FF_PROFILE_MPEG2_HIGH && s->chroma_format != 
> CHROMA_420) {
>  av_log(avctx, AV_LOG_ERROR,
> "Only High(1) and 4:2:2(0) profiles support 4:2:2 
> color sampling\n");
>  return -1;
> @@ -321,9 +328,9 @@ static void mpeg1_encode_sequence_header(MpegEncContext 
> *s)
>  put_header(s, EXT_START_CODE);
>  put_bits(&s->pb, 4, 1); // seq ext
>  
> -put_bits(&s->pb, 1, s->avctx->profile == FF_PROFILE_MPEG2_422); 
> // escx 1 for 4:2:2 profile
> +put_bits(&s->pb, 1, s->profile == FF_PROFILE_MPEG2_422); // escx 
> 1 for 4:2:2 profile
>  
> -put_bits(&s->pb, 3, s->avctx->profile); // profile
> +put_bits(&s->pb, 3, s->profile); // profile
>  put_bits(&s->pb, 4, s->avctx->level);   // level
>  
>  put_bits(&s->pb, 1, s->progressive_sequence);
> @@ -1165,6 +1172,11 @@ static const AVOption mpeg2_options[] = {
>  { "secam",NULL, 0, AV_OPT_TYPE_CONST,  {.i64 = 
> VIDEO_FORMAT_SECAM  },  0, 0, VE, "video_format" },
>  { "mac",  NULL, 0, AV_OPT_TYPE_CONST,  {.i64 = 
> VIDEO_FORMAT_MAC},  0, 0, VE, "video_format" },
>  { "unspecified",  NULL, 0, AV_OPT_TYPE_CONST,  {.i64 = 
> VIDEO_FORMAT_UNSPECIFIED},  0, 0, VE, "video_format" },
> +{ "profile",  "Set the profile",  OFFSET(profile),   
> AV_OPT_TYPE_INT,{ .i64 = FF_PROFILE_UNKNOWN }, FF_PROFILE_UNKNOWN, 
> FF_PROFILE_MPEG2_SIMPLE, VE, "profile" },
> +{ "422",  "",   0, AV_OPT_TYPE_CONST,{ .i64 = 
> FF_PROFILE_MPEG2_422 }, 0, 0, VE, "profile" },
> +{ "high", "",   0, AV_OPT_TYPE_CONST,{ .i64 = 
> FF_PROFILE_MPEG2_HIGH}, 0, 0, VE, "profile" },
> +{ "main", "",   0, AV_OPT_TYPE_CONST,{ .i64 = 
> FF_PROFILE_MPEG2_MAIN}, 0, 0, VE, "profile" },
> +{ "simple",   "",   0, AV_OPT_TYPE_CONST,{ .i64 = 
> FF_PROFILE_MPEG2_SIMPLE  }, 0, 0, VE, "profile" },
>  FF_MPV_COMMON_OPTS
>  { NULL },
>  };
> diff --git a/libavcodec/mpegvideo.h b/libavcodec/mpegvideo.h
> index 29e692f..cee423e 100644
> --- a/libavcodec/mpegvideo.h
> +++ b/libavcodec/mpegvideo.h
> @@ -456,6 +456,7 @@ typedef struct MpegEncContext {
>  int progressive_sequence;
>  int mpeg_f_code[2][2];
>  int a53_cc;
> +int profile;
>  
>  // picture structure defines are loaded from mpegutils.h
>  int picture_structure;
> -- 
> 2.9.5
> 

will apply the patch set.

-- 
Thanks,
Limin Wang
_

Re: [FFmpeg-devel] [PATCH v2] doc/filters.texi: complete rewrite of fps filter doc, v2.

[Nicolas George]

Carl Eugen Hoyos (12020-05-01):
> > -The desired output frame rate. The default is @code{25}.
> > +The output frame rate, in frames per second. May be an integer, real, or
> > +rational number, or an abbreviation. The default is @code{25}.
> I seriously wonder who really profits from this change but it may be ok.

I would argue: lazy beginners.

Non-beginners already know what a frame rate is, it completely basic
stuff.

Beginners who want to achieve something by themselves will peruse
https://ffmpeg.org/ffmpeg-all.html#Video-rate and now know what a frame
rate is.

If we want to enhance this, we just have to make sure the doc for every
AV_OPT_TYPE_VIDEO_RATE option has a link to #Video-rate. Nothing more.

Duplicating the explanations is a waste of everybody's time.

(I dream of a system where the doc would be not in a separate tree but
built as part of the library, and links like that happen automatically.)

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] [libavutil] Add saturated add/sub operations for int64_t.

[Carl Eugen Hoyos]

Am Fr., 1. Mai 2020 um 02:46 Uhr schrieb Dale Curtis :
>
> On Thu, Apr 30, 2020 at 5:21 PM James Almer  wrote:
>
> > On 4/30/2020 7:19 PM, Dale Curtis wrote:
> > > Many places are using their own custom code for handling overflow
> > > around timestamps or other int64_t values. There are enough of these
> > > now that having some common saturated math functions seems sound.
> > >
> > > This adds implementations that just use the builtin functions for
> > > recent gcc, clang when available or implements its own version for
> > > older compilers.
> >
> > These look like 64 bit versions of av_sat_add32 and av_sat_sub32, from
> > common.h, so you should probably add them there and rename them
> > accordingly.
> >
> >
> Ah! I was looking for those, but missed them. Thanks. Done.

This patch is not ok, please split it.

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 v3] avformat/dashenc: remove the arbitrary restrictions for filename

[lance . lmwang]

On Wed, Apr 08, 2020 at 08:45:09AM +0200, Andreas Rheinhardt wrote:
> lance.lmw...@gmail.com:
> > From: Limin Wang 
> > 
> > Signed-off-by: Limin Wang 
> > ---
> >  libavformat/dashenc.c | 26 +-
> >  1 file changed, 9 insertions(+), 17 deletions(-)
> > 
> > diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
> > index 94d463972a..903fdd0aa6 100644
> > --- a/libavformat/dashenc.c
> > +++ b/libavformat/dashenc.c
> > @@ -1832,28 +1832,20 @@ static void dashenc_delete_file(AVFormatContext *s, 
> > char *filename) {
> >  static int dashenc_delete_segment_file(AVFormatContext *s, const char* 
> > file)
> >  {
> >  DASHContext *c = s->priv_data;
> > -size_t dirname_len, file_len;
> > -char filename[1024];
> > -
> > -dirname_len = strlen(c->dirname);
> > -if (dirname_len >= sizeof(filename)) {
> > -av_log(s, AV_LOG_WARNING, "Cannot delete segments as the directory 
> > path is too long: %"PRIu64" characters: %s\n",
> > -(uint64_t)dirname_len, c->dirname);
> > -return AVERROR(ENAMETOOLONG);
> > -}
> > +AVBPrint buf;
> >  
> > -memcpy(filename, c->dirname, dirname_len);
> > +av_bprint_init(&buf, 0, AV_BPRINT_SIZE_UNLIMITED);
> >  
> > -file_len = strlen(file);
> > -if ((dirname_len + file_len) >= sizeof(filename)) {
> > -av_log(s, AV_LOG_WARNING, "Cannot delete segments as the path is 
> > too long: %"PRIu64" characters: %s%s\n",
> > -(uint64_t)(dirname_len + file_len), c->dirname, file);
> > -return AVERROR(ENAMETOOLONG);
> > +av_bprintf(&buf, "%s%s", c->dirname, file);
> > +if (!av_bprint_is_complete(&buf)) {
> > +av_bprint_finalize(&buf, NULL);
> > +av_log(s, AV_LOG_WARNING, "Out of memory for filename\n");
> > +return AVERROR(ENOMEM);
> >  }
> >  
> > -memcpy(filename + dirname_len, file, file_len + 1); // include the 
> > terminating zero
> > -dashenc_delete_file(s, filename);
> > +dashenc_delete_file(s, buf.str);
> >  
> > +av_bprint_finalize(&buf, NULL);
> >  return 0;
> >  }
> >  
> > 
> Looks good now.

will apply.

thx.
> 
> - Andreas
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

-- 
Thanks,
Limin Wang
___
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] [libavutil] Add saturated add/sub operations for int64_t.

[Nicolas George]

Carl Eugen Hoyos (12020-05-01):
> This patch is not ok, please split it.

What benefit would it serve?

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 v1] avformat/concat: priv_data should be freed internally

[lance . lmwang]

On Fri, Apr 03, 2020 at 09:03:06PM +0800, lance.lmw...@gmail.com wrote:
> From: Limin Wang 
> 
> Signed-off-by: Limin Wang 
> ---
>  libavformat/concat.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/libavformat/concat.c b/libavformat/concat.c
> index ea3bc1dfde..cfe14760eb 100644
> --- a/libavformat/concat.c
> +++ b/libavformat/concat.c
> @@ -75,7 +75,6 @@ static av_cold int concat_open(URLContext *h, const char 
> *uri, int flags)
>  if (uri[i] == *AV_CAT_SEPARATOR) {
>  /* integer overflow */
>  if (++len == UINT_MAX / sizeof(*nodes)) {
> -av_freep(&h->priv_data);
>  return AVERROR(ENAMETOOLONG);
>  }
>  }
> -- 
> 2.21.0
> 

will apply it.


-- 
Thanks,
Limin Wang
___
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 v1] doc/utils: add more examples for valid time duration

[lance . lmwang]

On Wed, Apr 08, 2020 at 10:10:44AM +0800, lance.lmw...@gmail.com wrote:
> From: Limin Wang 
> 
> Signed-off-by: Limin Wang 
> ---
>  doc/utils.texi | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/doc/utils.texi b/doc/utils.texi
> index 05a2f81626..ca599f145b 100644
> --- a/doc/utils.texi
> +++ b/doc/utils.texi
> @@ -126,6 +126,15 @@ The following examples are all valid time duration:
>  @item 55
>  55 seconds
>  
> +@item 0.2
> +0.2 seconds
> +
> +@item 200ms
> +200 milliseconds, that's 0.2s
> +
> +@item 20us
> +20 microseconds, that's 0.2s
> +

will apply.

>  @item 12:03:45
>  12 hours, 03 minutes and 45 seconds
>  
> -- 
> 2.21.0
> 

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

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

Re: [FFmpeg-devel] [PATCH v3 2/2] avcodec/mpeg12enc: Support mpeg2 encoder profile with const options

[Marton Balint]

On Fri, 1 May 2020, lance.lmw...@gmail.com wrote:


On Sat, Apr 04, 2020 at 08:25:31AM +0800, lance.lmw...@gmail.com wrote:

From: Limin Wang 

Signed-off-by: Limin Wang 
---
 doc/encoders.texi  |  8 
 libavcodec/mpeg12enc.c | 22 +-
 libavcodec/mpegvideo.h |  1 +
 3 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/doc/encoders.texi b/doc/encoders.texi
index e23b6b3..5022b94 100644
--- a/doc/encoders.texi
+++ b/doc/encoders.texi
@@ -2753,6 +2753,14 @@ For maximum compatibility, use @samp{component}.
 @item a53cc @var{boolean}
 Import closed captions (which must be ATSC compatible format) into output.
 Default is 1 (on).
+@item profile @var{integer}
+Select the mpeg2 profile to encode, possible values:
+@table @samp
+@item 422
+@item high
+@item main
+@item simple
+@end table
 @end table

 @section png
diff --git a/libavcodec/mpeg12enc.c b/libavcodec/mpeg12enc.c
index 643ba81..9fe0c8b 100644
--- a/libavcodec/mpeg12enc.c
+++ b/libavcodec/mpeg12enc.c
@@ -156,23 +156,30 @@ static av_cold int encode_init(AVCodecContext *avctx)
 }
 }

-if (avctx->profile == FF_PROFILE_UNKNOWN) {
+if (s->profile == FF_PROFILE_UNKNOWN)
+s->profile = avctx->profile;
+
+if (s->profile == FF_PROFILE_UNKNOWN) {
 if (avctx->level != FF_LEVEL_UNKNOWN) {
 av_log(avctx, AV_LOG_ERROR, "Set profile and level\n");
 return -1;
 }
 /* Main or 4:2:2 */
 avctx->profile = s->chroma_format == CHROMA_420 ? 
FF_PROFILE_MPEG2_MAIN : FF_PROFILE_MPEG2_422;
+s->profile = s->chroma_format == CHROMA_420 ? FF_PROFILE_MPEG2_MAIN : 
FF_PROFILE_MPEG2_422;
+} else if (s->profile < FF_PROFILE_MPEG2_422) {
+av_log(avctx, AV_LOG_ERROR, "Invalid mpeg2 profile set\n");
+return -1;
 }

 if (avctx->level == FF_LEVEL_UNKNOWN) {
-if (avctx->profile == FF_PROFILE_MPEG2_422) {   /* 4:2:2 */
+if (s->profile == FF_PROFILE_MPEG2_422) {   /* 4:2:2 */
 if (avctx->width <= 720 && avctx->height <= 608)
 avctx->level = 5;   /* Main */
 else
 avctx->level = 2;   /* High */
 } else {
-if (avctx->profile != FF_PROFILE_MPEG2_HIGH && s->chroma_format != 
CHROMA_420) {
+if (s->profile != FF_PROFILE_MPEG2_HIGH && s->chroma_format != 
CHROMA_420) {
 av_log(avctx, AV_LOG_ERROR,
"Only High(1) and 4:2:2(0) profiles support 4:2:2 color 
sampling\n");
 return -1;
@@ -321,9 +328,9 @@ static void mpeg1_encode_sequence_header(MpegEncContext *s)
 put_header(s, EXT_START_CODE);
 put_bits(&s->pb, 4, 1); // seq ext

-put_bits(&s->pb, 1, s->avctx->profile == FF_PROFILE_MPEG2_422); // 
escx 1 for 4:2:2 profile
+put_bits(&s->pb, 1, s->profile == FF_PROFILE_MPEG2_422); // escx 1 
for 4:2:2 profile

-put_bits(&s->pb, 3, s->avctx->profile); // profile
+put_bits(&s->pb, 3, s->profile); // profile
 put_bits(&s->pb, 4, s->avctx->level);   // level

 put_bits(&s->pb, 1, s->progressive_sequence);
@@ -1165,6 +1172,11 @@ static const AVOption mpeg2_options[] = {
 { "secam",NULL, 0, AV_OPT_TYPE_CONST,  {.i64 = VIDEO_FORMAT_SECAM  
},  0, 0, VE, "video_format" },
 { "mac",  NULL, 0, AV_OPT_TYPE_CONST,  {.i64 = VIDEO_FORMAT_MAC
},  0, 0, VE, "video_format" },
 { "unspecified",  NULL, 0, AV_OPT_TYPE_CONST,  {.i64 = 
VIDEO_FORMAT_UNSPECIFIED},  0, 0, VE, "video_format" },
+{ "profile",  "Set the profile",  OFFSET(profile),   AV_OPT_TYPE_INT,{ .i64 
= FF_PROFILE_UNKNOWN }, FF_PROFILE_UNKNOWN, FF_PROFILE_MPEG2_SIMPLE, VE, "profile" },
+{ "422",  "",   0, AV_OPT_TYPE_CONST,{ .i64 = FF_PROFILE_MPEG2_422 
}, 0, 0, VE, "profile" },
+{ "high", "",   0, AV_OPT_TYPE_CONST,{ .i64 = FF_PROFILE_MPEG2_HIGH
}, 0, 0, VE, "profile" },
+{ "main", "",   0, AV_OPT_TYPE_CONST,{ .i64 = FF_PROFILE_MPEG2_MAIN
}, 0, 0, VE, "profile" },
+{ "simple",   "",   0, AV_OPT_TYPE_CONST,{ .i64 = FF_PROFILE_MPEG2_SIMPLE  
}, 0, 0, VE, "profile" },
 FF_MPV_COMMON_OPTS
 { NULL },
 };
diff --git a/libavcodec/mpegvideo.h b/libavcodec/mpegvideo.h
index 29e692f..cee423e 100644
--- a/libavcodec/mpegvideo.h
+++ b/libavcodec/mpegvideo.h
@@ -456,6 +456,7 @@ typedef struct MpegEncContext {
 int progressive_sequence;
 int mpeg_f_code[2][2];
 int a53_cc;
+int profile;

 // picture structure defines are loaded from mpegutils.h
 int picture_structure;
--
2.9.5



will apply the patch set.


Hold on, this patch does not seem right. Is 422 as a named constant even 
works?


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

T

Re: [FFmpeg-devel] [PATCH v3 2/2] avcodec/mpeg12enc: Support mpeg2 encoder profile with const options

[lance . lmwang]

On Fri, May 01, 2020 at 12:32:44PM +0200, Marton Balint wrote:
> 
> 
> On Fri, 1 May 2020, lance.lmw...@gmail.com wrote:
> 
> > On Sat, Apr 04, 2020 at 08:25:31AM +0800, lance.lmw...@gmail.com wrote:
> > > From: Limin Wang 
> > > 
> > > Signed-off-by: Limin Wang 
> > > ---
> > >  doc/encoders.texi  |  8 
> > >  libavcodec/mpeg12enc.c | 22 +-
> > >  libavcodec/mpegvideo.h |  1 +
> > >  3 files changed, 26 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/doc/encoders.texi b/doc/encoders.texi
> > > index e23b6b3..5022b94 100644
> > > --- a/doc/encoders.texi
> > > +++ b/doc/encoders.texi
> > > @@ -2753,6 +2753,14 @@ For maximum compatibility, use @samp{component}.
> > >  @item a53cc @var{boolean}
> > >  Import closed captions (which must be ATSC compatible format) into 
> > > output.
> > >  Default is 1 (on).
> > > +@item profile @var{integer}
> > > +Select the mpeg2 profile to encode, possible values:
> > > +@table @samp
> > > +@item 422
> > > +@item high
> > > +@item main
> > > +@item simple
> > > +@end table
> > >  @end table
> > > 
> > >  @section png
> > > diff --git a/libavcodec/mpeg12enc.c b/libavcodec/mpeg12enc.c
> > > index 643ba81..9fe0c8b 100644
> > > --- a/libavcodec/mpeg12enc.c
> > > +++ b/libavcodec/mpeg12enc.c
> > > @@ -156,23 +156,30 @@ static av_cold int encode_init(AVCodecContext 
> > > *avctx)
> > >  }
> > >  }
> > > 
> > > -if (avctx->profile == FF_PROFILE_UNKNOWN) {
> > > +if (s->profile == FF_PROFILE_UNKNOWN)
> > > +s->profile = avctx->profile;
> > > +
> > > +if (s->profile == FF_PROFILE_UNKNOWN) {
> > >  if (avctx->level != FF_LEVEL_UNKNOWN) {
> > >  av_log(avctx, AV_LOG_ERROR, "Set profile and level\n");
> > >  return -1;
> > >  }
> > >  /* Main or 4:2:2 */
> > >  avctx->profile = s->chroma_format == CHROMA_420 ? 
> > > FF_PROFILE_MPEG2_MAIN : FF_PROFILE_MPEG2_422;
> > > +s->profile = s->chroma_format == CHROMA_420 ? 
> > > FF_PROFILE_MPEG2_MAIN : FF_PROFILE_MPEG2_422;
> > > +} else if (s->profile < FF_PROFILE_MPEG2_422) {
> > > +av_log(avctx, AV_LOG_ERROR, "Invalid mpeg2 profile set\n");
> > > +return -1;
> > >  }
> > > 
> > >  if (avctx->level == FF_LEVEL_UNKNOWN) {
> > > -if (avctx->profile == FF_PROFILE_MPEG2_422) {   /* 4:2:2 */
> > > +if (s->profile == FF_PROFILE_MPEG2_422) {   /* 4:2:2 */
> > >  if (avctx->width <= 720 && avctx->height <= 608)
> > >  avctx->level = 5;   /* Main */
> > >  else
> > >  avctx->level = 2;   /* High */
> > >  } else {
> > > -if (avctx->profile != FF_PROFILE_MPEG2_HIGH && 
> > > s->chroma_format != CHROMA_420) {
> > > +if (s->profile != FF_PROFILE_MPEG2_HIGH && s->chroma_format 
> > > != CHROMA_420) {
> > >  av_log(avctx, AV_LOG_ERROR,
> > > "Only High(1) and 4:2:2(0) profiles support 4:2:2 
> > > color sampling\n");
> > >  return -1;
> > > @@ -321,9 +328,9 @@ static void 
> > > mpeg1_encode_sequence_header(MpegEncContext *s)
> > >  put_header(s, EXT_START_CODE);
> > >  put_bits(&s->pb, 4, 1); // seq ext
> > > 
> > > -put_bits(&s->pb, 1, s->avctx->profile == 
> > > FF_PROFILE_MPEG2_422); // escx 1 for 4:2:2 profile
> > > +put_bits(&s->pb, 1, s->profile == FF_PROFILE_MPEG2_422); // 
> > > escx 1 for 4:2:2 profile
> > > 
> > > -put_bits(&s->pb, 3, s->avctx->profile); // profile
> > > +put_bits(&s->pb, 3, s->profile); // profile
> > >  put_bits(&s->pb, 4, s->avctx->level);   // level
> > > 
> > >  put_bits(&s->pb, 1, s->progressive_sequence);
> > > @@ -1165,6 +1172,11 @@ static const AVOption mpeg2_options[] = {
> > >  { "secam",NULL, 0, AV_OPT_TYPE_CONST,  {.i64 = 
> > > VIDEO_FORMAT_SECAM  },  0, 0, VE, "video_format" },
> > >  { "mac",  NULL, 0, AV_OPT_TYPE_CONST,  {.i64 = 
> > > VIDEO_FORMAT_MAC},  0, 0, VE, "video_format" },
> > >  { "unspecified",  NULL, 0, AV_OPT_TYPE_CONST,  {.i64 = 
> > > VIDEO_FORMAT_UNSPECIFIED},  0, 0, VE, "video_format" },
> > > +{ "profile",  "Set the profile",  OFFSET(profile),   
> > > AV_OPT_TYPE_INT,{ .i64 = FF_PROFILE_UNKNOWN }, FF_PROFILE_UNKNOWN, 
> > > FF_PROFILE_MPEG2_SIMPLE, VE, "profile" },
> > > +{ "422",  "",   0, AV_OPT_TYPE_CONST,{ .i64 = 
> > > FF_PROFILE_MPEG2_422 }, 0, 0, VE, "profile" },
> > > +{ "high", "",   0, AV_OPT_TYPE_CONST,{ .i64 = 
> > > FF_PROFILE_MPEG2_HIGH}, 0, 0, VE, "profile" },
> > > +{ "main", "",   0, AV_OPT_TYPE_CONST,{ .i64 = 
> > > FF_PROFILE_MPEG2_MAIN}, 0, 0, VE, "profile" },
> > > +{ "simple",   "",   0, AV_OPT_TYPE_CONST,{ .i64 = 
> > > FF_PROFILE_MPEG2_SIMPLE  }, 0, 0, VE, "profile" },
> > >

[FFmpeg-devel] [PATCH] avcodec/librav1e: Use the framerate when available for ratecontrol

[Derek Buitenhuis]

Rav1e currently uses the time base given to it only for ratecontrol... where
the inverse is taken and used as a framerate. So, do what we do in other 
wrappers
and use the framerate if we can.

Signed-off-by: Derek Buitenhuis 
---
Notably, this leaves pkt->pts still broken (it was broken before, too), but
after discussion with James and Lynne, we decided to fix this in the rav1e API
and bump the minimum version, instead of using a PTS queue in librav1e.c.
---
 libavcodec/librav1e.c | 19 +++
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/libavcodec/librav1e.c b/libavcodec/librav1e.c
index b8b1b4f8f1..b0ff60d8c7 100644
--- a/libavcodec/librav1e.c
+++ b/libavcodec/librav1e.c
@@ -186,10 +186,21 @@ static av_cold int librav1e_encode_init(AVCodecContext 
*avctx)
 return AVERROR_EXTERNAL;
 }
 
-rav1e_config_set_time_base(cfg, (RaRational) {
-   avctx->time_base.num * avctx->ticks_per_frame,
-   avctx->time_base.den
-   });
+/*
+ * Rav1e currently uses the time base given to it only for ratecontrol... 
where
+ * the inverse is taken and used as a framerate. So, do what we do in 
other wrappers
+ * and use the framerate if we can.
+ */
+if (avctx->framerate.num > 0 && avctx->framerate.den > 0) {
+rav1e_config_set_time_base(cfg, (RaRational) {
+   avctx->framerate.den, avctx->framerate.num
+   });
+} else {
+rav1e_config_set_time_base(cfg, (RaRational) {
+   avctx->time_base.num * 
avctx->ticks_per_frame,
+   avctx->time_base.den
+   });
+}
 
 if (avctx->flags & AV_CODEC_FLAG_PASS2) {
 if (!avctx->stats_in) {
-- 
2.26.2

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

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

Re: [FFmpeg-devel] [PATCH] [libavutil] Add saturated add/sub operations for int64_t.

[James Almer]

On 5/1/2020 6:36 AM, Carl Eugen Hoyos wrote:
> Am Fr., 1. Mai 2020 um 01:31 Uhr schrieb James Almer :
>>
>> On 4/30/2020 8:20 PM, Carl Eugen Hoyos wrote:
>>> Am Fr., 1. Mai 2020 um 00:20 Uhr schrieb Dale Curtis 
>>> :

 Many places are using their own custom code for handling overflow
 around timestamps or other int64_t values. There are enough of these
 now that having some common saturated math functions seems sound.
>>>
 This adds implementations that just use the builtin functions for
 recent gcc, clang when available or implements its own version for
 older compilers.
>>>
>>> Should be tested in configure and I believe this should be a separate patch.
>>
>> Not really, the AV_GCC_VERSION_AT_LEAST helper macro exists precisely
>> for this purpose.
> 
> The macro exists to avoid separate patches?

No, it exists to not require configure checks just to enable a path for
gcc/clang and another for other compilers.

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

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

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

Re: [FFmpeg-devel] [PATCH v3 2/2] avcodec/mpeg12enc: Support mpeg2 encoder profile with const options

[Marton Balint]

On Fri, 1 May 2020, lance.lmw...@gmail.com wrote:


On Fri, May 01, 2020 at 12:32:44PM +0200, Marton Balint wrote:



On Fri, 1 May 2020, lance.lmw...@gmail.com wrote:

> On Sat, Apr 04, 2020 at 08:25:31AM +0800, lance.lmw...@gmail.com wrote:
> > From: Limin Wang 
> > 
> > Signed-off-by: Limin Wang 

> > ---
> >  doc/encoders.texi  |  8 
> >  libavcodec/mpeg12enc.c | 22 +-
> >  libavcodec/mpegvideo.h |  1 +
> >  3 files changed, 26 insertions(+), 5 deletions(-)
> > 
> > diff --git a/doc/encoders.texi b/doc/encoders.texi

> > index e23b6b3..5022b94 100644
> > --- a/doc/encoders.texi
> > +++ b/doc/encoders.texi
> > @@ -2753,6 +2753,14 @@ For maximum compatibility, use @samp{component}.
> >  @item a53cc @var{boolean}
> >  Import closed captions (which must be ATSC compatible format) into output.
> >  Default is 1 (on).
> > +@item profile @var{integer}
> > +Select the mpeg2 profile to encode, possible values:
> > +@table @samp
> > +@item 422
> > +@item high
> > +@item main
> > +@item simple
> > +@end table
> >  @end table
> > 
> >  @section png

> > diff --git a/libavcodec/mpeg12enc.c b/libavcodec/mpeg12enc.c
> > index 643ba81..9fe0c8b 100644
> > --- a/libavcodec/mpeg12enc.c
> > +++ b/libavcodec/mpeg12enc.c
> > @@ -156,23 +156,30 @@ static av_cold int encode_init(AVCodecContext *avctx)
> >  }
> >  }
> > 
> > -if (avctx->profile == FF_PROFILE_UNKNOWN) {

> > +if (s->profile == FF_PROFILE_UNKNOWN)
> > +s->profile = avctx->profile;
> > +
> > +if (s->profile == FF_PROFILE_UNKNOWN) {
> >  if (avctx->level != FF_LEVEL_UNKNOWN) {
> >  av_log(avctx, AV_LOG_ERROR, "Set profile and level\n");
> >  return -1;
> >  }
> >  /* Main or 4:2:2 */
> >  avctx->profile = s->chroma_format == CHROMA_420 ? 
FF_PROFILE_MPEG2_MAIN : FF_PROFILE_MPEG2_422;
> > +s->profile = s->chroma_format == CHROMA_420 ? 
FF_PROFILE_MPEG2_MAIN : FF_PROFILE_MPEG2_422;
> > +} else if (s->profile < FF_PROFILE_MPEG2_422) {
> > +av_log(avctx, AV_LOG_ERROR, "Invalid mpeg2 profile set\n");
> > +return -1;
> >  }
> > 
> >  if (avctx->level == FF_LEVEL_UNKNOWN) {

> > -if (avctx->profile == FF_PROFILE_MPEG2_422) {   /* 4:2:2 */
> > +if (s->profile == FF_PROFILE_MPEG2_422) {   /* 4:2:2 */
> >  if (avctx->width <= 720 && avctx->height <= 608)
> >  avctx->level = 5;   /* Main */
> >  else
> >  avctx->level = 2;   /* High */
> >  } else {
> > -if (avctx->profile != FF_PROFILE_MPEG2_HIGH && 
s->chroma_format != CHROMA_420) {
> > +if (s->profile != FF_PROFILE_MPEG2_HIGH && s->chroma_format != 
CHROMA_420) {
> >  av_log(avctx, AV_LOG_ERROR,
> > "Only High(1) and 4:2:2(0) profiles support 4:2:2 color 
sampling\n");
> >  return -1;
> > @@ -321,9 +328,9 @@ static void mpeg1_encode_sequence_header(MpegEncContext 
*s)
> >  put_header(s, EXT_START_CODE);
> >  put_bits(&s->pb, 4, 1); // seq ext
> > 
> > -put_bits(&s->pb, 1, s->avctx->profile == FF_PROFILE_MPEG2_422); // escx 1 for 4:2:2 profile

> > +put_bits(&s->pb, 1, s->profile == FF_PROFILE_MPEG2_422); // 
escx 1 for 4:2:2 profile
> > 
> > -put_bits(&s->pb, 3, s->avctx->profile); // profile

> > +put_bits(&s->pb, 3, s->profile); // profile
> >  put_bits(&s->pb, 4, s->avctx->level);   // level
> > 
> >  put_bits(&s->pb, 1, s->progressive_sequence);

> > @@ -1165,6 +1172,11 @@ static const AVOption mpeg2_options[] = {
> >  { "secam",NULL, 0, AV_OPT_TYPE_CONST,  {.i64 = VIDEO_FORMAT_SECAM
  },  0, 0, VE, "video_format" },
> >  { "mac",  NULL, 0, AV_OPT_TYPE_CONST,  {.i64 = VIDEO_FORMAT_MAC  
  },  0, 0, VE, "video_format" },
> >  { "unspecified",  NULL, 0, AV_OPT_TYPE_CONST,  {.i64 = 
VIDEO_FORMAT_UNSPECIFIED},  0, 0, VE, "video_format" },
> > +{ "profile",  "Set the profile",  OFFSET(profile),   AV_OPT_TYPE_INT,{ 
.i64 = FF_PROFILE_UNKNOWN }, FF_PROFILE_UNKNOWN, FF_PROFILE_MPEG2_SIMPLE, VE, "profile" },
> > +{ "422",  "",   0, AV_OPT_TYPE_CONST,{ .i64 = FF_PROFILE_MPEG2_422   
  }, 0, 0, VE, "profile" },
> > +{ "high", "",   0, AV_OPT_TYPE_CONST,{ .i64 = FF_PROFILE_MPEG2_HIGH  
  }, 0, 0, VE, "profile" },
> > +{ "main", "",   0, AV_OPT_TYPE_CONST,{ .i64 = FF_PROFILE_MPEG2_MAIN  
  }, 0, 0, VE, "profile" },
> > +{ "simple",   "",   0, AV_OPT_TYPE_CONST,{ .i64 = 
FF_PROFILE_MPEG2_SIMPLE  }, 0, 0, VE, "profile" },
> >  FF_MPV_COMMON_OPTS
> >  { NULL },
> >  };
> > diff --git a/libavcodec/mpegvideo.h b/libavcodec/mpegvideo.h
> > index 29e692f..cee423e 100644
> > --- a/libavcodec/mpegvideo.h
> > +++ b/libavcodec/mpegvideo.h
> > @@ -456,6 +456,7 @@

[FFmpeg-devel] [PATCH 2/2] avformat/oggdec: Reallocate buffer before writing into it

[Michael Niedermayer]

Fixes: out of array write
Fixes: Regression since f619e1ec66b89215582eff4404b681b760540b4f

Signed-off-by: Michael Niedermayer 
---
 libavformat/oggdec.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/libavformat/oggdec.c b/libavformat/oggdec.c
index a9034ea61c..7dd48af66c 100644
--- a/libavformat/oggdec.c
+++ b/libavformat/oggdec.c
@@ -441,6 +441,12 @@ static int ogg_read_page(AVFormatContext *s, int *sid, int 
probing)
 
 os = ogg->streams + idx;
 
+ret = buf_realloc(os, size);
+if (ret < 0) {
+av_free(readout_buf);
+return ret;
+}
+
 memcpy(os->buf + os->bufpos, readout_buf, size);
 av_free(readout_buf);
 }
-- 
2.17.1

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

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

[FFmpeg-devel] [PATCH 1/2] avformat/oggdec: Factor buffer reallocation out

[Michael Niedermayer]

Signed-off-by: Michael Niedermayer 
---
 libavformat/oggdec.c | 25 +
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/libavformat/oggdec.c b/libavformat/oggdec.c
index c591bafddd..a9034ea61c 100644
--- a/libavformat/oggdec.c
+++ b/libavformat/oggdec.c
@@ -297,6 +297,20 @@ static int data_packets_seen(const struct ogg *ogg)
 return 0;
 }
 
+static int buf_realloc(struct ogg_stream *os, int size)
+{
+/* Even if invalid guarantee there's enough memory to read the page */
+if (os->bufsize - os->bufpos < size) {
+uint8_t *nb = av_realloc(os->buf, 2*os->bufsize + 
AV_INPUT_BUFFER_PADDING_SIZE);
+if (!nb)
+return AVERROR(ENOMEM);
+os->buf = nb;
+os->bufsize *= 2;
+}
+
+return 0;
+}
+
 static int ogg_read_page(AVFormatContext *s, int *sid, int probing)
 {
 AVIOContext *bc = s->pb;
@@ -378,14 +392,9 @@ static int ogg_read_page(AVFormatContext *s, int *sid, int 
probing)
 if (idx >= 0) {
 os = ogg->streams + idx;
 
-/* Even if invalid guarantee there's enough memory to read the page */
-if (os->bufsize - os->bufpos < size) {
-uint8_t *nb = av_realloc(os->buf, 2*os->bufsize + 
AV_INPUT_BUFFER_PADDING_SIZE);
-if (!nb)
-return AVERROR(ENOMEM);
-os->buf = nb;
-os->bufsize *= 2;
-}
+ret = buf_realloc(os, size);
+if (ret < 0)
+return ret;
 
 readout_buf = os->buf + os->bufpos;
 } else {
-- 
2.17.1

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

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

Re: [FFmpeg-devel] [PATCH 0/3] Patch set to delay output live stream

[Marton Balint]

On Thu, 30 Apr 2020, Tao Zhang wrote:


Andreas Rheinhardt  于2020年4月30日周四 下午4:23写道：


Tao Zhang:
> Marton Balint  于2020年4月30日周四 下午3:26写道：
>>
>>
>>
>> On Thu, 30 Apr 2020, Tao Zhang wrote:
>>
>>> Marton Balint  于2020年4月30日周四 上午4:55写道：



 On Thu, 30 Apr 2020, Tao Zhang wrote:

> Marton Balint  于2020年4月30日周四 上午12:03写道：
>>
>>
>>
>> On Wed, 29 Apr 2020, leozhang wrote:
>>
>>> In some applications, it is required to add delay to live streaming.
>>
>> In what applications? And if you do this, why not run
>>
>> sleep 20; ffmpeg 
> In live streaming applications, someone wouldn't want broadcast what's
> comming next immediately.
> Sleep 20 then ffmpeg is not ok, because the stream is still
> broadcasting immediately, and lost 20 seconds signal.

 So you want to buffer 20 seconds of input, and then start the output?
>>> yes
>>
>> Then your timing based approach is not the best way to do that. What you
>> want is a buffer fullness based approach. E.g. somewhere in the chain of
>> ffmpeg components you want to have a fixed buffer size of 20 seconds of
>> data, which is always kept filled.
> I don't think bitstream filter can have a buffer which is always
> filled with 20 seconds data, because bitstream filter don't handle
> timestamp or time base.
> Feel free to point out if I have wrong understanding about bitstream filter.

Indeed you have. Your understanding seems to be based on the old
bitstream filter API, the one before
33d18982fa03feb061c8f744a4f0a9175c1f63ab (from November 2013).

Learned it.
One question I met is the actual muxer (not fifo proxy muxer)
write_header function should be called after the delay, seems that I
can not achieve it by the bitstream filter in a clean way.


Yes, ffmpeg.c does not delay writing the header until the first 
packet. Ideally ffmpeg.c code should be unlcuttered to be able to delay 
writing the header at least until the first packet arrives, but it 
seems to me that would require quite a bit of nontrivial ffmpeg.c 
refactoring.


Is it a big issue if writing the header is not delayed? Also the fifo code 
has the abilty to retry if the RTMP strem times out or whatever, can't 
that be used to work around this?


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

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

Re: [FFmpeg-devel] [PATCH v3 2/2] avcodec/mpeg12enc: Support mpeg2 encoder profile with const options

[lance . lmwang]

On Fri, May 01, 2020 at 03:22:32PM +0200, Marton Balint wrote:
> 
> 
> On Fri, 1 May 2020, lance.lmw...@gmail.com wrote:
> 
> > On Fri, May 01, 2020 at 12:32:44PM +0200, Marton Balint wrote:
> > > 
> > > 
> > > On Fri, 1 May 2020, lance.lmw...@gmail.com wrote:
> > > 
> > > > On Sat, Apr 04, 2020 at 08:25:31AM +0800, lance.lmw...@gmail.com wrote:
> > > > > From: Limin Wang 
> > > > > > > Signed-off-by: Limin Wang 
> > > > > ---
> > > > >  doc/encoders.texi  |  8 
> > > > >  libavcodec/mpeg12enc.c | 22 +-
> > > > >  libavcodec/mpegvideo.h |  1 +
> > > > >  3 files changed, 26 insertions(+), 5 deletions(-)
> > > > > > > diff --git a/doc/encoders.texi b/doc/encoders.texi
> > > > > index e23b6b3..5022b94 100644
> > > > > --- a/doc/encoders.texi
> > > > > +++ b/doc/encoders.texi
> > > > > @@ -2753,6 +2753,14 @@ For maximum compatibility, use 
> > > > > @samp{component}.
> > > > >  @item a53cc @var{boolean}
> > > > >  Import closed captions (which must be ATSC compatible format) into 
> > > > > output.
> > > > >  Default is 1 (on).
> > > > > +@item profile @var{integer}
> > > > > +Select the mpeg2 profile to encode, possible values:
> > > > > +@table @samp
> > > > > +@item 422
> > > > > +@item high
> > > > > +@item main
> > > > > +@item simple
> > > > > +@end table
> > > > >  @end table
> > > > > > >  @section png
> > > > > diff --git a/libavcodec/mpeg12enc.c b/libavcodec/mpeg12enc.c
> > > > > index 643ba81..9fe0c8b 100644
> > > > > --- a/libavcodec/mpeg12enc.c
> > > > > +++ b/libavcodec/mpeg12enc.c
> > > > > @@ -156,23 +156,30 @@ static av_cold int encode_init(AVCodecContext 
> > > > > *avctx)
> > > > >  }
> > > > >  }
> > > > > > > -if (avctx->profile == FF_PROFILE_UNKNOWN) {
> > > > > +if (s->profile == FF_PROFILE_UNKNOWN)
> > > > > +s->profile = avctx->profile;
> > > > > +
> > > > > +if (s->profile == FF_PROFILE_UNKNOWN) {
> > > > >  if (avctx->level != FF_LEVEL_UNKNOWN) {
> > > > >  av_log(avctx, AV_LOG_ERROR, "Set profile and level\n");
> > > > >  return -1;
> > > > >  }
> > > > >  /* Main or 4:2:2 */
> > > > >  avctx->profile = s->chroma_format == CHROMA_420 ? 
> > > > > FF_PROFILE_MPEG2_MAIN : FF_PROFILE_MPEG2_422;
> > > > > +s->profile = s->chroma_format == CHROMA_420 ? 
> > > > > FF_PROFILE_MPEG2_MAIN : FF_PROFILE_MPEG2_422;
> > > > > +} else if (s->profile < FF_PROFILE_MPEG2_422) {
> > > > > +av_log(avctx, AV_LOG_ERROR, "Invalid mpeg2 profile set\n");
> > > > > +return -1;
> > > > >  }
> > > > > > >  if (avctx->level == FF_LEVEL_UNKNOWN) {
> > > > > -if (avctx->profile == FF_PROFILE_MPEG2_422) {   /* 4:2:2 */
> > > > > +if (s->profile == FF_PROFILE_MPEG2_422) {   /* 4:2:2 */
> > > > >  if (avctx->width <= 720 && avctx->height <= 608)
> > > > >  avctx->level = 5;   /* Main */
> > > > >  else
> > > > >  avctx->level = 2;   /* High */
> > > > >  } else {
> > > > > -if (avctx->profile != FF_PROFILE_MPEG2_HIGH && 
> > > > > s->chroma_format != CHROMA_420) {
> > > > > +if (s->profile != FF_PROFILE_MPEG2_HIGH && 
> > > > > s->chroma_format != CHROMA_420) {
> > > > >  av_log(avctx, AV_LOG_ERROR,
> > > > > "Only High(1) and 4:2:2(0) profiles support 
> > > > > 4:2:2 color sampling\n");
> > > > >  return -1;
> > > > > @@ -321,9 +328,9 @@ static void 
> > > > > mpeg1_encode_sequence_header(MpegEncContext *s)
> > > > >  put_header(s, EXT_START_CODE);
> > > > >  put_bits(&s->pb, 4, 1); // seq ext
> > > > > > > -put_bits(&s->pb, 1, s->avctx->profile ==
> > > FF_PROFILE_MPEG2_422); // escx 1 for 4:2:2 profile
> > > > > +put_bits(&s->pb, 1, s->profile == FF_PROFILE_MPEG2_422); 
> > > > > // escx 1 for 4:2:2 profile
> > > > > > > -put_bits(&s->pb, 3, s->avctx->profile); //
> > > profile
> > > > > +put_bits(&s->pb, 3, s->profile); // profile
> > > > >  put_bits(&s->pb, 4, s->avctx->level);   // level
> > > > > > >  put_bits(&s->pb, 1, s->progressive_sequence);
> > > > > @@ -1165,6 +1172,11 @@ static const AVOption mpeg2_options[] = {
> > > > >  { "secam",NULL, 0, AV_OPT_TYPE_CONST,  {.i64 = 
> > > > > VIDEO_FORMAT_SECAM  },  0, 0, VE, "video_format" },
> > > > >  { "mac",  NULL, 0, AV_OPT_TYPE_CONST,  {.i64 = 
> > > > > VIDEO_FORMAT_MAC},  0, 0, VE, "video_format" },
> > > > >  { "unspecified",  NULL, 0, AV_OPT_TYPE_CONST,  {.i64 = 
> > > > > VIDEO_FORMAT_UNSPECIFIED},  0, 0, VE, "video_format" },
> > > > > +{ "profile",  "Set the profile",  OFFSET(profile),   
> > > > > AV_OPT_TYPE_INT,{ .i64 = FF_PROFILE_UNKNOWN }, FF_PROFILE_UNKNOWN, 
> > > > > FF_PROFILE_MPEG2_SIMPLE, VE, "profile" },
> > > > > +{ 

Re: [FFmpeg-devel] [PATCH] doc/encoders: remove ffaac>fdk-aac claim

[Jean-Baptiste Kempf]

On Thu, Apr 30, 2020, at 21:00, Lou Logan wrote:
> -This encoder is the default AAC encoder, natively implemented into FFmpeg. 
> Its
> -quality is on par or better than libfdk_aac at the default bitrate of 
> 128kbps.
> -This encoder also implements more options, profiles and samplerates than
> -other encoders (with only the AAC-HE profile pending to be implemented) so 
> this
> -encoder has become the default and is the recommended choice.
> +This encoder is the default AAC encoder, natively implemented into FFmpeg.

Is it not true that the encoder implements more options, profiles and 
samplerates?

-- 
Jean-Baptiste Kempf -  President
+33 672 704 734
___
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] doc/encoders: remove ffaac>fdk-aac claim

[Nicolas George]

Jean-Baptiste Kempf (12020-05-01):
> Is it not true that the encoder implements more options, profiles and
> samplerates?

But even if it is true, does it belong in the introductory part of the
documentation? The many options will be visible in the list of options
anyway. This paragraph looks more like a sales pitch to me. "this
encoder has become the default and is the recommended choice" could be
in an encoding guide, but it has nothing to do in a reference
documentation.

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 v3 2/2] avcodec/mpeg12enc: Support mpeg2 encoder profile with const options

[Marton Balint]

On Fri, 1 May 2020, lance.lmw...@gmail.com wrote:


On Fri, May 01, 2020 at 03:22:32PM +0200, Marton Balint wrote:



On Fri, 1 May 2020, lance.lmw...@gmail.com wrote:

> On Fri, May 01, 2020 at 12:32:44PM +0200, Marton Balint wrote:
> > 
> > 
> > On Fri, 1 May 2020, lance.lmw...@gmail.com wrote:
> > 
> > > On Sat, Apr 04, 2020 at 08:25:31AM +0800, lance.lmw...@gmail.com wrote:

> > > > From: Limin Wang 
> > > > > > Signed-off-by: Limin Wang 
> > > > ---
> > > >  doc/encoders.texi  |  8 
> > > >  libavcodec/mpeg12enc.c | 22 +-
> > > >  libavcodec/mpegvideo.h |  1 +
> > > >  3 files changed, 26 insertions(+), 5 deletions(-)
> > > > > > diff --git a/doc/encoders.texi b/doc/encoders.texi
> > > > index e23b6b3..5022b94 100644
> > > > --- a/doc/encoders.texi
> > > > +++ b/doc/encoders.texi
> > > > @@ -2753,6 +2753,14 @@ For maximum compatibility, use @samp{component}.
> > > >  @item a53cc @var{boolean}
> > > >  Import closed captions (which must be ATSC compatible format) into 
output.
> > > >  Default is 1 (on).
> > > > +@item profile @var{integer}
> > > > +Select the mpeg2 profile to encode, possible values:
> > > > +@table @samp
> > > > +@item 422
> > > > +@item high
> > > > +@item main
> > > > +@item simple
> > > > +@end table
> > > >  @end table
> > > > > >  @section png
> > > > diff --git a/libavcodec/mpeg12enc.c b/libavcodec/mpeg12enc.c
> > > > index 643ba81..9fe0c8b 100644
> > > > --- a/libavcodec/mpeg12enc.c
> > > > +++ b/libavcodec/mpeg12enc.c
> > > > @@ -156,23 +156,30 @@ static av_cold int encode_init(AVCodecContext 
*avctx)
> > > >  }
> > > >  }
> > > > > > -if (avctx->profile == FF_PROFILE_UNKNOWN) {
> > > > +if (s->profile == FF_PROFILE_UNKNOWN)
> > > > +s->profile = avctx->profile;
> > > > +
> > > > +if (s->profile == FF_PROFILE_UNKNOWN) {
> > > >  if (avctx->level != FF_LEVEL_UNKNOWN) {
> > > >  av_log(avctx, AV_LOG_ERROR, "Set profile and level\n");
> > > >  return -1;
> > > >  }
> > > >  /* Main or 4:2:2 */
> > > >  avctx->profile = s->chroma_format == CHROMA_420 ? 
FF_PROFILE_MPEG2_MAIN : FF_PROFILE_MPEG2_422;
> > > > +s->profile = s->chroma_format == CHROMA_420 ? 
FF_PROFILE_MPEG2_MAIN : FF_PROFILE_MPEG2_422;
> > > > +} else if (s->profile < FF_PROFILE_MPEG2_422) {
> > > > +av_log(avctx, AV_LOG_ERROR, "Invalid mpeg2 profile set\n");
> > > > +return -1;
> > > >  }
> > > > > >  if (avctx->level == FF_LEVEL_UNKNOWN) {
> > > > -if (avctx->profile == FF_PROFILE_MPEG2_422) {   /* 4:2:2 */
> > > > +if (s->profile == FF_PROFILE_MPEG2_422) {   /* 4:2:2 */
> > > >  if (avctx->width <= 720 && avctx->height <= 608)
> > > >  avctx->level = 5;   /* Main */
> > > >  else
> > > >  avctx->level = 2;   /* High */
> > > >  } else {
> > > > -if (avctx->profile != FF_PROFILE_MPEG2_HIGH && 
s->chroma_format != CHROMA_420) {
> > > > +if (s->profile != FF_PROFILE_MPEG2_HIGH && 
s->chroma_format != CHROMA_420) {
> > > >  av_log(avctx, AV_LOG_ERROR,
> > > > "Only High(1) and 4:2:2(0) profiles support 4:2:2 
color sampling\n");
> > > >  return -1;
> > > > @@ -321,9 +328,9 @@ static void 
mpeg1_encode_sequence_header(MpegEncContext *s)
> > > >  put_header(s, EXT_START_CODE);
> > > >  put_bits(&s->pb, 4, 1); // seq ext
> > > > > > -put_bits(&s->pb, 1, s->avctx->profile ==
> > FF_PROFILE_MPEG2_422); // escx 1 for 4:2:2 profile
> > > > +put_bits(&s->pb, 1, s->profile == FF_PROFILE_MPEG2_422); 
// escx 1 for 4:2:2 profile
> > > > > > -put_bits(&s->pb, 3, s->avctx->profile); //
> > profile
> > > > +put_bits(&s->pb, 3, s->profile); // profile
> > > >  put_bits(&s->pb, 4, s->avctx->level);   // level
> > > > > >  put_bits(&s->pb, 1, s->progressive_sequence);
> > > > @@ -1165,6 +1172,11 @@ static const AVOption mpeg2_options[] = {
> > > >  { "secam",NULL, 0, AV_OPT_TYPE_CONST,  {.i64 = 
VIDEO_FORMAT_SECAM  },  0, 0, VE, "video_format" },
> > > >  { "mac",  NULL, 0, AV_OPT_TYPE_CONST,  {.i64 = VIDEO_FORMAT_MAC  
  },  0, 0, VE, "video_format" },
> > > >  { "unspecified",  NULL, 0, AV_OPT_TYPE_CONST,  {.i64 = 
VIDEO_FORMAT_UNSPECIFIED},  0, 0, VE, "video_format" },
> > > > +{ "profile",  "Set the profile",  OFFSET(profile),   
AV_OPT_TYPE_INT,{ .i64 = FF_PROFILE_UNKNOWN }, FF_PROFILE_UNKNOWN, FF_PROFILE_MPEG2_SIMPLE, VE, "profile" },
> > > > +{ "422",  "",   0, AV_OPT_TYPE_CONST,{ .i64 = 
FF_PROFILE_MPEG2_422 }, 0, 0, VE, "profile" },
> > > > +{ "high", "",   0, AV_OPT_TYPE_CONST,{ .i64 = 
FF_PROFILE_MPEG2_HIGH}, 0, 0, VE, "profile" },
> > > > +{ "main", "",  

[FFmpeg-devel] [PATCH 1/1] avformat hls check discard state of streams always

[vectronic]

The discard needs to be checked on a stream even after the first packet is 
read. The first packet has already been read as part of calling 
avformat_find_stream_info. This means that setting AVStream->discard on a 
stream after having determined the stream info for the HLS package had no 
effect and unwanted packets were returned by subsequent calls to 
hls_read_packet.

Signed-off-by: vectronic 
---
 libavformat/hls.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libavformat/hls.c b/libavformat/hls.c
index fc45719d1c..0740e9c546 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -2014,7 +2014,7 @@ fail:
 return ret;
 }
 
-static int recheck_discard_flags(AVFormatContext *s, int first)
+static int recheck_discard_flags(AVFormatContext *s)
 {
 HLSContext *c = s->priv_data;
 int i, changed = 0;
@@ -2041,7 +2041,7 @@ static int recheck_discard_flags(AVFormatContext *s, int 
first)
 pls->seek_stream_index = -1;
 }
 av_log(s, AV_LOG_INFO, "Now receiving playlist %d, segment %d\n", 
i, pls->cur_seq_no);
-} else if (first && !cur_needed && pls->needed) {
+} else if (!cur_needed && pls->needed) {
 ff_format_io_close(pls->parent, &pls->input);
 pls->input_read_done = 0;
 ff_format_io_close(pls->parent, &pls->input_next);
@@ -2101,7 +2101,7 @@ static int hls_read_packet(AVFormatContext *s, AVPacket 
*pkt)
 HLSContext *c = s->priv_data;
 int ret, i, minplaylist = -1;
 
-recheck_discard_flags(s, c->first_packet);
+recheck_discard_flags(s);
 c->first_packet = 0;
 
 for (i = 0; i < c->n_playlists; i++) {
-- 
2.24.2 (Apple Git-127)

___
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/hlsenc: add support for microseconds since epoch based sequence number

[Marton Balint]

On Thu, 30 Apr 2020, Steven Liu wrote:





2020年4月30日 上午12:29，Marton Balint  写道：



On Sat, 18 Apr 2020, Marton Balint wrote:


Sequence numbers of segments should be unique, if an encoder is using shorter
than 1 second segments and it is restarted, then future segments will be using
already used sequence numbers if initial sequence number is based on the number
of seconds since epoch and not microseconds.


Ping.

Sorry response so late.
LGTM


Thanks, applied.

Regards,
Marton
___
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] avutil/opt: only skip evaluation for rational options

[Marton Balint]

On Wed, 29 Apr 2020, Marton Balint wrote:




On Sat, 18 Apr 2020, Marton Balint wrote:

Fixes problems when non-rational options were set using rational 

expressions,

causing rounding errors and the option range limits not to be enforced
properly.

ffmpeg -f lavfi -i "sine=r=96000/2"

This caused an assertion failure with assert level 2.

Signed-off-by: Marton Balint 
---
libavutil/opt.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/libavutil/opt.c b/libavutil/opt.c
index bf2562737b..b792dec01c 100644
--- a/libavutil/opt.c
+++ b/libavutil/opt.c
@@ -229,13 +229,15 @@ static int set_string(void *obj, const AVOption *o, 

const char *val, uint8_t **d
static int set_string_number(void *obj, void *target_obj, const AVOption 

*o, const char *val, void *dst)

{
int ret = 0;
-int num, den;
-char c;

-if (sscanf(val, "%d%*1[:/]%d%c", &num, &den, &c) == 2) {
-if ((ret = write_number(obj, o, dst, 1, den, num)) >= 0)
-return ret;
-ret = 0;
+if (o->type == AV_OPT_TYPE_RATIONAL || o->type == 

AV_OPT_TYPE_VIDEO_RATE) {

+int num, den;
+char c;
+if (sscanf(val, "%d%*1[:/]%d%c", &num, &den, &c) == 2) {
+if ((ret = write_number(obj, o, dst, 1, den, num)) >= 0)
+return ret;
+ret = 0;
+}
}


Ping, will apply soon.


Applied.

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

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

[FFmpeg-devel] [PATCH 0/1] avformat hls check discard state of streams always

[vectronic]

After opening an HLS package with avformat_open_input() and then getting stream
info with avformat_find_stream_info() I was then setting some of the input 
streams
to be discarded via avStream->discard = AVDISCARD_ALL.

However subsequent calls to av_read_frame() were returning packets from the 
streams
which were set to be discarded.

This patch addresses this issue:

The discard state of streams within HLS read packet logic was only checking the 
discard state when the first
packet was read. The first packet has already been read as part of calling 
avformat_find_stream_info.

vectronic (1):
  avformat hls check discard state of streams always

 libavformat/hls.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

-- 
2.24.2 (Apple Git-127)

___
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] avcodec/decode: use a single list bsf for codec decode bsfs

[Marton Balint]

On Tue, 28 Apr 2020, Marton Balint wrote:




On Sun, 26 Apr 2020, James Almer wrote:


On 4/26/2020 5:34 AM, Marton Balint wrote:

 void avcodec_flush_buffers(AVCodecContext *avctx)
 {
 AVCodecInternal *avci = avctx->internal;
@@ -2117,7 +2001,7 @@ void avcodec_flush_buffers(AVCodecContext *avctx)
 avctx->pts_correction_last_pts =
 avctx->pts_correction_last_dts = INT64_MIN;

-bsfs_flush(avctx);
+av_bsf_flush(avci->filter.bsf);


This function can be called with encoders as well, and after this change
you'll be calling av_bsf_flush() with NULL as argument.

Easiest solution is to add an av_codec_is_decoder(avctx->codec) check
before calling it, i guess.


Ok, changed locally.


Will apply the series soon, I will probably squash the two patch, since 
nobody was against removing DecodeFilterContext.


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

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

Re: [FFmpeg-devel] [PATCH 3/3] fftools/ffmpeg_opt: use av_bsf_list_parse_str for parsing bsf lists

[Marton Balint]

On Sun, 26 Apr 2020, James Almer wrote:


On 4/25/2020 3:55 PM, Marton Balint wrote:

Signed-off-by: Marton Balint 
---
 fftools/ffmpeg_opt.c | 57 +++-
 1 file changed, 3 insertions(+), 54 deletions(-)

diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index b52aa28626..dc42fb19d6 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -1416,7 +1416,6 @@ static OutputStream *new_output_stream(OptionsContext *o, 
AVFormatContext *oc, e
 {
 OutputStream *ost;
 AVStream *st = avformat_new_stream(oc, NULL);
-AVBSFList *bsf_list = NULL;
 int idx  = oc->nb_streams - 1, ret = 0;
 const char *bsfs = NULL, *time_base = NULL;
 char *next, *codec_tag = NULL;
@@ -1536,60 +1535,10 @@ static OutputStream *new_output_stream(OptionsContext 
*o, AVFormatContext *oc, e
 MATCH_PER_STREAM_OPT(copy_prior_start, i, ost->copy_prior_start, oc ,st);

 MATCH_PER_STREAM_OPT(bitstream_filters, str, bsfs, oc, st);
-while (bsfs && *bsfs) {
-const AVBitStreamFilter *filter;
-char *bsf, *bsf_options_str, *bsf_name;
-AVBSFContext *bsf_ctx;
-
-bsf = av_get_token(&bsfs, ",");
-if (!bsf)
-exit_program(1);
-bsf_name = av_strtok(bsf, "=", &bsf_options_str);
-if (!bsf_name)
-exit_program(1);
-
-filter = av_bsf_get_by_name(bsf_name);
-if (!filter) {
-av_log(NULL, AV_LOG_FATAL, "Unknown bitstream filter %s\n", 
bsf_name);
-exit_program(1);
-}
-
-ret = av_bsf_alloc(filter, &bsf_ctx);
-if (ret < 0) {
-av_log(NULL, AV_LOG_ERROR, "Error allocating a bitstream filter 
context\n");
-exit_program(1);
-}
-
-if (bsf_options_str && filter->priv_class) {
-const AVOption *opt = av_opt_next(bsf_ctx->priv_data, NULL);
-const char * shorthand[2] = {NULL};
-
-if (opt)
-shorthand[0] = opt->name;
-
-ret = av_opt_set_from_string(bsf_ctx->priv_data, bsf_options_str, shorthand, 
"=", ":");
-if (ret < 0) {
-av_log(NULL, AV_LOG_ERROR, "Error parsing options for bitstream 
filter %s\n", bsf_name);
-exit_program(1);
-}
-}
-
-if (!bsf_list)
-bsf_list = av_bsf_list_alloc();
-if (!bsf_list || av_bsf_list_append(bsf_list, bsf_ctx) < 0) {
-av_log(NULL, AV_LOG_ERROR, "Failed to allocate or append to bsf 
list\n");
-exit_program(1);
-}
-
-av_freep(&bsf);
-
-if (*bsfs)
-bsfs++;
-}
-if (bsf_list) {
-ret = av_bsf_list_finalize(&bsf_list, &ost->bsf_ctx);
+if (bsfs && *bsfs) {
+ret = av_bsf_list_parse_str(bsfs, &ost->bsf_ctx);
 if (ret < 0) {
-av_log(NULL, AV_LOG_ERROR, "Failed to finalize bsf list\n");
+av_log(NULL, AV_LOG_ERROR, "Error parsing bitstream filter sequence 
'%s': %s\n", bsfs, av_err2str(ret));
 exit_program(1);
 }
 }


Maybe this patch could instead be merged with 1/3 and applied after 2/3?


Will do and will apply soon.

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

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

Re: [FFmpeg-devel] [PATCH 1/2] avformat/mpegtsenc: Don't use heap allocated array to store pids

[Marton Balint]

On Thu, 30 Apr 2020, Andriy Gelman wrote:


From: Andriy Gelman 

A temporary heap array currently stores pids from all streams.  It is
used to make sure there are no duplicated pids. However, this array is
not needed because the pids from past streams are stored in the
MpegTSWriteStream structs.



LGTM, thanks.

Marton


Signed-off-by: Andriy Gelman 
---
libavformat/mpegtsenc.c | 51 +++--
1 file changed, 14 insertions(+), 37 deletions(-)

diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
index f2be6c6632..8ca1ddf003 100644
--- a/libavformat/mpegtsenc.c
+++ b/libavformat/mpegtsenc.c
@@ -931,7 +931,6 @@ static int mpegts_init(AVFormatContext *s)
{
MpegTSWrite *ts = s->priv_data;
int i, j;
-int *pids;
int ret;

if (ts->m2ts_mode == -1) {
@@ -989,12 +988,6 @@ static int mpegts_init(AVFormatContext *s)
ts->sdt.write_packet = section_write_packet;
ts->sdt.opaque   = s;

-pids = av_malloc_array(s->nb_streams, sizeof(*pids));
-if (!pids) {
-ret = AVERROR(ENOMEM);
-goto fail;
-}
-
/* assign pids to each stream */
for (i = 0; i < s->nb_streams; i++) {
AVStream *st = s->streams[i];
@@ -1002,8 +995,7 @@ static int mpegts_init(AVFormatContext *s)

ts_st = av_mallocz(sizeof(MpegTSWriteStream));
if (!ts_st) {
-ret = AVERROR(ENOMEM);
-goto fail;
+return AVERROR(ENOMEM);
}
st->priv_data = ts_st;

@@ -1011,8 +1003,7 @@ static int mpegts_init(AVFormatContext *s)

ts_st->payload = av_mallocz(ts->pes_payload_size);
if (!ts_st->payload) {
-ret = AVERROR(ENOMEM);
-goto fail;
+return AVERROR(ENOMEM);
}

/* MPEG pid values < 16 are reserved. Applications which set st->id in
@@ -1043,8 +1034,7 @@ static int mpegts_init(AVFormatContext *s)
ts->m2ts_textsub_pid > M2TS_TEXTSUB_PID + 1||
ts_st->pid < 16) {
av_log(s, AV_LOG_ERROR, "Cannot automatically assign PID for stream 
%d\n", st->index);
-ret = AVERROR(EINVAL);
-goto fail;
+return AVERROR(EINVAL);
}
} else {
ts_st->pid = ts->start_pid + i;
@@ -1055,30 +1045,26 @@ static int mpegts_init(AVFormatContext *s)
if (ts_st->pid >= 0x1FFF) {
av_log(s, AV_LOG_ERROR,
   "Invalid stream id %d, must be less than 8191\n", st->id);
-ret = AVERROR(EINVAL);
-goto fail;
+return AVERROR(EINVAL);
}
for (j = 0; j < ts->nb_services; j++) {
if (ts->services[j]->pmt.pid > LAST_OTHER_PID) {
av_log(s, AV_LOG_ERROR,
   "Invalid PMT PID %d, must be less than %d\n", 
ts->services[j]->pmt.pid, LAST_OTHER_PID + 1);
-ret = AVERROR(EINVAL);
-goto fail;
+return AVERROR(EINVAL);
}
if (ts_st->pid == ts->services[j]->pmt.pid) {
av_log(s, AV_LOG_ERROR, "PID %d cannot be both elementary and PMT 
PID\n", ts_st->pid);
-ret = AVERROR(EINVAL);
-goto fail;
+return AVERROR(EINVAL);
}
}
for (j = 0; j < i; j++) {
-if (pids[j] == ts_st->pid) {
+MpegTSWriteStream *ts_st_prev = s->streams[j]->priv_data;
+if (ts_st_prev->pid == ts_st->pid) {
av_log(s, AV_LOG_ERROR, "Duplicate stream id %d\n", ts_st->pid);
-ret = AVERROR(EINVAL);
-goto fail;
+return AVERROR(EINVAL);
}
}
-pids[i]= ts_st->pid;
ts_st->payload_pts = AV_NOPTS_VALUE;
ts_st->payload_dts = AV_NOPTS_VALUE;
ts_st->first_pts_check = 1;
@@ -1089,35 +1075,30 @@ static int mpegts_init(AVFormatContext *s)
AVStream *ast;
ts_st->amux = avformat_alloc_context();
if (!ts_st->amux) {
-ret = AVERROR(ENOMEM);
-goto fail;
+return AVERROR(ENOMEM);
}
ts_st->amux->oformat =
av_guess_format((ts->flags & MPEGTS_FLAG_AAC_LATM) ? "latm" : 
"adts",
NULL, NULL);
if (!ts_st->amux->oformat) {
-ret = AVERROR(EINVAL);
-goto fail;
+return AVERROR(EINVAL);
}
if (!(ast = avformat_new_stream(ts_st->amux, NULL))) {
-ret = AVERROR(ENOMEM);
-goto fail;
+return AVERROR(ENOMEM);
}
ret = avcodec_parameters_copy(ast->codecpar, st->codecpar);
if (ret != 0)
-goto fail;
+return ret;
ast->time_base = st->time_base;
ret = avformat_write_header(ts_

Re: [FFmpeg-devel] [PATCH 2/2] avformat/mpegtsenc: Remove two duplicated fields

[Marton Balint]

On Thu, 30 Apr 2020, Andriy Gelman wrote:


From: Andriy Gelman 

ts->{tsid,onid} stores the values of 
ts->{transport_stream_id,original_network_id}

Signed-off-by: Andriy Gelman 
---
libavformat/mpegtsenc.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)


LGTM, thanks.

Marton



diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
index 8ca1ddf003..bf1a7ee13f 100644
--- a/libavformat/mpegtsenc.c
+++ b/libavformat/mpegtsenc.c
@@ -79,8 +79,6 @@ typedef struct MpegTSWrite {
int64_t sdt_period; /* SDT period in PCR time base */
int64_t pat_period; /* PAT/PMT period in PCR time base */
int nb_services;
-int onid;
-int tsid;
int64_t first_pcr;
int64_t next_pcr;
int mux_rate; ///< set to 1 when VBR
@@ -261,7 +259,7 @@ static void mpegts_write_pat(AVFormatContext *s)
put16(&q, service->sid);
put16(&q, 0xe000 | service->pmt.pid);
}
-mpegts_write_section1(&ts->pat, PAT_TID, ts->tsid, ts->tables_version, 0, 
0,
+mpegts_write_section1(&ts->pat, PAT_TID, ts->transport_stream_id, 
ts->tables_version, 0, 0,
  data, q - data);
}

@@ -731,7 +729,7 @@ static void mpegts_write_sdt(AVFormatContext *s)
int i, running_status, free_ca_mode, val;

q = data;
-put16(&q, ts->onid);
+put16(&q, ts->original_network_id);
*q++ = 0xff;
for (i = 0; i < ts->nb_services; i++) {
service = ts->services[i];
@@ -757,7 +755,7 @@ static void mpegts_write_sdt(AVFormatContext *s)
desc_list_len_ptr[0] = val >> 8;
desc_list_len_ptr[1] = val;
}
-mpegts_write_section1(&ts->sdt, SDT_TID, ts->tsid, ts->tables_version, 0, 
0,
+mpegts_write_section1(&ts->sdt, SDT_TID, ts->transport_stream_id, 
ts->tables_version, 0, 0,
  data, q - data);
}

@@ -960,8 +958,6 @@ static int mpegts_init(AVFormatContext *s)
// round up to a whole number of TS packets
ts->pes_payload_size = (ts->pes_payload_size + 14 + 183) / 184 * 184 - 14;

-ts->tsid = ts->transport_stream_id;
-ts->onid = ts->original_network_id;
if (!s->nb_programs) {
/* allocate a single DVB service */
if (!mpegts_add_service(s, ts->service_id, s->metadata, NULL))
--
2.25.1

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

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

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

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

Re: [FFmpeg-devel] [PATCH 1/1] ensure closed caption info which is visible in default stream dump is also available in results when -show_streams is used

[Marton Balint]

On Wed, 29 Apr 2020, vectronic wrote:


Signed-off-by: vectronic 
---
doc/ffprobe.xsd   | 1 +
fftools/ffprobe.c | 2 ++
tests/ref/fate/concat-demuxer-extended-lavf-mxf   | 2 +-
tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10   | 2 +-
tests/ref/fate/concat-demuxer-simple1-lavf-mxf| 2 +-
tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10| 2 +-
tests/ref/fate/concat-demuxer-simple2-lavf-ts | 2 +-
tests/ref/fate/ffprobe_compact| 4 ++--
tests/ref/fate/ffprobe_csv| 4 ++--
tests/ref/fate/ffprobe_default| 2 ++
tests/ref/fate/ffprobe_flat   | 2 ++
tests/ref/fate/ffprobe_ini| 2 ++
tests/ref/fate/ffprobe_json   | 2 ++
tests/ref/fate/ffprobe_xml| 4 ++--
tests/ref/fate/hapqa-extract-nosnappy-to-hapalphaonly-mov | 1 +
tests/ref/fate/hapqa-extract-nosnappy-to-hapq-mov | 1 +
tests/ref/fate/mov-zombie | 2 +-
tests/ref/fate/mxf-probe-d10  | 1 +
tests/ref/fate/mxf-probe-dnxhd| 1 +
tests/ref/fate/mxf-probe-dv25 | 1 +
20 files changed, 28 insertions(+), 12 deletions(-)


LGTM, will apply.

Regards,
Marton



diff --git a/doc/ffprobe.xsd b/doc/ffprobe.xsd
index 97dc67def6..71cbd23ec6 100644
--- a/doc/ffprobe.xsd
+++ b/doc/ffprobe.xsd
@@ -226,6 +226,7 @@
  
  
  
+  
  
  
  
diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
index 840fcb71e2..5515e1b31b 100644
--- a/fftools/ffprobe.c
+++ b/fftools/ffprobe.c
@@ -2547,6 +2547,7 @@ static int show_stream(WriterContext *w, AVFormatContext 
*fmt_ctx, int stream_id
if (dec_ctx) {
print_int("coded_width",  dec_ctx->coded_width);
print_int("coded_height", dec_ctx->coded_height);
+print_int("closed_captions", !!(dec_ctx->properties & 
FF_CODEC_PROPERTY_CLOSED_CAPTIONS));
}
#endif
print_int("has_b_frames", par->video_delay);
@@ -2951,6 +2952,7 @@ static int open_input_file(InputFile *ifile, const char 
*filename,
ist->dec_ctx->pkt_timebase = stream->time_base;
ist->dec_ctx->framerate = stream->avg_frame_rate;
#if FF_API_LAVF_AVCTX
+ist->dec_ctx->properties = stream->codec->properties;
ist->dec_ctx->coded_width = stream->codec->coded_width;
ist->dec_ctx->coded_height = stream->codec->coded_height;
#endif
diff --git a/tests/ref/fate/concat-demuxer-extended-lavf-mxf 
b/tests/ref/fate/concat-demuxer-extended-lavf-mxf
index 2fb5fce4b1..e49915e407 100644
--- a/tests/ref/fate/concat-demuxer-extended-lavf-mxf
+++ b/tests/ref/fate/concat-demuxer-extended-lavf-mxf
@@ -1 +1 @@
-a6fb9c37dc71cb43eb9664a8ae9f1c66 
*tests/data/fate/concat-demuxer-extended-lavf-mxf.ffprobe
+861b9c23587d0a09caa78c3651faf5a0 
*tests/data/fate/concat-demuxer-extended-lavf-mxf.ffprobe
diff --git a/tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10 
b/tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10
index 60d729b3da..e3e76f217a 100644
--- a/tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10
+++ b/tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10
@@ -1 +1 @@
-cb7c8eac6f8917e39658e1fa4a250da8 
*tests/data/fate/concat-demuxer-extended-lavf-mxf_d10.ffprobe
+d66177ea3922692bc91cd0f8aa907650 
*tests/data/fate/concat-demuxer-extended-lavf-mxf_d10.ffprobe
diff --git a/tests/ref/fate/concat-demuxer-simple1-lavf-mxf 
b/tests/ref/fate/concat-demuxer-simple1-lavf-mxf
index d18e35b7ba..a590f6f24b 100644
--- a/tests/ref/fate/concat-demuxer-simple1-lavf-mxf
+++ b/tests/ref/fate/concat-demuxer-simple1-lavf-mxf
@@ -120,5 +120,5 @@ 
audio|1|65280|1.36|65280|1.36|1920|0.04|N/A|N/A|3840|207872|K_|1
Strings Metadata
video|0|37|1.48|34|1.36|1|0.04|N/A|N/A|24786|212480|K_|1
Strings Metadata
-0|mpeg2video|4|video|1/25|[0][0][0][0]|0x|352|288|0|0|1|1:1|11:9|yuv420p|8|tv|unknown|unknown|unknown|left|progressive|N/A|1|N/A|25/1|25/1|1/25|N/A|N/A|N/A|N/A|N/A|N/A|N/A|N/A|N/A|51|0|0|0|0|0|0|0|0|0|0|0|0|0x060A2B340101010501010D001301
+0|mpeg2video|4|video|1/25|[0][0][0][0]|0x|352|288|0|0|0|1|1:1|11:9|yuv420p|8|tv|unknown|unknown|unknown|left|progressive|N/A|1|N/A|25/1|25/1|1/25|N/A|N/A|N/A|N/A|N/A|N/A|N/A|N/A|N/A|51|0|0|0|0|0|0|0|0|0|0|0|0|0x060A2B340101010501010D001301
1|pcm_s16le|unknown|audio|1/48000|[0][0][0][0]|0x|s16|48000|1|unknown|16|N/A|0/0|0/0|1/48000|0|0.00|N/A|N/A|768000|N/A|N/A|N/A|N/A|50|0|0|0|0|0|0|0|0|0|0|0|0|0x060A2B340101010501010D001301
diff --git a/tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10 
b/tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10
index e83d1bf847..79ce1e2306 100644

[FFmpeg-devel] [PATCH] avformat/matroskaenc: always reserve max aac private data

[John Stebbins]

When extra data is updated using AV_PKT_DATA_NEW_EXTRADATA, the data is
written plus extra space is reserved for the max possible size extradata.
But when extradata is written during mkv_write_header, no extra space is
reserved. So the side data update overwrites whatever follows the
extradata in the track header and results in an invalid file.
---
 libavformat/matroskaenc.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 784973a951..e6917002c4 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -728,8 +728,6 @@ static int mkv_write_native_codecprivate(AVFormatContext 
*s, AVIOContext *pb,
 case AV_CODEC_ID_AAC:
 if (par->extradata_size)
 avio_write(dyn_cp, par->extradata, par->extradata_size);
-else
-put_ebml_void(pb, MAX_PCE_SIZE + 2 + 4);
 break;
 default:
 if (par->codec_id == AV_CODEC_ID_PRORES &&
@@ -749,6 +747,7 @@ static int mkv_write_codecprivate(AVFormatContext *s, 
AVIOContext *pb,
 AVIOContext *dyn_cp;
 uint8_t *codecpriv;
 int ret, codecpriv_size;
+int64_t offset;
 
 ret = avio_open_dyn_buf(&dyn_cp);
 if (ret < 0)
@@ -802,9 +801,15 @@ static int mkv_write_codecprivate(AVFormatContext *s, 
AVIOContext *pb,
 }
 
 codecpriv_size = avio_get_dyn_buf(dyn_cp, &codecpriv);
+offset = avio_tell(pb);
 if (codecpriv_size)
 put_ebml_binary(pb, MATROSKA_ID_CODECPRIVATE, codecpriv,
 codecpriv_size);
+if (par->codec_id == AV_CODEC_ID_AAC) {
+int filler = MAX_PCE_SIZE + 2 + 4 - (avio_tell(pb) - offset);
+if (filler)
+put_ebml_void(pb, filler);
+}
 ffio_free_dyn_buf(&dyn_cp);
 return ret;
 }
@@ -2182,7 +2187,7 @@ static int mkv_check_new_extra_data(AVFormatContext *s, 
const AVPacket *pkt)
 switch (par->codec_id) {
 case AV_CODEC_ID_AAC:
 if (side_data_size && (s->pb->seekable & AVIO_SEEKABLE_NORMAL) && 
!mkv->is_live) {
-int filler, output_sample_rate = 0;
+int output_sample_rate = 0;
 ret = get_aac_sample_rates(s, side_data, side_data_size, 
&track->sample_rate,
&output_sample_rate);
 if (ret < 0)
@@ -2195,9 +2200,6 @@ static int mkv_check_new_extra_data(AVFormatContext *s, 
const AVPacket *pkt)
 memcpy(par->extradata, side_data, side_data_size);
 avio_seek(mkv->tracks_bc, track->codecpriv_offset, SEEK_SET);
 mkv_write_codecprivate(s, mkv->tracks_bc, par, 1, 0);
-filler = MAX_PCE_SIZE + 2 + 4 - (avio_tell(mkv->tracks_bc) - 
track->codecpriv_offset);
-if (filler)
-put_ebml_void(mkv->tracks_bc, filler);
 avio_seek(mkv->tracks_bc, track->sample_rate_offset, SEEK_SET);
 put_ebml_float(mkv->tracks_bc, MATROSKA_ID_AUDIOSAMPLINGFREQ, 
track->sample_rate);
 put_ebml_float(mkv->tracks_bc, MATROSKA_ID_AUDIOOUTSAMPLINGFREQ, 
output_sample_rate);
-- 
2.25.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/matroskaenc: always reserve max aac private data

[Andreas Rheinhardt]

John Stebbins:
> When extra data is updated using AV_PKT_DATA_NEW_EXTRADATA, the data is
> written plus extra space is reserved for the max possible size extradata.
> But when extradata is written during mkv_write_header, no extra space is
> reserved. So the side data update overwrites whatever follows the
> extradata in the track header and results in an invalid file.
> ---
>  libavformat/matroskaenc.c | 14 --
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index 784973a951..e6917002c4 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -728,8 +728,6 @@ static int mkv_write_native_codecprivate(AVFormatContext 
> *s, AVIOContext *pb,
>  case AV_CODEC_ID_AAC:
>  if (par->extradata_size)
>  avio_write(dyn_cp, par->extradata, par->extradata_size);
> -else
> -put_ebml_void(pb, MAX_PCE_SIZE + 2 + 4);
>  break;
>  default:
>  if (par->codec_id == AV_CODEC_ID_PRORES &&
> @@ -749,6 +747,7 @@ static int mkv_write_codecprivate(AVFormatContext *s, 
> AVIOContext *pb,
>  AVIOContext *dyn_cp;
>  uint8_t *codecpriv;
>  int ret, codecpriv_size;
> +int64_t offset;
>  
>  ret = avio_open_dyn_buf(&dyn_cp);
>  if (ret < 0)
> @@ -802,9 +801,15 @@ static int mkv_write_codecprivate(AVFormatContext *s, 
> AVIOContext *pb,
>  }
>  
>  codecpriv_size = avio_get_dyn_buf(dyn_cp, &codecpriv);
> +offset = avio_tell(pb);
>  if (codecpriv_size)
>  put_ebml_binary(pb, MATROSKA_ID_CODECPRIVATE, codecpriv,
>  codecpriv_size);
> +if (par->codec_id == AV_CODEC_ID_AAC) {
> +int filler = MAX_PCE_SIZE + 2 + 4 - (avio_tell(pb) - offset);
> +if (filler)
> +put_ebml_void(pb, filler);
> +}
>  ffio_free_dyn_buf(&dyn_cp);
>  return ret;
>  }
> @@ -2182,7 +2187,7 @@ static int mkv_check_new_extra_data(AVFormatContext *s, 
> const AVPacket *pkt)
>  switch (par->codec_id) {
>  case AV_CODEC_ID_AAC:
>  if (side_data_size && (s->pb->seekable & AVIO_SEEKABLE_NORMAL) && 
> !mkv->is_live) {
> -int filler, output_sample_rate = 0;
> +int output_sample_rate = 0;
>  ret = get_aac_sample_rates(s, side_data, side_data_size, 
> &track->sample_rate,
> &output_sample_rate);
>  if (ret < 0)
> @@ -2195,9 +2200,6 @@ static int mkv_check_new_extra_data(AVFormatContext *s, 
> const AVPacket *pkt)
>  memcpy(par->extradata, side_data, side_data_size);
>  avio_seek(mkv->tracks_bc, track->codecpriv_offset, SEEK_SET);
>  mkv_write_codecprivate(s, mkv->tracks_bc, par, 1, 0);
> -filler = MAX_PCE_SIZE + 2 + 4 - (avio_tell(mkv->tracks_bc) - 
> track->codecpriv_offset);
> -if (filler)
> -put_ebml_void(mkv->tracks_bc, filler);
>  avio_seek(mkv->tracks_bc, track->sample_rate_offset, SEEK_SET);
>  put_ebml_float(mkv->tracks_bc, MATROSKA_ID_AUDIOSAMPLINGFREQ, 
> track->sample_rate);
>  put_ebml_float(mkv->tracks_bc, MATROSKA_ID_AUDIOOUTSAMPLINGFREQ, 
> output_sample_rate);
> 
If we already had extradata when writing the header, then what
guarantees us that the new extradata is better and should be preferred
to the old extradata? (I don't know the details of AAC extradata, but is
it possible that the extradata is simply incompatible to the old one
(e.g. when a user splices together actually incompatible streams)?)

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

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

Re: [FFmpeg-devel] [PATCH] [libavutil] Add saturated add/sub operations for int64_t.

[Dale Curtis]

On Fri, May 1, 2020 at 6:12 AM James Almer  wrote:

> On 5/1/2020 6:36 AM, Carl Eugen Hoyos wrote:
> >
> > The macro exists to avoid separate patches?
>
> No, it exists to not require configure checks just to enable a path for
> gcc/clang and another for other compilers.
>

Since consensus seems to have landed on splitting the patches, I've done
so. This thread now contains just the default implementation.

- dale


sat_math_v2.patch
Description: Binary data
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[FFmpeg-devel] [PATCH 2/2] Use gcc/clang builtins for av_sat_(add|sub)_64_c if available.

[Dale Curtis]

Signed-off-by: Dale Curtis 
---
 libavutil/common.h | 10 ++
 1 file changed, 10 insertions(+)


sat_math_builtin_v0.patch
Description: Binary data
___
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/matroskaenc: always reserve max aac private data

[James Almer]

On 5/1/2020 2:09 PM, John Stebbins wrote:
> When extra data is updated using AV_PKT_DATA_NEW_EXTRADATA, the data is
> written plus extra space is reserved for the max possible size extradata.
> But when extradata is written during mkv_write_header, no extra space is
> reserved. So the side data update overwrites whatever follows the
> extradata in the track header and results in an invalid file.

In what scenario there's extradata during init() and then new extradata
propagated in a packet side data for AAC? And should the side data one
take priority over the original one?

With FLAC we know it must have priority because the encoder sends it at
the end of the encoding process with information that was not available
during init(), but afaik that's not the case with AAC.

> ---
>  libavformat/matroskaenc.c | 14 --
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index 784973a951..e6917002c4 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -728,8 +728,6 @@ static int mkv_write_native_codecprivate(AVFormatContext 
> *s, AVIOContext *pb,
>  case AV_CODEC_ID_AAC:
>  if (par->extradata_size)
>  avio_write(dyn_cp, par->extradata, par->extradata_size);
> -else
> -put_ebml_void(pb, MAX_PCE_SIZE + 2 + 4);
>  break;
>  default:
>  if (par->codec_id == AV_CODEC_ID_PRORES &&
> @@ -749,6 +747,7 @@ static int mkv_write_codecprivate(AVFormatContext *s, 
> AVIOContext *pb,
>  AVIOContext *dyn_cp;
>  uint8_t *codecpriv;
>  int ret, codecpriv_size;
> +int64_t offset;
>  
>  ret = avio_open_dyn_buf(&dyn_cp);
>  if (ret < 0)
> @@ -802,9 +801,15 @@ static int mkv_write_codecprivate(AVFormatContext *s, 
> AVIOContext *pb,
>  }
>  
>  codecpriv_size = avio_get_dyn_buf(dyn_cp, &codecpriv);
> +offset = avio_tell(pb);
>  if (codecpriv_size)
>  put_ebml_binary(pb, MATROSKA_ID_CODECPRIVATE, codecpriv,
>  codecpriv_size);
> +if (par->codec_id == AV_CODEC_ID_AAC) {
> +int filler = MAX_PCE_SIZE + 2 + 4 - (avio_tell(pb) - offset);
> +if (filler)
> +put_ebml_void(pb, filler);
> +}

Why are you adding this here instead of adapting the code in
mkv_write_native_codecprivate()?

Can't you just do something like

case AV_CODEC_ID_AAC:
if (par->extradata_size)
avio_write(dyn_cp, par->extradata, par->extradata_size);
put_ebml_void(pb, MAX_PCE_SIZE + 2 + 4 - avio_tell(dyn_cp));
break;

>  ffio_free_dyn_buf(&dyn_cp);
>  return ret;
>  }
> @@ -2182,7 +2187,7 @@ static int mkv_check_new_extra_data(AVFormatContext *s, 
> const AVPacket *pkt)
>  switch (par->codec_id) {
>  case AV_CODEC_ID_AAC:
>  if (side_data_size && (s->pb->seekable & AVIO_SEEKABLE_NORMAL) && 
> !mkv->is_live) {
> -int filler, output_sample_rate = 0;
> +int output_sample_rate = 0;
>  ret = get_aac_sample_rates(s, side_data, side_data_size, 
> &track->sample_rate,
> &output_sample_rate);
>  if (ret < 0)
> @@ -2195,9 +2200,6 @@ static int mkv_check_new_extra_data(AVFormatContext *s, 
> const AVPacket *pkt)
>  memcpy(par->extradata, side_data, side_data_size);
>  avio_seek(mkv->tracks_bc, track->codecpriv_offset, SEEK_SET);
>  mkv_write_codecprivate(s, mkv->tracks_bc, par, 1, 0);
> -filler = MAX_PCE_SIZE + 2 + 4 - (avio_tell(mkv->tracks_bc) - 
> track->codecpriv_offset);
> -if (filler)
> -put_ebml_void(mkv->tracks_bc, filler);
>  avio_seek(mkv->tracks_bc, track->sample_rate_offset, SEEK_SET);
>  put_ebml_float(mkv->tracks_bc, MATROSKA_ID_AUDIOSAMPLINGFREQ, 
> track->sample_rate);
>  put_ebml_float(mkv->tracks_bc, MATROSKA_ID_AUDIOOUTSAMPLINGFREQ, 
> output_sample_rate);
> 

___
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] [libavutil] Add saturated add/sub operations for int64_t.

[James Almer]

On 5/1/2020 2:23 PM, Dale Curtis wrote:
> On Fri, May 1, 2020 at 6:12 AM James Almer  wrote:
> 
>> On 5/1/2020 6:36 AM, Carl Eugen Hoyos wrote:
>>>
>>> The macro exists to avoid separate patches?
>>
>> No, it exists to not require configure checks just to enable a path for
>> gcc/clang and another for other compilers.
>>
> 
> Since consensus seems to have landed on splitting the patches, I've done
> so. This thread now contains just the default implementation.

That wasn't the consensus. Neither Nicholas or I thought it was
required, but i don't have strong feelings about it.

> diff --git a/libavutil/common.h b/libavutil/common.h
> index 142ff9abe7..e926e7cb02 100644
> --- a/libavutil/common.h
> +++ b/libavutil/common.h
> @@ -291,6 +291,36 @@ static av_always_inline int av_sat_dsub32_c(int a, int b)
>  return av_sat_sub32(a, av_sat_add32(b, b));
>  }
>  
> +/**
> + * Add two signed 64-bit values with saturation.
> + *
> + * @param  a one value
> + * @param  b another value
> + * @return sum with signed saturation
> + */
> +static int64_t av_sat_add64_c(int64_t a, int64_t b) {

Missing av_always_inline attribute

> +  if (b >= 0 && a >= INT64_MAX - b)
> +return INT64_MAX;
> +  if (b <= 0 && a <= INT64_MIN - b)
> +return INT64_MIN;
> +  return a + b;
> +}
> +
> +/**
> + * Subtract two signed 64-bit values with saturation.
> + *
> + * @param  a one value
> + * @param  b another value
> + * @return difference with signed saturation
> + */
> +static int64_t av_sat_sub64_c(int64_t a, int64_t b) {

Ditto

> +  if (b <= 0 && a >= INT64_MAX + b) {
> +return INT64_MAX;
> +  if (b >= 0 && a <= INT64_MIN + b) {
> +return INT64_MIN;
> +  return a - b;
> +}

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

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

Re: [FFmpeg-devel] [PATCH] avformat/matroskaenc: always reserve max aac private data

[Andreas Rheinhardt]

James Almer:
> On 5/1/2020 2:09 PM, John Stebbins wrote:
>> When extra data is updated using AV_PKT_DATA_NEW_EXTRADATA, the data is
>> written plus extra space is reserved for the max possible size extradata.
>> But when extradata is written during mkv_write_header, no extra space is
>> reserved. So the side data update overwrites whatever follows the
>> extradata in the track header and results in an invalid file.
> 
> In what scenario there's extradata during init() and then new extradata
> propagated in a packet side data for AAC? And should the side data one
> take priority over the original one?
> 
> With FLAC we know it must have priority because the encoder sends it at
> the end of the encoding process with information that was not available
> during init(), but afaik that's not the case with AAC.
> 
>> ---
>>  libavformat/matroskaenc.c | 14 --
>>  1 file changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
>> index 784973a951..e6917002c4 100644
>> --- a/libavformat/matroskaenc.c
>> +++ b/libavformat/matroskaenc.c
>> @@ -728,8 +728,6 @@ static int mkv_write_native_codecprivate(AVFormatContext 
>> *s, AVIOContext *pb,
>>  case AV_CODEC_ID_AAC:
>>  if (par->extradata_size)
>>  avio_write(dyn_cp, par->extradata, par->extradata_size);
>> -else
>> -put_ebml_void(pb, MAX_PCE_SIZE + 2 + 4);
>>  break;
>>  default:
>>  if (par->codec_id == AV_CODEC_ID_PRORES &&
>> @@ -749,6 +747,7 @@ static int mkv_write_codecprivate(AVFormatContext *s, 
>> AVIOContext *pb,
>>  AVIOContext *dyn_cp;
>>  uint8_t *codecpriv;
>>  int ret, codecpriv_size;
>> +int64_t offset;
>>  
>>  ret = avio_open_dyn_buf(&dyn_cp);
>>  if (ret < 0)
>> @@ -802,9 +801,15 @@ static int mkv_write_codecprivate(AVFormatContext *s, 
>> AVIOContext *pb,
>>  }
>>  
>>  codecpriv_size = avio_get_dyn_buf(dyn_cp, &codecpriv);
>> +offset = avio_tell(pb);
>>  if (codecpriv_size)
>>  put_ebml_binary(pb, MATROSKA_ID_CODECPRIVATE, codecpriv,
>>  codecpriv_size);
>> +if (par->codec_id == AV_CODEC_ID_AAC) {
>> +int filler = MAX_PCE_SIZE + 2 + 4 - (avio_tell(pb) - offset);
>> +if (filler)
>> +put_ebml_void(pb, filler);
>> +}
> 
> Why are you adding this here instead of adapting the code in
> mkv_write_native_codecprivate()?
> 
> Can't you just do something like
> 
> case AV_CODEC_ID_AAC:
> if (par->extradata_size)
> avio_write(dyn_cp, par->extradata, par->extradata_size);
> put_ebml_void(pb, MAX_PCE_SIZE + 2 + 4 - avio_tell(dyn_cp));
> break;
> 
Then mkv_write_codecprivate() would overwrite the beginning of the void
element when it writes the CodecPrivate, creating an invalid file.
Instead one could do something like

if (par->extradata_size)
put_ebml_binary(pb, MATROSKA_ID_CODECPRIVATE, par->extradata,
par->extradata_size);
put_ebml_void(pb, MAX_PCE_SIZE + 2 + 4 - what was just written (not only
par->extradata);

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

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

Re: [FFmpeg-devel] [PATCH] [libavutil] Add saturated add/sub operations for int64_t.

[Dale Curtis]

On Fri, May 1, 2020 at 10:32 AM James Almer  wrote:

> On 5/1/2020 2:23 PM, Dale Curtis wrote:
> > On Fri, May 1, 2020 at 6:12 AM James Almer  wrote:
> >
> >> On 5/1/2020 6:36 AM, Carl Eugen Hoyos wrote:
> >>>
> >>> The macro exists to avoid separate patches?
> >>
> >> No, it exists to not require configure checks just to enable a path for
> >> gcc/clang and another for other compilers.
> >>
> >
> > Since consensus seems to have landed on splitting the patches, I've done
> > so. This thread now contains just the default implementation.
>
> That wasn't the consensus. Neither Nicholas or I thought it was
> required, but i don't have strong feelings about it.
>

Ah sorry for misunderstanding, your response to Carl above only seemed to
refute the necessity of a configure patch. I also don't care either way.


> > +static int64_t av_sat_add64_c(int64_t a, int64_t b) {
>
> Missing av_always_inline attribute
>

Done.


>
> > +static int64_t av_sat_sub64_c(int64_t a, int64_t b) {
>
> Ditto
>

Done.

- dale


sat_math_v3.patch
Description: Binary data
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 2/2] Use gcc/clang builtins for av_sat_(add|sub)_64_c if available.

[Dale Curtis]

Rebased.

On Fri, May 1, 2020 at 10:24 AM Dale Curtis  wrote:

> Signed-off-by: Dale Curtis 
> ---
>  libavutil/common.h | 10 ++
>  1 file changed, 10 insertions(+)
>


sat_math_builtin_v1.patch
Description: Binary data
___
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/matroskaenc: always reserve max aac private data

[John Stebbins]

On Fri, 2020-05-01 at 19:22 +0200, Andreas Rheinhardt wrote:
> John Stebbins:
> > When extra data is updated using AV_PKT_DATA_NEW_EXTRADATA, the
> > data is
> > written plus extra space is reserved for the max possible size
> > extradata.
> > But when extradata is written during mkv_write_header, no extra
> > space is
> > reserved. So the side data update overwrites whatever follows the
> > extradata in the track header and results in an invalid file.
> > ---
> >  libavformat/matroskaenc.c | 14 --
> >  1 file changed, 8 insertions(+), 6 deletions(-)
> > 
> > diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> > index 784973a951..e6917002c4 100644
> > --- a/libavformat/matroskaenc.c
> > +++ b/libavformat/matroskaenc.c
> > @@ -728,8 +728,6 @@ static int
> > mkv_write_native_codecprivate(AVFormatContext *s, AVIOContext *pb,
> >  case AV_CODEC_ID_AAC:
> >  if (par->extradata_size)
> >  avio_write(dyn_cp, par->extradata, par-
> > >extradata_size);
> > -else
> > -put_ebml_void(pb, MAX_PCE_SIZE + 2 + 4);
> >  break;
> >  default:
> >  if (par->codec_id == AV_CODEC_ID_PRORES &&
> > @@ -749,6 +747,7 @@ static int
> > mkv_write_codecprivate(AVFormatContext *s, AVIOContext *pb,
> >  AVIOContext *dyn_cp;
> >  uint8_t *codecpriv;
> >  int ret, codecpriv_size;
> > +int64_t offset;
> >  
> >  ret = avio_open_dyn_buf(&dyn_cp);
> >  if (ret < 0)
> > @@ -802,9 +801,15 @@ static int
> > mkv_write_codecprivate(AVFormatContext *s, AVIOContext *pb,
> >  }
> >  
> >  codecpriv_size = avio_get_dyn_buf(dyn_cp, &codecpriv);
> > +offset = avio_tell(pb);
> >  if (codecpriv_size)
> >  put_ebml_binary(pb, MATROSKA_ID_CODECPRIVATE, codecpriv,
> >  codecpriv_size);
> > +if (par->codec_id == AV_CODEC_ID_AAC) {
> > +int filler = MAX_PCE_SIZE + 2 + 4 - (avio_tell(pb) -
> > offset);
> > +if (filler)
> > +put_ebml_void(pb, filler);
> > +}
> >  ffio_free_dyn_buf(&dyn_cp);
> >  return ret;
> >  }
> > @@ -2182,7 +2187,7 @@ static int
> > mkv_check_new_extra_data(AVFormatContext *s, const AVPacket *pkt)
> >  switch (par->codec_id) {
> >  case AV_CODEC_ID_AAC:
> >  if (side_data_size && (s->pb->seekable &
> > AVIO_SEEKABLE_NORMAL) && !mkv->is_live) {
> > -int filler, output_sample_rate = 0;
> > +int output_sample_rate = 0;
> >  ret = get_aac_sample_rates(s, side_data,
> > side_data_size, &track->sample_rate,
> > &output_sample_rate);
> >  if (ret < 0)
> > @@ -2195,9 +2200,6 @@ static int
> > mkv_check_new_extra_data(AVFormatContext *s, const AVPacket *pkt)
> >  memcpy(par->extradata, side_data, side_data_size);
> >  avio_seek(mkv->tracks_bc, track->codecpriv_offset,
> > SEEK_SET);
> >  mkv_write_codecprivate(s, mkv->tracks_bc, par, 1, 0);
> > -filler = MAX_PCE_SIZE + 2 + 4 - (avio_tell(mkv-
> > >tracks_bc) - track->codecpriv_offset);
> > -if (filler)
> > -put_ebml_void(mkv->tracks_bc, filler);
> >  avio_seek(mkv->tracks_bc, track->sample_rate_offset,
> > SEEK_SET);
> >  put_ebml_float(mkv->tracks_bc,
> > MATROSKA_ID_AUDIOSAMPLINGFREQ, track->sample_rate);
> >  put_ebml_float(mkv->tracks_bc,
> > MATROSKA_ID_AUDIOOUTSAMPLINGFREQ, output_sample_rate);
> > 
> If we already had extradata when writing the header, then what
> guarantees us that the new extradata is better and should be
> preferred
> to the old extradata? (I don't know the details of AAC extradata, but
> is
> it possible that the extradata is simply incompatible to the old one
> (e.g. when a user splices together actually incompatible streams)?)
> 

The test case is a TS file with aac audio.  It TS files, it is
certainly possible for stream parameters to change on the fly. But, if
the extradata changes, there's really no way to handle it in mkv since
this data is global in mkv.  So perhaps a better solution is to ignore
side data extradata if it's already been written once?

___
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/matroskaenc: always reserve max aac private data

[Nicolas George]

John Stebbins (12020-05-01):
> The test case is a TS file with aac audio.  It TS files, it is
> certainly possible for stream parameters to change on the fly. But, if
> the extradata changes, there's really no way to handle it in mkv since
> this data is global in mkv.  So perhaps a better solution is to ignore
> side data extradata if it's already been written once?

Would it not lead to a corrupted file, possibly unplayable?

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/matroskaenc: always reserve max aac private data

[Andreas Rheinhardt]

John Stebbins:
> On Fri, 2020-05-01 at 19:22 +0200, Andreas Rheinhardt wrote:
>> John Stebbins:
>>> When extra data is updated using AV_PKT_DATA_NEW_EXTRADATA, the
>>> data is
>>> written plus extra space is reserved for the max possible size
>>> extradata.
>>> But when extradata is written during mkv_write_header, no extra
>>> space is
>>> reserved. So the side data update overwrites whatever follows the
>>> extradata in the track header and results in an invalid file.
>>> ---
>>>  libavformat/matroskaenc.c | 14 --
>>>  1 file changed, 8 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
>>> index 784973a951..e6917002c4 100644
>>> --- a/libavformat/matroskaenc.c
>>> +++ b/libavformat/matroskaenc.c
>>> @@ -728,8 +728,6 @@ static int
>>> mkv_write_native_codecprivate(AVFormatContext *s, AVIOContext *pb,
>>>  case AV_CODEC_ID_AAC:
>>>  if (par->extradata_size)
>>>  avio_write(dyn_cp, par->extradata, par-
 extradata_size);
>>> -else
>>> -put_ebml_void(pb, MAX_PCE_SIZE + 2 + 4);
>>>  break;
>>>  default:
>>>  if (par->codec_id == AV_CODEC_ID_PRORES &&
>>> @@ -749,6 +747,7 @@ static int
>>> mkv_write_codecprivate(AVFormatContext *s, AVIOContext *pb,
>>>  AVIOContext *dyn_cp;
>>>  uint8_t *codecpriv;
>>>  int ret, codecpriv_size;
>>> +int64_t offset;
>>>  
>>>  ret = avio_open_dyn_buf(&dyn_cp);
>>>  if (ret < 0)
>>> @@ -802,9 +801,15 @@ static int
>>> mkv_write_codecprivate(AVFormatContext *s, AVIOContext *pb,
>>>  }
>>>  
>>>  codecpriv_size = avio_get_dyn_buf(dyn_cp, &codecpriv);
>>> +offset = avio_tell(pb);
>>>  if (codecpriv_size)
>>>  put_ebml_binary(pb, MATROSKA_ID_CODECPRIVATE, codecpriv,
>>>  codecpriv_size);
>>> +if (par->codec_id == AV_CODEC_ID_AAC) {
>>> +int filler = MAX_PCE_SIZE + 2 + 4 - (avio_tell(pb) -
>>> offset);
>>> +if (filler)
>>> +put_ebml_void(pb, filler);
>>> +}
>>>  ffio_free_dyn_buf(&dyn_cp);
>>>  return ret;
>>>  }
>>> @@ -2182,7 +2187,7 @@ static int
>>> mkv_check_new_extra_data(AVFormatContext *s, const AVPacket *pkt)
>>>  switch (par->codec_id) {
>>>  case AV_CODEC_ID_AAC:
>>>  if (side_data_size && (s->pb->seekable &
>>> AVIO_SEEKABLE_NORMAL) && !mkv->is_live) {
>>> -int filler, output_sample_rate = 0;
>>> +int output_sample_rate = 0;
>>>  ret = get_aac_sample_rates(s, side_data,
>>> side_data_size, &track->sample_rate,
>>> &output_sample_rate);
>>>  if (ret < 0)
>>> @@ -2195,9 +2200,6 @@ static int
>>> mkv_check_new_extra_data(AVFormatContext *s, const AVPacket *pkt)
>>>  memcpy(par->extradata, side_data, side_data_size);
>>>  avio_seek(mkv->tracks_bc, track->codecpriv_offset,
>>> SEEK_SET);
>>>  mkv_write_codecprivate(s, mkv->tracks_bc, par, 1, 0);
>>> -filler = MAX_PCE_SIZE + 2 + 4 - (avio_tell(mkv-
 tracks_bc) - track->codecpriv_offset);
>>> -if (filler)
>>> -put_ebml_void(mkv->tracks_bc, filler);
>>>  avio_seek(mkv->tracks_bc, track->sample_rate_offset,
>>> SEEK_SET);
>>>  put_ebml_float(mkv->tracks_bc,
>>> MATROSKA_ID_AUDIOSAMPLINGFREQ, track->sample_rate);
>>>  put_ebml_float(mkv->tracks_bc,
>>> MATROSKA_ID_AUDIOOUTSAMPLINGFREQ, output_sample_rate);
>>>
>> If we already had extradata when writing the header, then what
>> guarantees us that the new extradata is better and should be
>> preferred
>> to the old extradata? (I don't know the details of AAC extradata, but
>> is
>> it possible that the extradata is simply incompatible to the old one
>> (e.g. when a user splices together actually incompatible streams)?)
>>
> 
> The test case is a TS file with aac audio.  It TS files, it is
> certainly possible for stream parameters to change on the fly. But, if
> the extradata changes, there's really no way to handle it in mkv since
> this data is global in mkv.  So perhaps a better solution is to ignore
> side data extradata if it's already been written once?
> 
In this scenario there is no way to know whether the new extradata is
better than the old one. If we ignore the extradata in such scenarios,
then is such a file even decodable?

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

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

Re: [FFmpeg-devel] [PATCH] [libavutil] Add saturated add/sub operations for int64_t.

[Carl Eugen Hoyos]

Am Fr., 1. Mai 2020 um 19:41 Uhr schrieb Dale Curtis :
>
> On Fri, May 1, 2020 at 10:32 AM James Almer  wrote:
>
> > On 5/1/2020 2:23 PM, Dale Curtis wrote:
> > > On Fri, May 1, 2020 at 6:12 AM James Almer  wrote:
> > >
> > >> On 5/1/2020 6:36 AM, Carl Eugen Hoyos wrote:
> > >>>
> > >>> The macro exists to avoid separate patches?
> > >>
> > >> No, it exists to not require configure checks just to enable a path for
> > >> gcc/clang and another for other compilers.
> > >>
> > >
> > > Since consensus seems to have landed on splitting the patches, I've done
> > > so. This thread now contains just the default implementation.
> >
> > That wasn't the consensus. Neither Nicholas or I thought it was
> > required, but i don't have strong feelings about it.
> >
>
> Ah sorry for misunderstanding, your response to Carl above only seemed to
> refute the necessity of a configure patch. I also don't care either way.
>
>
> > > +static int64_t av_sat_add64_c(int64_t a, int64_t b) {
> >
> > Missing av_always_inline attribute
> >
>
> Done.
>
>
> >
> > > +static int64_t av_sat_sub64_c(int64_t a, int64_t b) {
> >
> > Ditto
> >
>
> Done.

Could you confirm that you attached the wrong patch?

Or is it just my system?

Sorry, 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] avformat/matroskaenc: always reserve max aac private data

[John Stebbins]

On Fri, 2020-05-01 at 14:27 -0300, James Almer wrote:
> On 5/1/2020 2:09 PM, John Stebbins wrote:
> > When extra data is updated using AV_PKT_DATA_NEW_EXTRADATA, the
> > data is
> > written plus extra space is reserved for the max possible size
> > extradata.
> > But when extradata is written during mkv_write_header, no extra
> > space is
> > reserved. So the side data update overwrites whatever follows the
> > extradata in the track header and results in an invalid file.
> 
> In what scenario there's extradata during init() and then new
> extradata
> propagated in a packet side data for AAC? And should the side data
> one
> take priority over the original one?
> 

This is partially a HandBrake issue.  Before ffmpeg ever had side data,
HandBrake parsed the extradata out of the aac stream in TS files
ourselves before initializing the muxer.  We still do this, so
extradata is supplied by us when the muxer it initialized and again by
side data generated in aac_adtstoasc bsf.

So remuxing ts to mkv with ffmpeg exe does not have this problem.  But
it is a problem when using the API in valid ways.

I don't think it matters which takes priority.  If stream parameters
change, there really is no way to handle in mkv since this is global
data in mkv.  So perhaps just ignore side data when extradata has
already been set once?

> With FLAC we know it must have priority because the encoder sends it
> at
> the end of the encoding process with information that was not
> available
> during init(), but afaik that's not the case with AAC.
> 
> > ---
> >  libavformat/matroskaenc.c | 14 --
> >  1 file changed, 8 insertions(+), 6 deletions(-)
> > 
> > diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> > index 784973a951..e6917002c4 100644
> > --- a/libavformat/matroskaenc.c
> > +++ b/libavformat/matroskaenc.c
> > @@ -728,8 +728,6 @@ static int
> > mkv_write_native_codecprivate(AVFormatContext *s, AVIOContext *pb,
> >  case AV_CODEC_ID_AAC:
> >  if (par->extradata_size)
> >  avio_write(dyn_cp, par->extradata, par-
> > >extradata_size);
> > -else
> > -put_ebml_void(pb, MAX_PCE_SIZE + 2 + 4);
> >  break;
> >  default:
> >  if (par->codec_id == AV_CODEC_ID_PRORES &&
> > @@ -749,6 +747,7 @@ static int
> > mkv_write_codecprivate(AVFormatContext *s, AVIOContext *pb,
> >  AVIOContext *dyn_cp;
> >  uint8_t *codecpriv;
> >  int ret, codecpriv_size;
> > +int64_t offset;
> >  
> >  ret = avio_open_dyn_buf(&dyn_cp);
> >  if (ret < 0)
> > @@ -802,9 +801,15 @@ static int
> > mkv_write_codecprivate(AVFormatContext *s, AVIOContext *pb,
> >  }
> >  
> >  codecpriv_size = avio_get_dyn_buf(dyn_cp, &codecpriv);
> > +offset = avio_tell(pb);
> >  if (codecpriv_size)
> >  put_ebml_binary(pb, MATROSKA_ID_CODECPRIVATE, codecpriv,
> >  codecpriv_size);
> > +if (par->codec_id == AV_CODEC_ID_AAC) {
> > +int filler = MAX_PCE_SIZE + 2 + 4 - (avio_tell(pb) -
> > offset);
> > +if (filler)
> > +put_ebml_void(pb, filler);
> > +}
> 
> Why are you adding this here instead of adapting the code in
> mkv_write_native_codecprivate()?
> 
> Can't you just do something like
> 
> case AV_CODEC_ID_AAC:
> if (par->extradata_size)
> avio_write(dyn_cp, par->extradata, par->extradata_size);
> put_ebml_void(pb, MAX_PCE_SIZE + 2 + 4 - avio_tell(dyn_cp));
> break;
> 
> >  ffio_free_dyn_buf(&dyn_cp);
> >  return ret;
> >  }
> > @@ -2182,7 +2187,7 @@ static int
> > mkv_check_new_extra_data(AVFormatContext *s, const AVPacket *pkt)
> >  switch (par->codec_id) {
> >  case AV_CODEC_ID_AAC:
> >  if (side_data_size && (s->pb->seekable &
> > AVIO_SEEKABLE_NORMAL) && !mkv->is_live) {
> > -int filler, output_sample_rate = 0;
> > +int output_sample_rate = 0;
> >  ret = get_aac_sample_rates(s, side_data,
> > side_data_size, &track->sample_rate,
> > &output_sample_rate);
> >  if (ret < 0)
> > @@ -2195,9 +2200,6 @@ static int
> > mkv_check_new_extra_data(AVFormatContext *s, const AVPacket *pkt)
> >  memcpy(par->extradata, side_data, side_data_size);
> >  avio_seek(mkv->tracks_bc, track->codecpriv_offset,
> > SEEK_SET);
> >  mkv_write_codecprivate(s, mkv->tracks_bc, par, 1, 0);
> > -filler = MAX_PCE_SIZE + 2 + 4 - (avio_tell(mkv-
> > >tracks_bc) - track->codecpriv_offset);
> > -if (filler)
> > -put_ebml_void(mkv->tracks_bc, filler);
> >  avio_seek(mkv->tracks_bc, track->sample_rate_offset,
> > SEEK_SET);
> >  put_ebml_float(mkv->tracks_bc,
> > MATROSKA_ID_AUDIOSAMPLINGFREQ, track->sample_rate);
> >  put_ebml_float(mkv->tracks_bc,
> > MATROSKA_ID_AUDIOOUTSAMPLINGFREQ, output_sample_rate);
> > 
> 
> ___
> ffmpeg-dev

Re: [FFmpeg-devel] [PATCH] avformat/matroskaenc: always reserve max aac private data

[Andreas Rheinhardt]

John Stebbins:
> On Fri, 2020-05-01 at 14:27 -0300, James Almer wrote:
>> On 5/1/2020 2:09 PM, John Stebbins wrote:
>>> When extra data is updated using AV_PKT_DATA_NEW_EXTRADATA, the
>>> data is
>>> written plus extra space is reserved for the max possible size
>>> extradata.
>>> But when extradata is written during mkv_write_header, no extra
>>> space is
>>> reserved. So the side data update overwrites whatever follows the
>>> extradata in the track header and results in an invalid file.
>>
>> In what scenario there's extradata during init() and then new
>> extradata
>> propagated in a packet side data for AAC? And should the side data
>> one
>> take priority over the original one?
>>
> 
> This is partially a HandBrake issue.  Before ffmpeg ever had side data,
> HandBrake parsed the extradata out of the aac stream in TS files
> ourselves before initializing the muxer.  We still do this, so
> extradata is supplied by us when the muxer it initialized and again by
> side data generated in aac_adtstoasc bsf.
> 
Does this mean that HandBrake's extradata and the aac_adtstoasc bsf
side-data extradata are actually one and the same?

- Andreas

> So remuxing ts to mkv with ffmpeg exe does not have this problem.  But
> it is a problem when using the API in valid ways.
> 
> I don't think it matters which takes priority.  If stream parameters
> change, there really is no way to handle in mkv since this is global
> data in mkv.  So perhaps just ignore side data when extradata has
> already been set once?
> 
___
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/matroskaenc: always reserve max aac private data

[John Stebbins]

On Fri, 2020-05-01 at 19:53 +0200, Nicolas George wrote:
> John Stebbins (12020-05-01):
> > The test case is a TS file with aac audio.  It TS files, it is
> > certainly possible for stream parameters to change on the fly. But,
> > if
> > the extradata changes, there's really no way to handle it in mkv
> > since
> > this data is global in mkv.  So perhaps a better solution is to
> > ignore
> > side data extradata if it's already been written once?
> 
> Would it not lead to a corrupted file, possibly unplayable?
> 
> 

If the parameters change on the fly, some part of the audio stream is
going to be unplayable.  Either the part after the change will be
unplayable if you ignore extradata changes, or everything preceeding 
the last extradata change will be unplayable if you rewrite codec
private on every change.

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

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

Re: [FFmpeg-devel] [PATCH 2/2] Use gcc/clang builtins for av_sat_(add|sub)_64_c if available.

[Carl Eugen Hoyos]

Am Fr., 1. Mai 2020 um 19:24 Uhr schrieb Dale Curtis :
>
> Signed-off-by: Dale Curtis 
> ---
>  libavutil/common.h | 10 ++
>  1 file changed, 10 insertions(+)

Imo, this is guaranteed to break x86 compilation with some unusual
toolchain.
I looked carefully at all occurrences of AV_GCC_VERSION_AT_LEAST()
and I believe they are in fact different (not for x86 or combined with other
checks).

If you believe that the request for a configure check is unreasonable,
please commit.

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] avformat/matroskaenc: always reserve max aac private data

[Nicolas George]

John Stebbins (12020-05-01):
> If the parameters change on the fly, some part of the audio stream is
> going to be unplayable.  Either the part after the change will be
> unplayable if you ignore extradata changes, or everything preceeding 
> the last extradata change will be unplayable if you rewrite codec
> private on every change.

Then the only acceptable solution is to report an error.

If we can detect that the new extradata is compatible and will not cause
the file to be unplayable, we can let it pass, but we should try to
NEVER allow to lose data without notifying users.

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/matroskaenc: always reserve max aac private data

[John Stebbins]

On Fri, 2020-05-01 at 19:55 +0200, Andreas Rheinhardt wrote:
> John Stebbins:
> > On Fri, 2020-05-01 at 19:22 +0200, Andreas Rheinhardt wrote:
> > > John Stebbins:
> > > > When extra data is updated using AV_PKT_DATA_NEW_EXTRADATA, the
> > > > data is
> > > > written plus extra space is reserved for the max possible size
> > > > extradata.
> > > > But when extradata is written during mkv_write_header, no extra
> > > > space is
> > > > reserved. So the side data update overwrites whatever follows
> > > > the
> > > > extradata in the track header and results in an invalid file.
> > > > ---
> > > >  libavformat/matroskaenc.c | 14 --
> > > >  1 file changed, 8 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/libavformat/matroskaenc.c
> > > > b/libavformat/matroskaenc.c
> > > > index 784973a951..e6917002c4 100644
> > > > --- a/libavformat/matroskaenc.c
> > > > +++ b/libavformat/matroskaenc.c
> > > > @@ -728,8 +728,6 @@ static int
> > > > mkv_write_native_codecprivate(AVFormatContext *s, AVIOContext
> > > > *pb,
> > > >  case AV_CODEC_ID_AAC:
> > > >  if (par->extradata_size)
> > > >  avio_write(dyn_cp, par->extradata, par-
> > > > > extradata_size);
> > > > -else
> > > > -put_ebml_void(pb, MAX_PCE_SIZE + 2 + 4);
> > > >  break;
> > > >  default:
> > > >  if (par->codec_id == AV_CODEC_ID_PRORES &&
> > > > @@ -749,6 +747,7 @@ static int
> > > > mkv_write_codecprivate(AVFormatContext *s, AVIOContext *pb,
> > > >  AVIOContext *dyn_cp;
> > > >  uint8_t *codecpriv;
> > > >  int ret, codecpriv_size;
> > > > +int64_t offset;
> > > >  
> > > >  ret = avio_open_dyn_buf(&dyn_cp);
> > > >  if (ret < 0)
> > > > @@ -802,9 +801,15 @@ static int
> > > > mkv_write_codecprivate(AVFormatContext *s, AVIOContext *pb,
> > > >  }
> > > >  
> > > >  codecpriv_size = avio_get_dyn_buf(dyn_cp, &codecpriv);
> > > > +offset = avio_tell(pb);
> > > >  if (codecpriv_size)
> > > >  put_ebml_binary(pb, MATROSKA_ID_CODECPRIVATE,
> > > > codecpriv,
> > > >  codecpriv_size);
> > > > +if (par->codec_id == AV_CODEC_ID_AAC) {
> > > > +int filler = MAX_PCE_SIZE + 2 + 4 - (avio_tell(pb) -
> > > > offset);
> > > > +if (filler)
> > > > +put_ebml_void(pb, filler);
> > > > +}
> > > >  ffio_free_dyn_buf(&dyn_cp);
> > > >  return ret;
> > > >  }
> > > > @@ -2182,7 +2187,7 @@ static int
> > > > mkv_check_new_extra_data(AVFormatContext *s, const AVPacket
> > > > *pkt)
> > > >  switch (par->codec_id) {
> > > >  case AV_CODEC_ID_AAC:
> > > >  if (side_data_size && (s->pb->seekable &
> > > > AVIO_SEEKABLE_NORMAL) && !mkv->is_live) {
> > > > -int filler, output_sample_rate = 0;
> > > > +int output_sample_rate = 0;
> > > >  ret = get_aac_sample_rates(s, side_data,
> > > > side_data_size, &track->sample_rate,
> > > > &output_sample_rate);
> > > >  if (ret < 0)
> > > > @@ -2195,9 +2200,6 @@ static int
> > > > mkv_check_new_extra_data(AVFormatContext *s, const AVPacket
> > > > *pkt)
> > > >  memcpy(par->extradata, side_data, side_data_size);
> > > >  avio_seek(mkv->tracks_bc, track->codecpriv_offset,
> > > > SEEK_SET);
> > > >  mkv_write_codecprivate(s, mkv->tracks_bc, par, 1,
> > > > 0);
> > > > -filler = MAX_PCE_SIZE + 2 + 4 - (avio_tell(mkv-
> > > > > tracks_bc) - track->codecpriv_offset);
> > > > -if (filler)
> > > > -put_ebml_void(mkv->tracks_bc, filler);
> > > >  avio_seek(mkv->tracks_bc, track-
> > > > >sample_rate_offset,
> > > > SEEK_SET);
> > > >  put_ebml_float(mkv->tracks_bc,
> > > > MATROSKA_ID_AUDIOSAMPLINGFREQ, track->sample_rate);
> > > >  put_ebml_float(mkv->tracks_bc,
> > > > MATROSKA_ID_AUDIOOUTSAMPLINGFREQ, output_sample_rate);
> > > > 
> > > If we already had extradata when writing the header, then what
> > > guarantees us that the new extradata is better and should be
> > > preferred
> > > to the old extradata? (I don't know the details of AAC extradata,
> > > but
> > > is
> > > it possible that the extradata is simply incompatible to the old
> > > one
> > > (e.g. when a user splices together actually incompatible
> > > streams)?)
> > > 
> > 
> > The test case is a TS file with aac audio.  It TS files, it is
> > certainly possible for stream parameters to change on the fly. But,
> > if
> > the extradata changes, there's really no way to handle it in mkv
> > since
> > this data is global in mkv.  So perhaps a better solution is to
> > ignore
> > side data extradata if it's already been written once?
> > 
> In this scenario there is no way to know whether the new extradata is
> better than the old one. If we ignore the extradata in such
> scenarios,
> then is such a file even decodable?
> 

It depends on if the extradata actually changed

Re: [FFmpeg-devel] [PATCH] avformat/matroskaenc: always reserve max aac private data

[John Stebbins]

On Fri, 2020-05-01 at 20:03 +0200, Andreas Rheinhardt wrote:
> John Stebbins:
> > On Fri, 2020-05-01 at 14:27 -0300, James Almer wrote:
> > > On 5/1/2020 2:09 PM, John Stebbins wrote:
> > > > When extra data is updated using AV_PKT_DATA_NEW_EXTRADATA, the
> > > > data is
> > > > written plus extra space is reserved for the max possible size
> > > > extradata.
> > > > But when extradata is written during mkv_write_header, no extra
> > > > space is
> > > > reserved. So the side data update overwrites whatever follows
> > > > the
> > > > extradata in the track header and results in an invalid file.
> > > 
> > > In what scenario there's extradata during init() and then new
> > > extradata
> > > propagated in a packet side data for AAC? And should the side
> > > data
> > > one
> > > take priority over the original one?
> > > 
> > 
> > This is partially a HandBrake issue.  Before ffmpeg ever had side
> > data,
> > HandBrake parsed the extradata out of the aac stream in TS files
> > ourselves before initializing the muxer.  We still do this, so
> > extradata is supplied by us when the muxer it initialized and again
> > by
> > side data generated in aac_adtstoasc bsf.
> > 
> Does this mean that HandBrake's extradata and the aac_adtstoasc bsf
> side-data extradata are actually one and the same?
> 

Yes

___
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/matroskaenc: always reserve max aac private data

[John Stebbins]

On Fri, 2020-05-01 at 20:10 +0200, Nicolas George wrote:
> John Stebbins (12020-05-01):
> > If the parameters change on the fly, some part of the audio stream
> > is
> > going to be unplayable.  Either the part after the change will be
> > unplayable if you ignore extradata changes, or everything
> > preceeding 
> > the last extradata change will be unplayable if you rewrite codec
> > private on every change.
> 
> Then the only acceptable solution is to report an error.
> 
> If we can detect that the new extradata is compatible and will not
> cause
> the file to be unplayable, we can let it pass, but we should try to
> NEVER allow to lose data without notifying users.
> 

Well, current code in aac_adtstoasc silently ignores any changes.  It
only generates extradata from the initial packet.  It's not checked in
subsequent packets.

So this would have to be "fixed" in aac_adtstoasc as well if you want
to add error logging.

This is also a bit beyond my expertise.  I don't know what would
constitute incompitible parameters beyond a few obvious things.  I'm
not well versed in aac details.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 2/2] Use gcc/clang builtins for av_sat_(add|sub)_64_c if available.

[James Almer]

On 5/1/2020 3:07 PM, Carl Eugen Hoyos wrote:
> Am Fr., 1. Mai 2020 um 19:24 Uhr schrieb Dale Curtis 
> :
>>
>> Signed-off-by: Dale Curtis 
>> ---
>>  libavutil/common.h | 10 ++
>>  1 file changed, 10 insertions(+)
> 
> Imo, this is guaranteed to break x86 compilation with some unusual
> toolchain.
> I looked carefully at all occurrences of AV_GCC_VERSION_AT_LEAST()
> and I believe they are in fact different (not for x86 or combined with other
> checks).

Can you elaborate on this? AV_GCC_VERSION_AT_LEAST() is a public macro
used in public headers to choose one or another path depending on if the
compiler is a recent enough GCC, or something else.
What toolchain could this break, and why specifically x86?
__builtin_*_overflow are arch agnostic GCC functions.

> 
> If you believe that the request for a configure check is unreasonable,
> please commit.
> 
> 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".
> 

___
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] doc/encoders: remove ffaac>fdk-aac claim

[Lou Logan]

On Fri, May 1, 2020, at 6:56 AM, Jean-Baptiste Kempf wrote:
>
> Is it not true that the encoder implements more options, profiles and 
> samplerates?

ffaacenc has 1 additional AVoption compared to libfdk_aac; if that
means anything.

ffaacenc supports 1 additional sample rate (7350) compared to
libfdk_aac.

Not sure about profiles, but it certainly doesn't support HE(v2).

I didn't compare aac_at (AudioToolbox).

But as Nicolas mentioned I don't think any of that needs to be stated.
We don't do it for others.

I realize that the libfdk_aac section also has a claim which should
be removed:

> This encoder is considered to produce output on par or worse at
> 128kbps to the the native FFmpeg AAC encoder but can often produce
> better sounding audio at identical or lower bitrates...

I'll remove this when/if I apply the 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] avformat/matroskaenc: always reserve max aac private data

[Nicolas George]

John Stebbins (12020-05-01):
> Well, current code in aac_adtstoasc silently ignores any changes.  It
> only generates extradata from the initial packet.  It's not checked in
> subsequent packets.
> 
> So this would have to be "fixed" in aac_adtstoasc as well if you want
> to add error logging.

Probably. I want to emphasize that bugs in our current code cannot be
considered precedent to accept similar bugs in new code.

> This is also a bit beyond my expertise.  I don't know what would
> constitute incompitible parameters beyond a few obvious things.  I'm
> not well versed in aac details.

Then for now, I would say that we can only accept when it is
bit-identical with the extradata already there. If we cannot test for
compatibility more finely, then we have to assume incompatibility and
reject every AV_PKT_DATA_NEW_EXTRADATA with an error.

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] [libavutil] Add saturated add/sub operations for int64_t.

[Dale Curtis]

On Fri, May 1, 2020 at 10:57 AM Carl Eugen Hoyos  wrote:

> Could you confirm that you attached the wrong patch?
>

No, I sent the patches without completing the rebase. Sorry.

- dale


sat_math_v4.patch
Description: Binary data
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 2/2] Use gcc/clang builtins for av_sat_(add|sub)_64_c if available.

[Dale Curtis]

On Fri, May 1, 2020 at 11:22 AM James Almer  wrote:

> On 5/1/2020 3:07 PM, Carl Eugen Hoyos wrote:
> > Am Fr., 1. Mai 2020 um 19:24 Uhr schrieb Dale Curtis <
> dalecur...@chromium.org>:
> >>
> >> Signed-off-by: Dale Curtis 
> >> ---
> >>  libavutil/common.h | 10 ++
> >>  1 file changed, 10 insertions(+)
> >
> > Imo, this is guaranteed to break x86 compilation with some unusual
> > toolchain.
> > I looked carefully at all occurrences of AV_GCC_VERSION_AT_LEAST()
> > and I believe they are in fact different (not for x86 or combined with
> other
> > checks).
>
> Can you elaborate on this? AV_GCC_VERSION_AT_LEAST() is a public macro
> used in public headers to choose one or another path depending on if the
> compiler is a recent enough GCC, or something else.
> What toolchain could this break, and why specifically x86?
> __builtin_*_overflow are arch agnostic GCC functions.
>
>
Yes this is accurate. Actual rebased patch.
https://gcc.gnu.org/onlinedocs/gcc/Integer-Overflow-Builtins.html
https://releases.llvm.org/4.0.0/tools/clang/docs/LanguageExtensions.html#checked-arithmetic-builtins


- dale
From 61af5bd4c9a1f937dd0df5e50127bdd2e41d6965 Mon Sep 17 00:00:00 2001
From: Dale Curtis 
Date: Fri, 1 May 2020 10:20:43 -0700
Subject: [PATCH 2/2] Use gcc/clang builtins for av_sat_(add|sub)_64_c if
 available.

Signed-off-by: Dale Curtis 
---
 libavutil/common.h | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/libavutil/common.h b/libavutil/common.h
index 11907e5ba7..c4f5eea409 100644
--- a/libavutil/common.h
+++ b/libavutil/common.h
@@ -299,11 +299,16 @@ static av_always_inline int av_sat_dsub32_c(int a, int b)
  * @return sum with signed saturation
  */
 static av_always_inline int64_t av_sat_add64_c(int64_t a, int64_t b) {
+#if AV_GCC_VERSION_AT_LEAST(5,0) || defined(__clang__)
+  int64_t tmp;
+  return !__builtin_add_overflow(a, b, &tmp) ? tmp : (tmp < 0 ? INT64_MAX : INT64_MIN);
+#else
   if (b >= 0 && a >= INT64_MAX - b)
 return INT64_MAX;
   if (b <= 0 && a <= INT64_MIN - b)
 return INT64_MIN;
   return a + b;
+#endif
 }
 
 /**
@@ -314,11 +319,16 @@ static av_always_inline int64_t av_sat_add64_c(int64_t a, int64_t b) {
  * @return difference with signed saturation
  */
 static av_always_inline int64_t av_sat_sub64_c(int64_t a, int64_t b) {
+#if AV_GCC_VERSION_AT_LEAST(5,0) || defined(__clang__)
+  int64_t tmp;
+  return !__builtin_sub_overflow(a, b, &tmp) ? tmp : (tmp < 0 ? INT64_MAX : INT64_MIN);
+#else
   if (b <= 0 && a >= INT64_MAX + b) {
 return INT64_MAX;
   if (b >= 0 && a <= INT64_MIN + b) {
 return INT64_MIN;
   return a - b;
+#endif
 }
 
 /**
-- 
2.24.1.windows.2

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

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

[FFmpeg-devel] fix:?==?utf-8?q? rtsp/transport parser should accept comma for parameters

[abalam]

I needed this fix to make [Shinobi](https://github.com/ShinobiCCTV/Shinobi) 
works with recent chinese cameras.

We can read that FFmpeg is perfectly following the [RTSP 
specs](https://tools.ietf.org/html/rfc2326#page-58) : 
`Transports are comma separated, listed in order of preference. Parameters may 
be added to each transport, separated by a semicolon.`
But considering:
- VLC is working smart with not perfect cameras ;
- RTSP module in FFmpeg is not programmed to handle more than one transport ; 
=> this fix could be useful to anyone

I also think this fix might be improved by a better scan of parameters if 
ffmpeg wants to handle more than one transport.

Details: 
I'm using [Shinobi](https://github.com/ShinobiCCTV/Shinobi) to record cameras 
rtsp streams
For several cameras, cheap chinese ones, VLC is working to get stream but 
FFmpeg always crashes after it receives the RTSP transports parameters 
available.

`Transport: 
RTP/AVP;unicast;mode=PLAY;source=192.168.66.164;client_port=11658-11659;server_port=4-40001,ssrc=`

After debugging the source code, it appears that FFmpeg is not considering that 
a comma `,` before `ssrc` is a correct separator and think it's a second 
transport.
Then it crashes because RTSP don't want to accept more than one transport.

( This was closed PR https://github.com/FFmpeg/FFmpeg/pull/336 )

Thank you for having read,
Keep good work and have a nice day!

---
 libavformat/rtsp.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index b2b3f32011..f77de10119 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -1001,11 +1001,9 @@ static void rtsp_parse_transport(AVFormatContext *s,

             while (*p != ';' && *p != '\0' && *p != ',')
                 p++;
-            if (*p == ';')
+            if (*p == ';' || *p == ',')
                 p++;
         }
-        if (*p == ',')
-            p++;

         reply->nb_transports++;
         if (reply->nb_transports >= RTSP_MAX_TRANSPORTS)
--
2.21.1 (Apple Git-122.3)
___
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/librav1e: Use the framerate when available for ratecontrol

[Derek Buitenhuis]

On 01/05/2020 17:46, Lynne wrote:
> May 1, 2020, 12:31 by derek.buitenh...@gmail.com:
> 
>> Rav1e currently uses the time base given to it only for ratecontrol... where
>> the inverse is taken and used as a framerate. So, do what we do in other 
>> wrappers
>> and use the framerate if we can.
>>
>> Signed-off-by: Derek Buitenhuis 
>> ---
>> Notably, this leaves pkt->pts still broken (it was broken before, too), but
>> after discussion with James and Lynne, we decided to fix this in the rav1e 
>> API
>> and bump the minimum version, instead of using a PTS queue in librav1e.c.
>>
> 
> LGTM

Pushed, thanks.

- 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 1/2] avformat/oggdec: Factor buffer reallocation out

[Michael Niedermayer]

On Fri, May 01, 2020 at 06:47:54PM +0200, Lynne wrote:
> May 1, 2020, 14:16 by mich...@niedermayer.cc:
> 
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavformat/oggdec.c | 25 +
> >  1 file changed, 17 insertions(+), 8 deletions(-)
> >
> > diff --git a/libavformat/oggdec.c b/libavformat/oggdec.c
> > index c591bafddd..a9034ea61c 100644
> > --- a/libavformat/oggdec.c
> > +++ b/libavformat/oggdec.c
> > @@ -297,6 +297,20 @@ static int data_packets_seen(const struct ogg *ogg)
> >  return 0;
> >  }
> >  
> > +static int buf_realloc(struct ogg_stream *os, int size)
> > +{
> > +/* Even if invalid guarantee there's enough memory to read the page */
> > +if (os->bufsize - os->bufpos < size) {
> > +uint8_t *nb = av_realloc(os->buf, 2*os->bufsize + 
> > AV_INPUT_BUFFER_PADDING_SIZE);
> > +if (!nb)
> > +return AVERROR(ENOMEM);
> > +os->buf = nb;
> > +os->bufsize *= 2;
> > +}
> >
> 
> Looking at the code, this is just av_fast_realloc.
> You could change it to that, or you could just allocate MAX_PAGE_SIZE at the 
> start and never reallocate. Its only 65k.
> Up to you, feel free to commit whatever you choose.

The code already allocates 65k at the start
so ill apply this so this bug/regression is fixed, we can then change it
dont want to leave this bug open until we found the perfect solution

thx

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

Old school: Use the lowest level language in which you can solve the problem
conveniently.
New school: Use the highest level language in which the latest supercomputer
can solve the problem without the user falling asleep waiting.


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/matroskaenc: always reserve max aac private data

[John Stebbins]

On Fri, 2020-05-01 at 20:28 +0200, Nicolas George wrote:
> John Stebbins (12020-05-01):
> > Well, current code in aac_adtstoasc silently ignores any
> > changes.  It
> > only generates extradata from the initial packet.  It's not checked
> > in
> > subsequent packets.
> > 
> > So this would have to be "fixed" in aac_adtstoasc as well if you
> > want
> > to add error logging.
> 
> Probably. I want to emphasize that bugs in our current code cannot be
> considered precedent to accept similar bugs in new code.
> 
> > This is also a bit beyond my expertise.  I don't know what would
> > constitute incompitible parameters beyond a few obvious
> > things.  I'm
> > not well versed in aac details.
> 
> Then for now, I would say that we can only accept when it is
> bit-identical with the extradata already there. If we cannot test for
> compatibility more finely, then we have to assume incompatibility and
> reject every AV_PKT_DATA_NEW_EXTRADATA with an error.
> 
> 

Seems reasonable.  To be clear, this should result in an error returned
up the call stack (i.e. av_interleaved_write_frame would ultimately
return an error), and an error log?

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

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

Re: [FFmpeg-devel] [PATCH 2/2] Use gcc/clang builtins for av_sat_(add|sub)_64_c if available.

[Carl Eugen Hoyos]

Am Fr., 1. Mai 2020 um 20:22 Uhr schrieb James Almer :
>
> On 5/1/2020 3:07 PM, Carl Eugen Hoyos wrote:
> > Am Fr., 1. Mai 2020 um 19:24 Uhr schrieb Dale Curtis 
> > :
> >>
> >> Signed-off-by: Dale Curtis 
> >> ---
> >>  libavutil/common.h | 10 ++
> >>  1 file changed, 10 insertions(+)
> >
> > Imo, this is guaranteed to break x86 compilation with some unusual
> > toolchain.
> > I looked carefully at all occurrences of AV_GCC_VERSION_AT_LEAST()
> > and I believe they are in fact different (not for x86 or combined with other
> > checks).
>
> Can you elaborate on this? AV_GCC_VERSION_AT_LEAST() is a public macro
> used in public headers to choose one or another path depending on if the
> compiler is a recent enough GCC, or something else.

> What toolchain could this break

It definitely breaks icc compilation on Linux.

> and why specifically x86?

I mentioned x86 because afaics, all existing relevant uses of
AV_GCC_VERSION_AT_LEAST() will not be triggered on x86.

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/2] Use gcc/clang builtins for av_sat_(add|sub)_64_c if available.

[James Almer]

On 5/1/2020 4:27 PM, Carl Eugen Hoyos wrote:
> Am Fr., 1. Mai 2020 um 20:22 Uhr schrieb James Almer :
>>
>> On 5/1/2020 3:07 PM, Carl Eugen Hoyos wrote:
>>> Am Fr., 1. Mai 2020 um 19:24 Uhr schrieb Dale Curtis 
>>> :

 Signed-off-by: Dale Curtis 
 ---
  libavutil/common.h | 10 ++
  1 file changed, 10 insertions(+)
>>>
>>> Imo, this is guaranteed to break x86 compilation with some unusual
>>> toolchain.
>>> I looked carefully at all occurrences of AV_GCC_VERSION_AT_LEAST()
>>> and I believe they are in fact different (not for x86 or combined with other
>>> checks).
>>
>> Can you elaborate on this? AV_GCC_VERSION_AT_LEAST() is a public macro
>> used in public headers to choose one or another path depending on if the
>> compiler is a recent enough GCC, or something else.
> 
>> What toolchain could this break
> 
> It definitely breaks icc compilation on Linux.

So ICC on Linux defines __GNUC__ >= 5 yet doesn't support these builtins?

> 
>> and why specifically x86?
> 
> I mentioned x86 because afaics, all existing relevant uses of
> AV_GCC_VERSION_AT_LEAST() will not be triggered on x86.

AV_GCC_VERSION_AT_LEAST is used in a lot of arch agnostic libavutil
headers, and in x86/intmath.h

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

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

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

Re: [FFmpeg-devel] [PATCH 2/2] Use gcc/clang builtins for av_sat_(add|sub)_64_c if available.

[Carl Eugen Hoyos]

Am Fr., 1. Mai 2020 um 21:34 Uhr schrieb James Almer :
>
> On 5/1/2020 4:27 PM, Carl Eugen Hoyos wrote:
> > Am Fr., 1. Mai 2020 um 20:22 Uhr schrieb James Almer :
> >>
> >> On 5/1/2020 3:07 PM, Carl Eugen Hoyos wrote:
> >>> Am Fr., 1. Mai 2020 um 19:24 Uhr schrieb Dale Curtis 
> >>> :
> 
>  Signed-off-by: Dale Curtis 
>  ---
>   libavutil/common.h | 10 ++
>   1 file changed, 10 insertions(+)
> >>>
> >>> Imo, this is guaranteed to break x86 compilation with some unusual
> >>> toolchain.
> >>> I looked carefully at all occurrences of AV_GCC_VERSION_AT_LEAST()
> >>> and I believe they are in fact different (not for x86 or combined with 
> >>> other
> >>> checks).
> >>
> >> Can you elaborate on this? AV_GCC_VERSION_AT_LEAST() is a public macro
> >> used in public headers to choose one or another path depending on if the
> >> compiler is a recent enough GCC, or something else.
> >
> >> What toolchain could this break
> >
> > It definitely breaks icc compilation on Linux.
>
> So ICC on Linux defines __GNUC__ >= 5 yet doesn't support these builtins?

No, but yes, this is the effect.

> >> and why specifically x86?
> >
> > I mentioned x86 because afaics, all existing relevant uses of
> > AV_GCC_VERSION_AT_LEAST() will not be triggered on x86.
>
> AV_GCC_VERSION_AT_LEAST is used in a lot of arch agnostic libavutil
> headers, and in x86/intmath.h

This is exactly what I meant above with "carefully".

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] avformat/matroskaenc: always reserve max aac private data

[Carl Eugen Hoyos]

Am Fr., 1. Mai 2020 um 21:13 Uhr schrieb John Stebbins
:
>
> On Fri, 2020-05-01 at 20:28 +0200, Nicolas George wrote:
> > John Stebbins (12020-05-01):
> > > Well, current code in aac_adtstoasc silently ignores any
> > > changes.  It
> > > only generates extradata from the initial packet.  It's not checked
> > > in
> > > subsequent packets.
> > >
> > > So this would have to be "fixed" in aac_adtstoasc as well if you
> > > want
> > > to add error logging.
> >
> > Probably. I want to emphasize that bugs in our current code cannot be
> > considered precedent to accept similar bugs in new code.
> >
> > > This is also a bit beyond my expertise.  I don't know what would
> > > constitute incompitible parameters beyond a few obvious
> > > things.  I'm
> > > not well versed in aac details.
> >
> > Then for now, I would say that we can only accept when it is
> > bit-identical with the extradata already there. If we cannot test for
> > compatibility more finely, then we have to assume incompatibility and
> > reject every AV_PKT_DATA_NEW_EXTRADATA with an error.
> >
> >
>
> Seems reasonable.  To be clear, this should result in an error returned
> up the call stack (i.e. av_interleaved_write_frame would ultimately
> return an error), and an error log?

Only in an error log.

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/2] Use gcc/clang builtins for av_sat_(add|sub)_64_c if available.

[James Almer]

On 5/1/2020 4:37 PM, Carl Eugen Hoyos wrote:
> Am Fr., 1. Mai 2020 um 21:34 Uhr schrieb James Almer :
>>
>> On 5/1/2020 4:27 PM, Carl Eugen Hoyos wrote:
>>> Am Fr., 1. Mai 2020 um 20:22 Uhr schrieb James Almer :

 On 5/1/2020 3:07 PM, Carl Eugen Hoyos wrote:
> Am Fr., 1. Mai 2020 um 19:24 Uhr schrieb Dale Curtis 
> :
>>
>> Signed-off-by: Dale Curtis 
>> ---
>>  libavutil/common.h | 10 ++
>>  1 file changed, 10 insertions(+)
>
> Imo, this is guaranteed to break x86 compilation with some unusual
> toolchain.
> I looked carefully at all occurrences of AV_GCC_VERSION_AT_LEAST()
> and I believe they are in fact different (not for x86 or combined with 
> other
> checks).

 Can you elaborate on this? AV_GCC_VERSION_AT_LEAST() is a public macro
 used in public headers to choose one or another path depending on if the
 compiler is a recent enough GCC, or something else.
>>>
 What toolchain could this break
>>>
>>> It definitely breaks icc compilation on Linux.
>>
>> So ICC on Linux defines __GNUC__ >= 5 yet doesn't support these builtins?
> 
> No, but yes, this is the effect.

So that means ICC is broken, defining support for features it doesn't
really has.
We can workaround that with a !defined(__INTEL_COMPILER) check, i guess,
but perhaps we may want to stop supporting broken compilers.

> 
 and why specifically x86?
>>>
>>> I mentioned x86 because afaics, all existing relevant uses of
>>> AV_GCC_VERSION_AT_LEAST() will not be triggered on x86.
>>
>> AV_GCC_VERSION_AT_LEAST is used in a lot of arch agnostic libavutil
>> headers, and in x86/intmath.h
> 
> This is exactly what I meant above with "carefully".

It would be nice if you could stop being so terse and explain what's on
your mind for once. You're not being clear at all, and i asked you to
elaborate, something you still haven't done.
___
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/matroskaenc: always reserve max aac private data

[Andreas Rheinhardt]

John Stebbins:
> On Fri, 2020-05-01 at 20:28 +0200, Nicolas George wrote:
>> John Stebbins (12020-05-01):
>>> Well, current code in aac_adtstoasc silently ignores any
>>> changes.  It
>>> only generates extradata from the initial packet.  It's not checked
>>> in
>>> subsequent packets.
>>>
>>> So this would have to be "fixed" in aac_adtstoasc as well if you
>>> want
>>> to add error logging.
>>
>> Probably. I want to emphasize that bugs in our current code cannot be
>> considered precedent to accept similar bugs in new code.
>>
>>> This is also a bit beyond my expertise.  I don't know what would
>>> constitute incompitible parameters beyond a few obvious
>>> things.  I'm
>>> not well versed in aac details.
>>
>> Then for now, I would say that we can only accept when it is
>> bit-identical with the extradata already there. If we cannot test for
>> compatibility more finely, then we have to assume incompatibility and
>> reject every AV_PKT_DATA_NEW_EXTRADATA with an error.
>>
>>
> 
> Seems reasonable.  To be clear, this should result in an error returned
> up the call stack (i.e. av_interleaved_write_frame would ultimately
> return an error), and an error log?
> 
I'd rather opt to only warn in such a case unless strict_std_compliance
is >= FF_COMPLIANCE_STRICT. And maybe we should also discard all future
packets from this stream until we get one with side data matching the
CodecPrivate one?

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

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

Re: [FFmpeg-devel] [PATCH 2/2] Use gcc/clang builtins for av_sat_(add|sub)_64_c if available.

[Carl Eugen Hoyos]

Am Fr., 1. Mai 2020 um 21:42 Uhr schrieb James Almer :
>
> On 5/1/2020 4:37 PM, Carl Eugen Hoyos wrote:
> > Am Fr., 1. Mai 2020 um 21:34 Uhr schrieb James Almer :
> >>
> >> On 5/1/2020 4:27 PM, Carl Eugen Hoyos wrote:
> >>> Am Fr., 1. Mai 2020 um 20:22 Uhr schrieb James Almer :
> 
>  On 5/1/2020 3:07 PM, Carl Eugen Hoyos wrote:
> > Am Fr., 1. Mai 2020 um 19:24 Uhr schrieb Dale Curtis 
> > :
> >>
> >> Signed-off-by: Dale Curtis 
> >> ---
> >>  libavutil/common.h | 10 ++
> >>  1 file changed, 10 insertions(+)
> >
> > Imo, this is guaranteed to break x86 compilation with some unusual
> > toolchain.
> > I looked carefully at all occurrences of AV_GCC_VERSION_AT_LEAST()
> > and I believe they are in fact different (not for x86 or combined with 
> > other
> > checks).
> 
>  Can you elaborate on this? AV_GCC_VERSION_AT_LEAST() is a public macro
>  used in public headers to choose one or another path depending on if the
>  compiler is a recent enough GCC, or something else.
> >>>
>  What toolchain could this break
> >>>
> >>> It definitely breaks icc compilation on Linux.
> >>
> >> So ICC on Linux defines __GNUC__ >= 5 yet doesn't support these builtins?
> >
> > No, but yes, this is the effect.
>
> So that means ICC is broken, defining support for features it doesn't
> really has.
> We can workaround that with a !defined(__INTEL_COMPILER) check, i guess,
> but perhaps we may want to stop supporting broken compilers.

Or we add the necessary two lines of configure code necessary to avoid
the issue.

>  and why specifically x86?
> >>>
> >>> I mentioned x86 because afaics, all existing relevant uses of
> >>> AV_GCC_VERSION_AT_LEAST() will not be triggered on x86.
> >>
> >> AV_GCC_VERSION_AT_LEAST is used in a lot of arch agnostic libavutil
> >> headers, and in x86/intmath.h
> >
> > This is exactly what I meant above with "carefully".
>
> It would be nice if you could stop being so terse and explain what's on
> your mind for once. You're not being clear at all, and i asked you to
> elaborate, something you still haven't done.

To make this crystal-clear:
It would be nice if you could stop writing offending and annoying mails as
you did today.

The intmath.h check is surrounded by an additional check that looks much
stricter to me than the AV_GCC_VERSION_AT_LEAST() check there.

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/2] Use gcc/clang builtins for av_sat_(add|sub)_64_c if available.

[Dale Curtis]

On Fri, May 1, 2020 at 12:37 PM Carl Eugen Hoyos  wrote:

> >
> > So ICC on Linux defines __GNUC__ >= 5 yet doesn't support these builtins?
>
> No, but yes, this is the effect.
>

Does this patch work instead on the ICC compiler? GCC 4.2+ have support for
__has_builtin() which shouldn't be masqueraded by the ICC.

- dale
From b52049eea61e602382684534c18fd1e301b13d46 Mon Sep 17 00:00:00 2001
From: Dale Curtis 
Date: Fri, 1 May 2020 10:20:43 -0700
Subject: [PATCH] Use gcc/clang builtins for av_sat_(add|sub)_64_c if
 available.

Signed-off-by: Dale Curtis 
---
 libavutil/common.h | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/libavutil/common.h b/libavutil/common.h
index 11907e5ba7..915e91f8f7 100644
--- a/libavutil/common.h
+++ b/libavutil/common.h
@@ -299,11 +299,16 @@ static av_always_inline int av_sat_dsub32_c(int a, int b)
  * @return sum with signed saturation
  */
 static av_always_inline int64_t av_sat_add64_c(int64_t a, int64_t b) {
+#if (AV_GCC_VERSION_AT_LEAST(5,0) && __has_builtin(__builtin_add_overflow)) || defined(__clang__)
+  int64_t tmp;
+  return !__builtin_add_overflow(a, b, &tmp) ? tmp : (tmp < 0 ? INT64_MAX : INT64_MIN);
+#else
   if (b >= 0 && a >= INT64_MAX - b)
 return INT64_MAX;
   if (b <= 0 && a <= INT64_MIN - b)
 return INT64_MIN;
   return a + b;
+#endif
 }
 
 /**
@@ -314,11 +319,16 @@ static av_always_inline int64_t av_sat_add64_c(int64_t a, int64_t b) {
  * @return difference with signed saturation
  */
 static av_always_inline int64_t av_sat_sub64_c(int64_t a, int64_t b) {
+#if (AV_GCC_VERSION_AT_LEAST(5,0) && __has_builtin(__builtin_sub_overflow)) || defined(__clang__)
+  int64_t tmp;
+  return !__builtin_sub_overflow(a, b, &tmp) ? tmp : (tmp < 0 ? INT64_MAX : INT64_MIN);
+#else
   if (b <= 0 && a >= INT64_MAX + b) {
 return INT64_MAX;
   if (b >= 0 && a <= INT64_MIN + b) {
 return INT64_MIN;
   return a - b;
+#endif
 }
 
 /**
-- 
2.24.1.windows.2

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

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

Re: [FFmpeg-devel] [PATCH 2/2] Use gcc/clang builtins for av_sat_(add|sub)_64_c if available.

[Dale Curtis]

On Fri, May 1, 2020 at 12:50 PM Dale Curtis  wrote:

> On Fri, May 1, 2020 at 12:37 PM Carl Eugen Hoyos 
> wrote:
>
>> >
>> > So ICC on Linux defines __GNUC__ >= 5 yet doesn't support these
>> builtins?
>>
>> No, but yes, this is the effect.
>>
>
> Does this patch work instead on the ICC compiler? GCC 4.2+ have support
> for __has_builtin() which shouldn't be masqueraded by the ICC.
>
>
Whoops, I misread the release notes. It's 6.0
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66970 I guess configure is the
only way unless we limit to GCC6+.

- dale
___
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] [libavutil] Add saturated add/sub operations for int64_t.

[Michael Niedermayer]

On Thu, Apr 30, 2020 at 05:39:43PM -0700, Dale Curtis wrote:
> On Thu, Apr 30, 2020 at 5:21 PM James Almer  wrote:
> 
> > On 4/30/2020 7:19 PM, Dale Curtis wrote:
> > > Many places are using their own custom code for handling overflow
> > > around timestamps or other int64_t values. There are enough of these
> > > now that having some common saturated math functions seems sound.
> > >
> > > This adds implementations that just use the builtin functions for
> > > recent gcc, clang when available or implements its own version for
> > > older compilers.
> >
> > These look like 64 bit versions of av_sat_add32 and av_sat_sub32, from
> > common.h, so you should probably add them there and rename them
> > accordingly.
> >
> >
> Ah! I was looking for those, but missed them. Thanks. Done.

one disadvantage the av_sat* functions have is the lack of inexact detection

In addition to av_sat*
In situations where its better to fail than to clip, something that
emulates what (+-Inf/)NaN is for float may make sense.
That would allow to simply check after a computation if any inexactness
occured

Such a thing could be usefull in situations where a exact value or an
error is wanted.

thx

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Elect your leaders based on what they did after the last election, not
based on what they say before an election.



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] fix:?==?utf-8?q? rtsp/transport parser should accept comma for parameters

[Marton Balint]

On Fri, 1 May 2020, abalam wrote:



I needed this fix to make [Shinobi](https://github.com/ShinobiCCTV/Shinobi) 
works with recent chinese cameras.

We can read that FFmpeg is perfectly following the [RTSP 
specs](https://tools.ietf.org/html/rfc2326#page-58) : 
`Transports are comma separated, listed in order of preference. Parameters may 
be added to each transport, separated by a semicolon.`
But considering:
- VLC is working smart with not perfect cameras ;
- RTSP module in FFmpeg is not programmed to handle more than one transport ; 
=> this fix could be useful to anyone

I also think this fix might be improved by a better scan of parameters if 
ffmpeg wants to handle more than one transport.

Details: 
I'm using [Shinobi](https://github.com/ShinobiCCTV/Shinobi) to record cameras 
rtsp streams
For several cameras, cheap chinese ones, VLC is working to get stream but 
FFmpeg always crashes after it receives the RTSP transports parameters 
available.

`Transport: 
RTP/AVP;unicast;mode=PLAY;source=192.168.66.164;client_port=11658-11659;server_port=4-40001,ssrc=`

After debugging the source code, it appears that FFmpeg is not considering that 
a comma `,` before `ssrc` is a correct separator and think it's a second 
transport.
Then it crashes because RTSP don't want to accept more than one transport.

( This was closed PR https://github.com/FFmpeg/FFmpeg/pull/336 )

Thank you for having read,
Keep good work and have a nice day!


I am not really familiar with RTSP code, but at least the parse function 
does seem to parse multiple transports. So effectively breaking it does 
not seem right. Maybe you should check out how VLC detects such broken 
devices, and implement something similar here, because probably VLC's 
workaround is proven to not cause valid transport specifications to be 
misinterpreted as a single transport.


Regards,
Marton



---
 libavformat/rtsp.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index b2b3f32011..f77de10119 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -1001,11 +1001,9 @@ static void rtsp_parse_transport(AVFormatContext *s,

             while (*p != ';' && *p != '\0' && *p != ',')
                 p++;
-            if (*p == ';')
+            if (*p == ';' || *p == ',')
                 p++;
         }
-        if (*p == ',')
-            p++;

         reply->nb_transports++;
         if (reply->nb_transports >= RTSP_MAX_TRANSPORTS)
--
2.21.1 (Apple Git-122.3)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

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

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

Re: [FFmpeg-devel] [PATCH] avformat/matroskaenc: always reserve max aac private data

[Nicolas George]

Andreas Rheinhardt (12020-05-01):
> I'd rather opt to only warn in such a case unless strict_std_compliance
> is >= FF_COMPLIANCE_STRICT. And maybe we should also discard all future
> packets from this stream until we get one with side data matching the
> CodecPrivate one?

It is not a matter of standard compliance, it is a matter of lost data.
If only one extradata is written, then the other is discarded, and
possibly all subsequent packets are un-decodable.

Without an AVERROR code up the stack, a GUI application (most of them
just let av_log go to stderr) could open a dialog "Data successfully
saved. Delete original? [Yes] No" and let actual data be lost.

We can have an option to ignore the error, but as it is, it really must
be an error condition by default. Exactly the same as a write error on
the file.

Regards,

-- 
  Nicolas George


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

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

Re: [FFmpeg-devel] [PATCH 2/2] Use gcc/clang builtins for av_sat_(add|sub)_64_c if available.

[James Almer]

On 5/1/2020 4:46 PM, Carl Eugen Hoyos wrote:
> Am Fr., 1. Mai 2020 um 21:42 Uhr schrieb James Almer :
>>
>> On 5/1/2020 4:37 PM, Carl Eugen Hoyos wrote:
>>> Am Fr., 1. Mai 2020 um 21:34 Uhr schrieb James Almer :

 On 5/1/2020 4:27 PM, Carl Eugen Hoyos wrote:
> Am Fr., 1. Mai 2020 um 20:22 Uhr schrieb James Almer :
>>
>> On 5/1/2020 3:07 PM, Carl Eugen Hoyos wrote:
>>> Am Fr., 1. Mai 2020 um 19:24 Uhr schrieb Dale Curtis 
>>> :

 Signed-off-by: Dale Curtis 
 ---
  libavutil/common.h | 10 ++
  1 file changed, 10 insertions(+)
>>>
>>> Imo, this is guaranteed to break x86 compilation with some unusual
>>> toolchain.
>>> I looked carefully at all occurrences of AV_GCC_VERSION_AT_LEAST()
>>> and I believe they are in fact different (not for x86 or combined with 
>>> other
>>> checks).
>>
>> Can you elaborate on this? AV_GCC_VERSION_AT_LEAST() is a public macro
>> used in public headers to choose one or another path depending on if the
>> compiler is a recent enough GCC, or something else.
>
>> What toolchain could this break
>
> It definitely breaks icc compilation on Linux.

 So ICC on Linux defines __GNUC__ >= 5 yet doesn't support these builtins?
>>>
>>> No, but yes, this is the effect.
>>
>> So that means ICC is broken, defining support for features it doesn't
>> really has.
>> We can workaround that with a !defined(__INTEL_COMPILER) check, i guess,
>> but perhaps we may want to stop supporting broken compilers.
> 
> Or we add the necessary two lines of configure code necessary to avoid
> the issue.

No, a pre-processor check is more than enough. If we really care about
ICC, then Dale Curtis can add !defined(__INTEL_COMPILER) to the check,
like it's done in plenty other places for this exact same purpose.

> 
>> and why specifically x86?
>
> I mentioned x86 because afaics, all existing relevant uses of
> AV_GCC_VERSION_AT_LEAST() will not be triggered on x86.

 AV_GCC_VERSION_AT_LEAST is used in a lot of arch agnostic libavutil
 headers, and in x86/intmath.h
>>>
>>> This is exactly what I meant above with "carefully".
>>
>> It would be nice if you could stop being so terse and explain what's on
>> your mind for once. You're not being clear at all, and i asked you to
>> elaborate, something you still haven't done.
> 
> To make this crystal-clear:
> It would be nice if you could stop writing offending and annoying mails as
> you did today.

How am i writing offending emails? I asked you to elaborate on something
and barely got any information in return, then asked you to be less
terse and actually elaborate on what you were trying to argue.

> 
> The intmath.h check is surrounded by an additional check that looks much
> stricter to me than the AV_GCC_VERSION_AT_LEAST() check there.

The AV_GCC_VERSION_AT_LEAST check in x86/intmath.h is wrapped in a
__GNUC__ check and a __BMI2__ check. The former has no effect as far as
the macro is concerned since it's already a part of it, and the latter
is yet another GCC define, hence being checked after __GNUC__.
And being in the x86 folder, it's of course triggered on x86 targets.

Meanwhile, all the AV_GCC_VERSION_AT_LEAST checks in attributes.h are
triggered for every single target, with considerations for clang and ICC
where required.

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

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

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

Re: [FFmpeg-devel] [PATCH 2/2] Use gcc/clang builtins for av_sat_(add|sub)_64_c if available.

[Carl Eugen Hoyos]

Am Fr., 1. Mai 2020 um 21:58 Uhr schrieb James Almer :
>
> On 5/1/2020 4:46 PM, Carl Eugen Hoyos wrote:
> > Am Fr., 1. Mai 2020 um 21:42 Uhr schrieb James Almer :
> >>
> >> On 5/1/2020 4:37 PM, Carl Eugen Hoyos wrote:
> >>> Am Fr., 1. Mai 2020 um 21:34 Uhr schrieb James Almer :
> 
>  On 5/1/2020 4:27 PM, Carl Eugen Hoyos wrote:
> > Am Fr., 1. Mai 2020 um 20:22 Uhr schrieb James Almer 
> > :
> >>
> >> On 5/1/2020 3:07 PM, Carl Eugen Hoyos wrote:
> >>> Am Fr., 1. Mai 2020 um 19:24 Uhr schrieb Dale Curtis 
> >>> :
> 
>  Signed-off-by: Dale Curtis 
>  ---
>   libavutil/common.h | 10 ++
>   1 file changed, 10 insertions(+)
> >>>
> >>> Imo, this is guaranteed to break x86 compilation with some unusual
> >>> toolchain.
> >>> I looked carefully at all occurrences of AV_GCC_VERSION_AT_LEAST()
> >>> and I believe they are in fact different (not for x86 or combined 
> >>> with other
> >>> checks).
> >>
> >> Can you elaborate on this? AV_GCC_VERSION_AT_LEAST() is a public macro
> >> used in public headers to choose one or another path depending on if 
> >> the
> >> compiler is a recent enough GCC, or something else.
> >
> >> What toolchain could this break
> >
> > It definitely breaks icc compilation on Linux.
> 
>  So ICC on Linux defines __GNUC__ >= 5 yet doesn't support these builtins?
> >>>
> >>> No, but yes, this is the effect.
> >>
> >> So that means ICC is broken, defining support for features it doesn't
> >> really has.
> >> We can workaround that with a !defined(__INTEL_COMPILER) check, i guess,
> >> but perhaps we may want to stop supporting broken compilers.
> >
> > Or we add the necessary two lines of configure code necessary to avoid
> > the issue.
>
> No, a pre-processor check is more than enough. If we really care about
> ICC, then Dale Curtis can add !defined(__INTEL_COMPILER) to the check,
> like it's done in plenty other places for this exact same purpose.

No, it is not enough.

> >> and why specifically x86?
> >
> > I mentioned x86 because afaics, all existing relevant uses of
> > AV_GCC_VERSION_AT_LEAST() will not be triggered on x86.
> 
>  AV_GCC_VERSION_AT_LEAST is used in a lot of arch agnostic libavutil
>  headers, and in x86/intmath.h
> >>>
> >>> This is exactly what I meant above with "carefully".
> >>
> >> It would be nice if you could stop being so terse and explain what's on
> >> your mind for once. You're not being clear at all, and i asked you to
> >> elaborate, something you still haven't done.
> >
> > To make this crystal-clear:
> > It would be nice if you could stop writing offending and annoying mails as
> > you did today.
>
> How am i writing offending emails? I asked you to elaborate on something
> and barely got any information in return, then asked you to be less
> terse and actually elaborate on what you were trying to argue.

This seems untrue to me.

> > The intmath.h check is surrounded by an additional check that looks much
> > stricter to me than the AV_GCC_VERSION_AT_LEAST() check there.
>
> The AV_GCC_VERSION_AT_LEAST check in x86/intmath.h is wrapped in a
> __GNUC__ check and a __BMI2__ check. The former has no effect as far as
> the macro is concerned since it's already a part of it, and the latter
> is yet another GCC define, hence being checked after __GNUC__.
> And being in the x86 folder, it's of course triggered on x86 targets.
>
> Meanwhile, all the AV_GCC_VERSION_AT_LEAST checks in attributes.h are
> triggered for every single target, with considerations for clang and ICC
> where required.

Which case in attributes.h can cause a compilation failure?
I am curious because iirc I wrote some of them, I definitely tested them.

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/2] Use gcc/clang builtins for av_sat_(add|sub)_64_c if available.

[James Almer]

On 5/1/2020 5:01 PM, Carl Eugen Hoyos wrote:
> Am Fr., 1. Mai 2020 um 21:58 Uhr schrieb James Almer :
>>
>> On 5/1/2020 4:46 PM, Carl Eugen Hoyos wrote:
>>> Am Fr., 1. Mai 2020 um 21:42 Uhr schrieb James Almer :

 On 5/1/2020 4:37 PM, Carl Eugen Hoyos wrote:
> Am Fr., 1. Mai 2020 um 21:34 Uhr schrieb James Almer :
>>
>> On 5/1/2020 4:27 PM, Carl Eugen Hoyos wrote:
>>> Am Fr., 1. Mai 2020 um 20:22 Uhr schrieb James Almer 
>>> :

 On 5/1/2020 3:07 PM, Carl Eugen Hoyos wrote:
> Am Fr., 1. Mai 2020 um 19:24 Uhr schrieb Dale Curtis 
> :
>>
>> Signed-off-by: Dale Curtis 
>> ---
>>  libavutil/common.h | 10 ++
>>  1 file changed, 10 insertions(+)
>
> Imo, this is guaranteed to break x86 compilation with some unusual
> toolchain.
> I looked carefully at all occurrences of AV_GCC_VERSION_AT_LEAST()
> and I believe they are in fact different (not for x86 or combined 
> with other
> checks).

 Can you elaborate on this? AV_GCC_VERSION_AT_LEAST() is a public macro
 used in public headers to choose one or another path depending on if 
 the
 compiler is a recent enough GCC, or something else.
>>>
 What toolchain could this break
>>>
>>> It definitely breaks icc compilation on Linux.
>>
>> So ICC on Linux defines __GNUC__ >= 5 yet doesn't support these builtins?
>
> No, but yes, this is the effect.

 So that means ICC is broken, defining support for features it doesn't
 really has.
 We can workaround that with a !defined(__INTEL_COMPILER) check, i guess,
 but perhaps we may want to stop supporting broken compilers.
>>>
>>> Or we add the necessary two lines of configure code necessary to avoid
>>> the issue.
>>
>> No, a pre-processor check is more than enough. If we really care about
>> ICC, then Dale Curtis can add !defined(__INTEL_COMPILER) to the check,
>> like it's done in plenty other places for this exact same purpose.
> 
> No, it is not enough.

Unless you explain why, with details, I'll have to ignore you.
One-liners that don't argue anything are simply not helpful and a waste
of time.

> 
 and why specifically x86?
>>>
>>> I mentioned x86 because afaics, all existing relevant uses of
>>> AV_GCC_VERSION_AT_LEAST() will not be triggered on x86.
>>
>> AV_GCC_VERSION_AT_LEAST is used in a lot of arch agnostic libavutil
>> headers, and in x86/intmath.h
>
> This is exactly what I meant above with "carefully".

 It would be nice if you could stop being so terse and explain what's on
 your mind for once. You're not being clear at all, and i asked you to
 elaborate, something you still haven't done.
>>>
>>> To make this crystal-clear:
>>> It would be nice if you could stop writing offending and annoying mails as
>>> you did today.
>>
>> How am i writing offending emails? I asked you to elaborate on something
>> and barely got any information in return, then asked you to be less
>> terse and actually elaborate on what you were trying to argue.
> 
> This seems untrue to me.

Yet you wrote another unhelpful one-liner with no information whatsoever
in this very email up there.

> 
>>> The intmath.h check is surrounded by an additional check that looks much
>>> stricter to me than the AV_GCC_VERSION_AT_LEAST() check there.
>>
>> The AV_GCC_VERSION_AT_LEAST check in x86/intmath.h is wrapped in a
>> __GNUC__ check and a __BMI2__ check. The former has no effect as far as
>> the macro is concerned since it's already a part of it, and the latter
>> is yet another GCC define, hence being checked after __GNUC__.
>> And being in the x86 folder, it's of course triggered on x86 targets.
>>
>> Meanwhile, all the AV_GCC_VERSION_AT_LEAST checks in attributes.h are
>> triggered for every single target, with considerations for clang and ICC
>> where required.
> 
> Which case in attributes.h can cause a compilation failure?
> I am curious because iirc I wrote some of them, I definitely tested them.
> 
> 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".
> 

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

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

Re: [FFmpeg-devel] [PATCH 2/2] Use gcc/clang builtins for av_sat_(add|sub)_64_c if available.

[James Almer]

On 5/1/2020 4:50 PM, Dale Curtis wrote:
> On Fri, May 1, 2020 at 12:37 PM Carl Eugen Hoyos  wrote:
> 
>>>
>>> So ICC on Linux defines __GNUC__ >= 5 yet doesn't support these builtins?
>>
>> No, but yes, this is the effect.
>>
> 
> Does this patch work instead on the ICC compiler? GCC 4.2+ have support for
> __has_builtin() which shouldn't be masqueraded by the ICC.

Just make the check

(AV_GCC_VERSION_AT_LEAST(5,1) || defined(__clang__)) &&
!defined(__INTEL_COMPILER)

And yes, make it 5.1 since that's the first release. 5.0 was the
in-development version.

> 
> - dale
> 
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
> 

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

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

Re: [FFmpeg-devel] [PATCH 2/2] Use gcc/clang builtins for av_sat_(add|sub)_64_c if available.

[Carl Eugen Hoyos]

Am Fr., 1. Mai 2020 um 22:06 Uhr schrieb James Almer :
>
> On 5/1/2020 4:50 PM, Dale Curtis wrote:
> > On Fri, May 1, 2020 at 12:37 PM Carl Eugen Hoyos  wrote:
> >
> >>>
> >>> So ICC on Linux defines __GNUC__ >= 5 yet doesn't support these builtins?
> >>
> >> No, but yes, this is the effect.
> >>
> >
> > Does this patch work instead on the ICC compiler? GCC 4.2+ have support for
> > __has_builtin() which shouldn't be masqueraded by the ICC.
>
> Just make the check
>
> (AV_GCC_VERSION_AT_LEAST(5,1) || defined(__clang__)) &&
> !defined(__INTEL_COMPILER)

And switch the conditions.

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] [libavutil] Add saturated add/sub operations for int64_t.

[Dale Curtis]

On Fri, May 1, 2020 at 12:53 PM Michael Niedermayer 
wrote:

> On Thu, Apr 30, 2020 at 05:39:43PM -0700, Dale Curtis wrote:
> > On Thu, Apr 30, 2020 at 5:21 PM James Almer  wrote:
> >
> > > On 4/30/2020 7:19 PM, Dale Curtis wrote:
> > > > Many places are using their own custom code for handling overflow
> > > > around timestamps or other int64_t values. There are enough of these
> > > > now that having some common saturated math functions seems sound.
> > > >
> > > > This adds implementations that just use the builtin functions for
> > > > recent gcc, clang when available or implements its own version for
> > > > older compilers.
> > >
> > > These look like 64 bit versions of av_sat_add32 and av_sat_sub32, from
> > > common.h, so you should probably add them there and rename them
> > > accordingly.
> > >
> > >
> > Ah! I was looking for those, but missed them. Thanks. Done.
>
> one disadvantage the av_sat* functions have is the lack of inexact
> detection
>
> In addition to av_sat*
> In situations where its better to fail than to clip, something that
> emulates what (+-Inf/)NaN is for float may make sense.
> That would allow to simply check after a computation if any inexactness
> occured
>
> Such a thing could be usefull in situations where a exact value or an
> error is wanted.
>
>
The __builtin functions provide exactly this API, we're just hiding it. I
could add something like:
int did_overflow = av_checked_sat_(add|sub)64(int64_t a, int64_t b,
int64_t* result)

|result| would still satuate and thus av_sat_(add|sub)64 could use it
without checking the return value, but those which want to check and abort
could do so. This is similar to the API shape we expose in Chromium modulo
the fact we enforce an assert.

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

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

Re: [FFmpeg-devel] [PATCH 2/2] Use gcc/clang builtins for av_sat_(add|sub)_64_c if available.

[Carl Eugen Hoyos]

Am Fr., 1. Mai 2020 um 22:05 Uhr schrieb James Almer :

> Unless you explain why, with details, I'll have to ignore you.

https://ffmpeg.org/pipermail/ffmpeg-devel/2020-May/261801.html

This email and a follow-up from you are offending and annoying, and
it was indeed a waste of time to discuss this with you.

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/2] Use gcc/clang builtins for av_sat_(add|sub)_64_c if available.

[Dale Curtis]

On Fri, May 1, 2020 at 1:12 PM Carl Eugen Hoyos  wrote:

> Am Fr., 1. Mai 2020 um 22:06 Uhr schrieb James Almer :
> > Just make the check
> >
> > (AV_GCC_VERSION_AT_LEAST(5,1) || defined(__clang__)) &&
> > !defined(__INTEL_COMPILER)
>
> And switch the conditions.
>

Thanks. Done.

- dale
From 84a19373b4aa2bd01bdd239263c585b957a7b713 Mon Sep 17 00:00:00 2001
From: Dale Curtis 
Date: Fri, 1 May 2020 10:20:43 -0700
Subject: [PATCH] Use gcc/clang builtins for av_sat_(add|sub)_64_c if
 available.

Signed-off-by: Dale Curtis 
---
 libavutil/common.h | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/libavutil/common.h b/libavutil/common.h
index 11907e5ba7..4957842163 100644
--- a/libavutil/common.h
+++ b/libavutil/common.h
@@ -299,11 +299,16 @@ static av_always_inline int av_sat_dsub32_c(int a, int b)
  * @return sum with signed saturation
  */
 static av_always_inline int64_t av_sat_add64_c(int64_t a, int64_t b) {
+#if (!defined(__INTEL_COMPILER) && AV_GCC_VERSION_AT_LEAST(5,1)) || defined(__clang__)
+  int64_t tmp;
+  return !__builtin_add_overflow(a, b, &tmp) ? tmp : (tmp < 0 ? INT64_MAX : INT64_MIN);
+#else
   if (b >= 0 && a >= INT64_MAX - b)
 return INT64_MAX;
   if (b <= 0 && a <= INT64_MIN - b)
 return INT64_MIN;
   return a + b;
+#endif
 }
 
 /**
@@ -314,11 +319,16 @@ static av_always_inline int64_t av_sat_add64_c(int64_t a, int64_t b) {
  * @return difference with signed saturation
  */
 static av_always_inline int64_t av_sat_sub64_c(int64_t a, int64_t b) {
+#if (!defined(__INTEL_COMPILER) && AV_GCC_VERSION_AT_LEAST(5,1)) || defined(__clang__)
+  int64_t tmp;
+  return !__builtin_sub_overflow(a, b, &tmp) ? tmp : (tmp < 0 ? INT64_MAX : INT64_MIN);
+#else
   if (b <= 0 && a >= INT64_MAX + b) {
 return INT64_MAX;
   if (b >= 0 && a <= INT64_MIN + b) {
 return INT64_MIN;
   return a - b;
+#endif
 }
 
 /**
-- 
2.24.1.windows.2

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

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

Re: [FFmpeg-devel] [PATCH] avcodec/librav1e: Use the framerate when available for ratecontrol

[Lynne]

May 1, 2020, 12:31 by derek.buitenh...@gmail.com:

> Rav1e currently uses the time base given to it only for ratecontrol... where
> the inverse is taken and used as a framerate. So, do what we do in other 
> wrappers
> and use the framerate if we can.
>
> Signed-off-by: Derek Buitenhuis 
> ---
> Notably, this leaves pkt->pts still broken (it was broken before, too), but
> after discussion with James and Lynne, we decided to fix this in the rav1e API
> and bump the minimum version, instead of using a PTS queue in librav1e.c.
>

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

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

Re: [FFmpeg-devel] [PATCH 1/2] avformat/oggdec: Factor buffer reallocation out

[Lynne]

May 1, 2020, 14:16 by mich...@niedermayer.cc:

> Signed-off-by: Michael Niedermayer 
> ---
>  libavformat/oggdec.c | 25 +
>  1 file changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/libavformat/oggdec.c b/libavformat/oggdec.c
> index c591bafddd..a9034ea61c 100644
> --- a/libavformat/oggdec.c
> +++ b/libavformat/oggdec.c
> @@ -297,6 +297,20 @@ static int data_packets_seen(const struct ogg *ogg)
>  return 0;
>  }
>  
> +static int buf_realloc(struct ogg_stream *os, int size)
> +{
> +/* Even if invalid guarantee there's enough memory to read the page */
> +if (os->bufsize - os->bufpos < size) {
> +uint8_t *nb = av_realloc(os->buf, 2*os->bufsize + 
> AV_INPUT_BUFFER_PADDING_SIZE);
> +if (!nb)
> +return AVERROR(ENOMEM);
> +os->buf = nb;
> +os->bufsize *= 2;
> +}
>

Looking at the code, this is just av_fast_realloc.
You could change it to that, or you could just allocate MAX_PAGE_SIZE at the 
start and never reallocate. Its only 65k.
Up to you, feel free to commit whatever you choose.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 2/2] avformat/oggdec: Reallocate buffer before writing into it

[Lynne]

May 1, 2020, 14:16 by mich...@niedermayer.cc:

> Fixes: out of array write
> Fixes: Regression since f619e1ec66b89215582eff4404b681b760540b4f
>
> Signed-off-by: Michael Niedermayer 
> ---
>  libavformat/oggdec.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/libavformat/oggdec.c b/libavformat/oggdec.c
> index a9034ea61c..7dd48af66c 100644
> --- a/libavformat/oggdec.c
> +++ b/libavformat/oggdec.c
> @@ -441,6 +441,12 @@ static int ogg_read_page(AVFormatContext *s, int *sid, 
> int probing)
>  
>  os = ogg->streams + idx;
>  
> +ret = buf_realloc(os, size);
> +if (ret < 0) {
> +av_free(readout_buf);
> +return ret;
> +}
> +
>  memcpy(os->buf + os->bufpos, readout_buf, size);
>  av_free(readout_buf); 
>

LGTM, though if you choose to allocate 65k at the start this would be 
unnecessary.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 2/2] Use gcc/clang builtins for av_sat_(add|sub)_64_c if available.

[Carl Eugen Hoyos]

Am Fr., 1. Mai 2020 um 22:16 Uhr schrieb Dale Curtis :
>
> On Fri, May 1, 2020 at 1:12 PM Carl Eugen Hoyos  wrote:
>
> > Am Fr., 1. Mai 2020 um 22:06 Uhr schrieb James Almer :
> > > Just make the check
> > >
> > > (AV_GCC_VERSION_AT_LEAST(5,1) || defined(__clang__)) &&
> > > !defined(__INTEL_COMPILER)
> >
> > And switch the conditions.
>
> Thanks. Done.

Is there a reason why this doesn't use __has_builtin(__builtin_add_overflow)
for clang?

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] ac3enc: fix AC3 downmix metadata issue

[Michael Niedermayer]

On Thu, Apr 30, 2020 at 07:53:03PM +0200, Paul B Mahol wrote:
> LGTM

will apply

thx

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

Dictatorship naturally arises out of democracy, and the most aggravated
form of tyranny and slavery out of the most extreme liberty. -- Plato


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]lavu/threadmessage: Properly declare a function pointer

[Carl Eugen Hoyos]

Hi!

Attached patch, inspired by a patch by Andreas, fixes the following
warning when -Wpedantic is used:
CC  libavutil/threadmessage.o
libavutil/threadmessage.c: In function ‘av_thread_message_flush’:
libavutil/threadmessage.c:222:23: warning: ISO C forbids
initialization between function pointer and ‘void *’ [-Wpedantic]
  222 | void *free_func = mq->free_func;
  |   ^~


Please comment, Carl Eugen
From 3f0b6c654b7473452638c1cc06dfe45eebb59079 Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos 
Date: Fri, 1 May 2020 23:42:01 +0200
Subject: [PATCH] lavu/threadmessage: Properly declare a function pointer.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fixes a pedantic warning:
libavutil/threadmessage.c:222:23: warning: ISO C forbids initialization between function pointer and ‘void *’
---
 libavutil/threadmessage.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavutil/threadmessage.c b/libavutil/threadmessage.c
index 764b7fb813..797ba6c44c 100644
--- a/libavutil/threadmessage.c
+++ b/libavutil/threadmessage.c
@@ -219,7 +219,7 @@ void av_thread_message_flush(AVThreadMessageQueue *mq)
 {
 #if HAVE_THREADS
 int used, off;
-void *free_func = mq->free_func;
+void(*free_func)(void *) = mq->free_func;
 
 pthread_mutex_lock(&mq->lock);
 used = av_fifo_size(mq->fifo);
-- 
2.24.1

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

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

Re: [FFmpeg-devel] [PATCH 2/2] Use gcc/clang builtins for av_sat_(add|sub)_64_c if available.

[Dale Curtis]

On Fri, May 1, 2020 at 2:00 PM Carl Eugen Hoyos  wrote:

> Am Fr., 1. Mai 2020 um 22:16 Uhr schrieb Dale Curtis <
> dalecur...@chromium.org>:
> >
> > On Fri, May 1, 2020 at 1:12 PM Carl Eugen Hoyos 
> wrote:
> >
> > > Am Fr., 1. Mai 2020 um 22:06 Uhr schrieb James Almer <
> jamr...@gmail.com>:
> > > > Just make the check
> > > >
> > > > (AV_GCC_VERSION_AT_LEAST(5,1) || defined(__clang__)) &&
> > > > !defined(__INTEL_COMPILER)
> > >
> > > And switch the conditions.
> >
> > Thanks. Done.
>
> Is there a reason why this doesn't use
> __has_builtin(__builtin_add_overflow)
> for clang?
>

Yes, prior to clang 10 it didn't work properly:
https://clang.llvm.org/docs/LanguageExtensions.html#has-builtin

- dale
___
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]lavfi/signature: Fix a cast of a function pointer

[Carl Eugen Hoyos]

Hi!

Attached patch fixes an ugly warning when compiling with -Wpedantic.
I am not in favour of adding casts to silence such warnings, but there
already is an incorrect cast.

Please comment, Carl Eugen
From ed41ff40b9e1bff357b52394dcfa1107dc4ada74 Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos 
Date: Fri, 1 May 2020 23:54:24 +0200
Subject: [PATCH] lavfi/signature: Fix a cast of a funtion pointer.

Fixes the following pedantic warning:
libavfilter/vf_signature.c:294:69: warning: ISO C forbids conversion of function pointer to object pointer type
---
 libavfilter/vf_signature.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavfilter/vf_signature.c b/libavfilter/vf_signature.c
index d07b213f31..3524ce5d50 100644
--- a/libavfilter/vf_signature.c
+++ b/libavfilter/vf_signature.c
@@ -291,7 +291,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *picref)
 }
 
 /* get threshold */
-qsort(sortsignature, elemcat->elem_count, sizeof(uint64_t), (void*) cmp);
+qsort(sortsignature, elemcat->elem_count, sizeof(uint64_t), (int (*)(const void *, const void *))cmp);
 th = sortsignature[(int) (elemcat->elem_count*0.333)];
 
 /* ternarize */
@@ -317,7 +317,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *picref)
 }
 
 /* confidence */
-qsort(conflist, DIFFELEM_SIZE, sizeof(uint64_t), (void*) cmp);
+qsort(conflist, DIFFELEM_SIZE, sizeof(uint64_t), (int (*)(const void *, const void *))cmp);
 fs->confidence = FFMIN(conflist[DIFFELEM_SIZE/2], 255);
 
 /* coarsesignature */
-- 
2.24.1

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

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

Re: [FFmpeg-devel] [PATCH 0/1] avformat hls check discard state of streams always

[Steven Liu]

> 2020年5月1日 下午11:24，vectronic  写道：
> 
> After opening an HLS package with avformat_open_input() and then getting 
> stream
> info with avformat_find_stream_info() I was then setting some of the input 
> streams
> to be discarded via avStream->discard = AVDISCARD_ALL.
> 
> However subsequent calls to av_read_frame() were returning packets from the 
> streams
> which were set to be discarded.
> 
> This patch addresses this issue:
> 
> The discard state of streams within HLS read packet logic was only checking 
> the discard state when the first
> packet was read. The first packet has already been read as part of calling 
> avformat_find_stream_info.
> 
> vectronic (1):
>  avformat hls check discard state of streams always
> 
> libavformat/hls.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
> 
> -- 
> 2.24.2 (Apple Git-127)
> 
> ___
> 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

Thanks

Steven Liu



___
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 v5] avformat/url: check url root node when rel include double dot and trim double dot

[Steven Liu]

> 2020年4月29日 下午12:50，Steven Liu  写道：
> 
> fix ticket: 8625
> and add testcase into url for double dot corner case
> 
> Signed-off-by: Steven Liu 
> ---
> libavformat/tests/url.c |  5 +++
> libavformat/url.c   | 77 ++---
> tests/ref/fate/url  |  5 +++
> 3 files changed, 83 insertions(+), 4 deletions(-)
> 
> diff --git a/libavformat/tests/url.c b/libavformat/tests/url.c
> index 5e484fd428..1d961a1b43 100644
> --- a/libavformat/tests/url.c
> +++ b/libavformat/tests/url.c
> @@ -56,6 +56,7 @@ int main(void)
> test("/foo/bar", "baz");
> test("/foo/bar", "../baz");
> test("/foo/bar", "/baz");
> +test("/foo/bar", "../../../baz");
> test("http://server/foo/";, "baz");
> test("http://server/foo/bar";, "baz");
> test("http://server/foo/";, "../baz");
> @@ -65,6 +66,10 @@ int main(void)
> test("http://server/foo/bar?param=value/with/slashes";, "/baz");
> test("http://server/foo/bar?param&otherparam";, "?someparam");
> test("http://server/foo/bar";, "//other/url");
> +test("http://server/foo/bar";, "../../../../../other/url");
> +test("http://server/foo/bar";, "/../../../../../other/url");
> +test("http://server/foo/bar";, "/test/../../../../../other/url");
> +test("http://server/foo/bar";, "/test/../../test/../../../other/url");
> 
> printf("\nTesting av_url_split:\n");
> test2("/foo/bar");
> diff --git a/libavformat/url.c b/libavformat/url.c
> index 596fb49cfc..7cd9e0c705 100644
> --- a/libavformat/url.c
> +++ b/libavformat/url.c
> @@ -21,6 +21,7 @@
> 
> 
> #include "avformat.h"
> +#include "internal.h"
> #include "config.h"
> #include "url.h"
> #if CONFIG_NETWORK
> @@ -77,10 +78,53 @@ int ff_url_join(char *str, int size, const char *proto,
> return strlen(str);
> }
> 
> +static void trim_double_dot_url(char *buf, const char *rel, int size)
> +{
> +const char *p = rel;
> +const char *root = rel;
> +char tmp_path[MAX_URL_SIZE] = {0, };
> +char *sep;
> +char *node;
> +
> +/* Get the path root of the url which start by "://" */
> +if (p && (sep = strstr(p, "://"))) {
> +sep += 3;
> +root = strchr(sep, '/');
> +}
> +
> +/* set new current position if the root node is changed */
> +p = root;
> +while (p && (node = strstr(p, ".."))) {
> +av_strlcat(tmp_path, p, node - p + strlen(tmp_path));
> +p = node + 3;
> +sep = strrchr(tmp_path, '/');
> +if (sep)
> +sep[0] = '\0';
> +else
> +tmp_path[0] = '\0';
> +}
> +
> +if (!av_stristart(p, "/", NULL) && root != rel)
> +av_strlcat(tmp_path, "/", size);
> +
> +av_strlcat(tmp_path, p, size);
> +/* start set buf after temp path process. */
> +av_strlcpy(buf, rel, root - rel + 1);
> +
> +if (!av_stristart(tmp_path, "/", NULL) && root != rel)
> +av_strlcat(buf, "/", size);
> +
> +av_strlcat(buf, tmp_path, size);
> +}
> +
> void ff_make_absolute_url(char *buf, int size, const char *base,
>   const char *rel)
> {
> char *sep, *path_query;
> +char *root, *p;
> +char tmp_path[MAX_URL_SIZE];
> +
> +memset(tmp_path, 0, sizeof(tmp_path));
> /* Absolute path, relative to the current server */
> if (base && strstr(base, "://") && rel[0] == '/') {
> if (base != buf)
> @@ -99,11 +143,14 @@ void ff_make_absolute_url(char *buf, int size, const 
> char *base,
> }
> }
> av_strlcat(buf, rel, size);
> +trim_double_dot_url(tmp_path, buf, size);
> +memset(buf, 0, size);
> +av_strlcpy(buf, tmp_path, size);
> return;
> }
> /* If rel actually is an absolute url, just copy it */
> if (!base || strstr(rel, "://") || rel[0] == '/') {
> -av_strlcpy(buf, rel, size);
> +trim_double_dot_url(buf, rel, size);
> return;
> }
> if (base != buf)
> @@ -117,19 +164,38 @@ void ff_make_absolute_url(char *buf, int size, const 
> char *base,
> /* Is relative path just a new query part? */
> if (rel[0] == '?') {
> av_strlcat(buf, rel, size);
> +trim_double_dot_url(tmp_path, buf, size);
> +memset(buf, 0, size);
> +av_strlcpy(buf, tmp_path, size);
> return;
> }
> 
> +root = p = buf;
> +/* Get the path root of the url which start by "://" */
> +if (p && strstr(p, "://")) {
> +sep = strstr(p, "://");
> +if (sep) {
> +sep += 3;
> +root = strchr(sep, '/');
> +}
> +}
> +
> /* Remove the file name from the base url */
> sep = strrchr(buf, '/');
> +if (sep <= root)
> +sep = root;
> +
> if (sep)
> sep[1] = '\0';
> else
> buf[0] = '\0';
> -while (av_strstart(rel, "../", NULL) && sep) {
> +while (av_strstart(rel, "..", NULL) && sep) {
> /* Remove the path delimiter at the end */
> -sep[0] = '\0';
> -sep = strrchr(buf,

Re: [FFmpeg-devel] [PATCH v4 7/9] lavc/libopenh264enc: add profile high option support

[Fu, Linjie]

> From: Martin Storsjö 
> Sent: Tuesday, April 28, 2020 04:09
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Cc: Fu, Linjie 
> Subject: Re: [FFmpeg-devel] [PATCH v4 7/9] lavc/libopenh264enc: add profile
> high option support
> 
> On Wed, 15 Apr 2020, Linjie Fu wrote:
> 
> > Add support for PRO_HIGH/PRO_BASELINE in SVC Encoding extention
> mode,
> > which determined by iEntropyCodingModeFlag in ParamTranscode().
> >
> >
>  param_svc.h#L394>
> >
> > Signed-off-by: Linjie Fu 
> > ---
> > libavcodec/libopenh264enc.c | 32 ++--
> > 1 file changed, 26 insertions(+), 6 deletions(-)
> >
> > diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
> > index a7c389e..0fe8cf4 100644
> > --- a/libavcodec/libopenh264enc.c
> > +++ b/libavcodec/libopenh264enc.c
> > @@ -42,7 +42,7 @@ typedef struct SVCContext {
> > ISVCEncoder *encoder;
> > int slice_mode;
> > int loopfilter;
> > -char *profile;
> > +int profile;
> > int max_nal_size;
> > int skip_frames;
> > int skipped;
> > @@ -73,7 +73,11 @@ static const AVOption options[] = {
> > #endif
> > #endif
> > { "loopfilter", "enable loop filter", OFFSET(loopfilter), 
> > AV_OPT_TYPE_INT,
> { .i64 = 1 }, 0, 1, VE },
> > -{ "profile", "set profile restrictions", OFFSET(profile),
> AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, VE },
> > +{ "profile", "Set profile restrictions", OFFSET(profile), 
> > AV_OPT_TYPE_INT,
> { .i64 = FF_PROFILE_UNKNOWN }, FF_PROFILE_UNKNOWN, 0x, VE,
> "profile" },
> > +#define PROFILE(name, value)  name, NULL, 0, AV_OPT_TYPE_CONST,
> { .i64 = value }, 0, 0, VE, "profile"
> > +{ PROFILE("constrained_baseline",
> FF_PROFILE_H264_CONSTRAINED_BASELINE) },
> > +{ PROFILE("high", FF_PROFILE_H264_HIGH) },
> > +#undef PROFILE
> > { "max_nal_size", "set maximum NAL size in bytes",
> OFFSET(max_nal_size), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, VE },
> > { "allow_skip_frames", "allow skipping frames to hit the target 
> > bitrate",
> OFFSET(skip_frames), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE },
> > { "cabac", "Enable cabac", OFFSET(cabac), AV_OPT_TYPE_INT, { .i64 = 0 },
> 0, 1, VE },
> > @@ -109,12 +113,28 @@ static av_cold int
> svc_encode_init_profile(AVCodecContext *avctx, SEncParamExt *
> > {
> > SVCContext *s = avctx->priv_data;
> >
> > -if (s->profile && !strcmp(s->profile, "main"))
> > -param->iEntropyCodingModeFlag = 1; //< 0:CAVLC  1:CABAC
> > -else if (!s->profile && s->cabac)
> > +if (s->profile == FF_PROFILE_UNKNOWN)
> > +s->profile = s->cabac ? FF_PROFILE_H264_HIGH :
> > +FF_PROFILE_H264_CONSTRAINED_BASELINE;
> > +
> > +switch (s->profile) {
> > +case FF_PROFILE_H264_HIGH:
> > param->iEntropyCodingModeFlag = 1;
> > -else
> > +av_log(avctx, AV_LOG_VERBOSE, "Using CABAC, "
> > +   "select EProfileIdc PRO_HIGH in libopenh264.\n");
> > +break;
> > +case FF_PROFILE_H264_CONSTRAINED_BASELINE:
> > +case FF_PROFILE_UNKNOWN:
> > +param->iEntropyCodingModeFlag = 0;
> > +av_log(avctx, AV_LOG_VERBOSE, "Using CAVLC, "
> > +   "select EProfileIdc PRO_BASELINE in libopenh264.\n");
> > +break;
> > +default:
> > param->iEntropyCodingModeFlag = 0;
> > +av_log(avctx, AV_LOG_WARNING, "Unsupported profile, "
> > +   "select EProfileIdc PRO_BASELINE in libopenh264.\n");
> > +break;
> > +}
> >
> > return 0;
> > }
> > --
> > 2.7.4
> 
> This is fine I guess - apparently it changed in OpenH264 1.8, before that,
> setting iEntropyCodingModeFlag = 1 got main profile, not high.

Yes, main profile was somehow removed in [1] v1.7, while high profile
was introduced in [2] v1.8. 

> The commit message feels a bit misleading for what the commit really does
> though. The commit message says "add ... support", which sounds like it
> would be a pure addition, while it is a bit of a rewrite, and actually
> removes support for "-profile main" (even though it might have given
> profile high with current OpenH264 versions).

> Maybe this would be clearer:
> 
> libopenh264enc: Rewrite profile handling
> 
> Support the profiles "constrained_baseline" and "high". Previously, the
> encoder wrapper supported the parameter value "main", but with recent
> versions of OpenH264, the produced bitstream signaled the profile high
> anyway.
> ---
> 
> That's at least clear with what the change actually does.
Yes, this would be more clear and reasonable, will update like this.

Actually, main profile currently seems to be available in ParamBaseTranscode() 
[3]
For SEncParamBase (did't try yet). IMHO it'll be better to add the PRO_MAIN 
support
back later.

[1] 
https://github.com/cisco/openh264/commit/f09c85f45d52c91234482b97b34fe01dadd002c7
[2] 
https://github.com/cisco/o

Re: [FFmpeg-devel] [PATCH 1/3] closed caption decoder: accept and decode a new codec type of 'raw 608 byte pairs'

[Roger Pack]

On Fri, May 1, 2020 at 12:22 AM Kieran Kunhya  wrote:
>
> On Fri, 1 May 2020 at 04:59, Roger Pack  wrote:
>
> > On Thu, Apr 30, 2020 at 4:30 AM Kieran Kunhya  wrote:
> > >
> > > On Thu, 30 Apr 2020 at 07:22, Roger Pack  wrote:
> > >
> > > > > > c9153590e5f167e41910d867639eb887164e28d2
> > > > 0001-closed-caption-decoder-accept-and-decode-a-new-codec.patch
> > > > > > From bf29fe5330e83e37cf064b18918185c6b00d9b9f Mon Sep 17 00:00:00
> > 2001
> > > > > > From: rogerdpack 
> > > > > > Date: Tue, 28 Apr 2020 05:15:15 +
> > > > > > Subject: [PATCH 1/3] closed caption decoder: accept and decode a
> > new
> > > > codec
> > > > > >  type of 'raw 608 byte pairs'
> > > >
> > >
> > > How does this content exist? Are you defining your own file format with
> > the
> > > byte pairs?
> >
> > It's transmitted as "raw byte pairs" by analog TV broadcast in the US.
> > https://en.wikipedia.org/wiki/EIA-608 has  details.
> >
> > I believe it transmits  one closed caption character (or control code)
> > per frame (basically a single byte pair on "line 21") so it can do 30
> > closed caption chars/second.  Anyway that's where it's originating
> > from.  Good question :)
> >
>
> Yes, but these byte pairs are usually in a container or exist in an
> analogue waveform.

It's from analogue waveform, it gets pulled out by the analog TV
decoder card into the byte pairs.
The container is basically "which frame" the line 21 data happened to
come in with.  It comes in with two bytes/frame.
It comes in with a timestamp given by (I presume) the timestamp of the
frame it accompanies as a pin in directshow (in windows).

Directshow with the tuner card presents the line 21 data as "VBI
packets" (with a header specifying which portion of the VBI it comes
from, line 21 "odd" or line 21 "even"), and a timestamp.

There is another directshow filter that basically filters out the VBI
data and passes on just the "raw byte pairs" for a given CC stream.
See attached, but only concentrate on the "VBI Codec" filter box.  It
presents  two "CC" pins, the first of which we accept the byte pairs
from (with patch 3/3) [1]

So it's an analog standard, which directshow handles this way, and
it's converted to byte pairs.

> Have you defined your own format of "raw byte pairs"? That doesn't exist in
> the wild as far as I know.

The "format" is normal EIA 608
https://en.wikipedia.org/wiki/EIA-608
The "characters" section.
There was already a codec named "AV_CODEC_ID_EIA_608", but it was
really EIA 608 encapsulated within an EIA 708 stream, so I went with a
new name.  The existing 608 decoder already decoded the "byte pairs"
internally I just needed to be able to access it without the
encapsulation for this particular input.
In the analog stream they "come in" on the line 21 data as EIA byte
pairs.  No 708 encapsulation, hence me calling it "raw"
(AV_CODEC_ID_EIA_608_RAW_BYTE_PAIRS).
So it seems standard.

Cheers.

-roger-

[1] If you care about the nitty gritty, here's the media types that
the pins present:

VBI pin:
Media type: MEDIATYPE_VBI {F72A76E1-EB0A-11D0-ACE4-C0CC16BA}
Subtype: KSDATAFORMAT_SUBTYPE_RAW8 {CA20D9A0-3E3E-11D1-9BF9-00C04FBBDEBF}
Format type: {F72A76E0-EB0A-11D0-ACE4-C0CC16BA}

CC pin:
Media type: MEDIATYPE_AUXLine21Data {670AEA80-3A82-11D0-B79B-00AA003767A7}
Subtype: MEDIASUBTYPE_Line21_BytePair {6E8D4A22-310C-11D0-B79A-00AA003767A7}
Format type: FORMAT_None {0F6417D6-C318-11D0-A43F-00A0C9223196}
___
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 v4 8/9] lavc/libopenh264enc: allow specifying the profile through AVCodecContext

[Fu, Linjie]

> From: ffmpeg-devel  On Behalf Of
> Martin Storsjö
> Sent: Tuesday, April 28, 2020 04:13
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Cc: Fu, Linjie 
> Subject: Re: [FFmpeg-devel] [PATCH v4 8/9] lavc/libopenh264enc: allow
> specifying the profile through AVCodecContext
> 
> On Wed, 15 Apr 2020, Linjie Fu wrote:
> 
> > Signed-off-by: Linjie Fu 
> > ---
> > libavcodec/libopenh264enc.c | 16 
> > 1 file changed, 16 insertions(+)
> >
> > diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
> > index 0fe8cf4..4d337d2 100644
> > --- a/libavcodec/libopenh264enc.c
> > +++ b/libavcodec/libopenh264enc.c
> > @@ -113,6 +113,22 @@ static av_cold int
> svc_encode_init_profile(AVCodecContext *avctx, SEncParamExt *
> > {
> > SVCContext *s = avctx->priv_data;
> >
> > +/* Allow specifying the libopenh264 profile through AVCodecContext.
> */
> > +if (FF_PROFILE_UNKNOWN == s->profile &&
> > +FF_PROFILE_UNKNOWN != avctx->profile)
> > +switch (avctx->profile) {
> > +case FF_PROFILE_H264_CONSTRAINED_BASELINE:
> > +s->profile = FF_PROFILE_H264_CONSTRAINED_BASELINE;
> > +break;
> > +case FF_PROFILE_H264_HIGH:
> > +s->profile = FF_PROFILE_H264_HIGH;
> > +break;
> > +default:
> > +av_log(avctx, AV_LOG_WARNING,
> > +   "Unsupported avctx->profile: %d.\n", avctx->profile);
> > +break;
> > +}
> > +
> > if (s->profile == FF_PROFILE_UNKNOWN)
> > s->profile = s->cabac ? FF_PROFILE_H264_HIGH :
> > FF_PROFILE_H264_CONSTRAINED_BASELINE;
> 
> With this in place, why even do the previous commit, why not just go for
> only using avctx->profile? Although as far as I can see, that field
> doesn't seem to have string options for sepcifying H264 profiles, so it
> can only be set via code or by specifying a numberic value - is that
> right?

Yes, code/numberic value works in this way, however maybe not straight
forward enough for user.

> Wouldn't it just be best to add the H264 profiles to options_table.h to
> keep allowing it to be set via a string, but remove the internal
> s->profile field here, as patch 7/9 already breaks handling of the profile
> field by stopping recognizing "main" and only recognizing "high" anyway.

Most sw codecs like libx264/libx265 and hw codecs like h264_vaapi/h264_nvenc
have the logic of :
1. derive the settings from avctx->profile which is determined in 
options_table.h;
2. If not, check the private option for specific encoder;

And specifically for libopenh264enc,  we allow one more logic:
3. determine the profile by s->cabac;

I admit it'll be good if settled this down to parse all possible profiles for 
each codec
(maybe according to libavcodec/profiles.c), hence we may remove 
similar/redundant
logics in specific encoder(x264/h264_vaapi/h264_nvenc).

Please help to comment whether it makes sense for you all, since this would 
impact
many codecs.

- Linjie



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

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

Re: [FFmpeg-devel] [PATCH] avfilter/af_loudnorm: Don't mix dB and linear values when calculating linear offset with inputs <3s

[Kyle Swanson]

Hi,

On Tue, Apr 28, 2020 at 12:58 AM Paul B Mahol  wrote:
>
> On 4/26/20, Sebastian Dröge  wrote:
> > From: Sebastian Dröge 
> >
> > s->target_i and global are in dB but s->target_tp and true_peak are
> > linear. Instead of mixing these in the calculations, convert the former
> > first to have all following calculations in the same unit.
> > ---
> >  libavfilter/af_loudnorm.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> >
>
> LGTM, but better ask and CC maintainer directly.

Tested and pushed. Thank you.

Thanks,
Kyle
___
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".