[FFmpeg-devel] [PATCH v4 2/2] avformat/rtmpproto: support enhanced rtmp

2023-08-27 Thread Steven Liu
add option named rtmp_enhanced_codec,
it would support hvc1,av01,vp09 now,
the fourcc is using Array of strings.

Signed-off-by: Steven Liu 
---
 doc/protocols.texi  | 11 +++
 libavformat/rtmpproto.c | 34 ++
 2 files changed, 45 insertions(+)

diff --git a/doc/protocols.texi b/doc/protocols.texi
index b3fad55591..c7637c11c9 100644
--- a/doc/protocols.texi
+++ b/doc/protocols.texi
@@ -896,6 +896,17 @@ be named, by prefixing the type with 'N' and specifying 
the name before
 the value (i.e. @code{NB:myFlag:1}). This option may be used multiple
 times to construct arbitrary AMF sequences.
 
+@item rtmp_enhanced_codecs
+Specify the list of codecs the client advertises to support in an
+enhanced RTMP stream. This option should be set to a comma separated
+list of fourcc values, like @code{hvc1,av01,vp09} for multiple codecs
+or @code{hvc1} for only one codec. The specified list will be presented
+in the "fourCcLive" property of the Connect Command Message.
+
+This option should set a string like @code{hvc1,av01,vp09}
+for multiple codecs, or @code{hvc1} for only one codec,
+set codec fourcc into fourCcLive property into Connect Command Message,
+
 @item rtmp_flashver
 Version of the Flash plugin used to run the SWF player. The default
 is LNX 9,0,124,2. (When publishing, the default is FMLE/3.0 (compatible;
diff --git a/libavformat/rtmpproto.c b/libavformat/rtmpproto.c
index f0ef223f05..98718bc6da 100644
--- a/libavformat/rtmpproto.c
+++ b/libavformat/rtmpproto.c
@@ -127,6 +127,7 @@ typedef struct RTMPContext {
 int   nb_streamid;///< The next stream id to 
return on createStream calls
 doubleduration;   ///< Duration of the stream in 
seconds as returned by the server (only valid if non-zero)
 int   tcp_nodelay;///< Use TCP_NODELAY to disable 
Nagle's algorithm if set to 1
+char  *enhanced_codecs;   ///< codec list in enhanced rtmp
 char  username[50];
 char  password[50];
 char  auth_params[500];
@@ -336,6 +337,38 @@ static int gen_connect(URLContext *s, RTMPContext *rt)
 ff_amf_write_field_name(&p, "app");
 ff_amf_write_string2(&p, rt->app, rt->auth_params);
 
+if (rt->enhanced_codecs) {
+uint32_t list_len = 0;
+char *fourcc_data = rt->enhanced_codecs;
+int fourcc_str_len = strlen(fourcc_data);
+
+// check the string, fourcc + ',' + ...  + end fourcc correct length 
should be (4+1)*n+4
+if ((fourcc_str_len + 1) % 5 != 0) {
+av_log(s, AV_LOG_ERROR, "Malformed rtmp_enhanched_codecs, "
+   "should be of the form hvc1[,av01][,vp09][,...]\n");
+return AVERROR(EINVAL);
+}
+
+list_len = (fourcc_str_len + 1) / 5;
+ff_amf_write_field_name(&p, "fourCcList");
+ff_amf_write_array_start(&p, list_len);
+
+while(fourcc_data - rt->enhanced_codecs < fourcc_str_len) {
+unsigned char fourcc[5];
+if (!strncmp(fourcc_data, "hvc1", 4) ||
+!strncmp(fourcc_data, "av01", 4) ||
+!strncmp(fourcc_data, "vp09", 4)) {
+av_strlcpy(fourcc, fourcc_data, sizeof(fourcc));
+ff_amf_write_string(&p, fourcc);
+} else {
+av_log(s, AV_LOG_ERROR, "Unsupported codec fourcc, 
%.*s\n", 4, fourcc_data);
+return AVERROR_PATCHWELCOME;
+}
+
+fourcc_data += 5;
+}
+}
+
 if (!rt->is_input) {
 ff_amf_write_field_name(&p, "type");
 ff_amf_write_string(&p, "nonprivate");
@@ -3104,6 +3137,7 @@ static const AVOption rtmp_options[] = {
 {"rtmp_conn", "Append arbitrary AMF data to the Connect message", 
OFFSET(conn), AV_OPT_TYPE_STRING, {.str = NULL }, 0, 0, DEC|ENC},
 {"rtmp_flashver", "Version of the Flash plugin used to run the SWF 
player.", OFFSET(flashver), AV_OPT_TYPE_STRING, {.str = NULL }, 0, 0, DEC|ENC},
 {"rtmp_flush_interval", "Number of packets flushed in the same request 
(RTMPT only).", OFFSET(flush_interval), AV_OPT_TYPE_INT, {.i64 = 10}, 0, 
INT_MAX, ENC},
+{"rtmp_enhanced_codecs", "Specify the codec(s) to use in an enhanced rtmp 
live stream", OFFSET(enhanced_codecs), AV_OPT_TYPE_STRING, {.str = NULL }, 0, 
0, ENC},
 {"rtmp_live", "Specify that the media is a live stream.", OFFSET(live), 
AV_OPT_TYPE_INT, {.i64 = -2}, INT_MIN, INT_MAX, DEC, "rtmp_live"},
 {"any", "both", 0, AV_OPT_TYPE_CONST, {.i64 = -2}, 0, 0, DEC, "rtmp_live"},
 {"live", "live stream", 0, AV_OPT_TYPE_CONST, {.i64 = -1}, 0, 0, DEC, 
"rtmp_live"},
-- 
2.40.0

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

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


[FFmpeg-devel] [PATCH v4 1/2] avformat/rtmppkt: add ff_amf_write_array for write

2023-08-27 Thread Steven Liu
Signed-off-by: Steven Liu 
---
 libavformat/rtmppkt.c | 6 ++
 libavformat/rtmppkt.h | 8 
 2 files changed, 14 insertions(+)

diff --git a/libavformat/rtmppkt.c b/libavformat/rtmppkt.c
index 4b97c0833f..a602bf6a96 100644
--- a/libavformat/rtmppkt.c
+++ b/libavformat/rtmppkt.c
@@ -40,6 +40,12 @@ void ff_amf_write_number(uint8_t **dst, double val)
 bytestream_put_be64(dst, av_double2int(val));
 }
 
+void ff_amf_write_array_start(uint8_t **dst, uint32_t length)
+{
+bytestream_put_byte(dst, AMF_DATA_TYPE_ARRAY);
+bytestream_put_be32(dst, length);
+}
+
 void ff_amf_write_string(uint8_t **dst, const char *str)
 {
 bytestream_put_byte(dst, AMF_DATA_TYPE_STRING);
diff --git a/libavformat/rtmppkt.h b/libavformat/rtmppkt.h
index a15d2a5773..7c580f2224 100644
--- a/libavformat/rtmppkt.h
+++ b/libavformat/rtmppkt.h
@@ -244,6 +244,14 @@ void ff_amf_write_null(uint8_t **dst);
  */
 void ff_amf_write_object_start(uint8_t **dst);
 
+/**
+ * Write marker and length for AMF array to buffer.
+ *
+ * @param dst pointer to the input buffer (will be modified)
+ * @param length value to write
+ */
+void ff_amf_write_array_start(uint8_t **dst, uint32_t length);
+
 /**
  * Write string used as field name in AMF object to buffer.
  *
-- 
2.40.0

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

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


Re: [FFmpeg-devel] [PATCH 2/2] avcodec/apedec: Implement interim mode detection

2023-08-27 Thread Michael Niedermayer
On Sat, Aug 26, 2023 at 06:53:50PM +0200, Michael Niedermayer wrote:
> Fixes: NoLegacy.ape
> Found-by: Matt Ashland 
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/apedec.c | 106 +++-
>  1 file changed, 84 insertions(+), 22 deletions(-)

Also, the same issue should exists in the mono codepath. I have
no testcase for mono, so left this unchanged for now, as i cannot
test it.
But if someone has a testcase, tell me

It also may be needed to wait till one of the CRCs to distinguish
between the 2 decoding forms. (this is how the SDK does it)

Waiting for some CRC is a bit inconvenient as we return data before
currently. And this quicker variant here works for all files i have

We also differ from the SDK by decoding both forms in parallel until
one fails. The advantage here is that if only one is decoded then
a random failure cannot be distinguished from "interim mode" which
would result in a bitstream error causing a switch to the wrong
mode.

Above is how its done ATM in git master. As more issues get submitted
that may change

thx

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

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire


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

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


Re: [FFmpeg-devel] [PATCH 2/2] avcodec/apedec: Implement interim mode detection

2023-08-27 Thread Michael Niedermayer
On Sat, Aug 26, 2023 at 08:07:14PM +0200, Michael Niedermayer wrote:
> On Sat, Aug 26, 2023 at 07:55:10PM +0200, Anton Khirnov wrote:
> > Quoting Michael Niedermayer (2023-08-26 18:53:50)
> > > Fixes: NoLegacy.ape
> > 
> > Can you make a FATE test?
> 
> The file seems 34mb, ill try to cut it down to something smaller

ive been able to cut it down to 100kb
tested on mips/arm/mingw32/64 and linux32/64 x86

test seems trivial, so ill apply it with my next push, test is below
for reference

commit 2c1ade30ad8f523d816a853f90512808b0c039e1 (HEAD -> master)
Author: Michael Niedermayer 
Date:   Sat Aug 26 20:21:32 2023 +0200

tests/fate: Add NoLegacy-cut.ape test

Signed-off-by: Michael Niedermayer 

diff --git a/tests/fate/monkeysaudio.mak b/tests/fate/monkeysaudio.mak
index d75937ec02..03c646cd47 100644
--- a/tests/fate/monkeysaudio.mak
+++ b/tests/fate/monkeysaudio.mak
@@ -16,5 +16,8 @@ $(foreach N,$(APE_VERSIONS),$(eval $(call 
FATE_APE_SUITE,$(N
 FATE_APE += fate-lossless-monkeysaudio-399
 fate-lossless-monkeysaudio-399: CMD = md5 -i 
$(TARGET_SAMPLES)/lossless-audio/luckynight-partial.ape -f s16le -af aresample

+FATE_APE += fate-lossless-monkeysaudio-legacy
+fate-lossless-monkeysaudio-legacy: CMD = md5 -i 
$(TARGET_SAMPLES)/lossless-audio/NoLegacy-cut.ape -f s32le -af aresample
+
 FATE_SAMPLES_AVCONV-$(call DEMDEC, APE, APE) += $(FATE_APE)
 fate-lossless-monkeysaudio: $(FATE_APE)
diff --git a/tests/ref/fate/lossless-monkeysaudio-legacy 
b/tests/ref/fate/lossless-monkeysaudio-legacy
new file mode 100644
index 00..33d4b56204
--- /dev/null
+++ b/tests/ref/fate/lossless-monkeysaudio-legacy
@@ -0,0 +1 @@
+2cd8a60478c77382dfed8b8bdf1f70e4


[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The day soldiers stop bringing you their problems is the day you have stopped 
leading them. They have either lost confidence that you can help or concluded 
you do not care. Either case is a failure of leadership. - Colin Powell


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] avcodec/apedec: Implement interim mode detection

2023-08-27 Thread Michael Niedermayer
On Sat, Aug 26, 2023 at 07:29:40PM +0200, Paul B Mahol wrote:
> On Sat, Aug 26, 2023 at 6:54 PM Michael Niedermayer 
> wrote:
> 
> > Fixes: NoLegacy.ape
> > Found-by: Matt Ashland 
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavcodec/apedec.c | 106 +++-
> >  1 file changed, 84 insertions(+), 22 deletions(-)
> >
> > diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c
> > index 8bd625ca05..249fc22e24 100644
> > --- a/libavcodec/apedec.c
> > +++ b/libavcodec/apedec.c
> > @@ -171,6 +171,9 @@ typedef struct APEContext {
> >  int32_t *decoded_buffer;
> >  int decoded_size;
> >  int32_t *decoded[MAX_CHANNELS];  ///< decoded data for each
> > channel
> > +int32_t *interim_buffer;
> > +int interim_size;
> > +int32_t *interim[MAX_CHANNELS];  ///< decoded data for each
> > channel
> >  int blocks_per_loop; ///< maximum number of
> > samples to decode for each call
> >
> >  int16_t* filterbuf[APE_FILTER_LEVELS];   ///< filter memory
> > @@ -187,6 +190,7 @@ typedef struct APEContext {
> >  const uint8_t *ptr;  ///< current position in
> > frame data
> >
> >  int error;
> > +int interim_mode;
> >
> >  void (*entropy_decode_mono)(struct APEContext *ctx, int
> > blockstodecode);
> >  void (*entropy_decode_stereo)(struct APEContext *ctx, int
> > blockstodecode);
> > @@ -223,6 +227,7 @@ static av_cold int ape_decode_close(AVCodecContext
> > *avctx)
> >  av_freep(&s->filterbuf[i]);
> >
> >  av_freep(&s->decoded_buffer);
> > +av_freep(&s->interim_buffer);
> >  av_freep(&s->data);
> >  s->decoded_size = s->data_size = 0;
> >
> > @@ -248,12 +253,15 @@ static av_cold int ape_decode_init(AVCodecContext
> > *avctx)
> >  switch (s->bps) {
> >  case 8:
> >  avctx->sample_fmt = AV_SAMPLE_FMT_U8P;
> > +s->interim_mode = 0;
> >  break;
> >  case 16:
> >  avctx->sample_fmt = AV_SAMPLE_FMT_S16P;
> > +s->interim_mode = 0;
> >  break;
> >  case 24:
> >  avctx->sample_fmt = AV_SAMPLE_FMT_S32P;
> > +s->interim_mode = -1;
> >  break;
> >  default:
> >  avpriv_request_sample(avctx,
> > @@ -1181,7 +1189,7 @@ static av_always_inline int
> > predictor_update_filter(APEPredictor64 *p,
> >  const int decoded,
> > const int filter,
> >  const int delayA,
> > const int delayB,
> >  const int adaptA,
> > const int adaptB,
> > -int compression_level)
> > +int interim_mode)
> >  {
> >  int64_t predictionA, predictionB;
> >  int32_t sign;
> > @@ -1209,7 +1217,7 @@ static av_always_inline int
> > predictor_update_filter(APEPredictor64 *p,
> >p->buf[delayB - 3] * p->coeffsB[filter][3] +
> >p->buf[delayB - 4] * p->coeffsB[filter][4];
> >
> > -if (compression_level < COMPRESSION_LEVEL_INSANE) {
> > +if (interim_mode < 1) {
> >  predictionA = (int32_t)predictionA;
> >  predictionB = (int32_t)predictionB;
> >  p->lastA[filter] = decoded + ((int32_t)(predictionA +
> > (predictionB >> 1)) >> 10);
> > @@ -1234,33 +1242,74 @@ static av_always_inline int
> > predictor_update_filter(APEPredictor64 *p,
> >
> >  static void predictor_decode_stereo_3950(APEContext *ctx, int count)
> >  {
> > -APEPredictor64 *p = &ctx->predictor64;
> > -int32_t *decoded0 = ctx->decoded[0];
> > -int32_t *decoded1 = ctx->decoded[1];
> > +APEPredictor64 *p_default = &ctx->predictor64;
> > +APEPredictor64 p_interim;
> > +int lcount = count;
> > +int num_passes = 1;
> >
> >  ape_apply_filters(ctx, ctx->decoded[0], ctx->decoded[1], count);
> > +if (ctx->interim_mode == -1) {
> > +p_interim = *p_default;
> > +num_passes ++;
> > +memcpy(ctx->interim[0], ctx->decoded[0],
> > sizeof(*ctx->interim[0])*count);
> > +memcpy(ctx->interim[1], ctx->decoded[1],
> > sizeof(*ctx->interim[1])*count);
> > +}
> >
> > -while (count--) {
> > -/* Predictor Y */
> > -*decoded0 = predictor_update_filter(p, *decoded0, 0, YDELAYA,
> > YDELAYB,
> > -YADAPTCOEFFSA, YADAPTCOEFFSB,
> > -ctx->compression_level);
> > -decoded0++;
> > -*decoded1 = predictor_update_filter(p, *decoded1, 1, XDELAYA,
> > XDELAYB,
> > -XADAPTCOEFFSA, XADAPTCOEFFSB,
> > -ctx->compression_level);
> > -decoded1++;
> > +for(int pass = 0; pass < num_passes; pass++) {
> >
> 
> Please fix your style in patches.

Noone can know from this what exact

[FFmpeg-devel] [PATCH] avfilter/vf_pad: respect frame colorspace tag in RGB to YUV conversion

2023-08-27 Thread Leo Izen
vf_pad calls ff_draw_init, which assumes BT.709 and TV range for its
YUV matricies. Since the filter only accepts RGB inputs for the color
argument, it needs to convert them to YUV for YUV input video, and it
should respect the tagged colormatrix when doing such a conversion, but
it does not. It can do this by calling ff_draw_init2, and this patch
causes the filter to re-init when the first frame is received, as that
is when that colormatrix tag becomes available.

If the filter is not initialized before the first frame, then an
assertion will fail in avfilter.c when it does sanity checks on
input/output dimensions, so the original initialization cannot be
skipped.

Signed-off-by: Leo Izen 
---
 libavfilter/vf_pad.c | 21 -
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/libavfilter/vf_pad.c b/libavfilter/vf_pad.c
index e52f7284d4..01de896772 100644
--- a/libavfilter/vf_pad.c
+++ b/libavfilter/vf_pad.c
@@ -100,9 +100,10 @@ typedef struct PadContext {
 FFDrawColor color;
 
 int eval_mode;  ///< expression evaluation mode
+int configured_input;   ///< if config_input has been called yet
 } PadContext;
 
-static int config_input(AVFilterLink *inlink)
+static int config_frame_input(AVFilterLink *inlink, const AVFrame *frame)
 {
 AVFilterContext *ctx = inlink->dst;
 PadContext *s = ctx->priv;
@@ -111,7 +112,10 @@ static int config_input(AVFilterLink *inlink)
 double var_values[VARS_NB], res;
 char *expr;
 
-ff_draw_init(&s->draw, inlink->format, 0);
+if (frame)
+ff_draw_init2(&s->draw, inlink->format, frame->colorspace, 
frame->color_range, 0);
+else
+ff_draw_init(&s->draw, inlink->format, 0);
 ff_draw_color(&s->draw, &s->color, s->rgba_color);
 
 var_values[VAR_IN_W]  = var_values[VAR_IW] = inlink->w;
@@ -216,6 +220,11 @@ eval_fail:
 
 }
 
+static int config_input(AVFilterLink *inlink)
+{
+return config_frame_input(inlink, NULL);
+}
+
 static int config_output(AVFilterLink *outlink)
 {
 PadContext *s = outlink->src->priv;
@@ -326,11 +335,11 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
 AVFilterLink *outlink = inlink->dst->outputs[0];
 AVFrame *out;
 int needs_copy;
-if(s->eval_mode == EVAL_MODE_FRAME && (
+if((s->eval_mode == EVAL_MODE_FRAME && (
in->width  != s->inlink_w
 || in->height != s->inlink_h
 || in->format != outlink->format
-|| in->sample_aspect_ratio.den != outlink->sample_aspect_ratio.den || 
in->sample_aspect_ratio.num != outlink->sample_aspect_ratio.num)) {
+|| in->sample_aspect_ratio.den != outlink->sample_aspect_ratio.den || 
in->sample_aspect_ratio.num != outlink->sample_aspect_ratio.num)) || 
!s->configured_input) {
 int ret;
 
 inlink->dst->inputs[0]->format = in->format;
@@ -341,7 +350,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
 inlink->dst->inputs[0]->sample_aspect_ratio.num = 
in->sample_aspect_ratio.num;
 
 
-if ((ret = config_input(inlink)) < 0) {
+if ((ret = config_frame_input(inlink, in)) < 0) {
 s->inlink_w = -1;
 return ret;
 }
@@ -349,6 +358,8 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
 s->inlink_w = -1;
 return ret;
 }
+
+s->configured_input = 1;
 }
 
 needs_copy = frame_needs_copy(s, in);
-- 
2.42.0

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

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


[FFmpeg-devel] sending RTSP keep alive even when paused

2023-08-27 Thread Tmc Tmc
Hi All,
I found a bug in ffmpeg's RTSP implementation.

The workflow is as follows:

1. Have a RTSP server that supports Pause.
2. Have ffmpeg play a video from that server (rtsp://:...).
3. Pause the video in ffmpeg.
4. ffmpeg does NOT properly send the keep alive when Paused
(either "GET_PARAMETER" or "OPTIONS"). This is the bug.
5. Since the RTSP server expects the keep alive, after the specified
timeout (usually 60s) it closes the connection.

The bug is in libavformat/rtspdec.c, rtsp_read_packet method.

This method first does ff_rtsp_fetch_packet, which fails when the server is
Paused, causing it to never send the "GET_PARAMETER" or "OPTIONS" which is
placed after this call.

The fix is to simply move that block of code before ff_rtsp_fetch_packet.
That is, move the entire if block:
if (!(rt->rtsp_flags & RTSP_FLAG_LISTEN)) { ... }
just before the call to:
ret = ff_rtsp_fetch_packet(s, pkt);

I have tested this and it seems to work well for our cases.

Your thoughts and comments on this are welcome.

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

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


Re: [FFmpeg-devel] [PATCH 6/6] doc/developer: deduplicate commit message rules

2023-08-27 Thread Stefano Sabatini
Il sab 26 ago 2023, 20:08 Anton Khirnov  ha scritto:

> The patches/committing section currently contains several
> partially-overlapping rules on commit messages. Merge and simplify them
> into one item.
> ---
>  doc/developer.texi | 41 ++---
>  1 file changed, 18 insertions(+), 23 deletions(-)
>
> diff --git a/doc/developer.texi b/doc/developer.texi
> index e4ba263581..fa417fc019 100644
> --- a/doc/developer.texi
> +++ b/doc/developer.texi
> @@ -340,13 +340,24 @@ missing samples or an implementation with a small
> subset of features.
>  Always check the mailing list for any reviewers with issues and test
>  FATE before you push.
>
> -@subheading Keep the main commit message short with an extended
> description below.
> -The commit message should have a short first line in the form of
> -a @samp{topic: short description} as a header, separated by a newline
> -from the body consisting of an explanation of why the change is necessary.
> -If the commit fixes a known bug on the bug tracker, the commit message
> -should include its bug ID. Referring to the issue on the bug tracker does
> -not exempt you from writing an excerpt of the bug in the commit message.
> +@subheading Commit messages
> +Commit messages are highly important tools for informing other developers
> on
> +what a given change does and why. Every commit must always have a properly
> +filled out commit message with the following format:
> +@example
> +area changed: Short 1 line description


Short => short given that we do not Capitalize

+
> +details describing what and why and giving references.
> +@end example
> +
> +If the commit addresses a known bug on our bug tracker or other external
> issue
> +(e.g. CVE), the commit message should include the relevant bug ID(s) or
> other
> +external identifiers. Note that this should be done in addition to a
> proper
> +explanation and not instead of it. Comments such as "fixed!" or "Changed
> it."
> +are not acceptable.
> +
> +When applying patches that have been discussed at length on the mailing
> list,
> +reference the thread in the commit message.
>
>  @subheading Testing must be adequate but not excessive.
>  If it works for you, others, and passes FATE then it should be OK to
> commit
> @@ -379,28 +390,12 @@ NOTE: If you had to put if()@{ .. @} over a large (>
> 5 lines) chunk of code,
>  then either do NOT change the indentation of the inner part within (do not
>  move it to the right)! or do so in a separate commit
>
> -@subheading Commit messages should always be filled out properly.
> -Always fill out the commit log message. Describe in a few lines what you
> -changed and why. You can refer to mailing list postings if you fix a
> -particular bug. Comments such as "fixed!" or "Changed it." are
> unacceptable.
> -Recommended format:
> -
> -@example
> -area changed: Short 1 line description
> -
> -details describing what and why and giving references.
> -@end example
> -
>  @subheading Credit the author of the patch.
>  Make sure the author of the commit is set correctly. (see git commit
> --author)
>  If you apply a patch, send an
>  answer to ffmpeg-devel (or wherever you got the patch from) saying that
>  you applied the patch.
>
> -@subheading Complex patches should refer to discussion surrounding them.
> -When applying patches that have been discussed (at length) on the mailing
> -list, reference the thread in the log message.
> -
>  @subheading Always wait long enough before pushing changes
>  Do NOT commit to code actively maintained by others without permission.
>  Send a patch to ffmpeg-devel. If no one answers within a reasonable
>

Lgtm otherwise, thanks.

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

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


Re: [FFmpeg-devel] [PATCH 5/6] doc/developer: drop an outdated item

2023-08-27 Thread Stefano Sabatini
Il sab 26 ago 2023, 20:08 Anton Khirnov  ha scritto:

> It dates back to pre-2005 days, when people generally tended to commit
> their work directly without going through the mailing list. Few
> developers do it today, and never outside of their standalone modules.
> This item is thus confusing and misleading and is better removed.
> ---
>  doc/developer.texi | 9 -
>  1 file changed, 9 deletions(-)
>

Agree, 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 4/6] doc/developer: add a code behaviour section to development policy

2023-08-27 Thread Stefano Sabatini
Il sab 26 ago 2023, 20:08 Anton Khirnov  ha scritto:

> Document our longstanding de facto policies on things like correctness,
> thread-safety, UB, etc.
>

UB?

---
>  doc/developer.texi | 50 +-
>  1 file changed, 36 insertions(+), 14 deletions(-)
>
> diff --git a/doc/developer.texi b/doc/developer.texi
> index df43119f98..afa0148137 100644
> --- a/doc/developer.texi
> +++ b/doc/developer.texi
> @@ -274,10 +274,6 @@ symbols. If in doubt, just avoid names starting with
> @code{_} altogether.
>  @section Miscellaneous conventions
>
>  @itemize @bullet
> -@item
> -fprintf and printf are forbidden in libavformat and libavcodec,
> -please use av_log() instead.
> -
>  @item
>  Casts should be used only when necessary. Unneeded parentheses
>  should also be avoided if they don't make the code easier to understand.
> @@ -286,6 +282,42 @@ should also be avoided if they don't make the code
> easier to understand.
>  @anchor{Development Policy}
>  @chapter Development Policy
>
> +@section Code behaviour
> +
> +@subheading Correctness
> +The code must be valid. It must not crash, abort, access invalid
> pointers, leak
> +memory, cause data races or signed integer overflow, or otherwise invoke
> +undefined behaviour.


Invoke => assume?

Error codes should be checked and, when applicable,
> +forwarded to the caller.
> +
> +@subheading Thread- and library-safety
> +Our libraries may be called by multiple independent callers in the same
> process.
> +These calls may happen from any number of threads and the different call
> sites
> +may not be aware of each other - e.g. a user program may be calling us
> directly,
>

Calling us -> calling the/our libraries

+and use one or more libraries that also call us. The code must behave
> correctly
> +under such conditions.
> +
> +@subheading Robustness
> +The code must treat as untrusted any bytestream received from a caller or
> read
> +from a file, network, etc. It must not misbehave when arbitrary data is
> sent to
> +it - typically it should print an error message and return
> +@code{AVERROR_INVALIDDATA} on encountering invalid input data.
> +
> +@subheading Memory allocation
> +The code must use the @code{av_malloc()} family of functions from
> +@file{libavutil/mem.h} to perform all memory allocation, except in
> special cases
> +(e.g. when interacting with an external library that requires a specific
> +allocator to be used).
> +
> +All allocations should be checked and @code{AVERROR(ENOMEM)} returned on
> +failure. A common mistake is that error paths leak memory - make sure
> that does
> +not happen.
> +
> +@subheading stdio
> +Our libraries must not access the stdio streams stdin/stdout/stderr
> directly
> +(e.g. via @code{printf()} family of functions), as that is not
> library-safe. For
> +logging, use @code{av_log()}.


Lgtm otherwise, thanks.

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

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


Re: [FFmpeg-devel] [PATCH 3/6] doc/developer: fix a nonsense statement

2023-08-27 Thread Stefano Sabatini
Il sab 26 ago 2023, 20:08 Anton Khirnov  ha scritto:

> Adding new fields to _functions_ makes no sense, it was supposed to be
> structs.
> ---
>  doc/developer.texi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/doc/developer.texi b/doc/developer.texi
> index d27716ab97..df43119f98 100644
> --- a/doc/developer.texi
> +++ b/doc/developer.texi
> @@ -425,7 +425,7 @@ number remains unchanged.
>  @subsection Adding new interfaces
>  Any new public identifiers in installed headers are considered new API -
> this
>  includes new functions, structs, macros, enum values, typedefs, new
> fields in
> -existing functions, new installed headers, etc. Consider the following
> +existing structs, new installed headers, etc. Consider the following
>  guidelines when adding new APIs.


Obviously ok, thanks.

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

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


Re: [FFmpeg-devel] [PATCH 2/6] doc/developer: merge the 'contributing code' section into its parent chapter

2023-08-27 Thread Stefano Sabatini
Il sab 26 ago 2023, 20:08 Anton Khirnov  ha scritto:

> The section consistes of a single short paragraph linking to other
> chapters. The enclosing chapter also has no other sections, all other
> text is placed in the chapter directly.
> Keeping a separate section for this paragraph just adds more clutter.
> ---
>  doc/developer.texi | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/doc/developer.texi b/doc/developer.texi
> index 85515f5d37..d27716ab97 100644
> --- a/doc/developer.texi
> +++ b/doc/developer.texi
> @@ -44,8 +44,6 @@ By supporting the project you find useful you ensure it
> continues to be
>  maintained and developed.
>  @end itemize
>
> -@section Contributing code
> -
>  All proposed code changes should be submitted for review to
>  @url{mailto:ffmpeg-devel@@ffmpeg.org, the development mailing list}, as
>  described in more detail in the @ref{Submitting patches} chapter. The code
>

Lgtm, thanks.

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

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


Re: [FFmpeg-devel] [PATCH 1/6] doc/developer: move a sentence to a more appropriate place

2023-08-27 Thread Stefano Sabatini
Il sab 26 ago 2023, 20:08 Anton Khirnov  ha scritto:

> It's targeted at our users, not developers, so it makes more sense to
> group it with other text targeted at our users.
> ---
>  doc/developer.texi | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/doc/developer.texi b/doc/developer.texi
> index 0c2f2cd7d1..85515f5d37 100644
> --- a/doc/developer.texi
> +++ b/doc/developer.texi
> @@ -24,6 +24,10 @@ generated from the headers
>  the examples under @file{doc/examples}
>  @end itemize
>
> +For more detailed legal information about the use of FFmpeg in
> +external programs read the @file{LICENSE} file in the source tree and
> +consult @url{https://ffmpeg.org/legal.html}.
> +
>  If you modify FFmpeg code for your own use case, you are highly
> encouraged to
>  @emph{submit your changes back to us}, using this document as a guide.
> There are
>  both pragmatic and ideological reasons to do so:
> @@ -40,10 +44,6 @@ By supporting the project you find useful you ensure it
> continues to be
>  maintained and developed.
>  @end itemize
>
> -For more detailed legal information about the use of FFmpeg in
> -external programs read the @file{LICENSE} file in the source tree and
> -consult @url{https://ffmpeg.org/legal.html}.
> -
>  @section Contributing code


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

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


Re: [FFmpeg-devel] [PATCH 1/2] lavfi/vf_libplacebo: switch to new pl_options struct

2023-08-27 Thread Niklas Haas
On Fri, 18 Aug 2023 18:50:43 +0200 Niklas Haas  wrote:
> From: Niklas Haas 
> 
> This new upstream struct simplifies params struct management by allowing
> them to all be contained in a single dynamically allocated struct. This
> commit switches to the new API in a backwards-compatible way.
> 
> The only nontrivial change that was required was to handle
> `sigmoid_params` in a way consistent with the rest of the params
> structs, instead of setting it directly to the upstream default.
> ---
>  libavfilter/vf_libplacebo.c | 89 -
>  1 file changed, 57 insertions(+), 32 deletions(-)
> 
> diff --git a/libavfilter/vf_libplacebo.c b/libavfilter/vf_libplacebo.c
> index 34879910538..b9effdbad95 100644
> --- a/libavfilter/vf_libplacebo.c
> +++ b/libavfilter/vf_libplacebo.c
> @@ -42,6 +42,25 @@ static inline AVFrame *pl_get_mapped_avframe(const struct 
> pl_frame *frame)
>  }
>  #endif
>  
> +#if PL_API_VER >= 309
> +#include 
> +#else
> +typedef struct pl_options_t {
> +// Backwards compatibility shim of this struct
> +struct pl_render_params params;
> +struct pl_deband_params deband_params;
> +struct pl_sigmoid_params sigmoid_params;
> +struct pl_color_adjustment color_adjustment;
> +struct pl_peak_detect_params peak_detect_params;
> +struct pl_color_map_params color_map_params;
> +struct pl_dither_params dither_params;
> +struct pl_cone_params cone_params;
> +} *pl_options;
> +
> +#define pl_options_alloc(log) av_mallocz(sizeof(struct pl_options_t))
> +#define pl_options_free(ptr)  av_freep(ptr)
> +#endif
> +
>  enum {
>  TONE_MAP_AUTO,
>  TONE_MAP_CLIP,
> @@ -175,7 +194,7 @@ typedef struct LibplaceboContext {
>  int color_trc;
>  
>  /* pl_render_params */
> -struct pl_render_params params;
> +pl_options opts;
>  char *upscaler;
>  char *downscaler;
>  char *frame_mixer;
> @@ -190,7 +209,6 @@ typedef struct LibplaceboContext {
>  int disable_fbos;
>  
>  /* pl_deband_params */
> -struct pl_deband_params deband_params;
>  int deband;
>  int deband_iterations;
>  float deband_threshold;
> @@ -198,7 +216,6 @@ typedef struct LibplaceboContext {
>  float deband_grain;
>  
>  /* pl_color_adjustment */
> -struct pl_color_adjustment color_adjustment;
>  float brightness;
>  float contrast;
>  float saturation;
> @@ -206,7 +223,6 @@ typedef struct LibplaceboContext {
>  float gamma;
>  
>  /* pl_peak_detect_params */
> -struct pl_peak_detect_params peak_detect_params;
>  int peakdetect;
>  float smoothing;
>  float min_peak;
> @@ -215,7 +231,6 @@ typedef struct LibplaceboContext {
>  float percentile;
>  
>  /* pl_color_map_params */
> -struct pl_color_map_params color_map_params;
>  int gamut_mode;
>  int tonemapping;
>  float tonemapping_param;
> @@ -239,13 +254,11 @@ typedef struct LibplaceboContext {
>  #endif
>  
>  /* pl_dither_params */
> -struct pl_dither_params dither_params;
>  int dithering;
>  int dither_lut_size;
>  int dither_temporal;
>  
>  /* pl_cone_params */
> -struct pl_cone_params cone_params;
>  int cones;
>  float cone_str;
>  
> @@ -363,6 +376,7 @@ static int update_settings(AVFilterContext *ctx)
>  {
>  int err = 0;
>  LibplaceboContext *s = ctx->priv;
> +pl_options opts = s->opts;
>  int gamut_mode = s->gamut_mode;
>  uint8_t color_rgba[4];
>  
> @@ -394,14 +408,16 @@ static int update_settings(AVFilterContext *ctx)
>  
>  RET(av_parse_color(color_rgba, s->fillcolor, -1, s));
>  
> -s->deband_params = *pl_deband_params(
> +opts->deband_params = *pl_deband_params(
>  .iterations = s->deband_iterations,
>  .threshold = s->deband_threshold,
>  .radius = s->deband_radius,
>  .grain = s->deband_grain,
>  );
>  
> -s->color_adjustment = (struct pl_color_adjustment) {
> +opts->sigmoid_params = pl_sigmoid_default_params;
> +
> +opts->color_adjustment = (struct pl_color_adjustment) {
>  .brightness = s->brightness,
>  .contrast = s->contrast,
>  .saturation = s->saturation,
> @@ -409,7 +425,7 @@ static int update_settings(AVFilterContext *ctx)
>  .gamma = s->gamma,
>  };
>  
> -s->peak_detect_params = *pl_peak_detect_params(
> +opts->peak_detect_params = *pl_peak_detect_params(
>  .smoothing_period = s->smoothing,
>  .minimum_peak = s->min_peak,
>  .scene_threshold_low = s->scene_low,
> @@ -422,7 +438,7 @@ static int update_settings(AVFilterContext *ctx)
>  #endif
>  );
>  
> -s->color_map_params = *pl_color_map_params(
> +opts->color_map_params = *pl_color_map_params(
>  #if FF_API_LIBPLACEBO_OPTS
>  # if PL_API_VER >= 269
>  .hybrid_mix = hybrid_mix,
> @@ -441,20 +457,20 @@ static int update_settings(AVFilterContext *ctx)
>  #endif
>  );
>  
> -set_gamut_mode(&s->color_map_params, gamut_mode);

[FFmpeg-devel] [PATCH 04/18 v2] fftools/ffmpeg_mux: stop rescaling timestamps in of_streamcopy()

2023-08-27 Thread Anton Khirnov
This function converts packet timestamps from the input stream timebase
to OutputStream.mux_timebase, which may or may not be equal to the
actual output AVStream timebase (and even when it is, this may not
always be the optimal choice due to bitstream filtering).

Just keep the timestamps in input stream timebase, they will be rescaled
as needed before bitstream filtering and/or sending the packet to the
muxer.

Move the av_rescale_delta() call for audio (needed to preserve accuracy
with coarse muxer timebases) to write_packet.

Drop now-unused OutputStream.mux_timebase.
---
 fftools/ffmpeg.h  |  2 --
 fftools/ffmpeg_enc.c  |  2 --
 fftools/ffmpeg_mux.c  | 44 ---
 fftools/ffmpeg_mux.h  |  2 +-
 fftools/ffmpeg_mux_init.c |  2 --
 5 files changed, 24 insertions(+), 28 deletions(-)

diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index d53181e427..ef5bb13908 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -523,8 +523,6 @@ typedef struct OutputStream {
 /* dts of the last packet sent to the muxing queue, in AV_TIME_BASE_Q */
 int64_t last_mux_dts;
 
-// the timebase of the packets sent to the muxer
-AVRational mux_timebase;
 AVRational enc_timebase;
 
 Encoder *enc;
diff --git a/fftools/ffmpeg_enc.c b/fftools/ffmpeg_enc.c
index 96424272bf..3c9fdd3237 100644
--- a/fftools/ffmpeg_enc.c
+++ b/fftools/ffmpeg_enc.c
@@ -463,8 +463,6 @@ int enc_open(OutputStream *ost, AVFrame *frame)
 if (ost->st->time_base.num <= 0 || ost->st->time_base.den <= 0)
 ost->st->time_base = av_add_q(ost->enc_ctx->time_base, (AVRational){0, 
1});
 
-ost->mux_timebase = enc_ctx->time_base;
-
 ret = of_stream_init(of, ost);
 if (ret < 0)
 return ret;
diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
index ab9bb398c1..54410aac5c 100644
--- a/fftools/ffmpeg_mux.c
+++ b/fftools/ffmpeg_mux.c
@@ -76,7 +76,23 @@ static int write_packet(Muxer *mux, OutputStream *ost, 
AVPacket *pkt)
 if (ost->type == AVMEDIA_TYPE_VIDEO && ost->vsync_method == VSYNC_DROP)
 pkt->pts = pkt->dts = AV_NOPTS_VALUE;
 
-av_packet_rescale_ts(pkt, pkt->time_base, ost->st->time_base);
+// rescale timestamps to the stream timebase
+if (ost->type == AVMEDIA_TYPE_AUDIO && !ost->enc) {
+// use av_rescale_delta() for streamcopying audio, to preserve
+// accuracy with coarse input timebases
+int duration = av_get_audio_frame_duration2(ost->st->codecpar, 
pkt->size);
+
+if (!duration)
+duration = ost->st->codecpar->frame_size;
+
+pkt->dts = av_rescale_delta(pkt->time_base, pkt->dts,
+(AVRational){1, 
ost->st->codecpar->sample_rate}, duration,
+&ms->ts_rescale_delta_last, 
ost->st->time_base);
+pkt->pts = pkt->dts;
+
+pkt->duration = av_rescale_q(pkt->duration, pkt->time_base, 
ost->st->time_base);
+} else
+av_packet_rescale_ts(pkt, pkt->time_base, ost->st->time_base);
 pkt->time_base = ost->st->time_base;
 
 if (!(s->oformat->flags & AVFMT_NOTIMESTAMPS)) {
@@ -392,7 +408,7 @@ int of_streamcopy(OutputStream *ost, const AVPacket *pkt, 
int64_t dts)
 OutputFile *of = output_files[ost->file_index];
 MuxStream  *ms = ms_from_ost(ost);
 int64_t start_time = (of->start_time == AV_NOPTS_VALUE) ? 0 : 
of->start_time;
-int64_t ost_tb_start_time = av_rescale_q(start_time, AV_TIME_BASE_Q, 
ost->mux_timebase);
+int64_t ts_offset;
 AVPacket *opkt = ms->pkt;
 int ret;
 
@@ -425,27 +441,17 @@ int of_streamcopy(OutputStream *ost, const AVPacket *pkt, 
int64_t dts)
 if (ret < 0)
 return ret;
 
-opkt->time_base = ost->mux_timebase;
+ts_offset = av_rescale_q(start_time, AV_TIME_BASE_Q, opkt->time_base);
 
 if (pkt->pts != AV_NOPTS_VALUE)
-opkt->pts = av_rescale_q(pkt->pts, pkt->time_base, opkt->time_base) - 
ost_tb_start_time;
+opkt->pts -= ts_offset;
 
 if (pkt->dts == AV_NOPTS_VALUE) {
 opkt->dts = av_rescale_q(dts, AV_TIME_BASE_Q, opkt->time_base);
 } else if (ost->st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) {
-int duration = av_get_audio_frame_duration2(ost->par_in, pkt->size);
-if(!duration)
-duration = ost->par_in->frame_size;
-opkt->dts = av_rescale_delta(pkt->time_base, pkt->dts,
-(AVRational){1, ost->par_in->sample_rate}, 
duration,
-&ms->ts_rescale_delta_last, 
opkt->time_base);
-/* dts will be set immediately afterwards to what pts is now */
-opkt->pts = opkt->dts - ost_tb_start_time;
-} else
-opkt->dts = av_rescale_q(pkt->dts, pkt->time_base, opkt->time_base);
-opkt->dts -= ost_tb_start_time;
-
-opkt->duration = av_rescale_q(pkt->duration, pkt->time_base, 
opkt->time_base);
+opkt->pts = opkt->dts - ts_offset;
+}
+opkt->dts -= ts_offset;