diff mbox series

[FFmpeg-devel] libavcodec/hevc_mp4toannexb_bsf: ignore extra data if possible

Message ID 20210910040948.14186-1-haihao.xiang@intel.com
State New
Headers show
Series [FFmpeg-devel] libavcodec/hevc_mp4toannexb_bsf: ignore extra data if possible | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_ppc success Make finished
andriy/make_fate_ppc success Make fate finished

Commit Message

Xiang, Haihao Sept. 10, 2021, 4:09 a.m. UTC 
It is possible that an IRAP frame in input AVPacket has SPS and PPS, and
these headers should take effect. Hence we should not prepend extra data
to IRAP frame in this case, otherwise an IRAP frame in output AVPacket
will have 2 SPS/PPS when extra data also has SPS and PPS, the second
SPS/PPS will override the first SPS/PPS and take effect.
---
 libavcodec/hevc_mp4toannexb_bsf.c | 7 +++++--
 tests/fate/hevc.mak               | 2 +-
 2 files changed, 6 insertions(+), 3 deletions(-)

Comments

galinart Sept. 13, 2021, 12:15 p.m. UTC | #1 
LGTM.

On Fri, 10 Sept 2021 at 05:10, Haihao Xiang <haihao.xiang@intel.com> wrote:
>
> It is possible that an IRAP frame in input AVPacket has SPS and PPS, and
> these headers should take effect. Hence we should not prepend extra data
> to IRAP frame in this case, otherwise an IRAP frame in output AVPacket
> will have 2 SPS/PPS when extra data also has SPS and PPS, the second
> SPS/PPS will override the first SPS/PPS and take effect.
> ---
>  libavcodec/hevc_mp4toannexb_bsf.c | 7 +++++--
>  tests/fate/hevc.mak               | 2 +-
>  2 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/libavcodec/hevc_mp4toannexb_bsf.c b/libavcodec/hevc_mp4toannexb_bsf.c
> index 790dfb0394..3b3732bbd0 100644
> --- a/libavcodec/hevc_mp4toannexb_bsf.c
> +++ b/libavcodec/hevc_mp4toannexb_bsf.c
> @@ -121,7 +121,7 @@ static int hevc_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *out)
>      HEVCBSFContext *s = ctx->priv_data;
>      AVPacket *in;
>      GetByteContext gb;
> -
> +    int has_sps = 0, has_pps = 0;
>      int got_irap = 0;
>      int i, ret = 0;
>
> @@ -155,10 +155,13 @@ static int hevc_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *out)
>          }
>
>          nalu_type = (bytestream2_peek_byte(&gb) >> 1) & 0x3f;
> +        has_sps = (has_sps || nalu_type == HEVC_NAL_SPS);
> +        has_pps = (has_pps || nalu_type == HEVC_NAL_PPS);
>
>          /* prepend extradata to IRAP frames */
>          is_irap       = nalu_type >= 16 && nalu_type <= 23;
> -        add_extradata = is_irap && !got_irap;
> +        /* ignore the extradata if IRAP frame has sps and pps */
> +        add_extradata = is_irap && !got_irap && !(has_sps && has_pps);
>          extra_size    = add_extradata * ctx->par_out->extradata_size;
>          got_irap     |= is_irap;
>
> diff --git a/tests/fate/hevc.mak b/tests/fate/hevc.mak
> index f6ea1df9a5..a4dac99b6a 100644
> --- a/tests/fate/hevc.mak
> +++ b/tests/fate/hevc.mak
> @@ -251,7 +251,7 @@ FATE_HEVC-$(call ALLYES, HEVC_DEMUXER MOV_DEMUXER HEVC_MP4TOANNEXB_BSF MOV_MUXER
>  fate-hevc-bsf-mp4toannexb: tests/data/hevc-mp4.mov
>  fate-hevc-bsf-mp4toannexb: CMD = md5 -i $(TARGET_PATH)/tests/data/hevc-mp4.mov -c:v copy -fflags +bitexact -f hevc
>  fate-hevc-bsf-mp4toannexb: CMP = oneline
> -fate-hevc-bsf-mp4toannexb: REF = 1873662a3af1848c37e4eb25722c8df9
> +fate-hevc-bsf-mp4toannexb: REF = 3c9d998a3aa2b9e0fb1c1f434952bf8b
>
>  fate-hevc-skiploopfilter: CMD = framemd5 -skip_loop_filter nokey -i $(TARGET_SAMPLES)/hevc-conformance/SAO_D_Samsung_5.bit -sws_flags bitexact
>  FATE_HEVC += fate-hevc-skiploopfilter
> --
> 2.17.1
>
> _______________________________________________
> 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".
Xiang, Haihao Sept. 22, 2021, 4:37 a.m. UTC | #2 
> LGTM.

Thanks for reviewing the patch. Could someone please merge this patch if no more
comment ?

Regards
Haihao

> 
> On Fri, 10 Sept 2021 at 05:10, Haihao Xiang <haihao.xiang@intel.com> wrote:
> > 
> > It is possible that an IRAP frame in input AVPacket has SPS and PPS, and
> > these headers should take effect. Hence we should not prepend extra data
> > to IRAP frame in this case, otherwise an IRAP frame in output AVPacket
> > will have 2 SPS/PPS when extra data also has SPS and PPS, the second
> > SPS/PPS will override the first SPS/PPS and take effect.
> > ---
> >  libavcodec/hevc_mp4toannexb_bsf.c | 7 +++++--
> >  tests/fate/hevc.mak               | 2 +-
> >  2 files changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/libavcodec/hevc_mp4toannexb_bsf.c
> > b/libavcodec/hevc_mp4toannexb_bsf.c
> > index 790dfb0394..3b3732bbd0 100644
> > --- a/libavcodec/hevc_mp4toannexb_bsf.c
> > +++ b/libavcodec/hevc_mp4toannexb_bsf.c
> > @@ -121,7 +121,7 @@ static int hevc_mp4toannexb_filter(AVBSFContext *ctx,
> > AVPacket *out)
> >      HEVCBSFContext *s = ctx->priv_data;
> >      AVPacket *in;
> >      GetByteContext gb;
> > -
> > +    int has_sps = 0, has_pps = 0;
> >      int got_irap = 0;
> >      int i, ret = 0;
> > 
> > @@ -155,10 +155,13 @@ static int hevc_mp4toannexb_filter(AVBSFContext *ctx,
> > AVPacket *out)
> >          }
> > 
> >          nalu_type = (bytestream2_peek_byte(&gb) >> 1) & 0x3f;
> > +        has_sps = (has_sps || nalu_type == HEVC_NAL_SPS);
> > +        has_pps = (has_pps || nalu_type == HEVC_NAL_PPS);
> > 
> >          /* prepend extradata to IRAP frames */
> >          is_irap       = nalu_type >= 16 && nalu_type <= 23;
> > -        add_extradata = is_irap && !got_irap;
> > +        /* ignore the extradata if IRAP frame has sps and pps */
> > +        add_extradata = is_irap && !got_irap && !(has_sps && has_pps);
> >          extra_size    = add_extradata * ctx->par_out->extradata_size;
> >          got_irap     |= is_irap;
> > 
> > diff --git a/tests/fate/hevc.mak b/tests/fate/hevc.mak
> > index f6ea1df9a5..a4dac99b6a 100644
> > --- a/tests/fate/hevc.mak
> > +++ b/tests/fate/hevc.mak
> > @@ -251,7 +251,7 @@ FATE_HEVC-$(call ALLYES, HEVC_DEMUXER MOV_DEMUXER
> > HEVC_MP4TOANNEXB_BSF MOV_MUXER
> >  fate-hevc-bsf-mp4toannexb: tests/data/hevc-mp4.mov
> >  fate-hevc-bsf-mp4toannexb: CMD = md5 -i $(TARGET_PATH)/tests/data/hevc-
> > mp4.mov -c:v copy -fflags +bitexact -f hevc
> >  fate-hevc-bsf-mp4toannexb: CMP = oneline
> > -fate-hevc-bsf-mp4toannexb: REF = 1873662a3af1848c37e4eb25722c8df9
> > +fate-hevc-bsf-mp4toannexb: REF = 3c9d998a3aa2b9e0fb1c1f434952bf8b
> > 
> >  fate-hevc-skiploopfilter: CMD = framemd5 -skip_loop_filter nokey -i
> > $(TARGET_SAMPLES)/hevc-conformance/SAO_D_Samsung_5.bit -sws_flags bitexact
> >  FATE_HEVC += fate-hevc-skiploopfilter
> > --
> > 2.17.1
> > 
> > _______________________________________________
> > 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".
> 
> _______________________________________________
> 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".
Xiang, Haihao Oct. 8, 2021, 6:08 a.m. UTC | #3 
On Wed, 2021-09-22 at 04:37 +0000, Xiang, Haihao wrote:
> > LGTM.
> 
> Thanks for reviewing the patch. Could someone please merge this patch if no
> more
> comment ?

Ping again, 

Thanks
Haihao

> 
> Regards
> Haihao
> 
> > 
> > On Fri, 10 Sept 2021 at 05:10, Haihao Xiang <haihao.xiang@intel.com> wrote:
> > > 
> > > It is possible that an IRAP frame in input AVPacket has SPS and PPS, and
> > > these headers should take effect. Hence we should not prepend extra data
> > > to IRAP frame in this case, otherwise an IRAP frame in output AVPacket
> > > will have 2 SPS/PPS when extra data also has SPS and PPS, the second
> > > SPS/PPS will override the first SPS/PPS and take effect.
> > > ---
> > >  libavcodec/hevc_mp4toannexb_bsf.c | 7 +++++--
> > >  tests/fate/hevc.mak               | 2 +-
> > >  2 files changed, 6 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/libavcodec/hevc_mp4toannexb_bsf.c
> > > b/libavcodec/hevc_mp4toannexb_bsf.c
> > > index 790dfb0394..3b3732bbd0 100644
> > > --- a/libavcodec/hevc_mp4toannexb_bsf.c
> > > +++ b/libavcodec/hevc_mp4toannexb_bsf.c
> > > @@ -121,7 +121,7 @@ static int hevc_mp4toannexb_filter(AVBSFContext *ctx,
> > > AVPacket *out)
> > >      HEVCBSFContext *s = ctx->priv_data;
> > >      AVPacket *in;
> > >      GetByteContext gb;
> > > -
> > > +    int has_sps = 0, has_pps = 0;
> > >      int got_irap = 0;
> > >      int i, ret = 0;
> > > 
> > > @@ -155,10 +155,13 @@ static int hevc_mp4toannexb_filter(AVBSFContext
> > > *ctx,
> > > AVPacket *out)
> > >          }
> > > 
> > >          nalu_type = (bytestream2_peek_byte(&gb) >> 1) & 0x3f;
> > > +        has_sps = (has_sps || nalu_type == HEVC_NAL_SPS);
> > > +        has_pps = (has_pps || nalu_type == HEVC_NAL_PPS);
> > > 
> > >          /* prepend extradata to IRAP frames */
> > >          is_irap       = nalu_type >= 16 && nalu_type <= 23;
> > > -        add_extradata = is_irap && !got_irap;
> > > +        /* ignore the extradata if IRAP frame has sps and pps */
> > > +        add_extradata = is_irap && !got_irap && !(has_sps && has_pps);
> > >          extra_size    = add_extradata * ctx->par_out->extradata_size;
> > >          got_irap     |= is_irap;
> > > 
> > > diff --git a/tests/fate/hevc.mak b/tests/fate/hevc.mak
> > > index f6ea1df9a5..a4dac99b6a 100644
> > > --- a/tests/fate/hevc.mak
> > > +++ b/tests/fate/hevc.mak
> > > @@ -251,7 +251,7 @@ FATE_HEVC-$(call ALLYES, HEVC_DEMUXER MOV_DEMUXER
> > > HEVC_MP4TOANNEXB_BSF MOV_MUXER
> > >  fate-hevc-bsf-mp4toannexb: tests/data/hevc-mp4.mov
> > >  fate-hevc-bsf-mp4toannexb: CMD = md5 -i $(TARGET_PATH)/tests/data/hevc-
> > > mp4.mov -c:v copy -fflags +bitexact -f hevc
> > >  fate-hevc-bsf-mp4toannexb: CMP = oneline
> > > -fate-hevc-bsf-mp4toannexb: REF = 1873662a3af1848c37e4eb25722c8df9
> > > +fate-hevc-bsf-mp4toannexb: REF = 3c9d998a3aa2b9e0fb1c1f434952bf8b
> > > 
> > >  fate-hevc-skiploopfilter: CMD = framemd5 -skip_loop_filter nokey -i
> > > $(TARGET_SAMPLES)/hevc-conformance/SAO_D_Samsung_5.bit -sws_flags bitexact
> > >  FATE_HEVC += fate-hevc-skiploopfilter
> > > --
> > > 2.17.1
> > > 
> > > _______________________________________________
> > > 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".
> > 
> > _______________________________________________
> > 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".
> 
> _______________________________________________
> 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/hevc_mp4toannexb_bsf.c b/libavcodec/hevc_mp4toannexb_bsf.c
index 790dfb0394..3b3732bbd0 100644
--- a/libavcodec/hevc_mp4toannexb_bsf.c
+++ b/libavcodec/hevc_mp4toannexb_bsf.c
@@ -121,7 +121,7 @@  static int hevc_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *out)
     HEVCBSFContext *s = ctx->priv_data;
     AVPacket *in;
     GetByteContext gb;
-
+    int has_sps = 0, has_pps = 0;
     int got_irap = 0;
     int i, ret = 0;
 
@@ -155,10 +155,13 @@  static int hevc_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *out)
         }
 
         nalu_type = (bytestream2_peek_byte(&gb) >> 1) & 0x3f;
+        has_sps = (has_sps || nalu_type == HEVC_NAL_SPS);
+        has_pps = (has_pps || nalu_type == HEVC_NAL_PPS);
 
         /* prepend extradata to IRAP frames */
         is_irap       = nalu_type >= 16 && nalu_type <= 23;
-        add_extradata = is_irap && !got_irap;
+        /* ignore the extradata if IRAP frame has sps and pps */
+        add_extradata = is_irap && !got_irap && !(has_sps && has_pps);
         extra_size    = add_extradata * ctx->par_out->extradata_size;
         got_irap     |= is_irap;
 
diff --git a/tests/fate/hevc.mak b/tests/fate/hevc.mak
index f6ea1df9a5..a4dac99b6a 100644
--- a/tests/fate/hevc.mak
+++ b/tests/fate/hevc.mak
@@ -251,7 +251,7 @@  FATE_HEVC-$(call ALLYES, HEVC_DEMUXER MOV_DEMUXER HEVC_MP4TOANNEXB_BSF MOV_MUXER
 fate-hevc-bsf-mp4toannexb: tests/data/hevc-mp4.mov
 fate-hevc-bsf-mp4toannexb: CMD = md5 -i $(TARGET_PATH)/tests/data/hevc-mp4.mov -c:v copy -fflags +bitexact -f hevc
 fate-hevc-bsf-mp4toannexb: CMP = oneline
-fate-hevc-bsf-mp4toannexb: REF = 1873662a3af1848c37e4eb25722c8df9
+fate-hevc-bsf-mp4toannexb: REF = 3c9d998a3aa2b9e0fb1c1f434952bf8b
 
 fate-hevc-skiploopfilter: CMD = framemd5 -skip_loop_filter nokey -i $(TARGET_SAMPLES)/hevc-conformance/SAO_D_Samsung_5.bit -sws_flags bitexact
 FATE_HEVC += fate-hevc-skiploopfilter