Re: [FFmpeg-devel] [PATCH 1/3] avcodec/av1dec: Pass AVCodecContext* as logctx in get_sw_pixel_format()

2023-09-12 Thread James Almer

On 9/12/2023 9:22 PM, Andreas Rheinhardt wrote:

Andreas Rheinhardt:

It indicates to the reader that said function does not modify
any state.

Signed-off-by: Andreas Rheinhardt 
---
  libavcodec/av1dec.c | 12 ++--
  1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
index 8f9c2dfefb..8f6c4f732e 100644
--- a/libavcodec/av1dec.c
+++ b/libavcodec/av1dec.c
@@ -440,7 +440,7 @@ static int get_tiles_info(AVCodecContext *avctx, const 
AV1RawTileGroup *tile_gro
  
  }
  
-static enum AVPixelFormat get_sw_pixel_format(AVCodecContext *avctx,

+static enum AVPixelFormat get_sw_pixel_format(void *logctx,
const AV1RawSequenceHeader *seq)
  {
  uint8_t bit_depth;
@@ -451,7 +451,7 @@ static enum AVPixelFormat 
get_sw_pixel_format(AVCodecContext *avctx,
  else if (seq->seq_profile <= 2)
  bit_depth = seq->color_config.high_bitdepth ? 10 : 8;
  else {
-av_log(avctx, AV_LOG_ERROR,
+av_log(logctx, AV_LOG_ERROR,
 "Unknown AV1 profile %d.\n", seq->seq_profile);
  return -1;
  }
@@ -467,7 +467,7 @@ static enum AVPixelFormat 
get_sw_pixel_format(AVCodecContext *avctx,
  else if (bit_depth == 12)
  pix_fmt = AV_PIX_FMT_YUV444P12;
  else
-av_log(avctx, AV_LOG_WARNING, "Unknown AV1 pixel format.\n");
+av_log(logctx, AV_LOG_WARNING, "Unknown AV1 pixel format.\n");
  } else if (seq->color_config.subsampling_x == 1 &&
 seq->color_config.subsampling_y == 0) {
  if (bit_depth == 8)
@@ -477,7 +477,7 @@ static enum AVPixelFormat 
get_sw_pixel_format(AVCodecContext *avctx,
  else if (bit_depth == 12)
  pix_fmt = AV_PIX_FMT_YUV422P12;
  else
-av_log(avctx, AV_LOG_WARNING, "Unknown AV1 pixel format.\n");
+av_log(logctx, AV_LOG_WARNING, "Unknown AV1 pixel format.\n");
  } else if (seq->color_config.subsampling_x == 1 &&
 seq->color_config.subsampling_y == 1) {
  if (bit_depth == 8)
@@ -487,7 +487,7 @@ static enum AVPixelFormat 
get_sw_pixel_format(AVCodecContext *avctx,
  else if (bit_depth == 12)
  pix_fmt = AV_PIX_FMT_YUV420P12;
  else
-av_log(avctx, AV_LOG_WARNING, "Unknown AV1 pixel format.\n");
+av_log(logctx, AV_LOG_WARNING, "Unknown AV1 pixel format.\n");
  }
  } else {
  if (bit_depth == 8)
@@ -497,7 +497,7 @@ static enum AVPixelFormat 
get_sw_pixel_format(AVCodecContext *avctx,
  else if (bit_depth == 12)
  pix_fmt = AV_PIX_FMT_GRAY12;
  else
-av_log(avctx, AV_LOG_WARNING, "Unknown AV1 pixel format.\n");
+av_log(logctx, AV_LOG_WARNING, "Unknown AV1 pixel format.\n");
  }
  
  return pix_fmt;


Will apply this patchset tonight unless there are objections.


Not against it, but this reminds me I'd really like a way to pass a 
const pointer to av_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 1/3] avcodec/av1dec: Pass AVCodecContext* as logctx in get_sw_pixel_format()

2023-09-12 Thread Andreas Rheinhardt
Andreas Rheinhardt:
> It indicates to the reader that said function does not modify
> any state.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/av1dec.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
> index 8f9c2dfefb..8f6c4f732e 100644
> --- a/libavcodec/av1dec.c
> +++ b/libavcodec/av1dec.c
> @@ -440,7 +440,7 @@ static int get_tiles_info(AVCodecContext *avctx, const 
> AV1RawTileGroup *tile_gro
>  
>  }
>  
> -static enum AVPixelFormat get_sw_pixel_format(AVCodecContext *avctx,
> +static enum AVPixelFormat get_sw_pixel_format(void *logctx,
>const AV1RawSequenceHeader 
> *seq)
>  {
>  uint8_t bit_depth;
> @@ -451,7 +451,7 @@ static enum AVPixelFormat 
> get_sw_pixel_format(AVCodecContext *avctx,
>  else if (seq->seq_profile <= 2)
>  bit_depth = seq->color_config.high_bitdepth ? 10 : 8;
>  else {
> -av_log(avctx, AV_LOG_ERROR,
> +av_log(logctx, AV_LOG_ERROR,
> "Unknown AV1 profile %d.\n", seq->seq_profile);
>  return -1;
>  }
> @@ -467,7 +467,7 @@ static enum AVPixelFormat 
> get_sw_pixel_format(AVCodecContext *avctx,
>  else if (bit_depth == 12)
>  pix_fmt = AV_PIX_FMT_YUV444P12;
>  else
> -av_log(avctx, AV_LOG_WARNING, "Unknown AV1 pixel format.\n");
> +av_log(logctx, AV_LOG_WARNING, "Unknown AV1 pixel 
> format.\n");
>  } else if (seq->color_config.subsampling_x == 1 &&
> seq->color_config.subsampling_y == 0) {
>  if (bit_depth == 8)
> @@ -477,7 +477,7 @@ static enum AVPixelFormat 
> get_sw_pixel_format(AVCodecContext *avctx,
>  else if (bit_depth == 12)
>  pix_fmt = AV_PIX_FMT_YUV422P12;
>  else
> -av_log(avctx, AV_LOG_WARNING, "Unknown AV1 pixel format.\n");
> +av_log(logctx, AV_LOG_WARNING, "Unknown AV1 pixel 
> format.\n");
>  } else if (seq->color_config.subsampling_x == 1 &&
> seq->color_config.subsampling_y == 1) {
>  if (bit_depth == 8)
> @@ -487,7 +487,7 @@ static enum AVPixelFormat 
> get_sw_pixel_format(AVCodecContext *avctx,
>  else if (bit_depth == 12)
>  pix_fmt = AV_PIX_FMT_YUV420P12;
>  else
> -av_log(avctx, AV_LOG_WARNING, "Unknown AV1 pixel format.\n");
> +av_log(logctx, AV_LOG_WARNING, "Unknown AV1 pixel 
> format.\n");
>  }
>  } else {
>  if (bit_depth == 8)
> @@ -497,7 +497,7 @@ static enum AVPixelFormat 
> get_sw_pixel_format(AVCodecContext *avctx,
>  else if (bit_depth == 12)
>  pix_fmt = AV_PIX_FMT_GRAY12;
>  else
> -av_log(avctx, AV_LOG_WARNING, "Unknown AV1 pixel format.\n");
> +av_log(logctx, AV_LOG_WARNING, "Unknown AV1 pixel format.\n");
>  }
>  
>  return pix_fmt;

Will apply this patchset tonight unless there are objections.

- Andreas

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

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


Re: [FFmpeg-devel] trac backups

2023-09-12 Thread Michael Niedermayer
On Wed, Sep 13, 2023 at 12:32:28AM +0200, Marvin Scholz wrote:
> 
> 
> On 13 Sep 2023, at 0:13, Michael Niedermayer wrote:
> 
> > Hi all
> >
> > our trac backups died 6 months ago
> > i just noticed as i checked the backups before deleting spam with trac-admin
> > backups are working again
> >
> > failure reason where expired gpg keys
> >
> > thx
> >
> > PS: yeah, iam slightly unhappy noone noticed this for 6 months
> 
> Who else other than you has access to the infrastructure?

all the root admins do
but that isnt the problem, even if 100 more people had access
the only way that was noticable it seems was if someone looked
either at the backups (which happily where generated with size 0 per file)
or the logs

ive added a gpg --recv-key so until the key server gets killed by the NSA
that might be avoided. Sadly gpg has no clean was to ignore key expiry

also ive pinged beastd (who wrote the trac backup scripts and is in CC)
and if he has no time, i guess ill have to look at why this was not more
vissible and how to fix that ...
But it seems theres no hurry

theres also the more radical solution of just making the backups public
as they are enrcypted anyway, it just doesnt feel like the first
choice. But it would avoid several issues like not noticing this or
other lost backups, ...

thx

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.


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] trac backups

2023-09-12 Thread Marvin Scholz



On 13 Sep 2023, at 0:13, Michael Niedermayer wrote:

> Hi all
>
> our trac backups died 6 months ago
> i just noticed as i checked the backups before deleting spam with trac-admin
> backups are working again
>
> failure reason where expired gpg keys
>
> thx
>
> PS: yeah, iam slightly unhappy noone noticed this for 6 months

Who else other than you has access to the infrastructure?

>
> -- 
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> When the tyrant has disposed of foreign enemies by conquest or treaty, and
> there is nothing more to fear from them, then he is always stirring up
> some war or other, in order that the people may require a leader. -- Plato
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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


[FFmpeg-devel] trac backups

2023-09-12 Thread Michael Niedermayer
Hi all

our trac backups died 6 months ago
i just noticed as i checked the backups before deleting spam with trac-admin
backups are working again

failure reason where expired gpg keys

thx

PS: yeah, iam slightly unhappy noone noticed this for 6 months

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When the tyrant has disposed of foreign enemies by conquest or treaty, and
there is nothing more to fear from them, then he is always stirring up
some war or other, in order that the people may require a leader. -- 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".


Re: [FFmpeg-devel] Trac spam

2023-09-12 Thread Michael Niedermayer
On Tue, Sep 12, 2023 at 07:03:33AM +0200, Michael Koch wrote:
> > I guess deleting users requires full admin rights, so i guess, just make a 
> > list
> of users which are in need of a deletion and post that and i or less likely
> some other admin will disable/delete them.
> 
> Please delete user "selune" in ticket 2104

user selune and the ticket comment where deleted using:
trac-admin ffmpeg session delete selune
trac-admin ffmpeg ticket remove_comment 2104 14

(happy i figured these out as it faster than using a web browser)

Removing a user does not remove her comments

I think you should have the power to edit comments, if you want.
seperate comment deletion powers i think need a future trac version
otherwise, please provide ticket number + comment number pairs in the future
as thats what the tools want

thx for your spam fighting!

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.


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

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


Re: [FFmpeg-devel] [PATCH 1/5] avformat/wtvdec: Don't truncate GUIDs

2023-09-12 Thread Peter Ross
On Tue, Sep 12, 2023 at 02:27:17PM +0200, Andreas Rheinhardt wrote:
> Each of the 16 bytes of a GUID is written as a two-character
> hex value and three hyphens, leading to a length of 35.
> GCC 13 emits a -Wformat-truncation= warning because of this.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
> Is there actually a reason this is using a different format than
> the one used by lavu/uuid.h?

i don't think so. wtvdec predates lavu/uuid.h.

patchset looks good.

-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)


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] [RFC PATCH 1/3] aacdec: always skip the first 2048 samples if there's no side data

2023-09-12 Thread Thierry Foucu
On Tue, Sep 12, 2023 at 9:25 AM Lynne  wrote:

> Sep 12, 2023, 09:43 by andreas.rheinha...@outlook.com:
>
> > Lynne:
> >
> >> For some reason, this was never set, which meant all **raw** AAC in ADTS
> >> streams, except faac, had extra samples at the start.
> >>
> >> Despite this being a standard MDCT-based codec with a frame size of
> 1024,
> >> hence a delay of 1024 samples at the start, all major encoders,
> excluding
> >> faac and FFmpeg, use 2048 samples of padding.
> >>
> >> The FFmpeg encoder will be modified to also output 2048 samples of
> padding
> >> at the start, to make it in line with other encoders.
> >>
> >
> > Does this also have actual advantages besides "being in line with other
> > encoders"?
> >
>
> Not really. I don't have an opinion on this. 1024 is the natural
> delay of the codec, so maybe it would be best to leave it at that.
>
>
> Note:
Not all encoders add 2048. Another version of the Fraunhofer encoder will
add only 1600 samples

and for HE-AAC of the same encoder will add 3200 samples.
Should we not then have an option to set it ?


> >> Yes, this leaves FATE pretty sad. Will fix it with the real version of
> the patch.
> >>
> >
> > Didn't we once guess the number of skip samples like this, only for this
> > guesswork to be removed intentionally? (This is not a rhetorical
> > question; I thought it to be true, but I see that there is still code
> > for faac in decode_fill(); maybe I misremember.)
> >
>
> I don't remember something like that. The faac workaround dates back
> from 2012 (bfe735b5824c7d10ba42932a17d786db50e3b2d4), and it's only for
> faac.
> It's less of a guess, as most encoders to use the FIL extension to signal
> themselves.
> ___
> 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".
>


-- 

Thierry Foucu
___
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] avcodec/h264dec: Fix data race when updating decode_error_flags

2023-09-12 Thread Andreas Rheinhardt
When using multi-threaded decoding, every decoding thread
has its own DBP consisting of H264Pictures and each of these
points to its own AVFrames. They are synced during
update_thread_context via av_frame_ref() and therefore
the threads actually decoding (as well as all the others)
must not modify any field that is copied by av_frame_ref()
after ff_thread_finish_setup().

Yet this is exactly what happens when an error occurs
during decoding and the AVFrame's decode_error_flags are updated.
Given that these errors only become apparent during decoding,
this can't be set before ff_thread_finish_setup() without
defeating the point of frame-threading; in practice,
this meant that the decoder did not set these flags correctly
in case frame-threading was in use. (This means that e.g.
the ffmpeg cli tool fails to output its "corrupt decoded frame"
message in a nondeterministic fashion.)

This commit fixes this by adding a new H264Picture field
that is actually propagated across threads; the field
is an AVBufferRef* whose data is an atomic_int; it is
atomic in order to allow multiple threads to update it
concurrently and not to provide synchronization
between the threads setting the field and the thread
ultimately returning the AVFrame.

This unfortunately has the overhead of one allocation
per H264Picture (both the original one as well as
creating a reference to an existing one), even in case
of no errors. In order to mitigate this, an AVBufferPool
has been used and only if frame-threading is actually
in use. This expense will be removed as soon as
a proper API for refcounted objects (not based upon
AVBuffer) is in place.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/h264_picture.c |  9 +
 libavcodec/h264_slice.c   |  7 +++
 libavcodec/h264dec.c  | 38 --
 libavcodec/h264dec.h  |  4 
 4 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/libavcodec/h264_picture.c b/libavcodec/h264_picture.c
index bd31f700cb..31b5e231c2 100644
--- a/libavcodec/h264_picture.c
+++ b/libavcodec/h264_picture.c
@@ -54,6 +54,7 @@ void ff_h264_unref_picture(H264Context *h, H264Picture *pic)
 av_buffer_unref(>motion_val_buf[i]);
 av_buffer_unref(>ref_index_buf[i]);
 }
+av_buffer_unref(>decode_error_flags);
 
 memset((uint8_t*)pic + off, 0, sizeof(*pic) - off);
 }
@@ -136,6 +137,10 @@ int ff_h264_ref_picture(H264Context *h, H264Picture *dst, 
H264Picture *src)
 dst->hwaccel_picture_private = dst->hwaccel_priv_buf->data;
 }
 
+ret = av_buffer_replace(>decode_error_flags, src->decode_error_flags);
+if (ret < 0)
+goto fail;
+
 h264_copy_picture_params(dst, src);
 
 return 0;
@@ -186,6 +191,10 @@ int ff_h264_replace_picture(H264Context *h, H264Picture 
*dst, const H264Picture
 
 dst->hwaccel_picture_private = src->hwaccel_picture_private;
 
+ret = av_buffer_replace(>decode_error_flags, src->decode_error_flags);
+if (ret < 0)
+goto fail;
+
 h264_copy_picture_params(dst, src);
 
 return 0;
diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
index 6cd7bb8fe7..71a878b80b 100644
--- a/libavcodec/h264_slice.c
+++ b/libavcodec/h264_slice.c
@@ -210,6 +210,13 @@ static int alloc_picture(H264Context *h, H264Picture *pic)
 if (ret < 0)
 goto fail;
 
+if (h->decode_error_flags_pool) {
+pic->decode_error_flags = 
av_buffer_pool_get(h->decode_error_flags_pool);
+if (!pic->decode_error_flags)
+goto fail;
+atomic_init((atomic_int*)pic->decode_error_flags->data, 0);
+}
+
 if (CONFIG_GRAY && !h->avctx->hwaccel && h->flags & AV_CODEC_FLAG_GRAY && 
pic->f->data[2]) {
 int h_chroma_shift, v_chroma_shift;
 av_pix_fmt_get_chroma_sub_sample(pic->f->format,
diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
index 8e90678125..796f80be8d 100644
--- a/libavcodec/h264dec.c
+++ b/libavcodec/h264dec.c
@@ -307,6 +307,12 @@ static int h264_init_context(AVCodecContext *avctx, 
H264Context *h)
 
 ff_h264_sei_uninit(>sei);
 
+if (avctx->active_thread_type & FF_THREAD_FRAME) {
+h->decode_error_flags_pool = av_buffer_pool_init(sizeof(atomic_int), 
NULL);
+if (!h->decode_error_flags_pool)
+return AVERROR(ENOMEM);
+}
+
 h->nb_slice_ctx = (avctx->active_thread_type & FF_THREAD_SLICE) ? 
avctx->thread_count : 1;
 h->slice_ctx = av_calloc(h->nb_slice_ctx, sizeof(*h->slice_ctx));
 if (!h->slice_ctx) {
@@ -353,6 +359,8 @@ static av_cold int h264_decode_end(AVCodecContext *avctx)
 
 h->cur_pic_ptr = NULL;
 
+av_buffer_pool_uninit(>decode_error_flags_pool);
+
 av_freep(>slice_ctx);
 h->nb_slice_ctx = 0;
 
@@ -739,7 +747,16 @@ static int decode_nal_units(H264Context *h, const uint8_t 
*buf, int buf_size)
 
 // set decode_error_flags to allow users to detect concealed decoding 
errors
 if ((ret < 0 || h->er.error_occurred) && h->cur_pic_ptr) {
-

[FFmpeg-devel] [PATCH 1/2] avcodec/error_resilience: Make applying decode_error_flags optional

2023-09-12 Thread Andreas Rheinhardt
Add a pointer parameter that if supplied will be used to return
the updated decode_error_flags. This will allow to fix several
races when using frame-threading; these resulted from AVFrame
that the earlier code updated concurrently being used as source
in an av_frame_ref() call in the decoder's update_thread_context.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/error_resilience.c |  7 +--
 libavcodec/error_resilience.h | 12 +++-
 libavcodec/h263dec.c  |  2 +-
 libavcodec/h264dec.c  |  2 +-
 libavcodec/mpeg12dec.c|  2 +-
 libavcodec/mss2.c |  2 +-
 libavcodec/rv10.c |  4 ++--
 libavcodec/rv34.c |  6 +++---
 libavcodec/vc1dec.c   |  2 +-
 9 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/libavcodec/error_resilience.c b/libavcodec/error_resilience.c
index 68e20925e0..880aea6f30 100644
--- a/libavcodec/error_resilience.c
+++ b/libavcodec/error_resilience.c
@@ -889,7 +889,7 @@ void ff_er_add_slice(ERContext *s, int startx, int starty,
 }
 }
 
-void ff_er_frame_end(ERContext *s)
+void ff_er_frame_end(ERContext *s, int *decode_error_flags)
 {
 int *linesize = NULL;
 int i, mb_x, mb_y, error, error_type, dc_error, mv_error, ac_error;
@@ -1114,7 +1114,10 @@ void ff_er_frame_end(ERContext *s)
 av_log(s->avctx, AV_LOG_INFO, "concealing %d DC, %d AC, %d MV errors in %c 
frame\n",
dc_error, ac_error, mv_error, 
av_get_picture_type_char(s->cur_pic.f->pict_type));
 
-s->cur_pic.f->decode_error_flags |= FF_DECODE_ERROR_CONCEALMENT_ACTIVE;
+if (decode_error_flags)
+*decode_error_flags |= FF_DECODE_ERROR_CONCEALMENT_ACTIVE;
+else
+s->cur_pic.f->decode_error_flags |= FF_DECODE_ERROR_CONCEALMENT_ACTIVE;
 
 is_intra_likely = is_intra_more_likely(s);
 
diff --git a/libavcodec/error_resilience.h b/libavcodec/error_resilience.h
index 47cc8a4fc6..b03f8ec896 100644
--- a/libavcodec/error_resilience.h
+++ b/libavcodec/error_resilience.h
@@ -90,7 +90,17 @@ typedef struct ERContext {
 } ERContext;
 
 void ff_er_frame_start(ERContext *s);
-void ff_er_frame_end(ERContext *s);
+
+/**
+ * Indicate that a frame has finished decoding and perform error concealment
+ * in case it has been enabled and is necessary and supported.
+ *
+ * @param s  ERContext in use
+ * @param decode_error_flags pointer where updated decode_error_flags are 
written
+ *   if supplied; if not, the new flags are directly
+ *   applied to the AVFrame whose errors are concealed
+ */
+void ff_er_frame_end(ERContext *s, int *decode_error_flags);
 void ff_er_add_slice(ERContext *s, int startx, int starty, int endx, int endy,
  int status);
 
diff --git a/libavcodec/h263dec.c b/libavcodec/h263dec.c
index 52e51dd489..9f63f1a7cb 100644
--- a/libavcodec/h263dec.c
+++ b/libavcodec/h263dec.c
@@ -622,7 +622,7 @@ retry:
 av_assert1(s->bitstream_buffer_size == 0);
 frame_end:
 if (!s->studio_profile)
-ff_er_frame_end(>er);
+ff_er_frame_end(>er, NULL);
 
 if (avctx->hwaccel) {
 ret = FF_HW_SIMPLE_CALL(avctx, end_frame);
diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
index f13b1081fc..8e90678125 100644
--- a/libavcodec/h264dec.c
+++ b/libavcodec/h264dec.c
@@ -779,7 +779,7 @@ end:
 if (sl->ref_count[1])
 ff_h264_set_erpic(>er.next_pic, sl->ref_list[1][0].parent);
 
-ff_er_frame_end(>er);
+ff_er_frame_end(>er, NULL);
 if (use_last_pic)
 memset(>ref_list[0][0], 0, sizeof(sl->ref_list[0][0]));
 }
diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index 1accd07e9e..4b5341b914 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -2033,7 +2033,7 @@ static int slice_end(AVCodecContext *avctx, AVFrame *pict)
 if (/* s->mb_y << field_pic == s->mb_height && */ !s->first_field && 
!s1->first_slice) {
 /* end of image */
 
-ff_er_frame_end(>er);
+ff_er_frame_end(>er, NULL);
 
 ff_mpv_frame_end(s);
 
diff --git a/libavcodec/mss2.c b/libavcodec/mss2.c
index 70aa56cb84..2237cc8bb1 100644
--- a/libavcodec/mss2.c
+++ b/libavcodec/mss2.c
@@ -422,7 +422,7 @@ static int decode_wmv9(AVCodecContext *avctx, const uint8_t 
*buf, int buf_size,
 ff_vc1_decode_blocks(v);
 
 if (v->end_mb_x == s->mb_width && s->end_mb_y == s->mb_height) {
-ff_er_frame_end(>er);
+ff_er_frame_end(>er, NULL);
 } else {
 av_log(v->s.avctx, AV_LOG_WARNING,
"disabling error correction due to block count mismatch %dx%d 
!= %dx%d\n",
diff --git a/libavcodec/rv10.c b/libavcodec/rv10.c
index 5edd934f82..6abceade4e 100644
--- a/libavcodec/rv10.c
+++ b/libavcodec/rv10.c
@@ -477,7 +477,7 @@ static int rv10_decode_packet(AVCodecContext *avctx, const 
uint8_t *buf,
 if ((s->mb_x == 0 && s->mb_y == 0) || !s->current_picture_ptr) {
 // FIXME write parser so we always have 

Re: [FFmpeg-devel] [PATCH 1/3] error_resilience: set the decode_error_flags outside

2023-09-12 Thread Michael Niedermayer
On Tue, Sep 12, 2023 at 01:40:13PM +0200, Thomas Guillem via ffmpeg-devel wrote:
> This will allow to fix data-races when ff_er_frame_end() is called after
> ff_thread_finish_setup()
> ---
>  libavcodec/error_resilience.c | 12 ++--
>  libavcodec/error_resilience.h |  2 +-
>  libavcodec/h263dec.c  |  6 --
>  libavcodec/h264dec.c  |  3 ++-
>  libavcodec/mpeg12dec.c|  3 ++-
>  libavcodec/mss2.c |  8 +---
>  libavcodec/rv10.c | 10 --
>  libavcodec/rv34.c | 12 +---
>  libavcodec/vc1dec.c   |  6 --
>  9 files changed, 41 insertions(+), 21 deletions(-)
> 

[...]

> diff --git a/libavcodec/error_resilience.h b/libavcodec/error_resilience.h
> index 47cc8a4fc6..a8cf73c72e 100644
> --- a/libavcodec/error_resilience.h
> +++ b/libavcodec/error_resilience.h
> @@ -90,7 +90,7 @@ typedef struct ERContext {
>  } ERContext;
>  
>  void ff_er_frame_start(ERContext *s);
> -void ff_er_frame_end(ERContext *s);
> +int ff_er_frame_end(ERContext *s);

The return code needs to be documented


>  void ff_er_add_slice(ERContext *s, int startx, int starty, int endx, int 
> endy,
>   int status);
>  
> diff --git a/libavcodec/h263dec.c b/libavcodec/h263dec.c
> index 52e51dd489..3e83d90586 100644
> --- a/libavcodec/h263dec.c
> +++ b/libavcodec/h263dec.c
> @@ -621,8 +621,10 @@ retry:
>  
>  av_assert1(s->bitstream_buffer_size == 0);
>  frame_end:
> -if (!s->studio_profile)
> -ff_er_frame_end(>er);
> +if (!s->studio_profile) {
> +if (ff_er_frame_end(>er) > 0)
> +s->current_picture.f->decode_error_flags |= 
> FF_DECODE_ERROR_CONCEALMENT_ACTIVE;
> +}
[...]
> +static void rv10_er_frame_end(MpegEncContext *s)
> +{
> +if (ff_er_frame_end(>er) > 0)
> +s->current_picture_ptr->f->decode_error_flags |= 
> FF_DECODE_ERROR_CONCEALMENT_ACTIVE;
> +}
[...]
> +static void rv34_er_frame_end(MpegEncContext *s)
> +{
> +if (ff_er_frame_end(>er) > 0)
> +s->current_picture_ptr->f->decode_error_flags |= 
> FF_DECODE_ERROR_CONCEALMENT_ACTIVE;
> +}
[...]
> +if (ff_er_frame_end(>er) > 0)
> +s->current_picture.f->decode_error_flags |= 
> FF_DECODE_ERROR_CONCEALMENT_ACTIVE;
> +}
>  }

This looks like duplicated code


[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

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


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

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


[FFmpeg-devel] [PATCH] avcodec/libkvazaar: Respect codec context color settings.

2023-09-12 Thread John Mather via ffmpeg-devel
This patch makes the libkvazaar encoder respect color settings that are
present on the codec context, including color range, primaries, transfer
function and colorspace.
---
 libavcodec/libkvazaar.c | 55 +
 1 file changed, 55 insertions(+)

diff --git a/libavcodec/libkvazaar.c b/libavcodec/libkvazaar.c
index 2ef34dd82e..cd87a9106d 100644
--- a/libavcodec/libkvazaar.c
+++ b/libavcodec/libkvazaar.c
@@ -56,6 +56,10 @@ static av_cold int libkvazaar_init(AVCodecContext *avctx)
 const kvz_api *const api = ctx->api = kvz_api_get(8);
 kvz_config *cfg = NULL;
 kvz_encoder *enc = NULL;
+int8_t kvz_fullrange = -1;
+int8_t kvz_colorprim = -1;
+int8_t kvz_transfer = -1;
+int8_t kvz_colormatrix = -1;
 
 /* Kvazaar requires width and height to be multiples of eight. */
 if (avctx->width % 8 || avctx->height % 8) {
@@ -101,6 +105,57 @@ FF_ENABLE_DEPRECATION_WARNINGS
 cfg->rc_algorithm = KVZ_LAMBDA;
 }
 
+switch (avctx->color_range) {
+case AVCOL_RANGE_JPEG: kvz_fullrange = 1; break;
+case AVCOL_RANGE_MPEG: kvz_fullrange = 0; break;
+default:   kvz_fullrange = 0;
+}
+cfg->vui.fullrange = kvz_fullrange;
+
+switch (avctx->color_primaries) {
+case AVCOL_PRI_BT709: kvz_colorprim = 1; break;
+case AVCOL_PRI_BT470M:kvz_colorprim = 4; break;
+case AVCOL_PRI_BT470BG:   kvz_colorprim = 5; break;
+case AVCOL_PRI_SMPTE170M: kvz_colorprim = 6; break;
+case AVCOL_PRI_SMPTE240M: kvz_colorprim = 7; break;
+case AVCOL_PRI_FILM:  kvz_colorprim = 8; break;
+case AVCOL_PRI_BT2020:kvz_colorprim = 9; break;
+default:  kvz_colorprim = 2;// undef
+}
+cfg->vui.colorprim = kvz_colorprim;
+
+switch (avctx->color_trc) {
+case AVCOL_TRC_BT709:kvz_transfer = 1;  break;
+case AVCOL_TRC_GAMMA22:  kvz_transfer = 4;  break;  // bt470m
+case AVCOL_TRC_GAMMA28:  kvz_transfer = 5;  break;  // bt470bg
+case AVCOL_TRC_SMPTE170M:kvz_transfer = 6;  break;
+case AVCOL_TRC_SMPTE240M:kvz_transfer = 7;  break;
+case AVCOL_TRC_LINEAR:   kvz_transfer = 8;  break;
+case AVCOL_TRC_LOG:  kvz_transfer = 9;  break;  // log100
+case AVCOL_TRC_LOG_SQRT: kvz_transfer = 10; break;  // log316
+case AVCOL_TRC_IEC61966_2_4: kvz_transfer = 11; break;
+case AVCOL_TRC_BT1361_ECG:   kvz_transfer = 12; break;  // bt1361e
+case AVCOL_TRC_IEC61966_2_1: kvz_transfer = 13; break;
+case AVCOL_TRC_BT2020_10:kvz_transfer = 14; break;
+case AVCOL_TRC_BT2020_12:kvz_transfer = 15; break;
+default: kvz_transfer = 2;  // undef
+}
+cfg->vui.transfer = kvz_transfer;
+
+switch (avctx->colorspace) {
+case AVCOL_SPC_RGB:kvz_colormatrix = 0;  break; // gbr
+case AVCOL_SPC_BT709:  kvz_colormatrix = 1;  break;
+case AVCOL_SPC_FCC:kvz_colormatrix = 5;  break;
+case AVCOL_SPC_BT470BG:kvz_colormatrix = 6;  break;
+case AVCOL_SPC_SMPTE170M:  kvz_colormatrix = 7;  break;
+case AVCOL_SPC_SMPTE240M:  kvz_colormatrix = 8;  break;
+case AVCOL_SPC_YCGCO:  kvz_colormatrix = 9;  break;
+case AVCOL_SPC_BT2020_NCL: kvz_colormatrix = 10; break; // bt2020nc
+case AVCOL_SPC_BT2020_CL:  kvz_colormatrix = 11; break; // bt2020c
+default:   kvz_colormatrix = 2; // undef
+}
+cfg->vui.colormatrix = kvz_colormatrix;
+
 if (ctx->kvz_params) {
 AVDictionary *dict = NULL;
 if (!av_dict_parse_string(, ctx->kvz_params, "=", ",", 0)) {
-- 
2.39.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 03/10] avformat/avformat: use the side data from AVStream.codecpar

2023-09-12 Thread James Almer

On 9/12/2023 1:43 PM, Andreas Rheinhardt wrote:

James Almer:

On 9/11/2023 4:19 PM, Andreas Rheinhardt wrote:

James Almer:

Deprecate AVStream.side_data and its helpers in favor of the AVStream's
codecpar.side_data.

This will considerably simplify the propagation of global side data
to decoders
and from encoders. Instead of having to do it inside packets, it will be
available during init().
Global and frame specific side data will therefore be distinct.

Signed-off-by: James Almer 
---> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index 1916aa2dc5..f78a027f64 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -164,6 +164,13 @@
    * decoding functions avcodec_send_packet() or
avcodec_decode_subtitle2() if the
    * caller wishes to decode the data.
    *
+ * There may be no overlap between the stream's @ref
AVCodecParameters.side_data
+ * "side data" and @ref AVPacket.side_data "side data" in packets.
I.e. a given
+ * side data is either exported by the demuxer in AVCodecParameters,
then it never
+ * appears in the packets, or the side data is exported through the
packets (always
+ * in the first packet where the value becomes known or changes),
then it does not
+ * appear in AVCodecParameters.
+ *


Is it actually certain that our demuxers currently abide by this? E.g.
in mpegts, stream parameters can change at any time, so why does it set
it at the stream level and not the packet level?


Does mpegts change AVStream.codecpar mid demuxing? wouldn't that be an
API violation? I assumed that was what AV_PKT_DATA_PARAM_CHANGE and
AV_PKT_DATA_NEW_EXTRADATA packet side data were for.



It seems that the dovi stream side data can be added (and potentially
even replaced) long after the stream has been created.


Yeah, as well as other codecpar fields. How is the library user even 
meant to know this happened? The doxy states codecpar is filled on 
stream creation or during avformat_find_stream_info(), so they would not 
expect it to change after that.


AV_PKT_DATA_PARAM_CHANGE seems to me that it's the proper way to 
propagate these changes, yet it looks underused and fuzzily defined.

Maybe it should be expanded and improved for this.


(The concat demuxer may even set extradata lateron and even change it
after it has been allocated; see match_streams() in
concat_read_packet(). The concat demuxer should actually add the global
side data of every input to the first packet from said input which means
that copying side data (whether in ff_stream_side_data_copy() or in
avcodec_parameters_copy() is potentially problematic.)




    * AVPacket.pts, AVPacket.dts and AVPacket.duration timing
information will be
    * set if known. They may also be unset (i.e. AV_NOPTS_VALUE for
    * pts/dts, 0 for duration) if the stream does not provide them.
The timing
@@ -209,6 +216,11 @@
    *   AVCodecParameters, rather than using @ref
avcodec_parameters_copy() during
    *   remuxing: there is no guarantee that the codec context values
remain valid
    *   for both input and output format contexts.
+ * - There may be no overlap between AVCodecParameters.side_data and
side data in
+ *   packets. I.e. a given side data is either set by the caller in
+ *   AVCodecParameters, then it never appears in the packets, or the
side data is
+ *   sent through the packets (always in the first packet where the
value becomes
+ *   known or changes), then it does not appear in AVCodecParameters.


I have to say, I don't really like this (and of course I am aware that
you are basically copying the doxy of AVPacketSideData here). As you


I can remove this part, to be added later if needed.


know, the Matroska muxer needs to add header fields in order to add
certain packet side data to blocks later. In case of seekable output,
one can update the header later, but in case of unseekable output that
is not true. I'd like there to be an easy way for the user to signal the
intention to send packet side data of a specific type later.


Maybe a new AVFMT_FLAG_?



That would potentially be a new AVFMT_FLAG_ for every side data type;
that makes no sense.


Yeah, missed the "specific type" part, and assumed you meant only a way 
to signal a simple "Keep an eye for side data in packets" scenario.





    * - The caller may fill in additional information, such as @ref
    *   AVFormatContext.metadata "global" or @ref AVStream.metadata
"per-stream"
    *   metadata, @ref AVFormatContext.chapters "chapters", @ref
@@ -937,6 +949,7 @@ typedef struct AVStream {
    */
   AVPacket attached_pic;
   +#if FF_API_AVSTREAM_SIDE_DATA
   /**
    * An array of side data that applies to the whole stream (i.e.
the
    * container does not allow it to change between packets).
@@ -953,13 +966,20 @@ typedef struct AVStream {
    *
    * Freed by libavformat in avformat_free_context().
    *
- * @see av_format_inject_global_side_data()
+ * @deprecated use AVStream's @ref 

Re: [FFmpeg-devel] [PATCH 03/10] avformat/avformat: use the side data from AVStream.codecpar

2023-09-12 Thread Andreas Rheinhardt
James Almer:
> On 9/11/2023 4:19 PM, Andreas Rheinhardt wrote:
>> James Almer:
>>> Deprecate AVStream.side_data and its helpers in favor of the AVStream's
>>> codecpar.side_data.
>>>
>>> This will considerably simplify the propagation of global side data
>>> to decoders
>>> and from encoders. Instead of having to do it inside packets, it will be
>>> available during init().
>>> Global and frame specific side data will therefore be distinct.
>>>
>>> Signed-off-by: James Almer 
>>> ---> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>>> index 1916aa2dc5..f78a027f64 100644
>>> --- a/libavformat/avformat.h
>>> +++ b/libavformat/avformat.h
>>> @@ -164,6 +164,13 @@
>>>    * decoding functions avcodec_send_packet() or
>>> avcodec_decode_subtitle2() if the
>>>    * caller wishes to decode the data.
>>>    *
>>> + * There may be no overlap between the stream's @ref
>>> AVCodecParameters.side_data
>>> + * "side data" and @ref AVPacket.side_data "side data" in packets.
>>> I.e. a given
>>> + * side data is either exported by the demuxer in AVCodecParameters,
>>> then it never
>>> + * appears in the packets, or the side data is exported through the
>>> packets (always
>>> + * in the first packet where the value becomes known or changes),
>>> then it does not
>>> + * appear in AVCodecParameters.
>>> + *
>>
>> Is it actually certain that our demuxers currently abide by this? E.g.
>> in mpegts, stream parameters can change at any time, so why does it set
>> it at the stream level and not the packet level?
> 
> Does mpegts change AVStream.codecpar mid demuxing? wouldn't that be an
> API violation? I assumed that was what AV_PKT_DATA_PARAM_CHANGE and
> AV_PKT_DATA_NEW_EXTRADATA packet side data were for.
> 

It seems that the dovi stream side data can be added (and potentially
even replaced) long after the stream has been created.
(The concat demuxer may even set extradata lateron and even change it
after it has been allocated; see match_streams() in
concat_read_packet(). The concat demuxer should actually add the global
side data of every input to the first packet from said input which means
that copying side data (whether in ff_stream_side_data_copy() or in
avcodec_parameters_copy() is potentially problematic.)

>>
>>>    * AVPacket.pts, AVPacket.dts and AVPacket.duration timing
>>> information will be
>>>    * set if known. They may also be unset (i.e. AV_NOPTS_VALUE for
>>>    * pts/dts, 0 for duration) if the stream does not provide them.
>>> The timing
>>> @@ -209,6 +216,11 @@
>>>    *   AVCodecParameters, rather than using @ref
>>> avcodec_parameters_copy() during
>>>    *   remuxing: there is no guarantee that the codec context values
>>> remain valid
>>>    *   for both input and output format contexts.
>>> + * - There may be no overlap between AVCodecParameters.side_data and
>>> side data in
>>> + *   packets. I.e. a given side data is either set by the caller in
>>> + *   AVCodecParameters, then it never appears in the packets, or the
>>> side data is
>>> + *   sent through the packets (always in the first packet where the
>>> value becomes
>>> + *   known or changes), then it does not appear in AVCodecParameters.
>>
>> I have to say, I don't really like this (and of course I am aware that
>> you are basically copying the doxy of AVPacketSideData here). As you
> 
> I can remove this part, to be added later if needed.
> 
>> know, the Matroska muxer needs to add header fields in order to add
>> certain packet side data to blocks later. In case of seekable output,
>> one can update the header later, but in case of unseekable output that
>> is not true. I'd like there to be an easy way for the user to signal the
>> intention to send packet side data of a specific type later.
> 
> Maybe a new AVFMT_FLAG_?
> 

That would potentially be a new AVFMT_FLAG_ for every side data type;
that makes no sense.

>>
>>>    * - The caller may fill in additional information, such as @ref
>>>    *   AVFormatContext.metadata "global" or @ref AVStream.metadata
>>> "per-stream"
>>>    *   metadata, @ref AVFormatContext.chapters "chapters", @ref
>>> @@ -937,6 +949,7 @@ typedef struct AVStream {
>>>    */
>>>   AVPacket attached_pic;
>>>   +#if FF_API_AVSTREAM_SIDE_DATA
>>>   /**
>>>    * An array of side data that applies to the whole stream (i.e.
>>> the
>>>    * container does not allow it to change between packets).
>>> @@ -953,13 +966,20 @@ typedef struct AVStream {
>>>    *
>>>    * Freed by libavformat in avformat_free_context().
>>>    *
>>> - * @see av_format_inject_global_side_data()
>>> + * @deprecated use AVStream's @ref AVCodecParameters.side_data
>>> + * "codecpar side data".
>>>    */
>>> +    attribute_deprecated
>>>   AVPacketSideData *side_data;
>>>   /**
>>>    * The number of elements in the AVStream.side_data array.
>>> + *
>>> + * @deprecated use AVStream's @ref AVCodecParameters.side_data
>>> + * "codecpar 

Re: [FFmpeg-devel] [PATCH 02/10] avcodec/codec_par: add side data to AVCodecParameters

2023-09-12 Thread James Almer

On 9/11/2023 2:45 PM, Andreas Rheinhardt wrote:

James Almer:

This will simplify the propagation of side data to decoders and from encoders.
Global side data will now reside in the AVCodecContext, thus be available
during init(), removing the need to propagate it inside packets.

Global and frame specific side data will therefore be distinct.

Signed-off-by: James Almer 
---
  libavcodec/codec_par.c | 81 ++
  libavcodec/codec_par.h |  6 
  2 files changed, 87 insertions(+)

diff --git a/libavcodec/codec_par.c b/libavcodec/codec_par.c
index a38a475dc7..c0c941c2b6 100644
--- a/libavcodec/codec_par.c
+++ b/libavcodec/codec_par.c
@@ -27,11 +27,13 @@
  #include "libavutil/mem.h"
  #include "avcodec.h"
  #include "codec_par.h"
+#include "packet.h"
  
  static void codec_parameters_reset(AVCodecParameters *par)

  {
  av_freep(>extradata);
  av_channel_layout_uninit(>ch_layout);
+av_packet_side_data_set_free(>side_data);
  
  memset(par, 0, sizeof(*par));
  
@@ -82,6 +84,8 @@ int avcodec_parameters_copy(AVCodecParameters *dst, const AVCodecParameters *src

  dst->ch_layout  = (AVChannelLayout){0};
  dst->extradata  = NULL;
  dst->extradata_size = 0;
+dst->side_data.sd= NULL;
+dst->side_data.nb_sd = 0;
  if (src->extradata) {
  dst->extradata = av_mallocz(src->extradata_size + 
AV_INPUT_BUFFER_PADDING_SIZE);
  if (!dst->extradata)
@@ -89,6 +93,32 @@ int avcodec_parameters_copy(AVCodecParameters *dst, const 
AVCodecParameters *src
  memcpy(dst->extradata, src->extradata, src->extradata_size);
  dst->extradata_size = src->extradata_size;
  }
+if (src->side_data.nb_sd) {
+const AVPacketSideDataSet *src_set = >side_data;
+AVPacketSideDataSet *dst_set = >side_data;
+
+dst_set->sd = av_calloc(src_set->nb_sd, sizeof(*dst_set->sd));
+if (!dst_set->sd)
+return AVERROR(ENOMEM);
+
+for (int i = 0; i < src_set->nb_sd; i++) {
+const AVPacketSideData *src_sd = src_set->sd[i];
+AVPacketSideData *dst_sd = av_mallocz(sizeof(*dst_sd));
+
+if (!dst_sd)
+return AVERROR(ENOMEM);
+
+dst_sd->data = av_memdup(src_sd->data, src_sd->size);
+if (!dst_sd->data) {
+return AVERROR(ENOMEM);
+av_free(dst_sd);
+}
+
+dst_sd->type = src_sd->type;
+dst_sd->size = src_sd->size;
+dst_set->sd[dst_set->nb_sd++] = dst_sd;
+}
+}
  
  ret = av_channel_layout_copy(>ch_layout, >ch_layout);

  if (ret < 0)
@@ -178,6 +208,32 @@ FF_ENABLE_DEPRECATION_WARNINGS
  par->extradata_size = codec->extradata_size;
  }
  
+if (codec->nb_coded_side_data) {

+AVPacketSideDataSet *dst_set = >side_data;
+
+dst_set->sd = av_calloc(codec->nb_coded_side_data, 
sizeof(*dst_set->sd));
+if (!dst_set->sd)
+return AVERROR(ENOMEM);
+
+for (int i = 0; i < codec->nb_coded_side_data; i++) {
+const AVPacketSideData *src_sd = >coded_side_data[i];
+AVPacketSideData *dst_sd = av_mallocz(sizeof(*dst_sd));
+
+if (!dst_sd)
+return AVERROR(ENOMEM);
+
+dst_sd->data = av_memdup(src_sd->data, src_sd->size);
+if (!dst_sd->data) {
+return AVERROR(ENOMEM);
+av_free(dst_sd);


No compiler warning for this?


Good catch. And no, i don't get one with GCC 13.




+}
+
+dst_sd->type = src_sd->type;
+dst_sd->size = src_sd->size;
+dst_set->sd[dst_set->nb_sd++] = dst_sd;
+}
+}
+
  return 0;
  }
  
@@ -262,5 +318,30 @@ FF_ENABLE_DEPRECATION_WARNINGS

  codec->extradata_size = par->extradata_size;
  }
  
+for (int i = 0; i < codec->nb_coded_side_data; i++)

+av_free(codec->coded_side_data[i].data);
+av_freep(>coded_side_data);
+codec->nb_coded_side_data = 0;
+if (par->side_data.nb_sd) {
+const AVPacketSideDataSet *src_set = >side_data;
+
+codec->coded_side_data = av_calloc(src_set->nb_sd, 
sizeof(*codec->coded_side_data));
+if (!codec->coded_side_data)
+return AVERROR(ENOMEM);
+
+for (int i = 0; i < src_set->nb_sd; i++) {
+const AVPacketSideData *src_sd = src_set->sd[i];
+AVPacketSideData *dst_sd = >coded_side_data[i];
+
+dst_sd->data = av_memdup(src_sd->data, src_sd->size);
+if (!dst_sd->data)
+return AVERROR(ENOMEM);
+
+dst_sd->type = src_sd->type;
+dst_sd->size = src_sd->size;
+codec->nb_coded_side_data++;
+}
+}
+
  return 0;
  }
diff --git a/libavcodec/codec_par.h b/libavcodec/codec_par.h
index add90fdb1e..169e595b7c 100644
--- a/libavcodec/codec_par.h
+++ b/libavcodec/codec_par.h
@@ -29,6 +29,7 @@
  #include 

Re: [FFmpeg-devel] [PATCH 03/10] avformat/avformat: use the side data from AVStream.codecpar

2023-09-12 Thread James Almer

On 9/11/2023 4:19 PM, Andreas Rheinhardt wrote:

James Almer:

Deprecate AVStream.side_data and its helpers in favor of the AVStream's
codecpar.side_data.

This will considerably simplify the propagation of global side data to decoders
and from encoders. Instead of having to do it inside packets, it will be
available during init().
Global and frame specific side data will therefore be distinct.

Signed-off-by: James Almer 
---> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index 1916aa2dc5..f78a027f64 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -164,6 +164,13 @@
   * decoding functions avcodec_send_packet() or avcodec_decode_subtitle2() if 
the
   * caller wishes to decode the data.
   *
+ * There may be no overlap between the stream's @ref 
AVCodecParameters.side_data
+ * "side data" and @ref AVPacket.side_data "side data" in packets. I.e. a given
+ * side data is either exported by the demuxer in AVCodecParameters, then it 
never
+ * appears in the packets, or the side data is exported through the packets 
(always
+ * in the first packet where the value becomes known or changes), then it does 
not
+ * appear in AVCodecParameters.
+ *


Is it actually certain that our demuxers currently abide by this? E.g.
in mpegts, stream parameters can change at any time, so why does it set
it at the stream level and not the packet level?


Does mpegts change AVStream.codecpar mid demuxing? wouldn't that be an 
API violation? I assumed that was what AV_PKT_DATA_PARAM_CHANGE and 
AV_PKT_DATA_NEW_EXTRADATA packet side data were for.





   * AVPacket.pts, AVPacket.dts and AVPacket.duration timing information will be
   * set if known. They may also be unset (i.e. AV_NOPTS_VALUE for
   * pts/dts, 0 for duration) if the stream does not provide them. The timing
@@ -209,6 +216,11 @@
   *   AVCodecParameters, rather than using @ref avcodec_parameters_copy() 
during
   *   remuxing: there is no guarantee that the codec context values remain 
valid
   *   for both input and output format contexts.
+ * - There may be no overlap between AVCodecParameters.side_data and side data 
in
+ *   packets. I.e. a given side data is either set by the caller in
+ *   AVCodecParameters, then it never appears in the packets, or the side data 
is
+ *   sent through the packets (always in the first packet where the value 
becomes
+ *   known or changes), then it does not appear in AVCodecParameters.


I have to say, I don't really like this (and of course I am aware that
you are basically copying the doxy of AVPacketSideData here). As you


I can remove this part, to be added later if needed.


know, the Matroska muxer needs to add header fields in order to add
certain packet side data to blocks later. In case of seekable output,
one can update the header later, but in case of unseekable output that
is not true. I'd like there to be an easy way for the user to signal the
intention to send packet side data of a specific type later.


Maybe a new AVFMT_FLAG_?




   * - The caller may fill in additional information, such as @ref
   *   AVFormatContext.metadata "global" or @ref AVStream.metadata "per-stream"
   *   metadata, @ref AVFormatContext.chapters "chapters", @ref
@@ -937,6 +949,7 @@ typedef struct AVStream {
   */
  AVPacket attached_pic;
  
+#if FF_API_AVSTREAM_SIDE_DATA

  /**
   * An array of side data that applies to the whole stream (i.e. the
   * container does not allow it to change between packets).
@@ -953,13 +966,20 @@ typedef struct AVStream {
   *
   * Freed by libavformat in avformat_free_context().
   *
- * @see av_format_inject_global_side_data()
+ * @deprecated use AVStream's @ref AVCodecParameters.side_data
+ * "codecpar side data".
   */
+attribute_deprecated
  AVPacketSideData *side_data;
  /**
   * The number of elements in the AVStream.side_data array.
+ *
+ * @deprecated use AVStream's @ref AVCodecParameters.side_data
+ * "codecpar side data".
   */
+attribute_deprecated
  intnb_side_data;
+#endif
  
  /**

   * Flags indicating events happening on the stream, a combination of
@@ -1715,11 +1735,18 @@ typedef struct AVFormatContext {
  int (*io_close2)(struct AVFormatContext *s, AVIOContext *pb);
  } AVFormatContext;
  
+#if FF_API_AVSTREAM_SIDE_DATA

  /**
   * This function will cause global side data to be injected in the next packet
   * of each stream as well as after any subsequent seek.
+ *
+ * @deprecated global side data is always available in every AVStream's
+ * @ref AVCodecParameters.side_data "codecpar side data" array.
+ * @see av_packet_side_data_set_get()
   */
+attribute_deprecated
  void av_format_inject_global_side_data(AVFormatContext *s);
+#endif
  
  /**

   * Returns the method used to set ctx->duration.
@@ -1844,6 +1871,7 @@ const AVClass *av_stream_get_class(void);
   */
  AVStream 

Re: [FFmpeg-devel] [RFC PATCH 1/3] aacdec: always skip the first 2048 samples if there's no side data

2023-09-12 Thread Lynne
Sep 12, 2023, 09:43 by andreas.rheinha...@outlook.com:

> Lynne:
>
>> For some reason, this was never set, which meant all **raw** AAC in ADTS
>> streams, except faac, had extra samples at the start.
>>
>> Despite this being a standard MDCT-based codec with a frame size of 1024,
>> hence a delay of 1024 samples at the start, all major encoders, excluding
>> faac and FFmpeg, use 2048 samples of padding.
>>
>> The FFmpeg encoder will be modified to also output 2048 samples of padding
>> at the start, to make it in line with other encoders.
>>
>
> Does this also have actual advantages besides "being in line with other
> encoders"?
>

Not really. I don't have an opinion on this. 1024 is the natural
delay of the codec, so maybe it would be best to leave it at that.


>> Yes, this leaves FATE pretty sad. Will fix it with the real version of the 
>> patch.
>>
>
> Didn't we once guess the number of skip samples like this, only for this
> guesswork to be removed intentionally? (This is not a rhetorical
> question; I thought it to be true, but I see that there is still code
> for faac in decode_fill(); maybe I misremember.)
>

I don't remember something like that. The faac workaround dates back
from 2012 (bfe735b5824c7d10ba42932a17d786db50e3b2d4), and it's only for faac.
It's less of a guess, as most encoders to use the FIL extension to signal
themselves.
___
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/avio: Don't use incompatible function pointer type for call

2023-09-12 Thread Andreas Rheinhardt
Andreas Rheinhardt:
> It is undefined behaviour even in cases where it works
> (it works because it is only a const uint8_t* vs. uint8_t* difference).
> 
> Instead add a cbuf parameter to pass a const buffer (for writing)
> as well as a parameter indicating whether we are reading or writing;
> retry_transfer_wrapper() itself then uses the correct function
> based upon this information.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
> Another approach; IMO the best of the three. What do you think?
> 
>  libavformat/avio.c | 16 +++-
>  1 file changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/libavformat/avio.c b/libavformat/avio.c
> index b6a192892a..b793a7546c 100644
> --- a/libavformat/avio.c
> +++ b/libavformat/avio.c
> @@ -348,10 +348,9 @@ fail:
>  }
>  
>  static inline int retry_transfer_wrapper(URLContext *h, uint8_t *buf,
> + const uint8_t *cbuf,
>   int size, int size_min,
> - int (*transfer_func)(URLContext *h,
> -  uint8_t *buf,
> -  int size))
> + int read)
>  {
>  int ret, len;
>  int fast_retries = 5;
> @@ -361,7 +360,8 @@ static inline int retry_transfer_wrapper(URLContext *h, 
> uint8_t *buf,
>  while (len < size_min) {
>  if (ff_check_interrupt(>interrupt_callback))
>  return AVERROR_EXIT;
> -ret = transfer_func(h, buf + len, size - len);
> +ret = read ? h->prot->url_read (h, buf + len, size - len):
> + h->prot->url_write(h, cbuf + len, size - len);
>  if (ret == AVERROR(EINTR))
>  continue;
>  if (h->flags & AVIO_FLAG_NONBLOCK)
> @@ -398,14 +398,14 @@ int ffurl_read2(void *urlcontext, uint8_t *buf, int 
> size)
>  
>  if (!(h->flags & AVIO_FLAG_READ))
>  return AVERROR(EIO);
> -return retry_transfer_wrapper(h, buf, size, 1, h->prot->url_read);
> +return retry_transfer_wrapper(h, buf, NULL, size, 1, 1);
>  }
>  
>  int ffurl_read_complete(URLContext *h, unsigned char *buf, int size)
>  {
>  if (!(h->flags & AVIO_FLAG_READ))
>  return AVERROR(EIO);
> -return retry_transfer_wrapper(h, buf, size, size, h->prot->url_read);
> +return retry_transfer_wrapper(h, buf, NULL, size, size, 1);
>  }
>  
>  #if FF_API_AVIO_WRITE_NONCONST
> @@ -422,9 +422,7 @@ int ffurl_write2(void *urlcontext, const uint8_t *buf, 
> int size)
>  if (h->max_packet_size && size > h->max_packet_size)
>  return AVERROR(EIO);
>  
> -return retry_transfer_wrapper(h, (unsigned char *)buf, size, size,
> -  (int (*)(struct URLContext *, uint8_t *, 
> int))
> -  h->prot->url_write);
> +return retry_transfer_wrapper(h, NULL, buf, size, size, 0);
>  }
>  
>  int64_t ffurl_seek2(void *urlcontext, int64_t pos, int whence)

Will apply this tonight unless there are objections.

- Andreas

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

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


Re: [FFmpeg-devel] [PATCH 3/3] h264: fix data-race with FF_DECODE_ERROR_DECODE_SLICES

2023-09-12 Thread Andreas Rheinhardt
Thomas Guillem via ffmpeg-devel:
> Same than the previous commit but with FF_DECODE_ERROR_DECODE_SLICES
> 
> Fix the following data-race:
> 
> WARNING: ThreadSanitizer: data race (pid=55935)
>   Write of size 4 at 0x7b509378 by thread T1 (mutexes: write M608):
> #0 decode_nal_units src/libavcodec/h264dec.c:742 (ffmpeg+0xb19dd6)
> #1 h264_decode_frame src/libavcodec/h264dec.c:1016 (ffmpeg+0xb19dd6)
> #2 frame_worker_thread src/libavcodec/pthread_frame.c:228 
> (ffmpeg+0xdeea7e)
> 
>   Previous read of size 4 at 0x7b509378 by thread T14 (mutexes: write 
> M610):
> #0 frame_copy_props src/libavutil/frame.c:321 (ffmpeg+0x1793759)
> #1 av_frame_replace src/libavutil/frame.c:530 (ffmpeg+0x1794714)
> #2 ff_thread_replace_frame src/libavcodec/utils.c:898 (ffmpeg+0xfb1d0f)
> #3 ff_h264_replace_picture src/libavcodec/h264_picture.c:159 
> (ffmpeg+0x149cd3d)
> #4 ff_h264_update_thread_context src/libavcodec/h264_slice.c:413 
> (ffmpeg+0x14abf04)
> #5 update_context_from_thread src/libavcodec/pthread_frame.c:355 
> (ffmpeg+0xdec39c)
> #6 submit_packet src/libavcodec/pthread_frame.c:494 (ffmpeg+0xdecee3)
> #7 ff_thread_decode_frame src/libavcodec/pthread_frame.c:545 
> (ffmpeg+0xdecee3)
> #8 decode_simple_internal src/libavcodec/decode.c:431 (ffmpeg+0x9e1e20)
> #9 decode_simple_receive_frame src/libavcodec/decode.c:607 
> (ffmpeg+0x9e1e20)
> #10 decode_receive_frame_internal src/libavcodec/decode.c:635 
> (ffmpeg+0x9e1e20)
> #11 avcodec_send_packet src/libavcodec/decode.c:732 (ffmpeg+0x9e28fa)
> #12 packet_decode src/fftools/ffmpeg_dec.c:555 (ffmpeg+0x229888)
> #13 decoder_thread src/fftools/ffmpeg_dec.c:702 (ffmpeg+0x229888)
> ---
>  libavcodec/h264dec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
> index 24e849fc5b..b82ca8f14f 100644
> --- a/libavcodec/h264dec.c
> +++ b/libavcodec/h264dec.c
> @@ -739,7 +739,7 @@ static int decode_nal_units(H264Context *h, const uint8_t 
> *buf, int buf_size)
>  
>  // set decode_error_flags to allow users to detect concealed decoding 
> errors
>  if ((ret < 0 || h->er.error_occurred) && h->cur_pic_ptr) {
> -h->cur_pic_ptr->f->decode_error_flags |= 
> FF_DECODE_ERROR_DECODE_SLICES;
> +h->cur_pic_ptr->decode_error_flags |= FF_DECODE_ERROR_DECODE_SLICES;
>  }
>  
>  ret = 0;

IIRC this does not work: The thread that decodes a frame is typically
not the same thread that outputs said frame. The H264Picture srcp in
output_frame() points to one of the H264Picture in the H264Context.DBP
of the outputting thread, not the decoding thread. The outputting
threads decode_error_flags will therefore always be zero.

My preferred way to fix this is to allocate the H264Pictures (or at
least the stuff needed by later decoding threads) separately and make
them shared between decoder threads (with the caveat that only the
actual decoding threads may modify them; and they may only do so in a
controlled manner (i.e. no changes after having signalled to finish the
picture etc.). But this would be a major rewrite which will probably
never happen.

In the meantime i will send an alternative patch for this.

- Andreas

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

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


[FFmpeg-devel] [PATCH 5/5] avformat/wtvdec: Avoid unnecessary allocations

2023-09-12 Thread Andreas Rheinhardt
Signed-off-by: Andreas Rheinhardt 
---
 libavformat/wtvdec.c | 47 ++--
 1 file changed, 19 insertions(+), 28 deletions(-)

diff --git a/libavformat/wtvdec.c b/libavformat/wtvdec.c
index 9fe00590c8..e70470f79b 100644
--- a/libavformat/wtvdec.c
+++ b/libavformat/wtvdec.c
@@ -460,71 +460,62 @@ done:
 
 static void get_tag(AVFormatContext *s, AVIOContext *pb, const char *key, int 
type, int length)
 {
-int buf_size;
-char *buf;
+char buf[LEN_PRETTY_GUID + 1], *bufp = buf;
+unsigned dict_flags = 0;
 
 if (!strcmp(key, "WM/MediaThumbType")) {
 avio_skip(pb, length);
 return;
 }
 
-buf_size = FFMIN(FFMAX(length + length / 2U, LEN_PRETTY_GUID) + 1, 
INT_MAX);
-buf = av_malloc(buf_size);
-if (!buf)
-return;
-
 if (type == 0 && length == 4) {
-snprintf(buf, buf_size, "%u", avio_rl32(pb));
+snprintf(buf, sizeof(buf), "%u", avio_rl32(pb));
 } else if (type == 1) {
-avio_get_str16le(pb, length, buf, buf_size);
-if (!strlen(buf)) {
-   av_free(buf);
+int buflen = FFMIN(length + length / 2U + 1, INT_MAX);
+bufp = av_malloc(buflen);
+if (!bufp)
+return;
+avio_get_str16le(pb, length, bufp, buflen);
+if (!*bufp) {
+   av_free(bufp);
return;
 }
+dict_flags = AV_DICT_DONT_STRDUP_VAL;
 } else if (type == 3 && length == 4) {
 strcpy(buf, avio_rl32(pb) ? "true" : "false");
 } else if (type == 4 && length == 8) {
 int64_t num = avio_rl64(pb);
 if (!strcmp(key, "WM/EncodingTime") ||
 !strcmp(key, "WM/MediaOriginalBroadcastDateTime")) {
-if (filetime_to_iso8601(buf, buf_size, num) < 0) {
-av_free(buf);
+if (filetime_to_iso8601(buf, sizeof(buf), num) < 0)
 return;
-}
 } else if (!strcmp(key, "WM/WMRVEncodeTime") ||
!strcmp(key, "WM/WMRVEndTime")) {
-if (crazytime_to_iso8601(buf, buf_size, num) < 0) {
-av_free(buf);
+if (crazytime_to_iso8601(buf, sizeof(buf), num) < 0)
 return;
-}
 } else if (!strcmp(key, "WM/WMRVExpirationDate")) {
-if (oledate_to_iso8601(buf, buf_size, num) < 0 ) {
-av_free(buf);
+if (oledate_to_iso8601(buf, sizeof(buf), num) < 0)
 return;
-}
 } else if (!strcmp(key, "WM/WMRVBitrate"))
-snprintf(buf, buf_size, "%f", av_int2double(num));
+snprintf(buf, sizeof(buf), "%f", av_int2double(num));
 else
-snprintf(buf, buf_size, "%"PRIi64, num);
+snprintf(buf, sizeof(buf), "%"PRIi64, num);
 } else if (type == 5 && length == 2) {
-snprintf(buf, buf_size, "%u", avio_rl16(pb));
+snprintf(buf, sizeof(buf), "%u", avio_rl16(pb));
 } else if (type == 6 && length == 16) {
 ff_asf_guid guid;
 avio_read(pb, guid, 16);
-snprintf(buf, buf_size, PRI_PRETTY_GUID, ARG_PRETTY_GUID(guid));
+snprintf(buf, sizeof(buf), PRI_PRETTY_GUID, ARG_PRETTY_GUID(guid));
 } else if (type == 2 && !strcmp(key, "WM/Picture")) {
 get_attachment(s, pb, length);
-av_freep();
 return;
 } else {
-av_freep();
 av_log(s, AV_LOG_WARNING, "unsupported metadata entry; key:%s, 
type:%d, length:0x%x\n", key, type, length);
 avio_skip(pb, length);
 return;
 }
 
-av_dict_set(>metadata, key, buf, 0);
-av_freep();
+av_dict_set(>metadata, key, bufp, dict_flags);
 }
 
 /**
-- 
2.34.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 4/5] avformat/wtvdec: Use smaller upper bound for buffer

2023-09-12 Thread Andreas Rheinhardt
Every code point in the BMP is representable with at most three bytes
in UTF-8 and every code point not in the BMP takes four bytes.
For each of the latter, the encoding of UTF-16 takes as many
bytes; for each of the former, it takes at most 3/2 as many.
Therefore one can decrease the size of the buffer allocated
here.

Signed-off-by: Andreas Rheinhardt 
---
 libavformat/wtvdec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/wtvdec.c b/libavformat/wtvdec.c
index 4ce4b6403e..9fe00590c8 100644
--- a/libavformat/wtvdec.c
+++ b/libavformat/wtvdec.c
@@ -468,7 +468,7 @@ static void get_tag(AVFormatContext *s, AVIOContext *pb, 
const char *key, int ty
 return;
 }
 
-buf_size = FFMIN(FFMAX(2U * length, LEN_PRETTY_GUID) + 1, INT_MAX);
+buf_size = FFMIN(FFMAX(length + length / 2U, LEN_PRETTY_GUID) + 1, 
INT_MAX);
 buf = av_malloc(buf_size);
 if (!buf)
 return;
-- 
2.34.1

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

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


[FFmpeg-devel] [PATCH 3/5] avformat/wtvdec: Fix signed integer overflow

2023-09-12 Thread Andreas Rheinhardt
Happens when length > INT_MAX / 2; use unsigned for the computation,
but restrict the value to INT_MAX, because avio_get_str16le()
accepts an int as buf_len argument. Notice that it can happen
that the string read by avio_get_str16le() is truncated in this case.

Signed-off-by: Andreas Rheinhardt 
---
 libavformat/wtvdec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/wtvdec.c b/libavformat/wtvdec.c
index 2de6dc2103..4ce4b6403e 100644
--- a/libavformat/wtvdec.c
+++ b/libavformat/wtvdec.c
@@ -468,7 +468,7 @@ static void get_tag(AVFormatContext *s, AVIOContext *pb, 
const char *key, int ty
 return;
 }
 
-buf_size = FFMAX(2*length, LEN_PRETTY_GUID) + 1;
+buf_size = FFMIN(FFMAX(2U * length, LEN_PRETTY_GUID) + 1, INT_MAX);
 buf = av_malloc(buf_size);
 if (!buf)
 return;
-- 
2.34.1

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

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


[FFmpeg-devel] [PATCH 2/5] avformat/wtvdec: Skip too big tags

2023-09-12 Thread Andreas Rheinhardt
get_tag() is not designed with negative length in mind;
in this case, it will allocate a very small buffer
(LEN_PRETTY_GUID + 1) and might call avio_get_str16le()
with a negative maxlen (which relies on these parameters
to be signed).

Signed-off-by: Andreas Rheinhardt 
---
 libavformat/wtvdec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/wtvdec.c b/libavformat/wtvdec.c
index 1103f5ba03..2de6dc2103 100644
--- a/libavformat/wtvdec.c
+++ b/libavformat/wtvdec.c
@@ -539,7 +539,7 @@ static void parse_legacy_attrib(AVFormatContext *s, 
AVIOContext *pb)
 ff_get_guid(pb, );
 type   = avio_rl32(pb);
 length = avio_rl32(pb);
-if (!length)
+if (length <= 0)
 break;
 if (ff_guidcmp(, ff_metadata_guid)) {
 av_log(s, AV_LOG_WARNING, "unknown guid "FF_PRI_GUID", expected 
metadata_guid; "
-- 
2.34.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 v27 2/2] avcodec/evc_decoder: Provided support for EVC decoder

2023-09-12 Thread Dawid Kozinski/Multimedia (PLT) /SRPOL/Staff Engineer/Samsung Electronics





> -Original Message-
> From: ffmpeg-devel  On Behalf Of James
> Almer
> Sent: poniedziałek, 11 września 2023 00:56
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v27 2/2] avcodec/evc_decoder: Provided
> support for EVC decoder
> 
> On 8/16/2023 8:11 AM, Dawid Kozinski wrote:
> > +/**
> > + * Initialize decoder
> > + * Create a decoder instance and allocate all the needed resources
> > + *
> > + * @param avctx codec context
> > + * @return 0 on success, negative error code on failure  */ static
> > +av_cold int libxevd_init(AVCodecContext *avctx) {
> > +XevdContext *xectx = avctx->priv_data;
> > +XEVD_CDSC *cdsc = &(xectx->cdsc);
> > +
> > +/* read configurations and set values for created descriptor
(XEVD_CDSC)
> */
> > +get_conf(avctx, cdsc);
> > +
> > +/* create decoder */
> > +xectx->id = xevd_create(&(xectx->cdsc), NULL);
> > +if (xectx->id == NULL) {
> > +av_log(avctx, AV_LOG_ERROR, "Cannot create XEVD encoder\n");
> > +return AVERROR_EXTERNAL;
> > +}
> > +
> > +xectx->draining_mode = 0;
> > +xectx->pkt = av_packet_alloc();
> 
> Unchecked allocation.
> 
> > +
> > +return 0;
> > +}
> > +
> > +/**
> > +  * Decode frame with decoupled packet/frame dataflow
> > +  *
> > +  * @param avctx codec context
> > +  * @param[out] frame decoded frame
> > +  *
> > +  * @return 0 on success, negative error code on failure
> > +  */
> > +static int libxevd_receive_frame(AVCodecContext *avctx, AVFrame
> > +*frame) {
> > +XevdContext *xectx = avctx->priv_data;
> > +AVPacket *pkt = xectx->pkt;
> > +XEVD_IMGB *imgb = NULL;
> > +
> > +int xevd_ret = 0;
> > +int ret = 0;
> > +
> > +if (!pkt)
> > +return AVERROR(ENOMEM);
> 
> This check should be in libxevd_init(), like i said above.
> 
> > +
> > +// obtain access unit (input data) - a set of NAL units that are
consecutive in
> decoding order and containing exactly one encoded image
> > +ret = ff_decode_get_packet(avctx, pkt);
> 
> You're unconditionally fetching a new packet every time receive_frame() is
> called. Is it guaranteed that the previous packet was fully consumed and
freed?
> 
> > +if (ret < 0 && ret != AVERROR_EOF) {
> > +av_packet_unref(pkt);
> > +
> > +return ret;
> > +} else if(ret == AVERROR_EOF && xectx->draining_mode == 0) { //
> > + End of stream situations. Enter draining mode
> > +
> > +xectx->draining_mode = 1;
> > +av_packet_unref(pkt);
> > +}
> > +
> > +if (pkt->size > 0) {
> > +int bs_read_pos = 0;
> > +XEVD_STAT stat;
> > +XEVD_BITB bitb;
> > +int nalu_size;
> > +AVPacket* pkt_au;
> > +imgb = NULL;
> > +
> > +pkt_au = av_packet_clone(pkt);
> 
> Unchecked allocation.
> 
> > +av_packet_unref(pkt);
> 
> You're unreferencing this packet here but then you check its fields below.
> 
> > +
> > +// get all nal units from AU
> > +while(pkt_au->size > (bs_read_pos + XEVD_NAL_UNIT_LENGTH_BYTE))
{
> > +memset(, 0, sizeof(XEVD_STAT));
> > +
> > +nalu_size = read_nal_unit_length(pkt_au->data +
bs_read_pos,
> XEVD_NAL_UNIT_LENGTH_BYTE, avctx);
> > +if (nalu_size == 0) {
> > +av_log(avctx, AV_LOG_ERROR, "Invalid bitstream\n");
> > +av_packet_free(_au);
> > +ret = AVERROR_INVALIDDATA;
> > +
> > +return ret;
> > +}
> > +bs_read_pos += XEVD_NAL_UNIT_LENGTH_BYTE;
> > +
> > +bitb.addr = pkt_au->data + bs_read_pos;
> > +bitb.ssize = nalu_size;
> > +bitb.pdata[0] = pkt_au;
> > +bitb.ts[XEVD_TS_DTS] = pkt_au->dts;
> > +
> > +/* main decoding block */
> > +xevd_ret = xevd_decode(xectx->id, , );
> > +if (XEVD_FAILED(xevd_ret)) {
> > +av_log(avctx, AV_LOG_ERROR, "Failed to decode
bitstream\n");
> > +av_packet_free(_au);
> > +ret = AVERROR_EXTERNAL;
> 
> You can just do return AVERROR_EXTERNAL;
> 
> > +
> > +return ret;
> > +}
> > +
> > +bs_read_pos += nalu_size;
> > +
> > +if (stat.nalu_type == XEVD_NUT_SPS) { // EVC stream
parameters
> changed
> > +if ((ret = export_stream_params(xectx, avctx)) != 0) {
> > +av_log(avctx, AV_LOG_ERROR, "Failed to export
stream
> params\n");
> > +av_packet_free(_au);
> > +
> > +return ret;
> > +}
> > +}
> > +
> > +if (stat.read != nalu_size)
> > +av_log(avctx, AV_LOG_INFO, "Different reading of
> > + bitstream (in:%d,read:%d)\n,", nalu_size, stat.read);
> > +
> > +// stat.fnum - has negative value if the decoded data is
not frame
> > +if (stat.fnum >= 0) {
> 
> This means there can be more than one frame after a call to
> 

[FFmpeg-devel] [PATCH 1/5] avformat/wtvdec: Don't truncate GUIDs

2023-09-12 Thread Andreas Rheinhardt
Each of the 16 bytes of a GUID is written as a two-character
hex value and three hyphens, leading to a length of 35.
GCC 13 emits a -Wformat-truncation= warning because of this.

Signed-off-by: Andreas Rheinhardt 
---
Is there actually a reason this is using a different format than
the one used by lavu/uuid.h?

 libavformat/wtvdec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/wtvdec.c b/libavformat/wtvdec.c
index b29ea7a923..1103f5ba03 100644
--- a/libavformat/wtvdec.c
+++ b/libavformat/wtvdec.c
@@ -43,7 +43,7 @@
 "%08"PRIx32"-%04"PRIx16"-%04"PRIx16"-%02x%02x%02x%02x%02x%02x%02x%02x"
 #define ARG_PRETTY_GUID(g) \
 
AV_RL32(g),AV_RL16(g+4),AV_RL16(g+6),g[8],g[9],g[10],g[11],g[12],g[13],g[14],g[15]
-#define LEN_PRETTY_GUID 34
+#define LEN_PRETTY_GUID 35
 
 /*
  * File system routines
-- 
2.34.1

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

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


Re: [FFmpeg-devel] [PATCH 1/4] lavc/aarch64: new optimization for 8-bit hevc_epel_uni_v

2023-09-12 Thread Martin Storsjö

Hi,

Sorry for not tending to your patches sooner.

Unfortunately, this patchset is impossible to apply - there seems to be 
garbled whitespace in the patch which would require me to manually apply 
all the changes.


Can you try sending the patches again in a way that doesn't corrupt 
whitespace? If not, can you push the branch somewhere where I can fetch 
it?


// Martin

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

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


[FFmpeg-devel] [PATCH 2/3] h264: fix data-race with FF_DECODE_ERROR_CONCEALMENT_ACTIVE

2023-09-12 Thread Thomas Guillem via ffmpeg-devel
Set the FF_DECODE_ERROR_CONCEALMENT_ACTIVE flags on the AVFrane before
outputing it. Store in in the H264Picture in the meantime, where it
won't be read/write by other threads.

Fix the following data-race:

WARNING: ThreadSanitizer: data race (pid=55134)
  Write of size 4 at 0x7b507f78 by thread T1 (mutexes: write M58):
#0 decode_nal_units src/libavcodec/h264dec.c:783 (ffmpeg+0xb1a678)
#1 h264_decode_frame src/libavcodec/h264dec.c:1014 (ffmpeg+0xb1a678)
#2 frame_worker_thread src/libavcodec/pthread_frame.c:228 (ffmpeg+0xdeea6e)

  Previous read of size 4 at 0x7b507f78 by thread T14 (mutexes: write M60):
#0 frame_copy_props src/libavutil/frame.c:321 (ffmpeg+0x1793739)
#1 av_frame_replace src/libavutil/frame.c:530 (ffmpeg+0x17946f4)
#2 ff_thread_replace_frame src/libavcodec/utils.c:898 (ffmpeg+0xfb1cff)
#3 ff_h264_replace_picture src/libavcodec/h264_picture.c:159 
(ffmpeg+0x149cd2d)
#4 ff_h264_update_thread_context src/libavcodec/h264_slice.c:413 
(ffmpeg+0x14abee4)
#5 update_context_from_thread src/libavcodec/pthread_frame.c:355 
(ffmpeg+0xdec38c)
#6 submit_packet src/libavcodec/pthread_frame.c:494 (ffmpeg+0xdeced3)
#7 ff_thread_decode_frame src/libavcodec/pthread_frame.c:545 
(ffmpeg+0xdeced3)
#8 decode_simple_internal src/libavcodec/decode.c:431 (ffmpeg+0x9e1e20)
#9 decode_simple_receive_frame src/libavcodec/decode.c:607 (ffmpeg+0x9e1e20)
#10 decode_receive_frame_internal src/libavcodec/decode.c:635 
(ffmpeg+0x9e1e20)
#11 avcodec_send_packet src/libavcodec/decode.c:732 (ffmpeg+0x9e28fa)
#12 packet_decode src/fftools/ffmpeg_dec.c:555 (ffmpeg+0x229888)
#13 decoder_thread src/fftools/ffmpeg_dec.c:702 (ffmpeg+0x229888)
---
 libavcodec/h264_slice.c | 1 +
 libavcodec/h264dec.c| 4 +++-
 libavcodec/h264dec.h| 2 ++
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
index 6cd7bb8fe7..249c764d13 100644
--- a/libavcodec/h264_slice.c
+++ b/libavcodec/h264_slice.c
@@ -535,6 +535,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
 pic->f->crop_right  = h->crop_right;
 pic->f->crop_top= h->crop_top;
 pic->f->crop_bottom = h->crop_bottom;
+pic->decode_error_flags = 0;
 
 pic->needs_fg = h->sei.common.film_grain_characteristics.present && 
!h->avctx->hwaccel &&
 !(h->avctx->export_side_data & AV_CODEC_EXPORT_DATA_FILM_GRAIN);
diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
index 553f300c3d..24e849fc5b 100644
--- a/libavcodec/h264dec.c
+++ b/libavcodec/h264dec.c
@@ -780,7 +780,7 @@ end:
 ff_h264_set_erpic(>er.next_pic, sl->ref_list[1][0].parent);
 
 if (ff_er_frame_end(>er) > 0)
-h->cur_pic_ptr->f->decode_error_flags |= 
FF_DECODE_ERROR_CONCEALMENT_ACTIVE;
+h->cur_pic_ptr->decode_error_flags |= 
FF_DECODE_ERROR_CONCEALMENT_ACTIVE;
 if (use_last_pic)
 memset(>ref_list[0][0], 0, sizeof(sl->ref_list[0][0]));
 }
@@ -849,6 +849,8 @@ static int output_frame(H264Context *h, AVFrame *dst, 
H264Picture *srcp)
 if (ret < 0)
 return ret;
 
+dst->decode_error_flags |= srcp->decode_error_flags;
+
 if (srcp->needs_fg && (ret = av_frame_copy_props(dst, srcp->f)) < 0)
 return ret;
 
diff --git a/libavcodec/h264dec.h b/libavcodec/h264dec.h
index beaab3902c..cb3d5cef00 100644
--- a/libavcodec/h264dec.h
+++ b/libavcodec/h264dec.h
@@ -152,6 +152,8 @@ typedef struct H264Picture {
 
 int mb_width, mb_height;
 int mb_stride;
+
+int decode_error_flags;
 } H264Picture;
 
 typedef struct H264Ref {
-- 
2.39.2

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

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


[FFmpeg-devel] [PATCH 3/3] h264: fix data-race with FF_DECODE_ERROR_DECODE_SLICES

2023-09-12 Thread Thomas Guillem via ffmpeg-devel
Same than the previous commit but with FF_DECODE_ERROR_DECODE_SLICES

Fix the following data-race:

WARNING: ThreadSanitizer: data race (pid=55935)
  Write of size 4 at 0x7b509378 by thread T1 (mutexes: write M608):
#0 decode_nal_units src/libavcodec/h264dec.c:742 (ffmpeg+0xb19dd6)
#1 h264_decode_frame src/libavcodec/h264dec.c:1016 (ffmpeg+0xb19dd6)
#2 frame_worker_thread src/libavcodec/pthread_frame.c:228 (ffmpeg+0xdeea7e)

  Previous read of size 4 at 0x7b509378 by thread T14 (mutexes: write M610):
#0 frame_copy_props src/libavutil/frame.c:321 (ffmpeg+0x1793759)
#1 av_frame_replace src/libavutil/frame.c:530 (ffmpeg+0x1794714)
#2 ff_thread_replace_frame src/libavcodec/utils.c:898 (ffmpeg+0xfb1d0f)
#3 ff_h264_replace_picture src/libavcodec/h264_picture.c:159 
(ffmpeg+0x149cd3d)
#4 ff_h264_update_thread_context src/libavcodec/h264_slice.c:413 
(ffmpeg+0x14abf04)
#5 update_context_from_thread src/libavcodec/pthread_frame.c:355 
(ffmpeg+0xdec39c)
#6 submit_packet src/libavcodec/pthread_frame.c:494 (ffmpeg+0xdecee3)
#7 ff_thread_decode_frame src/libavcodec/pthread_frame.c:545 
(ffmpeg+0xdecee3)
#8 decode_simple_internal src/libavcodec/decode.c:431 (ffmpeg+0x9e1e20)
#9 decode_simple_receive_frame src/libavcodec/decode.c:607 (ffmpeg+0x9e1e20)
#10 decode_receive_frame_internal src/libavcodec/decode.c:635 
(ffmpeg+0x9e1e20)
#11 avcodec_send_packet src/libavcodec/decode.c:732 (ffmpeg+0x9e28fa)
#12 packet_decode src/fftools/ffmpeg_dec.c:555 (ffmpeg+0x229888)
#13 decoder_thread src/fftools/ffmpeg_dec.c:702 (ffmpeg+0x229888)
---
 libavcodec/h264dec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
index 24e849fc5b..b82ca8f14f 100644
--- a/libavcodec/h264dec.c
+++ b/libavcodec/h264dec.c
@@ -739,7 +739,7 @@ static int decode_nal_units(H264Context *h, const uint8_t 
*buf, int buf_size)
 
 // set decode_error_flags to allow users to detect concealed decoding 
errors
 if ((ret < 0 || h->er.error_occurred) && h->cur_pic_ptr) {
-h->cur_pic_ptr->f->decode_error_flags |= FF_DECODE_ERROR_DECODE_SLICES;
+h->cur_pic_ptr->decode_error_flags |= FF_DECODE_ERROR_DECODE_SLICES;
 }
 
 ret = 0;
-- 
2.39.2

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

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


[FFmpeg-devel] [PATCH 1/3] error_resilience: set the decode_error_flags outside

2023-09-12 Thread Thomas Guillem via ffmpeg-devel
This will allow to fix data-races when ff_er_frame_end() is called after
ff_thread_finish_setup()
---
 libavcodec/error_resilience.c | 12 ++--
 libavcodec/error_resilience.h |  2 +-
 libavcodec/h263dec.c  |  6 --
 libavcodec/h264dec.c  |  3 ++-
 libavcodec/mpeg12dec.c|  3 ++-
 libavcodec/mss2.c |  8 +---
 libavcodec/rv10.c | 10 --
 libavcodec/rv34.c | 12 +---
 libavcodec/vc1dec.c   |  6 --
 9 files changed, 41 insertions(+), 21 deletions(-)

diff --git a/libavcodec/error_resilience.c b/libavcodec/error_resilience.c
index 68e20925e0..1f43b233ff 100644
--- a/libavcodec/error_resilience.c
+++ b/libavcodec/error_resilience.c
@@ -889,7 +889,7 @@ void ff_er_add_slice(ERContext *s, int startx, int starty,
 }
 }
 
-void ff_er_frame_end(ERContext *s)
+int ff_er_frame_end(ERContext *s)
 {
 int *linesize = NULL;
 int i, mb_x, mb_y, error, error_type, dc_error, mv_error, ac_error;
@@ -906,7 +906,7 @@ void ff_er_frame_end(ERContext *s)
 !er_supported(s)   ||
 atomic_load(>error_count) == 3 * s->mb_width *
   (s->avctx->skip_top + s->avctx->skip_bottom)) {
-return;
+return 0;
 }
 linesize = s->cur_pic.f->linesize;
 
@@ -921,7 +921,7 @@ void ff_er_frame_end(ERContext *s)
 
 if (mb_x == s->mb_width) {
 av_log(s->avctx, AV_LOG_DEBUG, "ignoring last missing slice\n");
-return;
+return 0;
 }
 }
 
@@ -960,7 +960,7 @@ void ff_er_frame_end(ERContext *s)
 s->cur_pic.ref_index[i]  = NULL;
 s->cur_pic.motion_val[i] = NULL;
 }
-return;
+return 0;
 }
 }
 
@@ -1114,8 +1114,6 @@ void ff_er_frame_end(ERContext *s)
 av_log(s->avctx, AV_LOG_INFO, "concealing %d DC, %d AC, %d MV errors in %c 
frame\n",
dc_error, ac_error, mv_error, 
av_get_picture_type_char(s->cur_pic.f->pict_type));
 
-s->cur_pic.f->decode_error_flags |= FF_DECODE_ERROR_CONCEALMENT_ACTIVE;
-
 is_intra_likely = is_intra_more_likely(s);
 
 /* set unknown mb-type to most likely */
@@ -1352,4 +1350,6 @@ void ff_er_frame_end(ERContext *s)
 memset(>cur_pic, 0, sizeof(ERPicture));
 memset(>last_pic, 0, sizeof(ERPicture));
 memset(>next_pic, 0, sizeof(ERPicture));
+
+return 1;
 }
diff --git a/libavcodec/error_resilience.h b/libavcodec/error_resilience.h
index 47cc8a4fc6..a8cf73c72e 100644
--- a/libavcodec/error_resilience.h
+++ b/libavcodec/error_resilience.h
@@ -90,7 +90,7 @@ typedef struct ERContext {
 } ERContext;
 
 void ff_er_frame_start(ERContext *s);
-void ff_er_frame_end(ERContext *s);
+int ff_er_frame_end(ERContext *s);
 void ff_er_add_slice(ERContext *s, int startx, int starty, int endx, int endy,
  int status);
 
diff --git a/libavcodec/h263dec.c b/libavcodec/h263dec.c
index 52e51dd489..3e83d90586 100644
--- a/libavcodec/h263dec.c
+++ b/libavcodec/h263dec.c
@@ -621,8 +621,10 @@ retry:
 
 av_assert1(s->bitstream_buffer_size == 0);
 frame_end:
-if (!s->studio_profile)
-ff_er_frame_end(>er);
+if (!s->studio_profile) {
+if (ff_er_frame_end(>er) > 0)
+s->current_picture.f->decode_error_flags |= 
FF_DECODE_ERROR_CONCEALMENT_ACTIVE;
+}
 
 if (avctx->hwaccel) {
 ret = FF_HW_SIMPLE_CALL(avctx, end_frame);
diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
index f13b1081fc..553f300c3d 100644
--- a/libavcodec/h264dec.c
+++ b/libavcodec/h264dec.c
@@ -779,7 +779,8 @@ end:
 if (sl->ref_count[1])
 ff_h264_set_erpic(>er.next_pic, sl->ref_list[1][0].parent);
 
-ff_er_frame_end(>er);
+if (ff_er_frame_end(>er) > 0)
+h->cur_pic_ptr->f->decode_error_flags |= 
FF_DECODE_ERROR_CONCEALMENT_ACTIVE;
 if (use_last_pic)
 memset(>ref_list[0][0], 0, sizeof(sl->ref_list[0][0]));
 }
diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index 1accd07e9e..b2cc78c3d3 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -2033,7 +2033,8 @@ static int slice_end(AVCodecContext *avctx, AVFrame *pict)
 if (/* s->mb_y << field_pic == s->mb_height && */ !s->first_field && 
!s1->first_slice) {
 /* end of image */
 
-ff_er_frame_end(>er);
+if (ff_er_frame_end(>er) > 0)
+s->current_picture_ptr->f->decode_error_flags |= 
FF_DECODE_ERROR_CONCEALMENT_ACTIVE;
 
 ff_mpv_frame_end(s);
 
diff --git a/libavcodec/mss2.c b/libavcodec/mss2.c
index 70aa56cb84..29cb4be614 100644
--- a/libavcodec/mss2.c
+++ b/libavcodec/mss2.c
@@ -421,8 +421,12 @@ static int decode_wmv9(AVCodecContext *avctx, const 
uint8_t *buf, int buf_size,
 
 ff_vc1_decode_blocks(v);
 
+f = s->current_picture.f;
+
 if (v->end_mb_x == s->mb_width && s->end_mb_y == s->mb_height) {
-ff_er_frame_end(>er);
+if 

Re: [FFmpeg-devel] [RFC PATCH 1/3] aacdec: always skip the first 2048 samples if there's no side data

2023-09-12 Thread Andreas Rheinhardt
Lynne:
> For some reason, this was never set, which meant all **raw** AAC in ADTS
> streams, except faac, had extra samples at the start.
> 
> Despite this being a standard MDCT-based codec with a frame size of 1024,
> hence a delay of 1024 samples at the start, all major encoders, excluding
> faac and FFmpeg, use 2048 samples of padding.
> 
> The FFmpeg encoder will be modified to also output 2048 samples of padding
> at the start, to make it in line with other encoders.

Does this also have actual advantages besides "being in line with other
encoders"?

> 
> Yes, this leaves FATE pretty sad. Will fix it with the real version of the 
> patch.
> 

Didn't we once guess the number of skip samples like this, only for this
guesswork to be removed intentionally? (This is not a rhetorical
question; I thought it to be true, but I see that there is still code
for faac in decode_fill(); maybe I misremember.)

- 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] [RFC PATCH 3/3] aacdec: allow to skip sbr start-up delay

2023-09-12 Thread Lynne
Sep 12, 2023, 08:14 by d...@lynne.ee:

> As it happens, there's no standard between startup delay for SBR between
> decoders either. libfdkaac uses 5056 samples, but Apple's encoder (via 
> afconvert)
> uses 3136.
>
> Currently, this only fixes libfdk-aac. Would like to have more samples from 
> more
> encoders so I can fix all known cases.
>

Wrong patch attached.

>From c0e5659e5a2d56e883f9e1bb8c6d5bca1059721a Mon Sep 17 00:00:00 2001
From: Lynne 
Date: Tue, 12 Sep 2023 08:06:40 +0200
Subject: [PATCH 3/3] aacdec: allow to skip sbr start-up delay

---
 libavcodec/aac.h |  1 +
 libavcodec/aacdec_template.c | 11 +++
 2 files changed, 12 insertions(+)

diff --git a/libavcodec/aac.h b/libavcodec/aac.h
index 285d3b7482..3f67f353b7 100644
--- a/libavcodec/aac.h
+++ b/libavcodec/aac.h
@@ -298,6 +298,7 @@ struct AACContext {
 AVCodecContext *avctx;
 AVFrame *frame;
 
+int sbr_state_changed;
 int is_saved; ///< Set if elements have stored overlap from previous frame.
 DynamicRangeControl che_drc;
 
diff --git a/libavcodec/aacdec_template.c b/libavcodec/aacdec_template.c
index 0e4a274fea..b6c6d19f61 100644
--- a/libavcodec/aacdec_template.c
+++ b/libavcodec/aacdec_template.c
@@ -1207,6 +1207,8 @@ static av_cold int aac_decode_init(AVCodecContext *avctx)
 avctx->extradata_size * 8LL,
 1)) < 0)
 return ret;
+
+ac->sbr_state_changed = ac->oc[1].m4ac.sbr == 1;
 } else {
 int sr, i;
 uint8_t layout_map[MAX_ELEM_ID*4][3];
@@ -3154,6 +3156,7 @@ static int aac_decode_frame_int(AVCodecContext *avctx, AVFrame *frame,
 int is_dmono, sce_count = 0;
 int payload_alignment;
 uint8_t che_presence[4][MAX_ELEM_ID] = {{0}};
+int sbr_state_start = ac->oc[1].m4ac.sbr;
 
 ac->frame = frame;
 
@@ -3346,6 +3349,14 @@ static int aac_decode_frame_int(AVCodecContext *avctx, AVFrame *frame,
 frame->data[0] = frame->data[1];
 }
 
+ac->sbr_state_changed |= (ac->oc[1].m4ac.sbr != sbr_state_start) && (ac->oc[1].m4ac.sbr == 1);
+
+if (ac->sbr_state_changed) {
+avctx->internal->skip_samples = 5056;
+avctx->internal->skip_samples_add = 1;
+ac->sbr_state_changed = 0;
+}
+
 return 0;
 fail:
 pop_output_configuration(ac);
-- 
2.40.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] [RFC PATCH 3/3] aacdec: allow to skip sbr start-up delay

2023-09-12 Thread Lynne
As it happens, there's no standard between startup delay for SBR between
decoders either. libfdkaac uses 5056 samples, but Apple's encoder (via 
afconvert)
uses 3136.

Currently, this only fixes libfdk-aac. Would like to have more samples from more
encoders so I can fix all known cases.

>From 079235e1f1a9caeadfd2b8d78b3fe2273d86018a Mon Sep 17 00:00:00 2001
From: Lynne 
Date: Fri, 11 Aug 2023 17:50:54 +0200
Subject: [PATCH 1/3] aacdec: always skip the first 2048 samples if there's no
 side data

For some reason, this was never set, which meant all **raw** AAC in ADTS
streams, except faac, had extra samples at the start.

Despite this being a standard MDCT-based codec with a frame size of 1024,
hence a delay of 1024 samples at the start, all major encoders, excluding
faac and FFmpeg, use 2048 samples of padding.

The FFmpeg encoder will be modified to also output 2048 samples of padding
at the start, to make it in line with other encoders.
---
 libavcodec/aacdec_template.c | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/libavcodec/aacdec_template.c b/libavcodec/aacdec_template.c
index f8039e490b..0e4a274fea 100644
--- a/libavcodec/aacdec_template.c
+++ b/libavcodec/aacdec_template.c
@@ -1273,6 +1273,9 @@ static av_cold int aac_decode_init(AVCodecContext *avctx)
 if (ret < 0)
 return ret;
 
+/* Usually overridden by side data */
+avctx->internal->skip_samples = 2048;
+
 return 0;
 }
 
@@ -2417,14 +2420,16 @@ static int decode_dynamic_range(DynamicRangeControl *che_drc,
 return n;
 }
 
-static int decode_fill(AACContext *ac, GetBitContext *gb, int len) {
+static int decode_fill(AACContext *ac, GetBitContext *gb, int len)
+{
 uint8_t buf[256];
-int i, major, minor;
+int i, major, minor, micro;
 
 if (len < 13+7*8)
 goto unknown;
 
-get_bits(gb, 13); len -= 13;
+get_bits(gb, 13);
+len -= 13;
 
 for(i=0; i+1=8; i++, len-=8)
 buf[i] = get_bits(gb, 8);
@@ -2434,7 +2439,11 @@ static int decode_fill(AACContext *ac, GetBitContext *gb, int len) {
 av_log(ac->avctx, AV_LOG_DEBUG, "FILL:%s\n", buf);
 
 if (sscanf(buf, "libfaac %d.%d", , ) == 2){
-ac->avctx->internal->skip_samples = 1024;
+ac->avctx->internal->skip_samples -= 1024;
+}
+
+if ((sscanf(buf, "avc %d.%d.%d", , , ) == 3)) {
+ac->avctx->internal->skip_samples -= 1024;
 }
 
 unknown:
-- 
2.40.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] [RFC PATCH 2/3] decode: allow decoders to override skip_samples

2023-09-12 Thread Lynne
Very hacky. There must be a better way of doing this.

>From 843809ac072bbaf9ae0d3d3946723f1fcfb07923 Mon Sep 17 00:00:00 2001
From: Lynne 
Date: Tue, 12 Sep 2023 08:00:02 +0200
Subject: [PATCH 2/3] decode: allow decoders to override skip_samples

---
 libavcodec/decode.c   | 5 -
 libavcodec/internal.h | 5 +
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 169ee79acd..9e8a4532f2 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -312,7 +312,10 @@ static int discard_samples(AVCodecContext *avctx, AVFrame *frame, int64_t *disca
 
 side = av_frame_get_side_data(frame, AV_FRAME_DATA_SKIP_SAMPLES);
 if (side && side->size >= 10) {
-avci->skip_samples = AV_RL32(side->data);
+if (avci->skip_samples_add)
+avci->skip_samples += AV_RL32(side->data);
+else
+avci->skip_samples = AV_RL32(side->data);
 avci->skip_samples = FFMAX(0, avci->skip_samples);
 discard_padding = AV_RL32(side->data + 4);
 av_log(avctx, AV_LOG_DEBUG, "skip %d / discard %d samples due to side data\n",
diff --git a/libavcodec/internal.h b/libavcodec/internal.h
index 83e0bc3fb2..8f59a117ba 100644
--- a/libavcodec/internal.h
+++ b/libavcodec/internal.h
@@ -118,6 +118,11 @@ typedef struct AVCodecInternal {
  */
 int skip_samples;
 
+/**
+ * In case there's side data, add it to skip_samples, instead of overriding it.
+ */
+int skip_samples_add;
+
 /**
  * hwaccel-specific private data
  */
-- 
2.40.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] [RFC PATCH 1/3] aacdec: always skip the first 2048 samples if there's no side data

2023-09-12 Thread Lynne
For some reason, this was never set, which meant all **raw** AAC in ADTS
streams, except faac, had extra samples at the start.

Despite this being a standard MDCT-based codec with a frame size of 1024,
hence a delay of 1024 samples at the start, all major encoders, excluding
faac and FFmpeg, use 2048 samples of padding.

The FFmpeg encoder will be modified to also output 2048 samples of padding
at the start, to make it in line with other encoders.

Yes, this leaves FATE pretty sad. Will fix it with the real version of the 
patch.


>From 079235e1f1a9caeadfd2b8d78b3fe2273d86018a Mon Sep 17 00:00:00 2001
From: Lynne 
Date: Fri, 11 Aug 2023 17:50:54 +0200
Subject: [PATCH 1/3] aacdec: always skip the first 2048 samples if there's no
 side data

For some reason, this was never set, which meant all **raw** AAC in ADTS
streams, except faac, had extra samples at the start.

Despite this being a standard MDCT-based codec with a frame size of 1024,
hence a delay of 1024 samples at the start, all major encoders, excluding
faac and FFmpeg, use 2048 samples of padding.

The FFmpeg encoder will be modified to also output 2048 samples of padding
at the start, to make it in line with other encoders.
---
 libavcodec/aacdec_template.c | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/libavcodec/aacdec_template.c b/libavcodec/aacdec_template.c
index f8039e490b..0e4a274fea 100644
--- a/libavcodec/aacdec_template.c
+++ b/libavcodec/aacdec_template.c
@@ -1273,6 +1273,9 @@ static av_cold int aac_decode_init(AVCodecContext *avctx)
 if (ret < 0)
 return ret;
 
+/* Usually overridden by side data */
+avctx->internal->skip_samples = 2048;
+
 return 0;
 }
 
@@ -2417,14 +2420,16 @@ static int decode_dynamic_range(DynamicRangeControl *che_drc,
 return n;
 }
 
-static int decode_fill(AACContext *ac, GetBitContext *gb, int len) {
+static int decode_fill(AACContext *ac, GetBitContext *gb, int len)
+{
 uint8_t buf[256];
-int i, major, minor;
+int i, major, minor, micro;
 
 if (len < 13+7*8)
 goto unknown;
 
-get_bits(gb, 13); len -= 13;
+get_bits(gb, 13);
+len -= 13;
 
 for(i=0; i+1=8; i++, len-=8)
 buf[i] = get_bits(gb, 8);
@@ -2434,7 +2439,11 @@ static int decode_fill(AACContext *ac, GetBitContext *gb, int len) {
 av_log(ac->avctx, AV_LOG_DEBUG, "FILL:%s\n", buf);
 
 if (sscanf(buf, "libfaac %d.%d", , ) == 2){
-ac->avctx->internal->skip_samples = 1024;
+ac->avctx->internal->skip_samples -= 1024;
+}
+
+if ((sscanf(buf, "avc %d.%d.%d", , , ) == 3)) {
+ac->avctx->internal->skip_samples -= 1024;
 }
 
 unknown:
-- 
2.40.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".