Re: [FFmpeg-devel] [PATCH 1/4] avcodec/rtv1: Check if the minimal size is available in decode_rtv1()

From: Michael Niedermayer <michael@niedermayer.cc>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH 1/4] avcodec/rtv1: Check if the minimal size is available in decode_rtv1()
Date: Sat, 23 Sep 2023 00:33:36 +0200
Message-ID: <20230922223336.GB3543730@pb2> (raw)
In-Reply-To: <CAPYw7P4ks5koF5XU=fwiBi6nAe6m+qvV9QUsFFs11wUcmUVLoA@mail.gmail.com>

[-- Attachment #1.1: Type: text/plain, Size: 2366 bytes --]

On Sat, Sep 23, 2023 at 12:01:17AM +0200, Paul B Mahol wrote:
> On 9/22/23, Michael Niedermayer <michael@niedermayer.cc> wrote:
> > On Fri, Sep 22, 2023 at 11:30:37PM +0200, Paul B Mahol wrote:
> >> On 9/22/23, Michael Niedermayer <michael@niedermayer.cc> wrote:
> >> > On Fri, Sep 22, 2023 at 09:32:47PM +0200, Paul B Mahol wrote:
> >> >> On 9/22/23, Michael Niedermayer <michael@niedermayer.cc> wrote:
> >> >> > On Thu, Jul 27, 2023 at 01:59:13AM +0200, Michael Niedermayer wrote:
> >> >> >> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> >> >> >> ---
> >> >> >>  libavcodec/rtv1.c | 6 +++++-
> >> >> >>  1 file changed, 5 insertions(+), 1 deletion(-)
> >> >> >
> >> >> > will apply 1-3 of this patchset
> >> >>
> >> >> Are you sure this does not break decoding?
> >> >
> >> > Well, its a loop over 4x4 blocks, a 16bit "skip" run so the minimum
> >> > check looks correct.
> >> > There are 2 end of bitstream checks for early exit but they look like
> >> > error handling not some normal exit as they leave the frame
> >> > uninitialized
> >> >
> >>
> >> FFmpeg default initialization code for AVFrame's buffers does it
> >> twice, so they are always zeroed or previous values of previous
> >> buffers in pool.
> >
> > its rare that correct frame decoding depends on internal AVFrame buffer
> > ordering
> >
> 
> Users are supposed to use error checking. And I think decoder returns
> error on missing frame data.

yes, the rtv1 decoder looks a bit sloppy written, not returning error
codes on what looks like error checks.
Its not the only code doing that, ive seen this in other files too

> 
> When we lost interest in preserving all decoded frame pixels as much
> as possible?

when patches using discard_damaged_percentage where getting blocked in review
while simpler but less ideal solutions made all reviewers happy

I can implement this using discard_damaged_percentage, then the user can
decide at which point a frame would be too damaged to decode/return
and also to drop none or all with damage as the user prefers

thx

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

Homeopathy is like voting while filling the ballot out with transparent ink.
Sometimes the outcome one wanted occurs. Rarely its worse than filling out
a ballot properly.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 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".