diff mbox series

[FFmpeg-devel] libavcodec/h264dec: export block type in H.264

Message ID 20200715210530.790655-1-yonglel@google.com
State New
Headers show
Series [FFmpeg-devel] libavcodec/h264dec: export block type in H.264
Related show

Checks

Context Check Description
andriy/default pending
andriy/make fail Make failed
andriy/default pending
andriy/make fail Make failed

Commit Message

Yongle Lin July 15, 2020, 9:05 p.m. UTC
---
 libavcodec/h264dec.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Lynne July 15, 2020, 9:15 p.m. UTC | #1
Jul 15, 2020, 22:05 by yongle.lin.94@gmail.com:

> ---
>  libavcodec/h264dec.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
> index 1e2ca68449..b3de5290d0 100644
> --- a/libavcodec/h264dec.c
> +++ b/libavcodec/h264dec.c
> @@ -816,6 +816,20 @@ static int h264_export_enc_params(AVFrame *f, H264Picture *p)
>  b->h     = 16;
>  
>  b->delta_qp = p->qscale_table[mb_xy] - par->qp;
> +
> +            int mb_type = p->mb_type[mb_xy];
> +            if (IS_PCM(mb_type))
> +                b->flags |= AV_VIDEO_ENC_BLOCK_INTRA;
> +            if (IS_SKIP(mb_type))
> +                b->flags |= AV_VIDEO_ENC_BLOCK_SKIP;
> +            if (!USES_LIST(mb_type, 1))
> +                b->ref[0] = p->ref_index[0];
> +            else if (!USES_LIST(mb_type, 0))
> +                b->ref[0] = p->ref_index[1];
> +            else {
> +                b->ref[0] = p->ref_index[0];
> +                b->ref[1] = p->ref_index[1];
> +            }
>  } 
>

Looks good to me, apart from the wrong bracket style, which anyone applying
can correct.
I think Anton or Mark need to take a look at this too, since they know h264 better
than I do.
Mark Thompson July 15, 2020, 11:09 p.m. UTC | #2
On 15/07/2020 22:05, Yongle Lin wrote:
> ---
>   libavcodec/h264dec.c | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
> 
> diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
> index 1e2ca68449..b3de5290d0 100644
> --- a/libavcodec/h264dec.c
> +++ b/libavcodec/h264dec.c
> @@ -816,6 +816,20 @@ static int h264_export_enc_params(AVFrame *f, H264Picture *p)
>               b->h     = 16;
>   
>               b->delta_qp = p->qscale_table[mb_xy] - par->qp;
> +
> +            int mb_type = p->mb_type[mb_xy];
> +            if (IS_PCM(mb_type))
> +                b->flags |= AV_VIDEO_ENC_BLOCK_INTRA;

Can you explain your definition of what makes a block "intra"?  If you really do mean to only include PCM blocks then maybe it should have a different name, because that isn't what I would expect.

> +            if (IS_SKIP(mb_type))
> +                b->flags |= AV_VIDEO_ENC_BLOCK_SKIP; > +            if (!USES_LIST(mb_type, 1))
> +                b->ref[0] = p->ref_index[0];
> +            else if (!USES_LIST(mb_type, 0))
> +                b->ref[0] = p->ref_index[1];
> +            else {
> +                b->ref[0] = p->ref_index[0];
> +                b->ref[1] = p->ref_index[1];

I don't think ref_index is anything to do with what you think it is.

Also note that code of this form must be insufficient due to the lack dependence on both the slice and the block.  (Slice headers define the two reference picture lists, and blocks within a slice 
choose pictures from those lists.)

> +            }
>           }
>   
>       return 0;
> 

- Mark
diff mbox series

Patch

diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
index 1e2ca68449..b3de5290d0 100644
--- a/libavcodec/h264dec.c
+++ b/libavcodec/h264dec.c
@@ -816,6 +816,20 @@  static int h264_export_enc_params(AVFrame *f, H264Picture *p)
             b->h     = 16;
 
             b->delta_qp = p->qscale_table[mb_xy] - par->qp;
+
+            int mb_type = p->mb_type[mb_xy];
+            if (IS_PCM(mb_type))
+                b->flags |= AV_VIDEO_ENC_BLOCK_INTRA;
+            if (IS_SKIP(mb_type))
+                b->flags |= AV_VIDEO_ENC_BLOCK_SKIP;
+            if (!USES_LIST(mb_type, 1))
+                b->ref[0] = p->ref_index[0];
+            else if (!USES_LIST(mb_type, 0))
+                b->ref[0] = p->ref_index[1];
+            else {
+                b->ref[0] = p->ref_index[0];
+                b->ref[1] = p->ref_index[1];
+            }
         }
 
     return 0;