diff mbox series

[FFmpeg-devel] hevcdec: skip slices with missing PPS instead of skipping the entire packet

Message ID 20211228134929.1164-1-h.leppkes@gmail.com
State New
Headers show
Series [FFmpeg-devel] hevcdec: skip slices with missing PPS instead of skipping the entire packet | expand

Checks

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
andriy/make_ppc success Make finished
andriy/make_fate_ppc fail Make fate failed

Commit Message

Hendrik Leppkes Dec. 28, 2021, 1:49 p.m. UTC 
Aborting decoding of the entire packet on a missing PPS can result in
missing the actual PPS on streams with badly ordered NALs, where the
SPS/PPS/VPS are stitched to the back of the previous frame, instead of
the beginning of the next frame.

Instead, skip the undecodable slice, and let the decoder process further
NALs in the same packet.
---
 libavcodec/hevcdec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

James Almer Dec. 28, 2021, 2:26 p.m. UTC | #1 
On 12/28/2021 10:49 AM, Hendrik Leppkes wrote:
> Aborting decoding of the entire packet on a missing PPS can result in
> missing the actual PPS on streams with badly ordered NALs, where the
> SPS/PPS/VPS are stitched to the back of the previous frame, instead of
> the beginning of the next frame.
> 
> Instead, skip the undecodable slice, and let the decoder process further
> NALs in the same packet.
> ---
>   libavcodec/hevcdec.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
> index 3aa70e2245..c2451d682e 100644
> --- a/libavcodec/hevcdec.c
> +++ b/libavcodec/hevcdec.c
> @@ -575,7 +575,7 @@ static int hls_slice_header(HEVCContext *s)
>       sh->pps_id = get_ue_golomb_long(gb);
>       if (sh->pps_id >= HEVC_MAX_PPS_COUNT || !s->ps.pps_list[sh->pps_id]) {
>           av_log(s->avctx, AV_LOG_ERROR, "PPS id out of range: %d\n", sh->pps_id);
> -        return AVERROR_INVALIDDATA;
> +        return 1; // skip slice with missing PPS

The decoder should IMO also set the output frame's decode_error_flags to 
FF_DECODE_ERROR_DECODE_SLICES in this scenario.

>       }
>       if (!sh->first_slice_in_pic_flag &&
>           s->ps.pps != (HEVCPPS*)s->ps.pps_list[sh->pps_id]->data) {
Hendrik Leppkes Dec. 28, 2021, 10:01 p.m. UTC | #2 
On Tue, Dec 28, 2021 at 3:26 PM James Almer <jamrial@gmail.com> wrote:
>
> On 12/28/2021 10:49 AM, Hendrik Leppkes wrote:
> > Aborting decoding of the entire packet on a missing PPS can result in
> > missing the actual PPS on streams with badly ordered NALs, where the
> > SPS/PPS/VPS are stitched to the back of the previous frame, instead of
> > the beginning of the next frame.
> >
> > Instead, skip the undecodable slice, and let the decoder process further
> > NALs in the same packet.
> > ---
> >   libavcodec/hevcdec.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
> > index 3aa70e2245..c2451d682e 100644
> > --- a/libavcodec/hevcdec.c
> > +++ b/libavcodec/hevcdec.c
> > @@ -575,7 +575,7 @@ static int hls_slice_header(HEVCContext *s)
> >       sh->pps_id = get_ue_golomb_long(gb);
> >       if (sh->pps_id >= HEVC_MAX_PPS_COUNT || !s->ps.pps_list[sh->pps_id]) {
> >           av_log(s->avctx, AV_LOG_ERROR, "PPS id out of range: %d\n", sh->pps_id);
> > -        return AVERROR_INVALIDDATA;
> > +        return 1; // skip slice with missing PPS
>
> The decoder should IMO also set the output frame's decode_error_flags to
> FF_DECODE_ERROR_DECODE_SLICES in this scenario.
>

There is a variety of error conditions that can lead to a slice being
skipped, not just this one. I can potentially add it here, but it
won't be a very exhaustive flag.
If its also the first slice, the frame will fail to decode entirely as
well, although the point primarily here is to allow parsing of
non-image NALs.

- Hendrik
James Almer Dec. 28, 2021, 11:08 p.m. UTC | #3 
On 12/28/2021 7:01 PM, Hendrik Leppkes wrote:
> On Tue, Dec 28, 2021 at 3:26 PM James Almer <jamrial@gmail.com> wrote:
>>
>> On 12/28/2021 10:49 AM, Hendrik Leppkes wrote:
>>> Aborting decoding of the entire packet on a missing PPS can result in
>>> missing the actual PPS on streams with badly ordered NALs, where the
>>> SPS/PPS/VPS are stitched to the back of the previous frame, instead of
>>> the beginning of the next frame.
>>>
>>> Instead, skip the undecodable slice, and let the decoder process further
>>> NALs in the same packet.
>>> ---
>>>    libavcodec/hevcdec.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
>>> index 3aa70e2245..c2451d682e 100644
>>> --- a/libavcodec/hevcdec.c
>>> +++ b/libavcodec/hevcdec.c
>>> @@ -575,7 +575,7 @@ static int hls_slice_header(HEVCContext *s)
>>>        sh->pps_id = get_ue_golomb_long(gb);
>>>        if (sh->pps_id >= HEVC_MAX_PPS_COUNT || !s->ps.pps_list[sh->pps_id]) {
>>>            av_log(s->avctx, AV_LOG_ERROR, "PPS id out of range: %d\n", sh->pps_id);
>>> -        return AVERROR_INVALIDDATA;
>>> +        return 1; // skip slice with missing PPS
>>
>> The decoder should IMO also set the output frame's decode_error_flags to
>> FF_DECODE_ERROR_DECODE_SLICES in this scenario.
>>
> 
> There is a variety of error conditions that can lead to a slice being
> skipped, not just this one. I can potentially add it here, but it
> won't be a very exhaustive flag.
> If its also the first slice, the frame will fail to decode entirely as
> well, although the point primarily here is to allow parsing of
> non-image NALs.

What h264 does is set an internal flag on any kind of slice error at 
assorted places, then set the AVFrame one before returning it based on 
that (Needed because of frame threading).

It's not a blocker in any case. But even if it's just set at this point 
for the time being, it would at least be a start.

> 
> - Hendrik
> _______________________________________________
> 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".
diff mbox series

Patch

diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
index 3aa70e2245..c2451d682e 100644
--- a/libavcodec/hevcdec.c
+++ b/libavcodec/hevcdec.c
@@ -575,7 +575,7 @@  static int hls_slice_header(HEVCContext *s)
     sh->pps_id = get_ue_golomb_long(gb);
     if (sh->pps_id >= HEVC_MAX_PPS_COUNT || !s->ps.pps_list[sh->pps_id]) {
         av_log(s->avctx, AV_LOG_ERROR, "PPS id out of range: %d\n", sh->pps_id);
-        return AVERROR_INVALIDDATA;
+        return 1; // skip slice with missing PPS
     }
     if (!sh->first_slice_in_pic_flag &&
         s->ps.pps != (HEVCPPS*)s->ps.pps_list[sh->pps_id]->data) {