Re: [FFmpeg-devel] [PATCH] avcodec/mediacodecdec: call MediaCodec.stop on close
> On Aug 8, 2024, at 00:27, sfan5 wrote:
>
> Hi all,
>
> attached is a small fix for the MediaCodec code. Tested on Android 14.
> This can free up vital resources in case of using multiple
> decoding instances and there are buffer references left over
> and not immediately cleaned up.
>
> Signed-off-by: sfan5
> ---
> libavcodec/mediacodecdec_common.c | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/libavcodec/mediacodecdec_common.c
> b/libavcodec/mediacodecdec_common.c
> index d6f91e6e89..c888dea8cf 100644
> --- a/libavcodec/mediacodecdec_common.c
> +++ b/libavcodec/mediacodecdec_common.c
> @@ -841,6 +841,18 @@ int ff_mediacodec_dec_flush(AVCodecContext *avctx,
> MediaCodecDecContext *s)
>
> int ff_mediacodec_dec_close(AVCodecContext *avctx, MediaCodecDecContext *s)
> {
> +if (!s)
> +return 0;
> +
> +if (s->codec) {
> +if (atomic_load(&s->hw_buffer_count) == 0) {
> +ff_AMediaCodec_stop(s->codec);
> +av_log(avctx, AV_LOG_DEBUG, "MediaCodec %p stopped\n", s->codec);
> +} else {
> +av_log(avctx, AV_LOG_DEBUG, "Not stopping MediaCodec (there are
> buffers pending)\n");
> +}
> +}
> +
Could you elaborate on how this resolved the issue?
If hw_buffer_count is zero, isn’t MediaCodecDecContext will be released
immediately?
If hw_buffer_count isn’t zero, the patch make no real change, so how does this
patch work?
> ff_mediacodec_dec_unref(s);
>
> return 0;
> --
> 2.46.0
>
>
> <0001-avcodec-mediacodecdec-call-MediaCodec.stop-on-close.patch>___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v1] libavdevice/gdigrab: change hwnd tail check fail logic to !=null
> On Aug 7, 2024, at 23:58, cyfdel-at-hotmail@ffmpeg.org wrote:
>
> From: eaphone
>
> ---
> libavdevice/gdigrab.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavdevice/gdigrab.c b/libavdevice/gdigrab.c
> index c91661c556..08a41c304b 100644
> --- a/libavdevice/gdigrab.c
> +++ b/libavdevice/gdigrab.c
> @@ -281,7 +281,7 @@ gdigrab_read_header(AVFormatContext *s1)
>
> hwnd = (HWND) strtoull(name, &p, 0);
>
> -if (p == NULL || p == name || p[0] == '\0')
> +if (p == NULL || p == name || p[0] != '\0')
> {
> av_log(s1, AV_LOG_ERROR,
>"Invalid window handle '%s', must be a valid integer.\n",
> name);
LGTM.
> --
> 2.45.2.windows.1
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/mov: ensure required number of bytes is read
On Thu, Aug 08, 2024 at 01:09:01PM -0300, James Almer wrote: > On 8/7/2024 11:09 AM, Kacper Michajłow wrote: > > Fixes: use-of-uninitialized-value > > > > Found by OSS-Fuzz. > > --- > > libavformat/mov.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libavformat/mov.c b/libavformat/mov.c > > index 1052691936..f2d8aee766 100644 > > --- a/libavformat/mov.c > > +++ b/libavformat/mov.c > > @@ -7096,7 +7096,7 @@ static int mov_read_free(MOVContext *c, AVIOContext > > *pb, MOVAtom atom) > > if (atom.size < 8) > > return 0; > > -ret = avio_read(pb, content, FFMIN(sizeof(content), atom.size)); > > +ret = ffio_read_size(pb, content, FFMIN(sizeof(content), atom.size)); > > if (ret < 0) > > return ret; > > Unrelated (somewhat) to this patch, but why does ffio_read_size() replace > EOF with INVALIDDATA? Is it a good idea to mask the former? EOF might be interpreted as normal / no error end of file i guess thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB If you drop bombs on a foreign country and kill a hundred thousand innocent people, expect your government to call the consequence "unprovoked inhuman terrorist attacks" and use it to justify dropping more bombs and killing more people. The technology changed, the idea is old. signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/mov: ensure required number of bytes is read
On Wed, Aug 07, 2024 at 04:09:20PM +0200, Kacper Michajłow wrote: > Fixes: use-of-uninitialized-value > > Found by OSS-Fuzz. > --- > libavformat/mov.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) will apply thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Any man who breaks a law that conscience tells him is unjust and willingly accepts the penalty by staying in jail in order to arouse the conscience of the community on the injustice of the law is at that moment expressing the very highest respect for law. - Martin Luther King Jr signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/mov: ensure required number of bytes is read
On 8/7/2024 11:09 AM, Kacper Michajłow wrote: Fixes: use-of-uninitialized-value Found by OSS-Fuzz. --- libavformat/mov.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavformat/mov.c b/libavformat/mov.c index 1052691936..f2d8aee766 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -7096,7 +7096,7 @@ static int mov_read_free(MOVContext *c, AVIOContext *pb, MOVAtom atom) if (atom.size < 8) return 0; -ret = avio_read(pb, content, FFMIN(sizeof(content), atom.size)); +ret = ffio_read_size(pb, content, FFMIN(sizeof(content), atom.size)); if (ret < 0) return ret; Unrelated (somewhat) to this patch, but why does ffio_read_size() replace EOF with INVALIDDATA? Is it a good idea to mask the former? ___ 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] Extend the PATH buffer to 2048 for RTSP
On Wed, Aug 07, 2024 at 09:03:01AM +0200, Stefano Mandelli wrote:
> Recently, I have been experiencing an increasing number
> of user that use ffmpeg to retrive RTSP stream from
> personal mediaproxies (e.g. MediaMtx) with
> authorization based on JWT. The current length of PATH
> does not permit to insert the token in the URL failing
> the authorization with no possibilities to get the video.
>
> VLC has just modified the RSTP max URL length, and it
> permits to use token inside the URL.
>
> For these reasons, I propose this patch to extend the
> PATH buffer from 1024 to 2048 in order to use tokens
> and the authorization process based on JWT.
>
>
> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
> index 19b93df839..d8f45cf8d5 100644
> --- a/libavformat/rtsp.c
> +++ b/libavformat/rtsp.c
> @@ -1727,7 +1727,7 @@ void ff_rtsp_close_connections(AVFormatContext *s)
> int ff_rtsp_connect(AVFormatContext *s)
> {
> RTSPState *rt = s->priv_data;
> -char proto[128], host[1024], path[1024];
> +char proto[128], host[1024], path[2048];
> char tcpname[1024], cmd[MAX_URL_SIZE], auth[128];
> const char *lower_rtsp_proto = "tcp";
>int port, err, tcp_fd;
error: corrupt patch at line 13
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Asymptotically faster algorithms should always be preferred if you have
asymptotical amounts of data
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 4/6] avformat/wtvdec: clear sectors
On Wed, Aug 07, 2024 at 10:01:27AM +1000, Peter Ross wrote:
> On Wed, Aug 07, 2024 at 12:18:51AM +0200, Michael Niedermayer wrote:
> > The code can leave uninitialized holes in the array.
> > Fixes: use of uninitialized values
> > Fixes:
> > 70883/clusterfuzz-testcase-minimized-ffmpeg_dem_WTV_fuzzer-6698694567591936
> >
> > Found-by: continuous fuzzing process
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer
> > ---
> > libavformat/wtvdec.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavformat/wtvdec.c b/libavformat/wtvdec.c
> > index e153034aa19..1a6c4c33481 100644
> > --- a/libavformat/wtvdec.c
> > +++ b/libavformat/wtvdec.c
> > @@ -185,7 +185,7 @@ static AVIOContext * wtvfile_open_sector(unsigned
> > first_sector, uint64_t length,
> > int nb_sectors1 = read_ints(s->pb, sectors1, WTV_SECTOR_SIZE / 4);
> > int i;
> >
> > -wf->sectors = av_malloc_array(nb_sectors1, 1 << WTV_SECTOR_BITS);
> > +wf->sectors = av_calloc(nb_sectors1, 1 << WTV_SECTOR_BITS);
> > if (!wf->sectors) {
> > av_free(wf);
> > return NULL;
>
> agree, please apply
will apply
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
You can kill me, but you cannot change the truth.
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 6/6] avformat/wtvdec: Check length of read mpeg2_descriptor
On Wed, Aug 07, 2024 at 10:02:09AM +1000, Peter Ross wrote: > On Wed, Aug 07, 2024 at 12:18:53AM +0200, Michael Niedermayer wrote: > > Fixes: Use of uninitialized value > > Fixes: > > 70900/clusterfuzz-testcase-minimized-ffmpeg_dem_WTV_fuzzer-6286909377150976 > > > > Found-by: continuous fuzzing process > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > Signed-off-by: Michael Niedermayer > > --- > > libavformat/wtvdec.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/libavformat/wtvdec.c b/libavformat/wtvdec.c > > index 1a6c4c33481..730c7fca783 100644 > > --- a/libavformat/wtvdec.c > > +++ b/libavformat/wtvdec.c > > @@ -846,7 +846,8 @@ static int parse_chunks(AVFormatContext *s, int mode, > > int64_t seekts, int *len_p > > } > > > > buf_size = FFMIN(len - consumed, sizeof(buf)); > > -avio_read(pb, buf, buf_size); > > +if (avio_read(pb, buf, buf_size) != buf_size) > > +return AVERROR_INVALIDDATA; > > consumed += buf_size; > > ff_parse_mpeg2_descriptor(s, st, 0, &pbuf, buf + buf_size, > > NULL, 0, 0, NULL); > > } > > please apply will apply thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Its not that you shouldnt use gotos but rather that you should write readable code and code with gotos often but not always is less readable 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 5/6] tools/target_dec_fuzzer: Use av_buffer_allocz() to avoid missing slices to have unpredictable content
On Wed, Aug 07, 2024 at 12:18:52AM +0200, Michael Niedermayer wrote: > Fixes: use of uninitialized values > Fixes: > 70885/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VP6F_fuzzer-4610946029387776 > (and likely others) > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer > --- > tools/target_dec_fuzzer.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) will apply [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB "Nothing to hide" only works if the folks in power share the values of you and everyone you know entirely and always will -- Tom Scott 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 5/6] tools/target_dec_fuzzer: Use av_buffer_allocz() to avoid missing slices to have unpredictable content
On 8/6/2024 7:18 PM, Michael Niedermayer wrote:
Fixes: use of uninitialized values
Fixes:
70885/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VP6F_fuzzer-4610946029387776
(and likely others)
Found-by: continuous fuzzing process
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer
---
tools/target_dec_fuzzer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/target_dec_fuzzer.c b/tools/target_dec_fuzzer.c
index d2d7e21dac7..794b5b92cc7 100644
--- a/tools/target_dec_fuzzer.c
+++ b/tools/target_dec_fuzzer.c
@@ -129,7 +129,7 @@ static int fuzz_video_get_buffer(AVCodecContext *ctx,
AVFrame *frame)
frame->extended_data = frame->data;
for (i = 0; i < 4 && size[i]; i++) {
-frame->buf[i] = av_buffer_alloc(size[i]);
+frame->buf[i] = av_buffer_allocz(size[i]);
if (!frame->buf[i])
goto fail;
frame->data[i] = frame->buf[i]->data;
Wouldn't this hide actual decoder bugs too?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/mediacodecdec: call MediaCodec.stop on close
Am 08.08.24 um 17:29 schrieb Zhao Zhili:
On Aug 8, 2024, at 00:27, sfan5 wrote:
Hi all,
attached is a small fix for the MediaCodec code. Tested on Android 14.
This can free up vital resources in case of using multiple
decoding instances and there are buffer references left over
and not immediately cleaned up.
Signed-off-by: sfan5
---
libavcodec/mediacodecdec_common.c | 12
1 file changed, 12 insertions(+)
diff --git a/libavcodec/mediacodecdec_common.c
b/libavcodec/mediacodecdec_common.c
index d6f91e6e89..c888dea8cf 100644
--- a/libavcodec/mediacodecdec_common.c
+++ b/libavcodec/mediacodecdec_common.c
@@ -841,6 +841,18 @@ int ff_mediacodec_dec_flush(AVCodecContext *avctx,
MediaCodecDecContext *s)
int ff_mediacodec_dec_close(AVCodecContext *avctx, MediaCodecDecContext *s)
{
+if (!s)
+return 0;
+
+if (s->codec) {
+if (atomic_load(&s->hw_buffer_count) == 0) {
+ff_AMediaCodec_stop(s->codec);
+av_log(avctx, AV_LOG_DEBUG, "MediaCodec %p stopped\n", s->codec);
+} else {
+av_log(avctx, AV_LOG_DEBUG, "Not stopping MediaCodec (there are buffers
pending)\n");
+}
+}
+
Could you elaborate on how this resolved the issue?
If hw_buffer_count is zero, isn’t MediaCodecDecContext will be released
immediately?
If hw_buffer_count isn’t zero, the patch make no real change, so how does this
patch work?
Sure.
here's the original report:
https://github.com/mpv-android/mpv-android/issues/966
summary:
mpv's hwdec code uses a single surface to facilitate zero-copy video
playback. Keeping an active MediaCodec instance connected to the surface
blocks the same surface from being used with a different MediaCodec
instance.
However mpv also keeps a reference to the last rendered frame alive even
when switching between files. It also flushes the codec when it's done
with a file. This leads to a situation where hw_buffer_count == 0 &&
refcount > 1.
If you were to say that this should be fixed in mpv I would agree and I
have indeed proposed a PR for this. But I noticed we're never calling
stop() in mediacodecdec and it's a very simple remedy.
___
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: Force vaapi image formats to NV12-only
attached
>From c6439f3a74529db25777029596791a62eb3c77d5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Llu=C3=ADs=20Batlle=20i=20Rossell?=
Date: Thu, 8 Aug 2024 20:32:03 +0200
Subject: [PATCH] Force vaapi image formats to NV12-only
Vaapi drivers often lack proper image converesions and not all
situations allow vaGetImage or vaPutImage with the image formats
reported by the API. NV12 seems allowed in all circumstances.
With this change now one can use the hwaccel directly without
explicit conversions to nv12 for frame downloading to work.
gstreamer adopted a similar approach:
https://bugzilla.gnome.org/show_bug.cgi?id=752958
---
libavutil/hwcontext_vaapi.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c
index 12bc95119a..d678e58d07 100644
--- a/libavutil/hwcontext_vaapi.c
+++ b/libavutil/hwcontext_vaapi.c
@@ -418,7 +418,12 @@ static int vaapi_device_init(AVHWDeviceContext *hwdev)
for (i = 0; i < image_count; i++) {
fourcc = image_list[i].fourcc;
pix_fmt = vaapi_pix_fmt_from_fourcc(fourcc);
-if (pix_fmt == AV_PIX_FMT_NONE) {
+if (pix_fmt != AV_PIX_FMT_NV12) {
+av_log(hwdev, AV_LOG_DEBUG, "Format %#x -> ignored.\n",
+ fourcc);
+continue;
+}
+else if (pix_fmt == AV_PIX_FMT_NONE) {
av_log(hwdev, AV_LOG_DEBUG, "Format %#x -> unknown.\n",
fourcc);
} else {
--
2.44.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 v2 1/3] avcodec/vvc_parser: move avctx->has_b_frames initialization to dec
From: Wu Jianhua
>From Jun Zhao :
> Should we relocate this to the decoder? Other codecs typically set this
> parameter in the decoder.
Signed-off-by: Wu Jianhua
---
libavcodec/vvc/dec.c| 1 +
libavcodec/vvc_parser.c | 3 ---
2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/libavcodec/vvc/dec.c b/libavcodec/vvc/dec.c
index d04f68e4cf..6e225d278a 100644
--- a/libavcodec/vvc/dec.c
+++ b/libavcodec/vvc/dec.c
@@ -748,6 +748,7 @@ static void export_frame_params(VVCContext *s, const
VVCFrameContext *fc)
c->coded_height = pps->height;
c->width= pps->width - ((pps->r->pps_conf_win_left_offset +
pps->r->pps_conf_win_right_offset) << sps->hshift[CHROMA]);
c->height = pps->height - ((pps->r->pps_conf_win_top_offset +
pps->r->pps_conf_win_bottom_offset) << sps->vshift[CHROMA]);
+c->has_b_frames =
sps->r->sps_dpb_params.dpb_max_num_reorder_pics[sps->r->sps_max_sublayers_minus1];
}
static int frame_setup(VVCFrameContext *fc, VVCContext *s)
diff --git a/libavcodec/vvc_parser.c b/libavcodec/vvc_parser.c
index 5373875aae..8d32d66573 100644
--- a/libavcodec/vvc_parser.c
+++ b/libavcodec/vvc_parser.c
@@ -185,9 +185,6 @@ static void set_parser_ctx(AVCodecParserContext *s,
AVCodecContext *avctx,
avctx->color_range =
sps->vui.vui_full_range_flag ? AVCOL_RANGE_JPEG : AVCOL_RANGE_MPEG;
-avctx->has_b_frames =
-
sps->sps_dpb_params.dpb_max_num_reorder_pics[sps->sps_max_sublayers_minus1];
-
if (sps->sps_ptl_dpb_hrd_params_present_flag &&
sps->sps_timing_hrd_params_present_flag) {
uint32_t num =
sps->sps_general_timing_hrd_parameters.num_units_in_tick;
--
2.44.0.windows.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 v2 2/3] avcodec/vvc/cabac: remove vvc_refill2
From: Wu Jianhua
See https://github.com/ffvvc/FFmpeg/issues/178
Signed-off-by: Wu Jianhua
---
libavcodec/cabac_functions.h | 2 +-
libavcodec/vvc/cabac.c | 28 +---
2 files changed, 2 insertions(+), 28 deletions(-)
diff --git a/libavcodec/cabac_functions.h b/libavcodec/cabac_functions.h
index c3f08d3410..9bee401f2c 100644
--- a/libavcodec/cabac_functions.h
+++ b/libavcodec/cabac_functions.h
@@ -85,7 +85,7 @@ static inline void renorm_cabac_decoder_once(CABACContext *c){
}
#endif
-#ifndef get_cabac_inline
+#if !defined(get_cabac_inline) || !defined(refill2)
static void refill2(CABACContext *c){
int i;
unsigned x;
diff --git a/libavcodec/vvc/cabac.c b/libavcodec/vvc/cabac.c
index 0d45eec751..c9b6f9bf3e 100644
--- a/libavcodec/vvc/cabac.c
+++ b/libavcodec/vvc/cabac.c
@@ -856,32 +856,6 @@ int ff_vvc_cabac_init(VVCLocalContext *lc,
return ret;
}
-//fixme
-static void vvc_refill2(CABACContext* c) {
-int i;
-unsigned x;
-#if !HAVE_FAST_CLZ
-x = c->low ^ (c->low - 1);
-i = 7 - ff_h264_norm_shift[x >> (CABAC_BITS - 1)];
-#else
-i = ff_ctz(c->low) - CABAC_BITS;
-#endif
-
-x = -CABAC_MASK;
-
-#if CABAC_BITS == 16
-x += (c->bytestream[0] << 9) + (c->bytestream[1] << 1);
-#else
-x += c->bytestream[0] << 1;
-#endif
-
-c->low += x << i;
-#if !UNCHECKED_BITSTREAM_READER
-if (c->bytestream < c->bytestream_end)
-#endif
-c->bytestream += CABAC_BITS / 8;
-}
-
static int inline vvc_get_cabac(CABACContext *c, VVCCabacState* base, const
int ctx)
{
VVCCabacState *s = base + ctx;
@@ -904,7 +878,7 @@ static int inline vvc_get_cabac(CABACContext *c,
VVCCabacState* base, const int
c->low <<= lps_mask;
if (!(c->low & CABAC_MASK))
-vvc_refill2(c);
+refill2(c);
s->state[0] = s->state[0] - (s->state[0] >> s->shift[0]) + (1023 * bit >>
s->shift[0]);
s->state[1] = s->state[1] - (s->state[1] >> s->shift[1]) + (16383 * bit >>
s->shift[1]);
return bit;
--
2.44.0.windows.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 v2 3/3] avcodec/vvc/dsp: prefix TxType and TxSize with VVC
From: Wu Jianhua
See https://github.com/ffvvc/FFmpeg/issues/180
Signed-off-by: Wu Jianhua
---
libavcodec/vvc/dsp.h | 28 ++--
libavcodec/vvc/dsp_template.c | 2 +-
libavcodec/vvc/intra.c| 26 +-
3 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/libavcodec/vvc/dsp.h b/libavcodec/vvc/dsp.h
index 0b49b97021..38ff492a23 100644
--- a/libavcodec/vvc/dsp.h
+++ b/libavcodec/vvc/dsp.h
@@ -27,21 +27,21 @@
#include
#include
-enum TxType {
-DCT2,
-DST7,
-DCT8,
-N_TX_TYPE,
+enum VVCTxType {
+VVC_DCT2,
+VVC_DST7,
+VVC_DCT8,
+VVC_N_TX_TYPE,
};
-enum TxSize {
-TX_SIZE_2,
-TX_SIZE_4,
-TX_SIZE_8,
-TX_SIZE_16,
-TX_SIZE_32,
-TX_SIZE_64,
-N_TX_SIZE,
+enum VVCTxSize {
+VVC_TX_SIZE_2,
+VVC_TX_SIZE_4,
+VVC_TX_SIZE_8,
+VVC_TX_SIZE_16,
+VVC_TX_SIZE_32,
+VVC_TX_SIZE_64,
+VVC_N_TX_SIZE,
};
typedef struct VVCInterDSPContext {
@@ -127,7 +127,7 @@ typedef struct VVCItxDSPContext {
void (*add_residual_joint)(uint8_t *dst, const int *res, int width, int
height, ptrdiff_t stride, int c_sign, int shift);
void (*pred_residual_joint)(int *buf, int width, int height, int c_sign,
int shift);
-void (*itx[N_TX_TYPE][N_TX_SIZE])(int *coeffs, ptrdiff_t step, size_t nz);
+void (*itx[VVC_N_TX_TYPE][VVC_N_TX_SIZE])(int *coeffs, ptrdiff_t step,
size_t nz);
void (*transform_bdpcm)(int *coeffs, int width, int height, int vertical,
int log2_transform_range);
} VVCItxDSPContext;
diff --git a/libavcodec/vvc/dsp_template.c b/libavcodec/vvc/dsp_template.c
index 8130abbccf..1aa1e027bd 100644
--- a/libavcodec/vvc/dsp_template.c
+++ b/libavcodec/vvc/dsp_template.c
@@ -97,7 +97,7 @@ static void FUNC(transform_bdpcm)(int *coeffs, const int
width, const int height
static void FUNC(ff_vvc_itx_dsp_init)(VVCItxDSPContext *const itx)
{
#define VVC_ITX(TYPE, type, s)
\
-itx->itx[TYPE][TX_SIZE_##s] = ff_vvc_inv_##type##_##s;
\
+itx->itx[VVC_##TYPE][VVC_##TX_SIZE_##s] =
ff_vvc_inv_##type##_##s; \
#define VVC_ITX_COMMON(TYPE, type)
\
VVC_ITX(TYPE, type, 4);
\
diff --git a/libavcodec/vvc/intra.c b/libavcodec/vvc/intra.c
index f77a012f09..73dca6dc85 100644
--- a/libavcodec/vvc/intra.c
+++ b/libavcodec/vvc/intra.c
@@ -128,15 +128,15 @@ static void ilfnst_transform(const VVCLocalContext *lc,
TransformBlock *tb)
}
//part of 8.7.4 Transformation process for scaled transform coefficients
-static void derive_transform_type(const VVCFrameContext *fc, const
VVCLocalContext *lc, const TransformBlock *tb, enum TxType *trh, enum TxType
*trv)
+static void derive_transform_type(const VVCFrameContext *fc, const
VVCLocalContext *lc, const TransformBlock *tb, enum VVCTxType *trh, enum
VVCTxType *trv)
{
const CodingUnit *cu = lc->cu;
-static const enum TxType mts_to_trh[] = {DCT2, DST7, DCT8, DST7, DCT8};
-static const enum TxType mts_to_trv[] = {DCT2, DST7, DST7, DCT8, DCT8};
+static const enum VVCTxType mts_to_trh[] = { VVC_DCT2, VVC_DST7, VVC_DCT8,
VVC_DST7, VVC_DCT8 };
+static const enum VVCTxType mts_to_trv[] = { VVC_DCT2, VVC_DST7, VVC_DST7,
VVC_DCT8, VVC_DCT8 };
const VVCSPS *sps = fc->ps.sps;
int implicit_mts_enabled = 0;
if (tb->c_idx || (cu->isp_split_type != ISP_NO_SPLIT && cu->lfnst_idx)) {
-*trh = *trv = DCT2;
+*trh = *trv = VVC_DCT2;
return;
}
@@ -152,11 +152,11 @@ static void derive_transform_type(const VVCFrameContext
*fc, const VVCLocalConte
const int w = tb->tb_width;
const int h = tb->tb_height;
if (cu->sbt_flag) {
-*trh = (cu->sbt_horizontal_flag || cu->sbt_pos_flag) ? DST7 :
DCT8;
-*trv = (!cu->sbt_horizontal_flag || cu->sbt_pos_flag) ? DST7 :
DCT8;
+*trh = (cu->sbt_horizontal_flag || cu->sbt_pos_flag) ? VVC_DST7 :
VVC_DCT8;
+*trv = (!cu->sbt_horizontal_flag || cu->sbt_pos_flag) ? VVC_DST7 :
VVC_DCT8;
} else {
-*trh = (w >= 4 && w <= 16) ? DST7 : DCT2;
-*trv = (h >= 4 && h <= 16) ? DST7 : DCT2;
+*trh = (w >= 4 && w <= 16) ? VVC_DST7 : VVC_DCT2;
+*trv = (h >= 4 && h <= 16) ? VVC_DST7 : VVC_DCT2;
}
return;
}
@@ -447,7 +447,7 @@ static void dequant(const VVCLocalContext *lc, const
TransformUnit *tu, Transfor
//transmatrix[0][0]
#define DCT_A 64
-static void itx_2d(const VVCFrameContext *fc, TransformBlock *tb, const enum
TxType trh, const enum TxType trv)
+static void itx_2d(const VVCFrameContext *fc, TransformBlock *tb, const enum
VVCTxType trh, const enum VVCTxType trv)
{
const VVCSPS *sps = fc->ps.sps;
const int w = tb->tb_width;
@@ -456,7 +456,7 @@ static void itx_2d(const VV
Re: [FFmpeg-devel] [PATCH v2 2/3] avcodec/vvc/cabac: remove vvc_refill2
toq...@outlook.com:
> From: Wu Jianhua
>
> See https://github.com/ffvvc/FFmpeg/issues/178
This link only sends one to a patchwork thread to read. The commit
message should instead explain why this is done on its own (and may
refer to the mailing list thread for a more detailed explanation).
Same for the next patch.
>
> Signed-off-by: Wu Jianhua
> ---
> libavcodec/cabac_functions.h | 2 +-
> libavcodec/vvc/cabac.c | 28 +---
> 2 files changed, 2 insertions(+), 28 deletions(-)
>
> diff --git a/libavcodec/cabac_functions.h b/libavcodec/cabac_functions.h
> index c3f08d3410..9bee401f2c 100644
> --- a/libavcodec/cabac_functions.h
> +++ b/libavcodec/cabac_functions.h
> @@ -85,7 +85,7 @@ static inline void renorm_cabac_decoder_once(CABACContext
> *c){
> }
> #endif
>
> -#ifndef get_cabac_inline
> +#if !defined(get_cabac_inline) || !defined(refill2)
> static void refill2(CABACContext *c){
> int i;
> unsigned x;
> diff --git a/libavcodec/vvc/cabac.c b/libavcodec/vvc/cabac.c
> index 0d45eec751..c9b6f9bf3e 100644
> --- a/libavcodec/vvc/cabac.c
> +++ b/libavcodec/vvc/cabac.c
> @@ -856,32 +856,6 @@ int ff_vvc_cabac_init(VVCLocalContext *lc,
> return ret;
> }
>
> -//fixme
> -static void vvc_refill2(CABACContext* c) {
> -int i;
> -unsigned x;
> -#if !HAVE_FAST_CLZ
> -x = c->low ^ (c->low - 1);
> -i = 7 - ff_h264_norm_shift[x >> (CABAC_BITS - 1)];
> -#else
> -i = ff_ctz(c->low) - CABAC_BITS;
> -#endif
> -
> -x = -CABAC_MASK;
> -
> -#if CABAC_BITS == 16
> -x += (c->bytestream[0] << 9) + (c->bytestream[1] << 1);
> -#else
> -x += c->bytestream[0] << 1;
> -#endif
> -
> -c->low += x << i;
> -#if !UNCHECKED_BITSTREAM_READER
> -if (c->bytestream < c->bytestream_end)
> -#endif
> -c->bytestream += CABAC_BITS / 8;
> -}
> -
> static int inline vvc_get_cabac(CABACContext *c, VVCCabacState* base, const
> int ctx)
> {
> VVCCabacState *s = base + ctx;
> @@ -904,7 +878,7 @@ static int inline vvc_get_cabac(CABACContext *c,
> VVCCabacState* base, const int
> c->low <<= lps_mask;
>
> if (!(c->low & CABAC_MASK))
> -vvc_refill2(c);
> +refill2(c);
> s->state[0] = s->state[0] - (s->state[0] >> s->shift[0]) + (1023 * bit
> >> s->shift[0]);
> s->state[1] = s->state[1] - (s->state[1] >> s->shift[1]) + (16383 * bit
> >> s->shift[1]);
> return bit;
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/mov: ensure required number of bytes is read
James Almer: > On 8/7/2024 11:09 AM, Kacper Michajłow wrote: >> Fixes: use-of-uninitialized-value >> >> Found by OSS-Fuzz. >> --- >> libavformat/mov.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/libavformat/mov.c b/libavformat/mov.c >> index 1052691936..f2d8aee766 100644 >> --- a/libavformat/mov.c >> +++ b/libavformat/mov.c >> @@ -7096,7 +7096,7 @@ static int mov_read_free(MOVContext *c, >> AVIOContext *pb, MOVAtom atom) >> if (atom.size < 8) >> return 0; >> - ret = avio_read(pb, content, FFMIN(sizeof(content), atom.size)); >> + ret = ffio_read_size(pb, content, FFMIN(sizeof(content), >> atom.size)); >> if (ret < 0) >> return ret; > > Unrelated (somewhat) to this patch, but why does ffio_read_size() > replace EOF with INVALIDDATA? Is it a good idea to mask the former? > ffio_read_size() is supposed to be used in scenarios where a certain number of bytes is supposed to be available (e.g. because some size field says that there have to be that many bytes of payload there). If we are at EOF when there is supposed to be data, the file is invalid. - 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 5/6] tools/target_dec_fuzzer: Use av_buffer_allocz() to avoid missing slices to have unpredictable content
On Thu, Aug 08, 2024 at 02:13:12PM -0300, James Almer wrote:
> On 8/6/2024 7:18 PM, Michael Niedermayer wrote:
> > Fixes: use of uninitialized values
> > Fixes:
> > 70885/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VP6F_fuzzer-4610946029387776
> > (and likely others)
> >
> > Found-by: continuous fuzzing process
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer
> > ---
> > tools/target_dec_fuzzer.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/target_dec_fuzzer.c b/tools/target_dec_fuzzer.c
> > index d2d7e21dac7..794b5b92cc7 100644
> > --- a/tools/target_dec_fuzzer.c
> > +++ b/tools/target_dec_fuzzer.c
> > @@ -129,7 +129,7 @@ static int fuzz_video_get_buffer(AVCodecContext *ctx,
> > AVFrame *frame)
> > frame->extended_data = frame->data;
> > for (i = 0; i < 4 && size[i]; i++) {
> > -frame->buf[i] = av_buffer_alloc(size[i]);
> > +frame->buf[i] = av_buffer_allocz(size[i]);
> > if (!frame->buf[i])
> > goto fail;
> > frame->data[i] = frame->buf[i]->data;
>
> Wouldn't this hide actual decoder bugs too?
iam not sure i understand what you mean
If decoders are fed with uninitialized buffers thats a
security issue because there are thousands if not ten thousands of
pathes if you consider the number of decoders and the number
of ways they can hit errors
Pathes in which these buffers are not filled completely, so each
of these pathes would then need to clear the right bits of data.
Basically that means implementing error concealment for every decoder.
AND making sure that error concealment code is 100% bugfree and leaves
never a spot uncleaned and never touched something that was not writen to
Security wise this is not possible for production code, its too
fragile (at least with the number of decoders and active maintainers we have)
(you want less code to have to be bugfree for security not more code having
to be bug free)
Now this is the fuzzer and not production code, ok. And of course is
great to have error concealment in every decoder
But then this leaves the question, who will do this work?
If noone does it then we will accumulate many msan bugs in ossfuzz that we wont
be able to do much with except ignore them.
This would make the fuzzer less efficient and it would confuse people looking
at the issues
Or the short punchy reply maybe is
Produce a volunteer who will fix these bugs before declaring them bugs.
And when doing so consider that we have bugfixes on the mailing list for which
we
seem to not even have the man power to review and apply them
so yeah my oppinion is the default should be the simple & easy to maintain way.
If someone declares their decoder to have flawless error concealment (and for
some
simple decoders that could be quite simple) these can always be excluded and use
uninitialized buffers in the fuzzer
thx
[...]
--
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] [PATCH v2 1/3] avcodec/libaribb24: change new lines to \n in ASS header
On Fri, May 10, 2024 at 11:31 PM Kacper Michajłow wrote: > > Fixes remaining \r\n is ASS header after 57c545090d. > > Signed-off-by: Kacper Michajłow > --- With an initial look this set looks good. If I understand correctly, the generic ASS encoder moved to outputting LF only in 7bf1b9b35769b37684dd2f18a54f01d852a540c8 and thus these modules which still output manual headers with CRLF cause mismatching endlines within a single document. Set still applies. Will try to give this a proper look and verify tomorrow, and then pull this in if it indeed does what it seems to do. Jan ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2 1/3] avcodec/libaribb24: change new lines to \n in ASS header
On Fri, 9 Aug 2024 at 00:04, Jan Ekström wrote: > > On Fri, May 10, 2024 at 11:31 PM Kacper Michajłow wrote: > > > > Fixes remaining \r\n is ASS header after 57c545090d. > > > > Signed-off-by: Kacper Michajłow > > --- > > With an initial look this set looks good. If I understand correctly, > the generic ASS encoder moved to outputting LF only in > 7bf1b9b35769b37684dd2f18a54f01d852a540c8 and thus these modules which > still output manual headers with CRLF cause mismatching endlines > within a single document. This patchest indeed fixes mismatched endlines, but the main thing it fixes is libzvbi_teletextdec converter which currently always returns AVERROR_BUG, because default style no longer matches (after 7bf1b9b35) the `strstr` expectation. https://github.com/FFmpeg/FFmpeg/blob/c390234da2e3c7a8884f5592f0b9b4928c482b3e/libavcodec/libzvbi-teletextdec.c#L94 event_pos = strstr(avctx->subtitle_header, "\r\n[Events]\r\n"); if (!event_pos) return AVERROR_BUG; - Kacper ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 5/6] tools/target_dec_fuzzer: Use av_buffer_allocz() to avoid missing slices to have unpredictable content
On Fri, 9 Aug 2024 at 00:06, Michael Niedermayer wrote:
>
> On Thu, Aug 08, 2024 at 02:13:12PM -0300, James Almer wrote:
> > On 8/6/2024 7:18 PM, Michael Niedermayer wrote:
> > > Fixes: use of uninitialized values
> > > Fixes:
> > > 70885/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VP6F_fuzzer-4610946029387776
> > > (and likely others)
> > >
> > > Found-by: continuous fuzzing process
> > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > Signed-off-by: Michael Niedermayer
> > > ---
> > > tools/target_dec_fuzzer.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/tools/target_dec_fuzzer.c b/tools/target_dec_fuzzer.c
> > > index d2d7e21dac7..794b5b92cc7 100644
> > > --- a/tools/target_dec_fuzzer.c
> > > +++ b/tools/target_dec_fuzzer.c
> > > @@ -129,7 +129,7 @@ static int fuzz_video_get_buffer(AVCodecContext *ctx,
> > > AVFrame *frame)
> > > frame->extended_data = frame->data;
> > > for (i = 0; i < 4 && size[i]; i++) {
> > > -frame->buf[i] = av_buffer_alloc(size[i]);
> > > +frame->buf[i] = av_buffer_allocz(size[i]);
> > > if (!frame->buf[i])
> > > goto fail;
> > > frame->data[i] = frame->buf[i]->data;
> >
> > Wouldn't this hide actual decoder bugs too?
>
> iam not sure i understand what you mean
In general, clearing buffers before processing makes MSAN less
effective in discovering invalid accesses because they would all
appear valid from its point of view. So, I guess the argument was that
this could hide actual decoder bugs since the buffers are already
initialized by the fuzzing binary itself, which, in theory, is
supposed to emulate the worst-case scenario for a tested decoder.
> If decoders are fed with uninitialized buffers thats a
> security issue because there are thousands if not ten thousands of
> pathes if you consider the number of decoders and the number
> of ways they can hit errors
Clearing those buffers in fuzzers does not alleviate this security
issue, as they may still be uninitialized in production code.
> Pathes in which these buffers are not filled completely, so each
> of these pathes would then need to clear the right bits of data.
> Basically that means implementing error concealment for every decoder.
> AND making sure that error concealment code is 100% bugfree and leaves
> never a spot uncleaned and never touched something that was not writen to
Isn't that the point of uninitialized access checking? I can't speak
to the scale of the problem because I don't know what the issues are.
In principle, you don't have to clear each uninitialized path of the
buffer that may occur due to an error. Instead, you should ensure that
the buffer is not accessed when the error occurs. If decoders rely on
external users to provide zeroed buffers to work correctly, then this
should be documented as an API requirement.
Outputting garbage on errors is acceptable, but if decoders process
uninitialized data internally when errors occur, they are, at best,
non-deterministic...
> Security wise this is not possible for production code, its too
> fragile (at least with the number of decoders and active maintainers we have)
> (you want less code to have to be bugfree for security not more code having
> to be bug free)
>
> Now this is the fuzzer and not production code, ok. And of course is
> great to have error concealment in every decoder
> But then this leaves the question, who will do this work?
> If noone does it then we will accumulate many msan bugs in ossfuzz that we
> wont
> be able to do much with except ignore them.
> This would make the fuzzer less efficient and it would confuse people looking
> at the issues
MSAN is not forgiving, and I can imagine that stabilizing it could
take time. However, suppressing the reports will not make it more
efficient. I might not fully understand what you meant, though.
That being said, I think the patch makes sense as a short-term
solution to suppress the bulk of reports and focus on the remaining
ones. However, it would be good to make it clear that, at some point,
it should be reverted. As it stands now, no one will remember why it
was zeroed out, and it could remain that way indefinitely. Perhaps it
should be configurable per decoder.
> Or the short punchy reply maybe is
> Produce a volunteer who will fix these bugs before declaring them bugs.
> And when doing so consider that we have bugfixes on the mailing list for
> which we
> seem to not even have the man power to review and apply them
>
> so yeah my oppinion is the default should be the simple & easy to maintain
> way.
> If someone declares their decoder to have flawless error concealment (and for
> some
> simple decoders that could be quite simple) these can always be excluded and
> use
> uninitialized buffers in the fuzzer
What is the problem with keeping those reports and letting "someone"
work on their decoder based on reports?
- Kacper
Re: [FFmpeg-devel] [PATCH] avcodec/mediacodecdec: call MediaCodec.stop on close
> On Aug 9, 2024, at 02:14, sfan5 wrote:
>
> Am 08.08.24 um 17:29 schrieb Zhao Zhili:
>>> On Aug 8, 2024, at 00:27, sfan5 wrote:
>>>
>>> Hi all,
>>>
>>> attached is a small fix for the MediaCodec code. Tested on Android 14.
>>>
>>> This can free up vital resources in case of using multiple
>>> decoding instances and there are buffer references left over
>>> and not immediately cleaned up.
>>>
>>> Signed-off-by: sfan5
>>> ---
>>> libavcodec/mediacodecdec_common.c | 12
>>> 1 file changed, 12 insertions(+)
>>>
>>> diff --git a/libavcodec/mediacodecdec_common.c
>>> b/libavcodec/mediacodecdec_common.c
>>> index d6f91e6e89..c888dea8cf 100644
>>> --- a/libavcodec/mediacodecdec_common.c
>>> +++ b/libavcodec/mediacodecdec_common.c
>>> @@ -841,6 +841,18 @@ int ff_mediacodec_dec_flush(AVCodecContext *avctx,
>>> MediaCodecDecContext *s)
>>>int ff_mediacodec_dec_close(AVCodecContext *avctx, MediaCodecDecContext
>>> *s)
>>> {
>>> +if (!s)
>>> +return 0;
>>> +
>>> +if (s->codec) {
>>> +if (atomic_load(&s->hw_buffer_count) == 0) {
>>> +ff_AMediaCodec_stop(s->codec);
>>> +av_log(avctx, AV_LOG_DEBUG, "MediaCodec %p stopped\n",
>>> s->codec);
>>> +} else {
>>> +av_log(avctx, AV_LOG_DEBUG, "Not stopping MediaCodec (there
>>> are buffers pending)\n");
>>> +}
>>> +}
>>> +
>> Could you elaborate on how this resolved the issue?
>> If hw_buffer_count is zero, isn’t MediaCodecDecContext will be released
>> immediately?
>> If hw_buffer_count isn’t zero, the patch make no real change, so how does
>> this patch work?
>
> Sure.
>
> here's the original report:
> https://github.com/mpv-android/mpv-android/issues/966
>
> summary:
>
> mpv's hwdec code uses a single surface to facilitate zero-copy video
> playback. Keeping an active MediaCodec instance connected to the surface
> blocks the same surface from being used with a different MediaCodec instance.
>
> However mpv also keeps a reference to the last rendered frame alive even when
> switching between files. It also flushes the codec when it's done with a
> file. This leads to a situation where hw_buffer_count == 0 && refcount > 1.
>
> If you were to say that this should be fixed in mpv I would agree and I have
> indeed proposed a PR for this. But I noticed we're never calling stop() in
> mediacodecdec and it's a very simple remedy.
OK. Please add the background to the commit message. It’s not obvious how to
trigger the "hw_buffer_count == 0 && refcount > 1” case.
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with
> subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] PATCH: Force vaapi image formats to NV12-only
> vaapi drivers often lack proper image converesions and not all
> situations allow vagetimage or vaputimage with the image formats
> reported by the api. nv12 seems allowed in all circumstances.
>
> with this change now one can use the hwaccel directly without
> explicit conversions to nv12 for frame downloading to work.
>
> gstreamer adopted a similar approach:
> https://bugzilla.gnome.org/show_bug.cgi?id=752958
> ---
> libavutil/hwcontext_vaapi.c | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c
> index 12bc95119a..d678e58d07 100644
> --- a/libavutil/hwcontext_vaapi.c
> +++ b/libavutil/hwcontext_vaapi.c
> @@ -418,7 +418,12 @@ static int vaapi_device_init(avhwdevicecontext *hwdev)
> for (i = 0; i < image_count; i++) {
> fourcc = image_list[i].fourcc;
> pix_fmt = vaapi_pix_fmt_from_fourcc(fourcc);
> -if (pix_fmt == av_pix_fmt_none) {
> +if (pix_fmt != av_pix_fmt_nv12) {
> +av_log(hwdev, av_log_debug, "format %#x -> ignored.\n",
> + fourcc);
> +continue;
> +}
> +else if (pix_fmt == av_pix_fmt_none) {
> av_log(hwdev, av_log_debug, "format %#x -> unknown.\n",
> fourcc);
> } else {
> --
> 2.44.1
Isn’t it break all pixel formats with bit depth > 8?
I think we already have hwcontext API to select sw_format, this isn’t a bug
inside ffmpeg.
> On Aug 9, 2024, at 02:37, Lluís Batlle i Rossell wrote:
>
> attached
> <0001-Force-vaapi-image-formats-to-NV12-only.patch>___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
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".
