Re: [FFmpeg-devel] [PATCH] lavc/hevc_parser: add 4 bytes startcode condition in hevc_find_frame_end
> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Michael Niedermayer
> Sent: Friday, November 30, 2018 07:01
> To: FFmpeg development discussions and patches de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] lavc/hevc_parser: add 4 bytes
> startcode condition in hevc_find_frame_end
>
> On Thu, Nov 29, 2018 at 04:14:29AM +, Fu, Linjie wrote:
> > > -Original Message-
> > > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> Behalf
> > > Of Michael Niedermayer
> > > Sent: Thursday, November 29, 2018 02:14
> > > To: FFmpeg development discussions and patches > > de...@ffmpeg.org>
> > > Subject: Re: [FFmpeg-devel] [PATCH] lavc/hevc_parser: add 4 bytes
> > > startcode condition in hevc_find_frame_end
> > >
> > > On Tue, Nov 27, 2018 at 08:16:55PM +0800, 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.
> > > >
> > > > Signed-off-by: Linjie Fu
> > > > ---
> > > > libavcodec/hevc_parser.c | 15 ++-
> > > > 1 file changed, 10 insertions(+), 5 deletions(-)
> > >
> > > breaks
> > > make fate-hevc-bsf-mp4toannexb
> > > TESThevc-bsf-mp4toannexb
> > > --- - 2018-11-28 19:12:47.869732022 +0100
> > > +++ tests/data/fate/hevc-bsf-mp4toannexb 2018-11-28
> > > 19:12:47.864276885 +0100
> > > @@ -1 +1 @@
> > > -1873662a3af1848c37e4eb25722c8df9
> > > +73019329ed7f81c24f9af67c34c640c0
> > > Test hevc-bsf-mp4toannexb failed. Look at tests/data/fate/hevc-bsf-
> > > mp4toannexb.err for details.
> > > make: *** [fate-hevc-bsf-mp4toannexb] Error 1
> > >
> >
> > Investigate the root cause:
> >
> > In fate test:
> > Step 1. ffmpeg -i hevc-conformance/WPP_A_ericsson_MAIN10_2.bit -c
> copy -flags +bitexact hevc-mp4.mov
> > Step 2. ffmpeg -i hevc-mp4_after.mov -c:v copy -fflags +bitexact -f hevc
> hevc.out
> > Step3. md5sum and compared with the reference
> >
> > Compare the output file in Step 1 before and after the patch, and found it
> changed:
> > -rw-r--r-- 1 root root 68460 11月 29 11:23 hevc-mp4_after.mov
> > -rw-r--r-- 1 root root 68507 11月 29 11:31 hevc-mp4_before.mov
> >
> > Compare the original file with two output files:
> > SUFFIX_SEI bit information in the first frame for example :
> >
> > WPP_A_ericsson_MAIN10_2.bit: { 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 }
> >
> > hevc-mp4_before.mov: { 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 }
> >
> > hevc-mp4_after.mov: { 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 }
> >
> > Before applying the patch, the output file has an extra 0x00 at the end of
> SUFFIX_SEI (because of blindly taking startcode as 3 bytes and left one byte).
> > After applying the patch, the output file has the same bit info like the
> original file.
> > So, I think this patch works.
>
> if the patch break make fate then it doesnt work.
> If a change cause by a patch is correct it needs to update the
> test / its checksum
> This is also important so that anyone looking at the patch knows which tests
> change and anyone testing knows how to detect a test failure vs a passed
> test
Thanks, sent a [v2] patch with the updated FATE checksum.
---
Linjie
> [...]
> --
> Michael GnuPG fingerprint:
> 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Those who are best at talking, realize last or never when they are wrong.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavc/hevc_parser: add 4 bytes startcode condition in hevc_find_frame_end
On Thu, Nov 29, 2018 at 04:14:29AM +, Fu, Linjie wrote:
> > -Original Message-
> > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> > Of Michael Niedermayer
> > Sent: Thursday, November 29, 2018 02:14
> > To: FFmpeg development discussions and patches > de...@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH] lavc/hevc_parser: add 4 bytes
> > startcode condition in hevc_find_frame_end
> >
> > On Tue, Nov 27, 2018 at 08:16:55PM +0800, 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.
> > >
> > > Signed-off-by: Linjie Fu
> > > ---
> > > libavcodec/hevc_parser.c | 15 ++-
> > > 1 file changed, 10 insertions(+), 5 deletions(-)
> >
> > breaks
> > make fate-hevc-bsf-mp4toannexb
> > TESThevc-bsf-mp4toannexb
> > --- - 2018-11-28 19:12:47.869732022 +0100
> > +++ tests/data/fate/hevc-bsf-mp4toannexb2018-11-28
> > 19:12:47.864276885 +0100
> > @@ -1 +1 @@
> > -1873662a3af1848c37e4eb25722c8df9
> > +73019329ed7f81c24f9af67c34c640c0
> > Test hevc-bsf-mp4toannexb failed. Look at tests/data/fate/hevc-bsf-
> > mp4toannexb.err for details.
> > make: *** [fate-hevc-bsf-mp4toannexb] Error 1
> >
>
> Investigate the root cause:
>
> In fate test:
> Step 1. ffmpeg -i hevc-conformance/WPP_A_ericsson_MAIN10_2.bit -c copy -flags
> +bitexact hevc-mp4.mov
> Step 2. ffmpeg -i hevc-mp4_after.mov -c:v copy -fflags +bitexact -f hevc
> hevc.out
> Step3. md5sum and compared with the reference
>
> Compare the output file in Step 1 before and after the patch, and found it
> changed:
> -rw-r--r-- 1 root root 68460 11月 29 11:23 hevc-mp4_after.mov
> -rw-r--r-- 1 root root 68507 11月 29 11:31 hevc-mp4_before.mov
>
> Compare the original file with two output files:
> SUFFIX_SEI bit information in the first frame for example :
>
> WPP_A_ericsson_MAIN10_2.bit: { 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 }
>
> hevc-mp4_before.mov: { 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 }
>
> hevc-mp4_after.mov: { 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 }
>
> Before applying the patch, the output file has an extra 0x00 at the end of
> SUFFIX_SEI (because of blindly taking startcode as 3 bytes and left one byte).
> After applying the patch, the output file has the same bit info like the
> original file.
> So, I think this patch works.
if the patch break make fate then it doesnt work.
If a change cause by a patch is correct it needs to update the
test / its checksum
This is also important so that anyone looking at the patch knows which tests
change and anyone testing knows how to detect a test failure vs a passed test
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Those who are best at talking, realize last or never when they are wrong.
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavc/hevc_parser: add 4 bytes startcode condition in hevc_find_frame_end
> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Michael Niedermayer
> Sent: Thursday, November 29, 2018 02:14
> To: FFmpeg development discussions and patches de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] lavc/hevc_parser: add 4 bytes
> startcode condition in hevc_find_frame_end
>
> On Tue, Nov 27, 2018 at 08:16:55PM +0800, 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.
> >
> > Signed-off-by: Linjie Fu
> > ---
> > libavcodec/hevc_parser.c | 15 ++-
> > 1 file changed, 10 insertions(+), 5 deletions(-)
>
> breaks
> make fate-hevc-bsf-mp4toannexb
> TESThevc-bsf-mp4toannexb
> --- - 2018-11-28 19:12:47.869732022 +0100
> +++ tests/data/fate/hevc-bsf-mp4toannexb 2018-11-28
> 19:12:47.864276885 +0100
> @@ -1 +1 @@
> -1873662a3af1848c37e4eb25722c8df9
> +73019329ed7f81c24f9af67c34c640c0
> Test hevc-bsf-mp4toannexb failed. Look at tests/data/fate/hevc-bsf-
> mp4toannexb.err for details.
> make: *** [fate-hevc-bsf-mp4toannexb] Error 1
>
Investigate the root cause:
In fate test:
Step 1. ffmpeg -i hevc-conformance/WPP_A_ericsson_MAIN10_2.bit -c copy -flags
+bitexact hevc-mp4.mov
Step 2. ffmpeg -i hevc-mp4_after.mov -c:v copy -fflags +bitexact -f hevc
hevc.out
Step3. md5sum and compared with the reference
Compare the output file in Step 1 before and after the patch, and found it
changed:
-rw-r--r-- 1 root root 68460 11月 29 11:23 hevc-mp4_after.mov
-rw-r--r-- 1 root root 68507 11月 29 11:31 hevc-mp4_before.mov
Compare the original file with two output files:
SUFFIX_SEI bit information in the first frame for example :
WPP_A_ericsson_MAIN10_2.bit: { 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 }
hevc-mp4_before.mov: { 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 }
hevc-mp4_after.mov: { 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 }
Before applying the patch, the output file has an extra 0x00 at the end of
SUFFIX_SEI (because of blindly taking startcode as 3 bytes and left one byte).
After applying the patch, the output file has the same bit info like the
original file.
So, I think this patch works.
Thanks,
- Linjie
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavc/hevc_parser: add 4 bytes startcode condition in hevc_find_frame_end
On Tue, Nov 27, 2018 at 08:16:55PM +0800, 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. > > Signed-off-by: Linjie Fu > --- > libavcodec/hevc_parser.c | 15 ++- > 1 file changed, 10 insertions(+), 5 deletions(-) breaks make fate-hevc-bsf-mp4toannexb TESThevc-bsf-mp4toannexb --- - 2018-11-28 19:12:47.869732022 +0100 +++ tests/data/fate/hevc-bsf-mp4toannexb2018-11-28 19:12:47.864276885 +0100 @@ -1 +1 @@ -1873662a3af1848c37e4eb25722c8df9 +73019329ed7f81c24f9af67c34c640c0 Test hevc-bsf-mp4toannexb failed. Look at tests/data/fate/hevc-bsf-mp4toannexb.err for details. make: *** [fate-hevc-bsf-mp4toannexb] Error 1 [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Frequently ignored answer#1 FFmpeg bugs should be sent to our bugtracker. User questions about the command line tools should be sent to the ffmpeg-user ML. And questions about how to use libav* should be sent to the libav-user ML. signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] lavc/hevc_parser: add 4 bytes startcode condition in hevc_find_frame_end
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.
Signed-off-by: Linjie Fu
---
libavcodec/hevc_parser.c | 15 ++-
1 file changed, 10 insertions(+), 5 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,
--
2.17.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
