| Message ID | 20231001180106.27631-1-michael@niedermayer.cc |
|---|---|
| State | Accepted |
| Commit | 5ddab49d48343385eadb3a435a5491c476b66ecc |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/h2645_parse: Avoid EAGAIN | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
Sent from my mobile device On Sun, 1 Oct 2023, 20:01 Michael Niedermayer, <michael@niedermayer.cc> wrote: > EAGAIN causes an assertion failure when it is returned from the decoder > > Fixes: Assertion consumed != (-(11)) failed at libavcodec/decode.c:462 > Fixes: assertion_IOT_instruction_decode_c_462/poc > > Found-by: Hardik Shah of Vehere (Dawn Treaders team) > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/h2645_parse.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavcodec/h2645_parse.h b/libavcodec/h2645_parse.h > index 787ce971ee4..128dea09efb 100644 > --- a/libavcodec/h2645_parse.h > +++ b/libavcodec/h2645_parse.h > @@ -123,7 +123,7 @@ static inline int get_nalsize(int nal_length_size, > const uint8_t *buf, > > if (*buf_index >= buf_size - nal_length_size) { > // the end of the buffer is reached, refill it > - return AVERROR(EAGAIN); > + return AVERROR_INVALIDDATA; > } > > for (i = 0; i < nal_length_size; i++) > -- > 2.17.1 > But these are not the same for an API user? Kieran >
On Sun, Oct 01, 2023 at 08:24:57PM +0200, Kieran Kunhya wrote: > Sent from my mobile device > > On Sun, 1 Oct 2023, 20:01 Michael Niedermayer, <michael@niedermayer.cc> > wrote: > > > EAGAIN causes an assertion failure when it is returned from the decoder > > > > Fixes: Assertion consumed != (-(11)) failed at libavcodec/decode.c:462 > > Fixes: assertion_IOT_instruction_decode_c_462/poc > > > > Found-by: Hardik Shah of Vehere (Dawn Treaders team) > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavcodec/h2645_parse.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libavcodec/h2645_parse.h b/libavcodec/h2645_parse.h > > index 787ce971ee4..128dea09efb 100644 > > --- a/libavcodec/h2645_parse.h > > +++ b/libavcodec/h2645_parse.h > > @@ -123,7 +123,7 @@ static inline int get_nalsize(int nal_length_size, > > const uint8_t *buf, > > > > if (*buf_index >= buf_size - nal_length_size) { > > // the end of the buffer is reached, refill it > > - return AVERROR(EAGAIN); > > + return AVERROR_INVALIDDATA; > > } > > > > for (i = 0; i < nal_length_size; i++) > > -- > > 2.17.1 > > > > But these are not the same for an API user? What public API returns this ? (and doesnt crash) What public API documents this to be returned ? thx [...]
On 10/1/2023 3:24 PM, Kieran Kunhya wrote: > Sent from my mobile device > > On Sun, 1 Oct 2023, 20:01 Michael Niedermayer, <michael@niedermayer.cc> > wrote: > >> EAGAIN causes an assertion failure when it is returned from the decoder >> >> Fixes: Assertion consumed != (-(11)) failed at libavcodec/decode.c:462 >> Fixes: assertion_IOT_instruction_decode_c_462/poc >> >> Found-by: Hardik Shah of Vehere (Dawn Treaders team) >> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> >> --- >> libavcodec/h2645_parse.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/libavcodec/h2645_parse.h b/libavcodec/h2645_parse.h >> index 787ce971ee4..128dea09efb 100644 >> --- a/libavcodec/h2645_parse.h >> +++ b/libavcodec/h2645_parse.h >> @@ -123,7 +123,7 @@ static inline int get_nalsize(int nal_length_size, >> const uint8_t *buf, >> >> if (*buf_index >= buf_size - nal_length_size) { >> // the end of the buffer is reached, refill it >> - return AVERROR(EAGAIN); >> + return AVERROR_INVALIDDATA; >> } >> >> for (i = 0; i < nal_length_size; i++) >> -- >> 2.17.1 >> > > But these are not the same for an API user? I think that when get_nalsize() was written, this specific path was meant to be handled by internal callers (e.g, ff_h2645_packet_split) in some custom way, but ultimately none did, and simply treated all return codes < 0 as errors, propagating them all the way to the library user. So either code using get_nalsize() gets adapted to properly handle EAGAIN as "refill the buffer" (judging by the comment above), or just remove all pretensions of there being situations other than fatal errors and success, which is what Michael did. > > Kieran > >> > _______________________________________________ > 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".
On Sun, Oct 01, 2023 at 08:01:06PM +0200, Michael Niedermayer wrote: > EAGAIN causes an assertion failure when it is returned from the decoder > > Fixes: Assertion consumed != (-(11)) failed at libavcodec/decode.c:462 > Fixes: assertion_IOT_instruction_decode_c_462/poc > > Found-by: Hardik Shah of Vehere (Dawn Treaders team) > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/h2645_parse.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) will apply [...]
diff --git a/libavcodec/h2645_parse.h b/libavcodec/h2645_parse.h index 787ce971ee4..128dea09efb 100644 --- a/libavcodec/h2645_parse.h +++ b/libavcodec/h2645_parse.h @@ -123,7 +123,7 @@ static inline int get_nalsize(int nal_length_size, const uint8_t *buf, if (*buf_index >= buf_size - nal_length_size) { // the end of the buffer is reached, refill it - return AVERROR(EAGAIN); + return AVERROR_INVALIDDATA; } for (i = 0; i < nal_length_size; i++)
EAGAIN causes an assertion failure when it is returned from the decoder Fixes: Assertion consumed != (-(11)) failed at libavcodec/decode.c:462 Fixes: assertion_IOT_instruction_decode_c_462/poc Found-by: Hardik Shah of Vehere (Dawn Treaders team) Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/h2645_parse.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)