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

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

[Michael Niedermayer]

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/rtv1.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/libavcodec/rtv1.c b/libavcodec/rtv1.c
index 4b202e6a21..95fa9210d8 100644
--- a/libavcodec/rtv1.c
+++ b/libavcodec/rtv1.c
@@ -44,6 +44,8 @@ static int decode_rtv1(GetByteContext *gb, uint8_t *dst, ptrdiff_t linesize,
     uint8_t block[8] = { 0 };
     int run = 0;
 
+    if (bytestream2_get_bytes_left(gb) < (width / 4) * (height / 4) / 0xFFFF * 4)
+        return AVERROR_INVALIDDATA;
     for (int y = 0; y < height; y += 4) {
         for (int x = 0; x < width * 4; x += 16) {
             int mode = 0;
@@ -126,7 +128,9 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *p,
     dst = p->data[0] + p->linesize[0] * (avctx->coded_height - 1);
     linesize = -p->linesize[0];
 
-    decode_rtv1(&gb, dst, linesize, width, height, flags, dsp->dxt1_block);
+    ret = decode_rtv1(&gb, dst, linesize, width, height, flags, dsp->dxt1_block);
+    if (ret < 0)
+        return ret;
 
     p->pict_type = AV_PICTURE_TYPE_I;
     p->flags |= AV_FRAME_FLAG_KEY;
-- 
2.17.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-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 2/4] tools/target_dec_fuzzer: Adjust threshold for rtv1

[Michael Niedermayer]

Fixes: 60499/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_RTV1_fuzzer-5020295866744832
Fixes: Timeout

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 tools/target_dec_fuzzer.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/target_dec_fuzzer.c b/tools/target_dec_fuzzer.c
index 165951dc9d..570291fa79 100644
--- a/tools/target_dec_fuzzer.c
+++ b/tools/target_dec_fuzzer.c
@@ -274,6 +274,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
     case AV_CODEC_ID_RKA:         maxsamples /= 256;   break;
     case AV_CODEC_ID_RSCC:        maxpixels  /= 256;   break;
     case AV_CODEC_ID_RASC:        maxpixels  /= 16;    break;
+    case AV_CODEC_ID_RTV1:        maxpixels  /= 16;    break;
     case AV_CODEC_ID_SANM:        maxpixels  /= 16;    break;
     case AV_CODEC_ID_SCPR:        maxpixels  /= 32;    break;
     case AV_CODEC_ID_SCREENPRESSO:maxpixels  /= 64;    break;
-- 
2.17.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-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 3/4] avcodec/vvc_parser: Avoid undefined overflow in POC computation

[Michael Niedermayer]

The comments to the function say that it does not implement the spec and
instead follows VTM.
This patch is quite likely not the right solution and more intended to show
the issue to people knowing the specific part of VTM ...

Fixes: signed integer overflow: 2147483392 + 256 cannot be represented in type 'int'
Fixes: 60505/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-6216675924770816

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/vvc_parser.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/vvc_parser.c b/libavcodec/vvc_parser.c
index 3951ebe50a..c661595e1e 100644
--- a/libavcodec/vvc_parser.c
+++ b/libavcodec/vvc_parser.c
@@ -225,10 +225,10 @@ static void get_slice_poc(VVCParserContext *s, int *poc,
         } else {
             if ((poc_lsb < prev_poc_lsb) && ((prev_poc_lsb - poc_lsb) >=
                 (max_poc_lsb / 2)))
-                poc_msb = prev_poc_msb + max_poc_lsb;
+                poc_msb = prev_poc_msb + (unsigned)max_poc_lsb;
             else if ((poc_lsb > prev_poc_lsb) && ((poc_lsb - prev_poc_lsb) >
                      (max_poc_lsb / 2)))
-                poc_msb = prev_poc_msb - max_poc_lsb;
+                poc_msb = prev_poc_msb - (unsigned)max_poc_lsb;
             else
                 poc_msb = prev_poc_msb;
         }
-- 
2.17.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-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 4/4] avcodec/evc_ps: Check num_ref_pic_list_in_sps

[Michael Niedermayer]

Fixes: out of array write
Fixes: 60798/clusterfuzz-testcase-minimized-ffmpeg_BSF_EVC_FRAME_MERGE_fuzzer-4633529766772736

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/evc_ps.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/libavcodec/evc_ps.c b/libavcodec/evc_ps.c
index 04ee6a45e6..64384a392c 100644
--- a/libavcodec/evc_ps.c
+++ b/libavcodec/evc_ps.c
@@ -243,11 +243,20 @@ int ff_evc_parse_sps(GetBitContext *gb, EVCParamSets *ps)
         sps->rpl1_same_as_rpl0_flag = get_bits1(gb);
         sps->num_ref_pic_list_in_sps[0] = get_ue_golomb(gb);
 
+        if ((unsigned)sps->num_ref_pic_list_in_sps[0] >= EVC_MAX_NUM_RPLS) {
+            ret = AVERROR_INVALIDDATA;
+            goto fail;
+        }
+
         for (int i = 0; i < sps->num_ref_pic_list_in_sps[0]; ++i)
             ref_pic_list_struct(gb, &sps->rpls[0][i]);
 
         if (!sps->rpl1_same_as_rpl0_flag) {
             sps->num_ref_pic_list_in_sps[1] = get_ue_golomb(gb);
+            if ((unsigned)sps->num_ref_pic_list_in_sps[1] >= EVC_MAX_NUM_RPLS) {
+                ret = AVERROR_INVALIDDATA;
+                goto fail;
+            }
             for (int i = 0; i < sps->num_ref_pic_list_in_sps[1]; ++i)
                 ref_pic_list_struct(gb, &sps->rpls[1][i]);
         }
-- 
2.17.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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 4/4] avcodec/evc_ps: Check num_ref_pic_list_in_sps

[James Almer]

On 7/26/2023 8:59 PM, Michael Niedermayer wrote:
> Fixes: out of array write
> Fixes: 60798/clusterfuzz-testcase-minimized-ffmpeg_BSF_EVC_FRAME_MERGE_fuzzer-4633529766772736
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>   libavcodec/evc_ps.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/libavcodec/evc_ps.c b/libavcodec/evc_ps.c
> index 04ee6a45e6..64384a392c 100644
> --- a/libavcodec/evc_ps.c
> +++ b/libavcodec/evc_ps.c
> @@ -243,11 +243,20 @@ int ff_evc_parse_sps(GetBitContext *gb, EVCParamSets *ps)
>           sps->rpl1_same_as_rpl0_flag = get_bits1(gb);
>           sps->num_ref_pic_list_in_sps[0] = get_ue_golomb(gb);
>   
> +        if ((unsigned)sps->num_ref_pic_list_in_sps[0] >= EVC_MAX_NUM_RPLS) {

EVC_MAX_NUM_RPLS should be 64, not 32 as it's currently defined. So 
change that too and it LGTM.

> +            ret = AVERROR_INVALIDDATA;
> +            goto fail;
> +        }
> +
>           for (int i = 0; i < sps->num_ref_pic_list_in_sps[0]; ++i)
>               ref_pic_list_struct(gb, &sps->rpls[0][i]);
>   
>           if (!sps->rpl1_same_as_rpl0_flag) {
>               sps->num_ref_pic_list_in_sps[1] = get_ue_golomb(gb);
> +            if ((unsigned)sps->num_ref_pic_list_in_sps[1] >= EVC_MAX_NUM_RPLS) {
> +                ret = AVERROR_INVALIDDATA;
> +                goto fail;
> +            }
>               for (int i = 0; i < sps->num_ref_pic_list_in_sps[1]; ++i)
>                   ref_pic_list_struct(gb, &sps->rpls[1][i]);
>           }
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 4/4] avcodec/evc_ps: Check num_ref_pic_list_in_sps

[Michael Niedermayer]

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

On Wed, Jul 26, 2023 at 09:19:10PM -0300, James Almer wrote:
> On 7/26/2023 8:59 PM, Michael Niedermayer wrote:
> > Fixes: out of array write
> > Fixes: 60798/clusterfuzz-testcase-minimized-ffmpeg_BSF_EVC_FRAME_MERGE_fuzzer-4633529766772736
> > 
> > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >   libavcodec/evc_ps.c | 9 +++++++++
> >   1 file changed, 9 insertions(+)
> > 
> > diff --git a/libavcodec/evc_ps.c b/libavcodec/evc_ps.c
> > index 04ee6a45e6..64384a392c 100644
> > --- a/libavcodec/evc_ps.c
> > +++ b/libavcodec/evc_ps.c
> > @@ -243,11 +243,20 @@ int ff_evc_parse_sps(GetBitContext *gb, EVCParamSets *ps)
> >           sps->rpl1_same_as_rpl0_flag = get_bits1(gb);
> >           sps->num_ref_pic_list_in_sps[0] = get_ue_golomb(gb);
> > +        if ((unsigned)sps->num_ref_pic_list_in_sps[0] >= EVC_MAX_NUM_RPLS) {
> 
> EVC_MAX_NUM_RPLS should be 64, not 32 as it's currently defined. So change
> that too and it LGTM.

ok will do

thx for reviewing

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

"You are 36 times more likely to die in a bathtub than at the hands of a
terrorist. Also, you are 2.5 times more likely to become a president and
2 times more likely to become an astronaut, than to die in a terrorist
attack." -- Thoughty2


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

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

[Michael Niedermayer]

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

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

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

He who knows, does not speak. He who speaks, does not know. -- Lao Tsu

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

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

[Paul B Mahol]

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?

It would not be first time you intentionally break decoders because of hacks.


>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> He who knows, does not speak. He who speaks, does not know. -- Lao Tsu
>
_______________________________________________
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".

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

[Michael Niedermayer]

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

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

Do you have some files so i can double check this is not breaking anything ?

thx

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

Those who would give up essential Liberty, to purchase a little
temporary Safety, deserve neither Liberty nor Safety -- Benjamin Franklin

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

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

[Paul B Mahol]

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.

> Do you have some files so i can double check this is not breaking anything

Search trac tickets.

> ?
>
> thx
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Those who would give up essential Liberty, to purchase a little
> temporary Safety, deserve neither Liberty nor Safety -- Benjamin Franklin
>
_______________________________________________
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".

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

[Michael Niedermayer]

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

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


> 
> > Do you have some files so i can double check this is not breaking anything
> 
> Search trac tickets.

thanks, found a zip with 2 video files
ill check it


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

"I am not trying to be anyone's saviour, I'm trying to think about the
 future and not be sad" - Elon Musk


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

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

[Paul B Mahol]

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.

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

>
>>
>> > Do you have some files so i can double check this is not breaking
>> > anything
>>
>> Search trac tickets.
>
> thanks, found a zip with 2 video files
> ill check it
>
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> "I am not trying to be anyone's saviour, I'm trying to think about the
>  future and not be sad" - Elon Musk
>
>
_______________________________________________
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".

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

[Michael Niedermayer]

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

Re: [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@niedermayer.cc> wrote:
> 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

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