Re: [FFmpeg-devel] [PATCH, v2] lavc/hevc_parser: add 4 bytes startcode condition and update FATE
> -Original Message- > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf > Of Fu, Linjie > Sent: Wednesday, January 9, 2019 10:51 > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH, v2] lavc/hevc_parser: add 4 bytes > startcode condition and update FATE > > > -Original Message- > > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On > Behalf > > Of Mark Thompson > > Sent: Monday, December 10, 2018 01:40 > > To: ffmpeg-devel@ffmpeg.org > > Subject: Re: [FFmpeg-devel] [PATCH, v2] lavc/hevc_parser: add 4 bytes > > startcode condition and update FATE > > > > On 07/12/2018 11:33, Linjie Fu wrote: > > > The startcode before VPS,SPS,PPS and the first NALU in an AU is 4 bytes. > > > Blindly taking the startcode as 3 bytes will leave 0x00 in last packet > > > and may lead to some warnings in parse_nal_units when s->flags is set to > > > PARSER_FLAG_COMPLETE_FRAMES. > > > > > > Add 4 bytes startcode condition in hevc_find_frame_end. > > > Modify the code to print the buf_size like in H264 and reduce the > > duplication. > > > > > > Update the FATE checksum for fate-hevc-bsf-mp4toannexb: > > > The ref output has redundant 0x00 at the end of the SUFFIX_SEI: > > > { 50 01 84 31 00 19 39 77 d0 7b 3f bd 1e 09 38 9a 79 41 > > > c0 16 11 da 18 aa 0a db 2b 19 fa 47 3f 0f 67 4a 91 9c a1 > > > 12 72 67 d6 f0 8f 66 ee 95 f9 e2 b9 ba 23 9a e8 80 00 } > > > > > > To verify the output of FATE: > > > ffmpeg -i hevc-conformance/WPP_A_ericsson_MAIN10_2.bit -c copy - > flags > > \ > > > +bitexact hevc-mp4.mov > > > ffmpeg -i hevc-mp4.mov -c:v copy -fflags +bitexact -f hevc hevc.out > > > > > > Signed-off-by: Linjie Fu > > > --- > > > [v2]: Update FATE checksum > > > > > > libavcodec/hevc_parser.c | 15 ++- > > > tests/fate/hevc.mak | 2 +- > > > 2 files changed, 11 insertions(+), 6 deletions(-) > > > > > > diff --git a/libavcodec/hevc_parser.c b/libavcodec/hevc_parser.c > > > index 369d1338d0..aa216e3c8d 100644 > > > --- a/libavcodec/hevc_parser.c > > > +++ b/libavcodec/hevc_parser.c > > > @@ -32,6 +32,7 @@ > > > #include "parser.h" > > > > > > #define START_CODE 0x01 ///< start_code_prefix_one_3bytes > > > +#define START_CODE_4 0x0001 ///< start_code_4bytes > > > > (There is no syntax element called "start_code_4bytes".) > > > > > > > > #define IS_IRAP_NAL(nal) (nal->type >= 16 && nal->type <= 23) > > > #define IS_IDR_NAL(nal) (nal->type == HEVC_NAL_IDR_W_RADL || nal- > > >type == HEVC_NAL_IDR_N_LP) > > > @@ -239,7 +240,7 @@ static int parse_nal_units(AVCodecParserContext > *s, > > const uint8_t *buf, > > > } > > > } > > > /* didn't find a picture! */ > > > -av_log(avctx, AV_LOG_ERROR, "missing picture in access unit\n"); > > > +av_log(avctx, AV_LOG_ERROR, "missing picture in access unit with > > size %d\n", buf_size); > > > return -1; > > > } > > > > > > @@ -267,8 +268,7 @@ static int > > hevc_find_frame_end(AVCodecParserContext *s, const uint8_t *buf, > > > if ((nut >= HEVC_NAL_VPS && nut <= HEVC_NAL_EOB_NUT) || nut > == > > HEVC_NAL_SEI_PREFIX || > > > (nut >= 41 && nut <= 44) || (nut >= 48 && nut <= 55)) { > > > if (pc->frame_start_found) { > > > -pc->frame_start_found = 0; > > > -return i - 5; > > > +goto found; > > > } > > > } else if (nut <= HEVC_NAL_RASL_R || > > > (nut >= HEVC_NAL_BLA_W_LP && nut <= > HEVC_NAL_CRA_NUT)) > > { > > > @@ -277,14 +277,19 @@ static int > > hevc_find_frame_end(AVCodecParserContext *s, const uint8_t *buf, > > > if (!pc->frame_start_found) { > > > pc->frame_start_found = 1; > > > } else { // First slice of next frame found > > > -pc->frame_start_found = 0; > > > -return i - 5; > > > +goto found; > > > } > > > } > > > } > > &
Re: [FFmpeg-devel] [PATCH, v2] lavc/hevc_parser: add 4 bytes startcode condition and update FATE
> -Original Message- > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf > Of Mark Thompson > Sent: Monday, December 10, 2018 01:40 > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH, v2] lavc/hevc_parser: add 4 bytes > startcode condition and update FATE > > On 07/12/2018 11:33, Linjie Fu wrote: > > The startcode before VPS,SPS,PPS and the first NALU in an AU is 4 bytes. > > Blindly taking the startcode as 3 bytes will leave 0x00 in last packet > > and may lead to some warnings in parse_nal_units when s->flags is set to > > PARSER_FLAG_COMPLETE_FRAMES. > > > > Add 4 bytes startcode condition in hevc_find_frame_end. > > Modify the code to print the buf_size like in H264 and reduce the > duplication. > > > > Update the FATE checksum for fate-hevc-bsf-mp4toannexb: > > The ref output has redundant 0x00 at the end of the SUFFIX_SEI: > > { 50 01 84 31 00 19 39 77 d0 7b 3f bd 1e 09 38 9a 79 41 > > c0 16 11 da 18 aa 0a db 2b 19 fa 47 3f 0f 67 4a 91 9c a1 > > 12 72 67 d6 f0 8f 66 ee 95 f9 e2 b9 ba 23 9a e8 80 00 } > > > > To verify the output of FATE: > > ffmpeg -i hevc-conformance/WPP_A_ericsson_MAIN10_2.bit -c copy -flags > \ > > +bitexact hevc-mp4.mov > > ffmpeg -i hevc-mp4.mov -c:v copy -fflags +bitexact -f hevc hevc.out > > > > Signed-off-by: Linjie Fu > > --- > > [v2]: Update FATE checksum > > > > libavcodec/hevc_parser.c | 15 ++- > > tests/fate/hevc.mak | 2 +- > > 2 files changed, 11 insertions(+), 6 deletions(-) > > > > diff --git a/libavcodec/hevc_parser.c b/libavcodec/hevc_parser.c > > index 369d1338d0..aa216e3c8d 100644 > > --- a/libavcodec/hevc_parser.c > > +++ b/libavcodec/hevc_parser.c > > @@ -32,6 +32,7 @@ > > #include "parser.h" > > > > #define START_CODE 0x01 ///< start_code_prefix_one_3bytes > > +#define START_CODE_4 0x0001 ///< start_code_4bytes > > (There is no syntax element called "start_code_4bytes".) > > > > > #define IS_IRAP_NAL(nal) (nal->type >= 16 && nal->type <= 23) > > #define IS_IDR_NAL(nal) (nal->type == HEVC_NAL_IDR_W_RADL || nal- > >type == HEVC_NAL_IDR_N_LP) > > @@ -239,7 +240,7 @@ static int parse_nal_units(AVCodecParserContext *s, > const uint8_t *buf, > > } > > } > > /* didn't find a picture! */ > > -av_log(avctx, AV_LOG_ERROR, "missing picture in access unit\n"); > > +av_log(avctx, AV_LOG_ERROR, "missing picture in access unit with > size %d\n", buf_size); > > return -1; > > } > > > > @@ -267,8 +268,7 @@ static int > hevc_find_frame_end(AVCodecParserContext *s, const uint8_t *buf, > > if ((nut >= HEVC_NAL_VPS && nut <= HEVC_NAL_EOB_NUT) || nut == > HEVC_NAL_SEI_PREFIX || > > (nut >= 41 && nut <= 44) || (nut >= 48 && nut <= 55)) { > > if (pc->frame_start_found) { > > -pc->frame_start_found = 0; > > -return i - 5; > > +goto found; > > } > > } else if (nut <= HEVC_NAL_RASL_R || > > (nut >= HEVC_NAL_BLA_W_LP && nut <= HEVC_NAL_CRA_NUT)) > { > > @@ -277,14 +277,19 @@ static int > hevc_find_frame_end(AVCodecParserContext *s, const uint8_t *buf, > > if (!pc->frame_start_found) { > > pc->frame_start_found = 1; > > } else { // First slice of next frame found > > -pc->frame_start_found = 0; > > -return i - 5; > > +goto found; > > } > > } > > } > > } > > > > return END_NOT_FOUND; > > + > > +found: > > +pc->frame_start_found = 0; > > +if (((pc->state64 >> 3 * 8) & 0x) == START_CODE_4) > > +return i - 6; > > +return i - 5; > > } > > Maybe? Special-casing four bytes doesn't make me feel good about this, > given that leading_zero_8bits can be included as many times as you like at > the beginning of a NAL unit. > > If there are a large number of zeroes between two frames, which packet do > you want them to end up in? This changes it from "all at the end of the first > frame" to "all except the last one at the end of the first frame", and it's > not > immediately obvious to me why that would be the right cho
Re: [FFmpeg-devel] [PATCH, v2] lavc/hevc_parser: add 4 bytes startcode condition and update FATE
> -Original Message- > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf > Of Mark Thompson > Sent: Monday, December 10, 2018 01:40 > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH, v2] lavc/hevc_parser: add 4 bytes > startcode condition and update FATE > > > > > #define START_CODE 0x01 ///< start_code_prefix_one_3bytes > > +#define START_CODE_4 0x0001 ///< start_code_4bytes > > (There is no syntax element called "start_code_4bytes".) > Deleted. > > > > #define IS_IRAP_NAL(nal) (nal->type >= 16 && nal->type <= 23) > > #define IS_IDR_NAL(nal) (nal->type == HEVC_NAL_IDR_W_RADL || nal- > >type == HEVC_NAL_IDR_N_LP) > > @@ -239,7 +240,7 @@ static int parse_nal_units(AVCodecParserContext *s, > const uint8_t *buf, > > } > > } > > /* didn't find a picture! */ > > -av_log(avctx, AV_LOG_ERROR, "missing picture in access unit\n"); > > +av_log(avctx, AV_LOG_ERROR, "missing picture in access unit with > size %d\n", buf_size); > > return -1; > > } > > > > @@ -267,8 +268,7 @@ static int > hevc_find_frame_end(AVCodecParserContext *s, const uint8_t *buf, > > if ((nut >= HEVC_NAL_VPS && nut <= HEVC_NAL_EOB_NUT) || nut == > HEVC_NAL_SEI_PREFIX || > > (nut >= 41 && nut <= 44) || (nut >= 48 && nut <= 55)) { > > if (pc->frame_start_found) { > > -pc->frame_start_found = 0; > > -return i - 5; > > +goto found; > > } > > } else if (nut <= HEVC_NAL_RASL_R || > > (nut >= HEVC_NAL_BLA_W_LP && nut <= HEVC_NAL_CRA_NUT)) > { > > @@ -277,14 +277,19 @@ static int > hevc_find_frame_end(AVCodecParserContext *s, const uint8_t *buf, > > if (!pc->frame_start_found) { > > pc->frame_start_found = 1; > > } else { // First slice of next frame found > > -pc->frame_start_found = 0; > > -return i - 5; > > +goto found; > > } > > } > > } > > } > > > > return END_NOT_FOUND; > > + > > +found: > > +pc->frame_start_found = 0; > > +if (((pc->state64 >> 3 * 8) & 0x) == START_CODE_4) > > +return i - 6; > > +return i - 5; > > } > > Maybe? Special-casing four bytes doesn't make me feel good about this, > given that leading_zero_8bits can be included as many times as you like at > the beginning of a NAL unit. > > If there are a large number of zeroes between two frames, which packet do > you want them to end up in? This changes it from "all at the end of the first > frame" to "all except the last one at the end of the first frame", and it's > not > immediately obvious to me why that would be the right choice. Why not, for > example, put all of the zeroes into the second frame? Modified and sent a new patch to find all leading_zero_8bits and put them into the second frame. Thanks --- Linjie Fu ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH, v2] lavc/hevc_parser: add 4 bytes startcode condition and update FATE
On 07/12/2018 11:33, Linjie Fu wrote: > The startcode before VPS,SPS,PPS and the first NALU in an AU is 4 bytes. > Blindly taking the startcode as 3 bytes will leave 0x00 in last packet > and may lead to some warnings in parse_nal_units when s->flags is set to > PARSER_FLAG_COMPLETE_FRAMES. > > Add 4 bytes startcode condition in hevc_find_frame_end. > Modify the code to print the buf_size like in H264 and reduce the duplication. > > Update the FATE checksum for fate-hevc-bsf-mp4toannexb: > The ref output has redundant 0x00 at the end of the SUFFIX_SEI: > { 50 01 84 31 00 19 39 77 d0 7b 3f bd 1e 09 38 9a 79 41 > c0 16 11 da 18 aa 0a db 2b 19 fa 47 3f 0f 67 4a 91 9c a1 > 12 72 67 d6 f0 8f 66 ee 95 f9 e2 b9 ba 23 9a e8 80 00 } > > To verify the output of FATE: > ffmpeg -i hevc-conformance/WPP_A_ericsson_MAIN10_2.bit -c copy -flags \ > +bitexact hevc-mp4.mov > ffmpeg -i hevc-mp4.mov -c:v copy -fflags +bitexact -f hevc hevc.out > > Signed-off-by: Linjie Fu > --- > [v2]: Update FATE checksum > > libavcodec/hevc_parser.c | 15 ++- > tests/fate/hevc.mak | 2 +- > 2 files changed, 11 insertions(+), 6 deletions(-) > > diff --git a/libavcodec/hevc_parser.c b/libavcodec/hevc_parser.c > index 369d1338d0..aa216e3c8d 100644 > --- a/libavcodec/hevc_parser.c > +++ b/libavcodec/hevc_parser.c > @@ -32,6 +32,7 @@ > #include "parser.h" > > #define START_CODE 0x01 ///< start_code_prefix_one_3bytes > +#define START_CODE_4 0x0001 ///< start_code_4bytes (There is no syntax element called "start_code_4bytes".) > > #define IS_IRAP_NAL(nal) (nal->type >= 16 && nal->type <= 23) > #define IS_IDR_NAL(nal) (nal->type == HEVC_NAL_IDR_W_RADL || nal->type == > HEVC_NAL_IDR_N_LP) > @@ -239,7 +240,7 @@ static int parse_nal_units(AVCodecParserContext *s, const > uint8_t *buf, > } > } > /* didn't find a picture! */ > -av_log(avctx, AV_LOG_ERROR, "missing picture in access unit\n"); > +av_log(avctx, AV_LOG_ERROR, "missing picture in access unit with size > %d\n", buf_size); > return -1; > } > > @@ -267,8 +268,7 @@ static int hevc_find_frame_end(AVCodecParserContext *s, > const uint8_t *buf, > if ((nut >= HEVC_NAL_VPS && nut <= HEVC_NAL_EOB_NUT) || nut == > HEVC_NAL_SEI_PREFIX || > (nut >= 41 && nut <= 44) || (nut >= 48 && nut <= 55)) { > if (pc->frame_start_found) { > -pc->frame_start_found = 0; > -return i - 5; > +goto found; > } > } else if (nut <= HEVC_NAL_RASL_R || > (nut >= HEVC_NAL_BLA_W_LP && nut <= HEVC_NAL_CRA_NUT)) { > @@ -277,14 +277,19 @@ static int hevc_find_frame_end(AVCodecParserContext *s, > const uint8_t *buf, > if (!pc->frame_start_found) { > pc->frame_start_found = 1; > } else { // First slice of next frame found > -pc->frame_start_found = 0; > -return i - 5; > +goto found; > } > } > } > } > > return END_NOT_FOUND; > + > +found: > +pc->frame_start_found = 0; > +if (((pc->state64 >> 3 * 8) & 0x) == START_CODE_4) > +return i - 6; > +return i - 5; > } Maybe? Special-casing four bytes doesn't make me feel good about this, given that leading_zero_8bits can be included as many times as you like at the beginning of a NAL unit. If there are a large number of zeroes between two frames, which packet do you want them to end up in? This changes it from "all at the end of the first frame" to "all except the last one at the end of the first frame", and it's not immediately obvious to me why that would be the right choice. Why not, for example, put all of the zeroes into the second frame? > static int hevc_parse(AVCodecParserContext *s, AVCodecContext *avctx, > diff --git a/tests/fate/hevc.mak b/tests/fate/hevc.mak > index db3ea19340..ab8da53535 100644 > --- a/tests/fate/hevc.mak > +++ b/tests/fate/hevc.mak > @@ -238,7 +238,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 = 73019329ed7f81c24f9af67c34c640c0 > > 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 > Thanks, - Mark ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH, v2] lavc/hevc_parser: add 4 bytes startcode condition and update FATE
The startcode before VPS,SPS,PPS and the first NALU in an AU is 4 bytes. Blindly taking the startcode as 3 bytes will leave 0x00 in last packet and may lead to some warnings in parse_nal_units when s->flags is set to PARSER_FLAG_COMPLETE_FRAMES. Add 4 bytes startcode condition in hevc_find_frame_end. Modify the code to print the buf_size like in H264 and reduce the duplication. Update the FATE checksum for fate-hevc-bsf-mp4toannexb: The ref output has redundant 0x00 at the end of the SUFFIX_SEI: { 50 01 84 31 00 19 39 77 d0 7b 3f bd 1e 09 38 9a 79 41 c0 16 11 da 18 aa 0a db 2b 19 fa 47 3f 0f 67 4a 91 9c a1 12 72 67 d6 f0 8f 66 ee 95 f9 e2 b9 ba 23 9a e8 80 00 } To verify the output of FATE: ffmpeg -i hevc-conformance/WPP_A_ericsson_MAIN10_2.bit -c copy -flags \ +bitexact hevc-mp4.mov ffmpeg -i hevc-mp4.mov -c:v copy -fflags +bitexact -f hevc hevc.out Signed-off-by: Linjie Fu --- [v2]: Update FATE checksum libavcodec/hevc_parser.c | 15 ++- tests/fate/hevc.mak | 2 +- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/libavcodec/hevc_parser.c b/libavcodec/hevc_parser.c index 369d1338d0..aa216e3c8d 100644 --- a/libavcodec/hevc_parser.c +++ b/libavcodec/hevc_parser.c @@ -32,6 +32,7 @@ #include "parser.h" #define START_CODE 0x01 ///< start_code_prefix_one_3bytes +#define START_CODE_4 0x0001 ///< start_code_4bytes #define IS_IRAP_NAL(nal) (nal->type >= 16 && nal->type <= 23) #define IS_IDR_NAL(nal) (nal->type == HEVC_NAL_IDR_W_RADL || nal->type == HEVC_NAL_IDR_N_LP) @@ -239,7 +240,7 @@ static int parse_nal_units(AVCodecParserContext *s, const uint8_t *buf, } } /* didn't find a picture! */ -av_log(avctx, AV_LOG_ERROR, "missing picture in access unit\n"); +av_log(avctx, AV_LOG_ERROR, "missing picture in access unit with size %d\n", buf_size); return -1; } @@ -267,8 +268,7 @@ static int hevc_find_frame_end(AVCodecParserContext *s, const uint8_t *buf, if ((nut >= HEVC_NAL_VPS && nut <= HEVC_NAL_EOB_NUT) || nut == HEVC_NAL_SEI_PREFIX || (nut >= 41 && nut <= 44) || (nut >= 48 && nut <= 55)) { if (pc->frame_start_found) { -pc->frame_start_found = 0; -return i - 5; +goto found; } } else if (nut <= HEVC_NAL_RASL_R || (nut >= HEVC_NAL_BLA_W_LP && nut <= HEVC_NAL_CRA_NUT)) { @@ -277,14 +277,19 @@ static int hevc_find_frame_end(AVCodecParserContext *s, const uint8_t *buf, if (!pc->frame_start_found) { pc->frame_start_found = 1; } else { // First slice of next frame found -pc->frame_start_found = 0; -return i - 5; +goto found; } } } } return END_NOT_FOUND; + +found: +pc->frame_start_found = 0; +if (((pc->state64 >> 3 * 8) & 0x) == START_CODE_4) +return i - 6; +return i - 5; } static int hevc_parse(AVCodecParserContext *s, AVCodecContext *avctx, diff --git a/tests/fate/hevc.mak b/tests/fate/hevc.mak index db3ea19340..ab8da53535 100644 --- a/tests/fate/hevc.mak +++ b/tests/fate/hevc.mak @@ -238,7 +238,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 = 73019329ed7f81c24f9af67c34c640c0 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 http://ffmpeg.org/mailman/listinfo/ffmpeg-devel