[FFmpeg-devel,3/5] h264dec: add idr_pic_id to slice context

From: Ezequiel Garcia <ezequiel@collabora.com>

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 libavcodec/h264_slice.c | 2 +-
 libavcodec/h264dec.h    | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

On 09/12/2020 20:25, Jonas Karlman wrote:
> From: Ezequiel Garcia <ezequiel@collabora.com>
> 
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
>   libavcodec/h264_slice.c | 2 +-
>   libavcodec/h264dec.h    | 1 +
>   2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
> index fa7a639053..8a3ce1a688 100644
> --- a/libavcodec/h264_slice.c
> +++ b/libavcodec/h264_slice.c
> @@ -1824,7 +1824,7 @@ static int h264_slice_header_parse(const H264Context *h, H264SliceContext *sl,
>       }
>   
>       if (nal->type == H264_NAL_IDR_SLICE)
> -        get_ue_golomb_long(&sl->gb); /* idr_pic_id */
> +        sl->idr_pic_id = get_ue_golomb_long(&sl->gb);
>   
>       if (sps->poc_type == 0) {
>           sl->poc_lsb = get_bits(&sl->gb, sps->log2_max_poc_lsb);
> diff --git a/libavcodec/h264dec.h b/libavcodec/h264dec.h
> index 29c4d4e42c..2ca9965b41 100644
> --- a/libavcodec/h264dec.h
> +++ b/libavcodec/h264dec.h
> @@ -335,6 +335,7 @@ typedef struct H264SliceContext {
>       int delta_poc[2];
>       int curr_pic_num;
>       int max_pic_num;
> +    int idr_pic_id;
>   } H264SliceContext;
>   
>   /**
> 

This feels very suspicious: idr_pic_id is unrelated to decoding, so a decoder which doesn't ignore must be doing something wrong?  (It's for detecting frame boundaries under packet loss so you don't splice together parts of unrelated intra frames.)

- Mark

On 2020-12-09 23:23, Mark Thompson wrote:
> On 09/12/2020 20:25, Jonas Karlman wrote:
>> From: Ezequiel Garcia <ezequiel@collabora.com>
>>
>> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>> ---
>>   libavcodec/h264_slice.c | 2 +-
>>   libavcodec/h264dec.h    | 1 +
>>   2 files changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
>> index fa7a639053..8a3ce1a688 100644
>> --- a/libavcodec/h264_slice.c
>> +++ b/libavcodec/h264_slice.c
>> @@ -1824,7 +1824,7 @@ static int h264_slice_header_parse(const H264Context *h, H264SliceContext *sl,
>>       }
>>   
>>       if (nal->type == H264_NAL_IDR_SLICE)
>> -        get_ue_golomb_long(&sl->gb); /* idr_pic_id */
>> +        sl->idr_pic_id = get_ue_golomb_long(&sl->gb);
>>   
>>       if (sps->poc_type == 0) {
>>           sl->poc_lsb = get_bits(&sl->gb, sps->log2_max_poc_lsb);
>> diff --git a/libavcodec/h264dec.h b/libavcodec/h264dec.h
>> index 29c4d4e42c..2ca9965b41 100644
>> --- a/libavcodec/h264dec.h
>> +++ b/libavcodec/h264dec.h
>> @@ -335,6 +335,7 @@ typedef struct H264SliceContext {
>>       int delta_poc[2];
>>       int curr_pic_num;
>>       int max_pic_num;
>> +    int idr_pic_id;
>>   } H264SliceContext;
>>   
>>   /**
>>
> 
> This feels very suspicious: idr_pic_id is unrelated to decoding, so a decoder which doesn't ignore must be doing something wrong?  (It's for detecting frame boundaries under packet loss so you don't splice together parts of unrelated intra frames.)

The Hantro VPU IP block seems to want this information together with the bit size of the ref_pic_marking for proper decoding, see [1] for how hwregs is configured.

I have not done any test with setting a forced idr_pic_id=0 in hw block.

Docs only mention the following:
 reference picture related

 h264_idrp_id
 instantaneous decoding refresh picture id

I will run some limited tests to see if decoded frame output changes when this hw reg is skipped.

[1] https://git.linuxtv.org/media_tree.git/tree/drivers/staging/media/hantro/hantro_g1_h264_dec.c#n89

Best regards,
Jonas

> 
> - Mark
>