Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH] avcodec/vvc/refs: remove dead code
@ 2025-06-26 21:49 Marvin Scholz
  2025-06-26 22:50 ` Frank Plowman
  0 siblings, 1 reply; 3+ messages in thread
From: Marvin Scholz @ 2025-06-26 21:49 UTC (permalink / raw)
  To: ffmpeg-devel

The ret value is already checked earlier, making this condition
impossible to ever happen.

Fix CID 1648350
---
 libavcodec/vvc/refs.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/libavcodec/vvc/refs.c b/libavcodec/vvc/refs.c
index 79967b77d3..e52cc0c10d 100644
--- a/libavcodec/vvc/refs.c
+++ b/libavcodec/vvc/refs.c
@@ -310,8 +310,6 @@ int ff_vvc_output_frame(VVCContext *s, VVCFrameContext *fc, AVFrame *out, const
                 ff_vvc_unref_frame(fc, frame, VVC_FRAME_FLAG_OUTPUT | VVC_FRAME_FLAG_BUMPING);
             else
                 ff_vvc_unref_frame(fc, frame, VVC_FRAME_FLAG_OUTPUT);
-            if (ret < 0)
-                return ret;
 
             av_log(s->avctx, AV_LOG_DEBUG,
                    "Output frame with POC %d.\n", frame->poc);
-- 
2.39.5 (Apple Git-154)

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

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [FFmpeg-devel] [PATCH] avcodec/vvc/refs: remove dead code
  2025-06-26 21:49 [FFmpeg-devel] [PATCH] avcodec/vvc/refs: remove dead code Marvin Scholz
@ 2025-06-26 22:50 ` Frank Plowman
  2025-06-26 23:23   ` [FFmpeg-devel] [PATCH v2] avcodec/vvc/refs: remove early return Marvin Scholz
  0 siblings, 1 reply; 3+ messages in thread
From: Frank Plowman @ 2025-06-26 22:50 UTC (permalink / raw)
  To: ffmpeg-devel


[-- Attachment #1.1.1.1: Type: text/plain, Size: 1395 bytes --]

On 27/06/2025 22:49, Marvin Scholz wrote:
> The ret value is already checked earlier, making this condition
> impossible to ever happen.
> 
> Fix CID 1648350
> ---
>  libavcodec/vvc/refs.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/libavcodec/vvc/refs.c b/libavcodec/vvc/refs.c
> index 79967b77d3..e52cc0c10d 100644
> --- a/libavcodec/vvc/refs.c
> +++ b/libavcodec/vvc/refs.c
> @@ -310,8 +310,6 @@ int ff_vvc_output_frame(VVCContext *s, VVCFrameContext *fc, AVFrame *out, const
>                  ff_vvc_unref_frame(fc, frame, VVC_FRAME_FLAG_OUTPUT | VVC_FRAME_FLAG_BUMPING);
>              else
>                  ff_vvc_unref_frame(fc, frame, VVC_FRAME_FLAG_OUTPUT);
> -            if (ret < 0)
> -                return ret;
>  
>              av_log(s->avctx, AV_LOG_DEBUG,
>                     "Output frame with POC %d.\n", frame->poc);

I agree there shouldn't be two checks on ret here.  I am not sure this
is the right one to remove however.  Perhaps it is better to remove the
first check and only check ret after having unreferenced the source
frame, as was the behaviour prior to
a8d949bd96364892903bedbbc2eef11b712e5500 ?  I'm not certain, but it
looks to me as though that commit might have introduced a memory leak:
if av_frame_ref fails, then the source frame is never unreferenced and
its data never freed.

-- 
Thanks,
Frank

[-- Attachment #1.1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 1091 bytes --]

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

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

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [FFmpeg-devel] [PATCH v2] avcodec/vvc/refs: remove early return
  2025-06-26 22:50 ` Frank Plowman
@ 2025-06-26 23:23   ` Marvin Scholz
  0 siblings, 0 replies; 3+ messages in thread
From: Marvin Scholz @ 2025-06-26 23:23 UTC (permalink / raw)
  To: ffmpeg-devel

The ret value is checked later on again, so this check
is redundant and would cause the frame to not be unrefd on
failure as well.

So remove this check and add one before av_frame_remove_side_data
to ensure it is not called with an invalid frame.

Fix CID 1648350
---
 libavcodec/vvc/refs.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/libavcodec/vvc/refs.c b/libavcodec/vvc/refs.c
index 79967b77d3..1840caa4ec 100644
--- a/libavcodec/vvc/refs.c
+++ b/libavcodec/vvc/refs.c
@@ -300,16 +300,15 @@ int ff_vvc_output_frame(VVCContext *s, VVCFrameContext *fc, AVFrame *out, const
                 frame->frame->flags |= AV_FRAME_FLAG_CORRUPT;
 
             ret = av_frame_ref(out, frame->needs_fg ? frame->frame_grain : frame->frame);
-            if (ret < 0)
-                return ret;
 
-            if (!(s->avctx->export_side_data & AV_CODEC_EXPORT_DATA_FILM_GRAIN))
+            if (!ret && !(s->avctx->export_side_data & AV_CODEC_EXPORT_DATA_FILM_GRAIN))
                 av_frame_remove_side_data(out, AV_FRAME_DATA_FILM_GRAIN_PARAMS);
 
             if (frame->flags & VVC_FRAME_FLAG_BUMPING)
                 ff_vvc_unref_frame(fc, frame, VVC_FRAME_FLAG_OUTPUT | VVC_FRAME_FLAG_BUMPING);
             else
                 ff_vvc_unref_frame(fc, frame, VVC_FRAME_FLAG_OUTPUT);
+
             if (ret < 0)
                 return ret;
 
-- 
2.39.5 (Apple Git-154)

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

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-06-26 23:24 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-06-26 21:49 [FFmpeg-devel] [PATCH] avcodec/vvc/refs: remove dead code Marvin Scholz
2025-06-26 22:50 ` Frank Plowman
2025-06-26 23:23   ` [FFmpeg-devel] [PATCH v2] avcodec/vvc/refs: remove early return Marvin Scholz

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git