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

[Paul B Mahol]

On 9/23/23, Michael Niedermayer <michael at niedermayer.cc> wrote:
> On Sat, Sep 23, 2023 at 12:01:17AM +0200, Paul B Mahol wrote:
>> On 9/22/23, Michael Niedermayer <michael at niedermayer.cc> wrote:
>> > On Fri, Sep 22, 2023 at 11:30:37PM +0200, Paul B Mahol wrote:
>> >> On 9/22/23, Michael Niedermayer <michael at niedermayer.cc> wrote:
>> >> > On Fri, Sep 22, 2023 at 09:32:47PM +0200, Paul B Mahol wrote:
>> >> >> On 9/22/23, Michael Niedermayer <michael at niedermayer.cc> wrote:
>> >> >> > On Thu, Jul 27, 2023 at 01:59:13AM +0200, Michael Niedermayer
>> >> >> > wrote:
>> >> >> >> Signed-off-by: Michael Niedermayer <michael at 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

You are last person here to call some decoder(s) are sloppy written.

>
>
>>
>> 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.
>