diff mbox series

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

Message ID 20201209202513.27449-4-jonas@kwiboo.se
State New
Headers show
Series Add V4L2 request API H.264 hwaccel
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate warning Make fate failed

Commit Message

Jonas Karlman Dec. 9, 2020, 8:25 p.m. UTC
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(-)

Comments

Mark Thompson Dec. 9, 2020, 10:23 p.m. UTC | #1
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
Jonas Karlman Dec. 9, 2020, 10:58 p.m. UTC | #2
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
>
diff mbox series

Patch

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;
 
 /**