Re: [FFmpeg-devel] [PATCH V2] avcodec/h264_mp4toannexb_bsf: add No IDR frame situation
-Original Message-
From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of
Michael Niedermayer
Sent: Friday, August 24, 2018 05:30
To: FFmpeg development discussions and patches
Subject: Re: [FFmpeg-devel] [PATCH V2] avcodec/h264_mp4toannexb_bsf: add No IDR
frame situation
On Thu, Aug 16, 2018 at 03:07:50PM +0800, Linjie Fu wrote:
> Fix the live stream encoding problem using qsv when the first frame is
> not an IDR frame.
>
> Add the extradata information when the IDR frame is missing in the
> first GOP.
>
> Fix the bug reported in ticket #6418.
>
> [PATCH V2] Fix the coding style.
>
> Signed-off-by: Linjie Fu
> ---
> libavcodec/h264_mp4toannexb_bsf.c | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/libavcodec/h264_mp4toannexb_bsf.c
> b/libavcodec/h264_mp4toannexb_bsf.c
> index 794c82e650..fbb9f1fe20 100644
> --- a/libavcodec/h264_mp4toannexb_bsf.c
> +++ b/libavcodec/h264_mp4toannexb_bsf.c
> @@ -33,6 +33,7 @@ typedef struct H264BSFContext {
> int32_t pps_offset;
> uint8_t length_size;
> uint8_t new_idr;
> +uint8_t new_nal_slice;
> uint8_t idr_sps_seen;
> uint8_t idr_pps_seen;
> int extradata_parsed;
> @@ -243,6 +244,7 @@ static int h264_mp4toannexb_filter(AVBSFContext *ctx,
> AVPacket *out)
> buf, nal_size, 1)) < 0)
> goto fail;
> s->new_idr = 0;
> +s->new_nal_slice = 1;
> /* if only SPS has been seen, also insert PPS */
> } else if (s->new_idr && unit_type == H264_NAL_IDR_SLICE &&
> s->idr_sps_seen && !s->idr_pps_seen) {
> if (s->pps_offset == -1) { @@ -253,6 +255,12 @@ static
> int h264_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *out)
> ctx->par_out->extradata +
> s->pps_offset, ctx->par_out->extradata_size - s->pps_offset,
> buf, nal_size, 1)) < 0)
> goto fail;
> +} else if (s->new_idr && !s->new_nal_slice && H264_NAL_SLICE ==
> unit_type && !s->idr_sps_seen && !s->idr_pps_seen){
> +av_log(ctx, AV_LOG_WARNING, "first H264_NAL_SLICE when there is
> no IDR.\n");
> +if ((ret = alloc_and_copy(out, ctx->par_out->extradata,
> ctx->par_out->extradata_size, buf, nal_size, 1)) < 0)
> +goto fail;
> +s->new_nal_slice = 1;
> +s->new_idr = 0;
The commit message is insufficient
this adds the extradata if new_idr is set not just in the absence of IDR at the
begin
also teh addded code is identical to the first if() just a different NAL is
checked for, this code duplication is not a good idea in already complex code
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
When you are offended at any man's fault, turn to yourself and study your own
failings. Then you will forget your anger. -- Epictetus
Thanks for your review.
I will adjust the code to reduce the duplication and make the commit message
clearer.
I add " new_nal_slice " to indicate the newly coming H264_NAL_IDR_SLICE and
H264_NAL_SLICE, because the new_idr is always "1" at the beginning of the
filter regardless of the nal type of slice.
This patch aims at the following issues:
1. the IDR frame is missing in the first GOP of a stream (common in live
stream, IDR.No.Inband.SPPS.mkv in ticket #6418)
2. there is no IDR frame in the input stream. (No.Inband.SPPS.No.IDR.mkv in
ticket #6418)
In issue 1, ffmpeg just drop all the frames in the first GOP without the patch
then continue to decode the following frames. (qsv and libopenh264)
In issue 2, ffmpeg could not decode any frames of the stream through
h264_mp4toannexb_filter without the patch. (qsv and libopenh264)
After applying this patch, all the frames could be decoded correctly in both
issues. (qsv works well, libopenh264 has to check the slice_nal_type in the
decode step in addition to the h264_mp4toannexb_filter, still can't decode.)
Thanks again.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH V2] avcodec/h264_mp4toannexb_bsf: add No IDR frame situation
On Thu, Aug 16, 2018 at 03:07:50PM +0800, Linjie Fu wrote:
> Fix the live stream encoding problem using qsv when the first frame
> is not an IDR frame.
>
> Add the extradata information when the IDR frame is missing in the
> first GOP.
>
> Fix the bug reported in ticket #6418.
>
> [PATCH V2] Fix the coding style.
>
> Signed-off-by: Linjie Fu
> ---
> libavcodec/h264_mp4toannexb_bsf.c | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/libavcodec/h264_mp4toannexb_bsf.c
> b/libavcodec/h264_mp4toannexb_bsf.c
> index 794c82e650..fbb9f1fe20 100644
> --- a/libavcodec/h264_mp4toannexb_bsf.c
> +++ b/libavcodec/h264_mp4toannexb_bsf.c
> @@ -33,6 +33,7 @@ typedef struct H264BSFContext {
> int32_t pps_offset;
> uint8_t length_size;
> uint8_t new_idr;
> +uint8_t new_nal_slice;
> uint8_t idr_sps_seen;
> uint8_t idr_pps_seen;
> int extradata_parsed;
> @@ -243,6 +244,7 @@ static int h264_mp4toannexb_filter(AVBSFContext *ctx,
> AVPacket *out)
> buf, nal_size, 1)) < 0)
> goto fail;
> s->new_idr = 0;
> +s->new_nal_slice = 1;
> /* if only SPS has been seen, also insert PPS */
> } else if (s->new_idr && unit_type == H264_NAL_IDR_SLICE &&
> s->idr_sps_seen && !s->idr_pps_seen) {
> if (s->pps_offset == -1) {
> @@ -253,6 +255,12 @@ static int h264_mp4toannexb_filter(AVBSFContext *ctx,
> AVPacket *out)
> ctx->par_out->extradata +
> s->pps_offset, ctx->par_out->extradata_size - s->pps_offset,
> buf, nal_size, 1)) < 0)
> goto fail;
> +} else if (s->new_idr && !s->new_nal_slice && H264_NAL_SLICE ==
> unit_type && !s->idr_sps_seen && !s->idr_pps_seen){
> +av_log(ctx, AV_LOG_WARNING, "first H264_NAL_SLICE when there is
> no IDR.\n");
> +if ((ret = alloc_and_copy(out, ctx->par_out->extradata,
> ctx->par_out->extradata_size, buf, nal_size, 1)) < 0)
> +goto fail;
> +s->new_nal_slice = 1;
> +s->new_idr = 0;
The commit message is insufficient
this adds the extradata if new_idr is set not just in the absence of IDR at
the begin
also teh addded code is identical to the first if() just a different NAL is
checked for, this code duplication is not a good idea in already complex
code
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
When you are offended at any man's fault, turn to yourself and study your
own failings. Then you will forget your anger. -- Epictetus
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH V2] avcodec/h264_mp4toannexb_bsf: add No IDR frame situation
-Original Message-
From: Fu, Linjie
Sent: Thursday, August 16, 2018 15:08
To: ffmpeg-devel@ffmpeg.org
Cc: Fu, Linjie
Subject: [PATCH V2] avcodec/h264_mp4toannexb_bsf: add No IDR frame situation
Fix the live stream encoding problem using qsv when the first frame is not an
IDR frame.
Add the extradata information when the IDR frame is missing in the first GOP.
Fix the bug reported in ticket #6418.
[PATCH V2] Fix the coding style.
Signed-off-by: Linjie Fu
---
libavcodec/h264_mp4toannexb_bsf.c | 8
1 file changed, 8 insertions(+)
diff --git a/libavcodec/h264_mp4toannexb_bsf.c
b/libavcodec/h264_mp4toannexb_bsf.c
index 794c82e650..fbb9f1fe20 100644
--- a/libavcodec/h264_mp4toannexb_bsf.c
+++ b/libavcodec/h264_mp4toannexb_bsf.c
@@ -33,6 +33,7 @@ typedef struct H264BSFContext {
int32_t pps_offset;
uint8_t length_size;
uint8_t new_idr;
+uint8_t new_nal_slice;
uint8_t idr_sps_seen;
uint8_t idr_pps_seen;
int extradata_parsed;
@@ -243,6 +244,7 @@ static int h264_mp4toannexb_filter(AVBSFContext *ctx,
AVPacket *out)
buf, nal_size, 1)) < 0)
goto fail;
s->new_idr = 0;
+s->new_nal_slice = 1;
/* if only SPS has been seen, also insert PPS */
} else if (s->new_idr && unit_type == H264_NAL_IDR_SLICE &&
s->idr_sps_seen && !s->idr_pps_seen) {
if (s->pps_offset == -1) {
@@ -253,6 +255,12 @@ static int h264_mp4toannexb_filter(AVBSFContext *ctx,
AVPacket *out)
ctx->par_out->extradata +
s->pps_offset, ctx->par_out->extradata_size - s->pps_offset,
buf, nal_size, 1)) < 0)
goto fail;
+} else if (s->new_idr && !s->new_nal_slice && H264_NAL_SLICE ==
unit_type && !s->idr_sps_seen && !s->idr_pps_seen){
+av_log(ctx, AV_LOG_WARNING, "first H264_NAL_SLICE when there is no
IDR.\n");
+if ((ret = alloc_and_copy(out, ctx->par_out->extradata,
ctx->par_out->extradata_size, buf, nal_size, 1)) < 0)
+goto fail;
+s->new_nal_slice = 1;
+s->new_idr = 0;
} else {
if ((ret=alloc_and_copy(out, NULL, 0, buf, nal_size, unit_type ==
H264_NAL_SPS || unit_type == H264_NAL_PPS)) < 0)
goto fail;
--
2.17.1
Ping.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH V2] avcodec/h264_mp4toannexb_bsf: add No IDR frame situation
Fix the live stream encoding problem using qsv when the first frame
is not an IDR frame.
Add the extradata information when the IDR frame is missing in the
first GOP.
Fix the bug reported in ticket #6418.
[PATCH V2] Fix the coding style.
Signed-off-by: Linjie Fu
---
libavcodec/h264_mp4toannexb_bsf.c | 8
1 file changed, 8 insertions(+)
diff --git a/libavcodec/h264_mp4toannexb_bsf.c
b/libavcodec/h264_mp4toannexb_bsf.c
index 794c82e650..fbb9f1fe20 100644
--- a/libavcodec/h264_mp4toannexb_bsf.c
+++ b/libavcodec/h264_mp4toannexb_bsf.c
@@ -33,6 +33,7 @@ typedef struct H264BSFContext {
int32_t pps_offset;
uint8_t length_size;
uint8_t new_idr;
+uint8_t new_nal_slice;
uint8_t idr_sps_seen;
uint8_t idr_pps_seen;
int extradata_parsed;
@@ -243,6 +244,7 @@ static int h264_mp4toannexb_filter(AVBSFContext *ctx,
AVPacket *out)
buf, nal_size, 1)) < 0)
goto fail;
s->new_idr = 0;
+s->new_nal_slice = 1;
/* if only SPS has been seen, also insert PPS */
} else if (s->new_idr && unit_type == H264_NAL_IDR_SLICE &&
s->idr_sps_seen && !s->idr_pps_seen) {
if (s->pps_offset == -1) {
@@ -253,6 +255,12 @@ static int h264_mp4toannexb_filter(AVBSFContext *ctx,
AVPacket *out)
ctx->par_out->extradata +
s->pps_offset, ctx->par_out->extradata_size - s->pps_offset,
buf, nal_size, 1)) < 0)
goto fail;
+} else if (s->new_idr && !s->new_nal_slice && H264_NAL_SLICE ==
unit_type && !s->idr_sps_seen && !s->idr_pps_seen){
+av_log(ctx, AV_LOG_WARNING, "first H264_NAL_SLICE when there is no
IDR.\n");
+if ((ret = alloc_and_copy(out, ctx->par_out->extradata,
ctx->par_out->extradata_size, buf, nal_size, 1)) < 0)
+goto fail;
+s->new_nal_slice = 1;
+s->new_idr = 0;
} else {
if ((ret=alloc_and_copy(out, NULL, 0, buf, nal_size, unit_type ==
H264_NAL_SPS || unit_type == H264_NAL_PPS)) < 0)
goto fail;
--
2.17.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
