| Message ID | 20210714143401.890-3-jamrial@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/3] avcodec: add a parser flag to enable keyframe tagging heuristics | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
James Almer: > Tag only packets containing with IDR pictures as keyframes by default, same as > the decoder. > Fixes MP4 and Matroska muxers marking incorrect samples as Sync Samples in > scenarios where this AVParser is used. > 1. Could you please explain where you got the info that Matroska keyframes need to be ISOBMFF Sync Samples from? Because it's actually undefined what it exactly is; in case of AV1 (the only codec with a detailed codec mapping) said mapping allows delayed random access points to be marked as keyframes (without providing anything to actually signal that these are delayed random access points), so a key frame is simply a random access point. And that is how it is de-facto handled with all other codecs as well. IMO it is ISOBMFF which is the outlier here. > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavcodec/h264_parser.c | 16 ++++++++++------ > tests/fate-run.sh | 4 ++-- > tests/fate/ffmpeg.mak | 2 +- > tests/fate/lavf-container.mak | 12 ++++++------ > tests/fate/matroska.mak | 2 +- > tests/ref/fate/copy-trac2211-avi | 2 +- > tests/ref/fate/matroska-h264-remux | 4 ++-- > tests/ref/fate/segment-mp4-to-ts | 10 +++++----- > tests/ref/lavf-fate/h264.mp4 | 4 ++-- > 9 files changed, 30 insertions(+), 26 deletions(-) > > diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c > index d3c56cc188..532dc462b0 100644 > --- a/libavcodec/h264_parser.c > +++ b/libavcodec/h264_parser.c > @@ -344,9 +344,11 @@ static inline int parse_nal_units(AVCodecParserContext *s, > get_ue_golomb_long(&nal.gb); // skip first_mb_in_slice > slice_type = get_ue_golomb_31(&nal.gb); > s->pict_type = ff_h264_golomb_to_pict_type[slice_type % 5]; > - if (p->sei.recovery_point.recovery_frame_cnt >= 0) { > - /* key frame, since recovery_frame_cnt is set */ > - s->key_frame = 1; > + if (s->flags & PARSER_FLAG_USE_KEYFRAME_HEURISTICS) { > + if (p->sei.recovery_point.recovery_frame_cnt >= 0) { > + /* key frame, since recovery_frame_cnt is set */ > + s->key_frame = 1; > + } 2. This change means that files that don't use IDR pictures, but only recovery point SEIs won't have packets marked as keyframes at all any more; this affects every open-gop video. This is way worse than an incorrect sync sample list created by the mp4 muxer. (When using x264 with open-gop, it even means that every stream so encoded will only have one keyframe (the IDR frame at the beginning), because for some reason x264 never uses IDR frames in open-gop mode even at scenecuts (where the gop is closed and where using an IDR frame instead of a recovery point SEI would actually save a few bytes).) > } > pps_id = get_ue_golomb(&nal.gb); > if (pps_id >= MAX_PPS_COUNT) { > @@ -370,9 +372,11 @@ static inline int parse_nal_units(AVCodecParserContext *s, > p->ps.sps = p->ps.pps->sps; > sps = p->ps.sps; > > - // heuristic to detect non marked keyframes > - if (p->ps.sps->ref_frame_count <= 1 && p->ps.pps->ref_count[0] <= 1 && s->pict_type == AV_PICTURE_TYPE_I) > - s->key_frame = 1; > + if (s->flags & PARSER_FLAG_USE_KEYFRAME_HEURISTICS) { > + // heuristic to detect non marked keyframes > + if (p->ps.sps->ref_frame_count <= 1 && p->ps.pps->ref_count[0] <= 1 && s->pict_type == AV_PICTURE_TYPE_I) > + s->key_frame = 1; > + } > > p->poc.frame_num = get_bits(&nal.gb, sps->log2_max_frame_num); > > diff --git a/tests/fate-run.sh b/tests/fate-run.sh > index ba437dfbb8..8680e35524 100755 > --- a/tests/fate-run.sh > +++ b/tests/fate-run.sh > @@ -339,8 +339,8 @@ lavf_container_fate() > outdir="tests/data/lavf-fate" > file=${outdir}/lavf.$t > input="${target_samples}/$1" > - do_avconv $file -auto_conversion_filters $DEC_OPTS $2 -i "$input" "$ENC_OPTS -metadata title=lavftest" -vcodec copy -acodec copy > - do_avconv_crc $file -auto_conversion_filters $DEC_OPTS -i $target_path/$file $3 > + do_avconv $file -auto_conversion_filters $DEC_OPTS $2 -i "$input" "$ENC_OPTS -metadata title=lavftest" -vcodec copy -acodec copy $3 > + do_avconv_crc $file -auto_conversion_filters $DEC_OPTS -i $target_path/$file $4 > } > > lavf_image(){ > diff --git a/tests/fate/ffmpeg.mak b/tests/fate/ffmpeg.mak > index 4dfb77d250..57d16fba6f 100644 > --- a/tests/fate/ffmpeg.mak > +++ b/tests/fate/ffmpeg.mak > @@ -110,7 +110,7 @@ fate-copy-trac4914-avi: CMD = transcode mpegts $(TARGET_SAMPLES)/mpeg2/xdcam8mp2 > FATE_STREAMCOPY-$(call ALLYES, H264_DEMUXER AVI_MUXER) += fate-copy-trac2211-avi > fate-copy-trac2211-avi: $(SAMPLES)/h264/bbc2.sample.h264 > fate-copy-trac2211-avi: CMD = transcode "h264 -r 14" $(TARGET_SAMPLES)/h264/bbc2.sample.h264\ > - avi "-c:a copy -c:v copy" > + avi "-c:a copy -c:v copy -copyinkf" > > FATE_STREAMCOPY-$(call ENCDEC, APNG, APNG) += fate-copy-apng > fate-copy-apng: fate-lavf-apng > diff --git a/tests/fate/lavf-container.mak b/tests/fate/lavf-container.mak > index 9e0eed4851..40250badc1 100644 > --- a/tests/fate/lavf-container.mak > +++ b/tests/fate/lavf-container.mak > @@ -71,13 +71,13 @@ FATE_LAVF_CONTAINER_FATE = $(FATE_LAVF_CONTAINER_FATE-yes:%=fate-lavf-fate-%) > $(FATE_LAVF_CONTAINER_FATE): REF = $(SRC_PATH)/tests/ref/lavf-fate/$(@:fate-lavf-fate-%=%) > $(FATE_LAVF_CONTAINER_FATE): $(AREF) $(VREF) > > -fate-lavf-fate-av1.mp4: CMD = lavf_container_fate "av1-test-vectors/av1-1-b8-05-mv.ivf" "" "-c:v copy" > -fate-lavf-fate-av1.mkv: CMD = lavf_container_fate "av1-test-vectors/av1-1-b8-05-mv.ivf" "" "-c:v copy" > -fate-lavf-fate-h264.mp4: CMD = lavf_container_fate "h264/intra_refresh.h264" "" "-c:v copy" > +fate-lavf-fate-av1.mp4: CMD = lavf_container_fate "av1-test-vectors/av1-1-b8-05-mv.ivf" "" "" "-c:v copy" > +fate-lavf-fate-av1.mkv: CMD = lavf_container_fate "av1-test-vectors/av1-1-b8-05-mv.ivf" "" "" "-c:v copy" > +fate-lavf-fate-h264.mp4: CMD = lavf_container_fate "h264/intra_refresh.h264" "" "-copyinkf" "-c:v copy -copyinkf" 3. You are adding copyinkf twice here; are you sure that the new parameter to lavf_container_fate is even necessary? > fate-lavf-fate-vp3.ogg: CMD = lavf_container_fate "vp3/coeff_level64.mkv" "-idct auto" > -fate-lavf-fate-vp8.ogg: CMD = lavf_container_fate "vp8/RRSF49-short.webm" "" "-acodec copy" > -fate-lavf-fate-latm: CMD = lavf_container_fate "aac/al04_44.mp4" "" "-acodec copy" > -fate-lavf-fate-mp3: CMD = lavf_container_fate "mp3-conformance/he_32khz.bit" "" "-acodec copy" > +fate-lavf-fate-vp8.ogg: CMD = lavf_container_fate "vp8/RRSF49-short.webm" "" "" "-acodec copy" > +fate-lavf-fate-latm: CMD = lavf_container_fate "aac/al04_44.mp4" "" "" "-acodec copy" > +fate-lavf-fate-mp3: CMD = lavf_container_fate "mp3-conformance/he_32khz.bit" "" "" "-acodec copy" > fate-lavf-fate-qtrle_mace6.mov: CMD = lavf_container_fate "qtrle/Animation-16Greys.mov" "-idct auto" > fate-lavf-fate-cram.avi: CMD = lavf_container_fate "cram/toon.avi" "-idct auto" > > diff --git a/tests/fate/matroska.mak b/tests/fate/matroska.mak > index ca7193a055..545a0d1d50 100644 > --- a/tests/fate/matroska.mak > +++ b/tests/fate/matroska.mak > @@ -105,7 +105,7 @@ FATE_MATROSKA_FFMPEG_FFPROBE-$(call ALLYES, FILE_PROTOCOL MPEGTS_DEMUXER \ > MATROSKA_DEMUXER H264_DECODER \ > FRAMECRC_MUXER PIPE_PROTOCOL) \ > += fate-matroska-h264-remux > -fate-matroska-h264-remux: CMD = transcode mpegts $(TARGET_SAMPLES)/h264/h264_intra_first-small.ts matroska "-map 0:0 -map 0 -c:v copy -sar:0 3:4 -bsf:v:1 h264_metadata=aud=remove:delete_filler=1 -disposition:v +hearing_impaired -af aresample -c:a:0 pcm_s32le -c:a:1 pcm_s32be -disposition:a:0 original -metadata:s:a:0 title=swedish_silence -metadata:s:a:1 title=norwegian_silence -disposition:a:1 dub" "-map 0:v" "" "-show_entries stream=index,codec_name:stream_tags=title,language" > +fate-matroska-h264-remux: CMD = transcode mpegts $(TARGET_SAMPLES)/h264/h264_intra_first-small.ts matroska "-map 0:0 -map 0 -c:v copy -copyinkf -sar:0 3:4 -bsf:v:1 h264_metadata=aud=remove:delete_filler=1 -disposition:v +hearing_impaired -af aresample -c:a:0 pcm_s32le -c:a:1 pcm_s32be -disposition:a:0 original -metadata:s:a:0 title=swedish_silence -metadata:s:a:1 title=norwegian_silence -disposition:a:1 dub" "-map 0:v" "" "-show_entries stream=index,codec_name:stream_tags=title,language" > > # Tests writing BlockAdditional and BlockGroups with ReferenceBlock elements; > # it also tests setting a track as suitable for hearing impaired. > diff --git a/tests/ref/fate/copy-trac2211-avi b/tests/ref/fate/copy-trac2211-avi > index 06d81e537d..1f71ae65f2 100644 > --- a/tests/ref/fate/copy-trac2211-avi > +++ b/tests/ref/fate/copy-trac2211-avi > @@ -1,4 +1,4 @@ > -0920978f3f8196413c43f0033b55a5b6 *tests/data/fate/copy-trac2211-avi.avi > +ee1e66eac40569ae3cf9552286900900 *tests/data/fate/copy-trac2211-avi.avi > 1777956 tests/data/fate/copy-trac2211-avi.avi > #tb 0: 1/14 > #media_type 0: video > diff --git a/tests/ref/fate/matroska-h264-remux b/tests/ref/fate/matroska-h264-remux > index 14e6758fa0..7b852f8266 100644 > --- a/tests/ref/fate/matroska-h264-remux > +++ b/tests/ref/fate/matroska-h264-remux > @@ -1,5 +1,5 @@ > -ded6da7e46ce7df1232b116afb0b2f0a *tests/data/fate/matroska-h264-remux.matroska > -2036083 tests/data/fate/matroska-h264-remux.matroska > +d5fc08094380fc8aba485c09b596ceee *tests/data/fate/matroska-h264-remux.matroska > +2371935 tests/data/fate/matroska-h264-remux.matroska The increase in filesize shown here is due to your copyinkf (which you had to add because without it no video packets would be in the output at all): with it, the undecodable frames before the actual keyframes are not dropped. And the keyframes are not marked as such. So copyinkf can't even be used as a workaround. > #tb 0: 1/25 > #media_type 0: video > #codec_id 0: rawvideo > diff --git a/tests/ref/fate/segment-mp4-to-ts b/tests/ref/fate/segment-mp4-to-ts > index 8b0746fa92..5c456cd0bc 100644 > --- a/tests/ref/fate/segment-mp4-to-ts > +++ b/tests/ref/fate/segment-mp4-to-ts > @@ -25,7 +25,7 @@ > 0, 57600, 64800, 3600, 1182, 0xbe1a4847, F=0x0, S=1, 1 > 0, 61200, 61200, 3600, 809, 0x8d948a4e, F=0x0, S=1, 1 > 0, 64800, 68400, 3600, 656, 0x4fa03c2b, F=0x0, S=1, 1 > -0, 68400, 86400, 3600, 26555, 0x5629b584, S=1, 1 > +0, 68400, 86400, 3600, 26555, 0x5629b584, F=0x0, S=1, 1 > 0, 72000, 79200, 3600, 1141, 0x761b31e8, F=0x0, S=1, 1 > 0, 75600, 75600, 3600, 717, 0x57746351, F=0x0, S=1, 1 > 0, 79200, 82800, 3600, 693, 0x78b24263, F=0x0, S=1, 1 > @@ -49,7 +49,7 @@ > 0, 144000, 151200, 3600, 1271, 0x46006870, F=0x0, S=1, 1 > 0, 147600, 147600, 3600, 849, 0x94dc99c7, F=0x0, S=1, 1 > 0, 151200, 154800, 3600, 753, 0xf4236cab, F=0x0, S=1, 1 > -0, 154800, 172800, 3600, 25825, 0xd5464dee, S=1, 1 > +0, 154800, 172800, 3600, 25825, 0xd5464dee, F=0x0, S=1, 1 > 0, 158400, 165600, 3600, 1206, 0x8ce84344, F=0x0, S=1, 1 > 0, 162000, 162000, 3600, 867, 0x312fa07d, F=0x0, S=1, 1 > 0, 165600, 169200, 3600, 719, 0x810666d1, F=0x0, S=1, 1 > @@ -73,7 +73,7 @@ > 0, 230400, 237600, 3600, 1545, 0x0099fc98, F=0x0, S=1, 1 > 0, 234000, 234000, 3600, 929, 0xfd72d049, F=0x0, S=1, 1 > 0, 237600, 241200, 3600, 829, 0xcfda9e96, F=0x0, S=1, 1 > -0, 241200, 259200, 3600, 24220, 0x5ca21d71, S=1, 1 > +0, 241200, 259200, 3600, 24220, 0x5ca21d71, F=0x0, S=1, 1 > 0, 244800, 252000, 3600, 1422, 0xcde6cc34, F=0x0, S=1, 1 > 0, 248400, 248400, 3600, 883, 0xedacbe25, F=0x0, S=1, 1 > 0, 252000, 255600, 3600, 768, 0x89d774bc, F=0x0, S=1, 1 > @@ -97,7 +97,7 @@ > 0, 316800, 324000, 3600, 1501, 0xb3b8f001, F=0x0, S=1, 1 > 0, 320400, 320400, 3600, 941, 0x92b0cb18, F=0x0, S=1, 1 > 0, 324000, 327600, 3600, 823, 0x3d548355, F=0x0, S=1, 1 > -0, 327600, 345600, 3600, 24042, 0x441e94fb, S=1, 1 > +0, 327600, 345600, 3600, 24042, 0x441e94fb, F=0x0, S=1, 1 > 0, 331200, 338400, 3600, 1582, 0x4f5d1049, F=0x0, S=1, 1 > 0, 334800, 334800, 3600, 945, 0x4f3cc9e8, F=0x0, S=1, 1 > 0, 338400, 342000, 3600, 815, 0x0ca790a4, F=0x0, S=1, 1 > @@ -121,7 +121,7 @@ > 0, 403200, 410400, 3600, 359, 0x11bdae52, F=0x0, S=1, 1 > 0, 406800, 406800, 3600, 235, 0xbec26964, F=0x0, S=1, 1 > 0, 410400, 414000, 3600, 221, 0x8380682c, F=0x0, S=1, 1 > -0, 414000, 432000, 3600, 22588, 0xf0ecf072, S=1, 1 > +0, 414000, 432000, 3600, 22588, 0xf0ecf072, F=0x0, S=1, 1 > 0, 417600, 424800, 3600, 383, 0x4f3bb571, F=0x0, S=1, 1 > 0, 421200, 421200, 3600, 257, 0x22e87802, F=0x0, S=1, 1 > 0, 424800, 428400, 3600, 261, 0xdb988134, F=0x0, S=1, 1 > diff --git a/tests/ref/lavf-fate/h264.mp4 b/tests/ref/lavf-fate/h264.mp4 > index a9c3823c2c..54d8c407d2 100644 > --- a/tests/ref/lavf-fate/h264.mp4 > +++ b/tests/ref/lavf-fate/h264.mp4 > @@ -1,3 +1,3 @@ > -fe299ea5205b71a48281f917b1256a5d *tests/data/lavf-fate/lavf.h264.mp4 > -547928 tests/data/lavf-fate/lavf.h264.mp4 > +badb54efedaf0c7f725158b85339a8f4 *tests/data/lavf-fate/lavf.h264.mp4 > +548177 tests/data/lavf-fate/lavf.h264.mp4 > tests/data/lavf-fate/lavf.h264.mp4 CRC=0x9da2c999 >
On 7/17/2021 9:30 PM, Andreas Rheinhardt wrote: > James Almer: >> Tag only packets containing with IDR pictures as keyframes by default, same as >> the decoder. >> Fixes MP4 and Matroska muxers marking incorrect samples as Sync Samples in >> scenarios where this AVParser is used. >> > > 1. Could you please explain where you got the info that Matroska > keyframes need to be ISOBMFF Sync Samples from? Because it's actually > undefined what it exactly is; in case of AV1 (the only codec with a > detailed codec mapping) said mapping allows delayed random access points > to be marked as keyframes (without providing anything to actually signal > that these are delayed random access points), so a key frame is simply a > random access point. And that is how it is de-facto handled with all > other codecs as well. IMO it is ISOBMFF which is the outlier here. It was an assumption considering the Matroska h264 encapsulation is taken verbatim from ISOBMFF. But you're right, MKVToolnix does mark those as key. > >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavcodec/h264_parser.c | 16 ++++++++++------ >> tests/fate-run.sh | 4 ++-- >> tests/fate/ffmpeg.mak | 2 +- >> tests/fate/lavf-container.mak | 12 ++++++------ >> tests/fate/matroska.mak | 2 +- >> tests/ref/fate/copy-trac2211-avi | 2 +- >> tests/ref/fate/matroska-h264-remux | 4 ++-- >> tests/ref/fate/segment-mp4-to-ts | 10 +++++----- >> tests/ref/lavf-fate/h264.mp4 | 4 ++-- >> 9 files changed, 30 insertions(+), 26 deletions(-) >> >> diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c >> index d3c56cc188..532dc462b0 100644 >> --- a/libavcodec/h264_parser.c >> +++ b/libavcodec/h264_parser.c >> @@ -344,9 +344,11 @@ static inline int parse_nal_units(AVCodecParserContext *s, >> get_ue_golomb_long(&nal.gb); // skip first_mb_in_slice >> slice_type = get_ue_golomb_31(&nal.gb); >> s->pict_type = ff_h264_golomb_to_pict_type[slice_type % 5]; >> - if (p->sei.recovery_point.recovery_frame_cnt >= 0) { >> - /* key frame, since recovery_frame_cnt is set */ >> - s->key_frame = 1; >> + if (s->flags & PARSER_FLAG_USE_KEYFRAME_HEURISTICS) { >> + if (p->sei.recovery_point.recovery_frame_cnt >= 0) { >> + /* key frame, since recovery_frame_cnt is set */ >> + s->key_frame = 1; >> + } > > 2. This change means that files that don't use IDR pictures, but only > recovery point SEIs won't have packets marked as keyframes at all any > more; this affects every open-gop video. This is way worse than an > incorrect sync sample list created by the mp4 muxer. I worked around this for the mpegts muxer as Kieran requested, but if Matroska is the same then the situation changes. I can add a user settable demuxer option for the autoinserted parser instead of hardcoding the behavior, and leave the current behavior as the default. > > (When using x264 with open-gop, it even means that every stream so > encoded will only have one keyframe (the IDR frame at the beginning), > because for some reason x264 never uses IDR frames in open-gop mode even > at scenecuts (where the gop is closed and where using an IDR frame > instead of a recovery point SEI would actually save a few bytes).) > >> } >> pps_id = get_ue_golomb(&nal.gb); >> if (pps_id >= MAX_PPS_COUNT) { >> @@ -370,9 +372,11 @@ static inline int parse_nal_units(AVCodecParserContext *s, >> p->ps.sps = p->ps.pps->sps; >> sps = p->ps.sps; >> >> - // heuristic to detect non marked keyframes >> - if (p->ps.sps->ref_frame_count <= 1 && p->ps.pps->ref_count[0] <= 1 && s->pict_type == AV_PICTURE_TYPE_I) >> - s->key_frame = 1; >> + if (s->flags & PARSER_FLAG_USE_KEYFRAME_HEURISTICS) { >> + // heuristic to detect non marked keyframes >> + if (p->ps.sps->ref_frame_count <= 1 && p->ps.pps->ref_count[0] <= 1 && s->pict_type == AV_PICTURE_TYPE_I) >> + s->key_frame = 1; >> + } >> >> p->poc.frame_num = get_bits(&nal.gb, sps->log2_max_frame_num); >> >> diff --git a/tests/fate-run.sh b/tests/fate-run.sh >> index ba437dfbb8..8680e35524 100755 >> --- a/tests/fate-run.sh >> +++ b/tests/fate-run.sh >> @@ -339,8 +339,8 @@ lavf_container_fate() >> outdir="tests/data/lavf-fate" >> file=${outdir}/lavf.$t >> input="${target_samples}/$1" >> - do_avconv $file -auto_conversion_filters $DEC_OPTS $2 -i "$input" "$ENC_OPTS -metadata title=lavftest" -vcodec copy -acodec copy >> - do_avconv_crc $file -auto_conversion_filters $DEC_OPTS -i $target_path/$file $3 >> + do_avconv $file -auto_conversion_filters $DEC_OPTS $2 -i "$input" "$ENC_OPTS -metadata title=lavftest" -vcodec copy -acodec copy $3 >> + do_avconv_crc $file -auto_conversion_filters $DEC_OPTS -i $target_path/$file $4 >> } >> >> lavf_image(){ >> diff --git a/tests/fate/ffmpeg.mak b/tests/fate/ffmpeg.mak >> index 4dfb77d250..57d16fba6f 100644 >> --- a/tests/fate/ffmpeg.mak >> +++ b/tests/fate/ffmpeg.mak >> @@ -110,7 +110,7 @@ fate-copy-trac4914-avi: CMD = transcode mpegts $(TARGET_SAMPLES)/mpeg2/xdcam8mp2 >> FATE_STREAMCOPY-$(call ALLYES, H264_DEMUXER AVI_MUXER) += fate-copy-trac2211-avi >> fate-copy-trac2211-avi: $(SAMPLES)/h264/bbc2.sample.h264 >> fate-copy-trac2211-avi: CMD = transcode "h264 -r 14" $(TARGET_SAMPLES)/h264/bbc2.sample.h264\ >> - avi "-c:a copy -c:v copy" >> + avi "-c:a copy -c:v copy -copyinkf" >> >> FATE_STREAMCOPY-$(call ENCDEC, APNG, APNG) += fate-copy-apng >> fate-copy-apng: fate-lavf-apng >> diff --git a/tests/fate/lavf-container.mak b/tests/fate/lavf-container.mak >> index 9e0eed4851..40250badc1 100644 >> --- a/tests/fate/lavf-container.mak >> +++ b/tests/fate/lavf-container.mak >> @@ -71,13 +71,13 @@ FATE_LAVF_CONTAINER_FATE = $(FATE_LAVF_CONTAINER_FATE-yes:%=fate-lavf-fate-%) >> $(FATE_LAVF_CONTAINER_FATE): REF = $(SRC_PATH)/tests/ref/lavf-fate/$(@:fate-lavf-fate-%=%) >> $(FATE_LAVF_CONTAINER_FATE): $(AREF) $(VREF) >> >> -fate-lavf-fate-av1.mp4: CMD = lavf_container_fate "av1-test-vectors/av1-1-b8-05-mv.ivf" "" "-c:v copy" >> -fate-lavf-fate-av1.mkv: CMD = lavf_container_fate "av1-test-vectors/av1-1-b8-05-mv.ivf" "" "-c:v copy" >> -fate-lavf-fate-h264.mp4: CMD = lavf_container_fate "h264/intra_refresh.h264" "" "-c:v copy" >> +fate-lavf-fate-av1.mp4: CMD = lavf_container_fate "av1-test-vectors/av1-1-b8-05-mv.ivf" "" "" "-c:v copy" >> +fate-lavf-fate-av1.mkv: CMD = lavf_container_fate "av1-test-vectors/av1-1-b8-05-mv.ivf" "" "" "-c:v copy" >> +fate-lavf-fate-h264.mp4: CMD = lavf_container_fate "h264/intra_refresh.h264" "" "-copyinkf" "-c:v copy -copyinkf" > > 3. You are adding copyinkf twice here; are you sure that the new > parameter to lavf_container_fate is even necessary? When i tested i recall it was needed because one command in lavf_container_fate() remuxes the input file to create the output, and the second remuxed the newly created output with the framecrc pseudo muxer. > >> fate-lavf-fate-vp3.ogg: CMD = lavf_container_fate "vp3/coeff_level64.mkv" "-idct auto" >> -fate-lavf-fate-vp8.ogg: CMD = lavf_container_fate "vp8/RRSF49-short.webm" "" "-acodec copy" >> -fate-lavf-fate-latm: CMD = lavf_container_fate "aac/al04_44.mp4" "" "-acodec copy" >> -fate-lavf-fate-mp3: CMD = lavf_container_fate "mp3-conformance/he_32khz.bit" "" "-acodec copy" >> +fate-lavf-fate-vp8.ogg: CMD = lavf_container_fate "vp8/RRSF49-short.webm" "" "" "-acodec copy" >> +fate-lavf-fate-latm: CMD = lavf_container_fate "aac/al04_44.mp4" "" "" "-acodec copy" >> +fate-lavf-fate-mp3: CMD = lavf_container_fate "mp3-conformance/he_32khz.bit" "" "" "-acodec copy" >> fate-lavf-fate-qtrle_mace6.mov: CMD = lavf_container_fate "qtrle/Animation-16Greys.mov" "-idct auto" >> fate-lavf-fate-cram.avi: CMD = lavf_container_fate "cram/toon.avi" "-idct auto" >> >> diff --git a/tests/fate/matroska.mak b/tests/fate/matroska.mak >> index ca7193a055..545a0d1d50 100644 >> --- a/tests/fate/matroska.mak >> +++ b/tests/fate/matroska.mak >> @@ -105,7 +105,7 @@ FATE_MATROSKA_FFMPEG_FFPROBE-$(call ALLYES, FILE_PROTOCOL MPEGTS_DEMUXER \ >> MATROSKA_DEMUXER H264_DECODER \ >> FRAMECRC_MUXER PIPE_PROTOCOL) \ >> += fate-matroska-h264-remux >> -fate-matroska-h264-remux: CMD = transcode mpegts $(TARGET_SAMPLES)/h264/h264_intra_first-small.ts matroska "-map 0:0 -map 0 -c:v copy -sar:0 3:4 -bsf:v:1 h264_metadata=aud=remove:delete_filler=1 -disposition:v +hearing_impaired -af aresample -c:a:0 pcm_s32le -c:a:1 pcm_s32be -disposition:a:0 original -metadata:s:a:0 title=swedish_silence -metadata:s:a:1 title=norwegian_silence -disposition:a:1 dub" "-map 0:v" "" "-show_entries stream=index,codec_name:stream_tags=title,language" >> +fate-matroska-h264-remux: CMD = transcode mpegts $(TARGET_SAMPLES)/h264/h264_intra_first-small.ts matroska "-map 0:0 -map 0 -c:v copy -copyinkf -sar:0 3:4 -bsf:v:1 h264_metadata=aud=remove:delete_filler=1 -disposition:v +hearing_impaired -af aresample -c:a:0 pcm_s32le -c:a:1 pcm_s32be -disposition:a:0 original -metadata:s:a:0 title=swedish_silence -metadata:s:a:1 title=norwegian_silence -disposition:a:1 dub" "-map 0:v" "" "-show_entries stream=index,codec_name:stream_tags=title,language" >> >> # Tests writing BlockAdditional and BlockGroups with ReferenceBlock elements; >> # it also tests setting a track as suitable for hearing impaired. >> diff --git a/tests/ref/fate/copy-trac2211-avi b/tests/ref/fate/copy-trac2211-avi >> index 06d81e537d..1f71ae65f2 100644 >> --- a/tests/ref/fate/copy-trac2211-avi >> +++ b/tests/ref/fate/copy-trac2211-avi >> @@ -1,4 +1,4 @@ >> -0920978f3f8196413c43f0033b55a5b6 *tests/data/fate/copy-trac2211-avi.avi >> +ee1e66eac40569ae3cf9552286900900 *tests/data/fate/copy-trac2211-avi.avi >> 1777956 tests/data/fate/copy-trac2211-avi.avi >> #tb 0: 1/14 >> #media_type 0: video >> diff --git a/tests/ref/fate/matroska-h264-remux b/tests/ref/fate/matroska-h264-remux >> index 14e6758fa0..7b852f8266 100644 >> --- a/tests/ref/fate/matroska-h264-remux >> +++ b/tests/ref/fate/matroska-h264-remux >> @@ -1,5 +1,5 @@ >> -ded6da7e46ce7df1232b116afb0b2f0a *tests/data/fate/matroska-h264-remux.matroska >> -2036083 tests/data/fate/matroska-h264-remux.matroska >> +d5fc08094380fc8aba485c09b596ceee *tests/data/fate/matroska-h264-remux.matroska >> +2371935 tests/data/fate/matroska-h264-remux.matroska > > The increase in filesize shown here is due to your copyinkf (which you > had to add because without it no video packets would be in the output at > all): with it, the undecodable frames before the actual keyframes are > not dropped. And the keyframes are not marked as such. So copyinkf can't > even be used as a workaround. > >> #tb 0: 1/25 >> #media_type 0: video >> #codec_id 0: rawvideo >> diff --git a/tests/ref/fate/segment-mp4-to-ts b/tests/ref/fate/segment-mp4-to-ts >> index 8b0746fa92..5c456cd0bc 100644 >> --- a/tests/ref/fate/segment-mp4-to-ts >> +++ b/tests/ref/fate/segment-mp4-to-ts >> @@ -25,7 +25,7 @@ >> 0, 57600, 64800, 3600, 1182, 0xbe1a4847, F=0x0, S=1, 1 >> 0, 61200, 61200, 3600, 809, 0x8d948a4e, F=0x0, S=1, 1 >> 0, 64800, 68400, 3600, 656, 0x4fa03c2b, F=0x0, S=1, 1 >> -0, 68400, 86400, 3600, 26555, 0x5629b584, S=1, 1 >> +0, 68400, 86400, 3600, 26555, 0x5629b584, F=0x0, S=1, 1 >> 0, 72000, 79200, 3600, 1141, 0x761b31e8, F=0x0, S=1, 1 >> 0, 75600, 75600, 3600, 717, 0x57746351, F=0x0, S=1, 1 >> 0, 79200, 82800, 3600, 693, 0x78b24263, F=0x0, S=1, 1 >> @@ -49,7 +49,7 @@ >> 0, 144000, 151200, 3600, 1271, 0x46006870, F=0x0, S=1, 1 >> 0, 147600, 147600, 3600, 849, 0x94dc99c7, F=0x0, S=1, 1 >> 0, 151200, 154800, 3600, 753, 0xf4236cab, F=0x0, S=1, 1 >> -0, 154800, 172800, 3600, 25825, 0xd5464dee, S=1, 1 >> +0, 154800, 172800, 3600, 25825, 0xd5464dee, F=0x0, S=1, 1 >> 0, 158400, 165600, 3600, 1206, 0x8ce84344, F=0x0, S=1, 1 >> 0, 162000, 162000, 3600, 867, 0x312fa07d, F=0x0, S=1, 1 >> 0, 165600, 169200, 3600, 719, 0x810666d1, F=0x0, S=1, 1 >> @@ -73,7 +73,7 @@ >> 0, 230400, 237600, 3600, 1545, 0x0099fc98, F=0x0, S=1, 1 >> 0, 234000, 234000, 3600, 929, 0xfd72d049, F=0x0, S=1, 1 >> 0, 237600, 241200, 3600, 829, 0xcfda9e96, F=0x0, S=1, 1 >> -0, 241200, 259200, 3600, 24220, 0x5ca21d71, S=1, 1 >> +0, 241200, 259200, 3600, 24220, 0x5ca21d71, F=0x0, S=1, 1 >> 0, 244800, 252000, 3600, 1422, 0xcde6cc34, F=0x0, S=1, 1 >> 0, 248400, 248400, 3600, 883, 0xedacbe25, F=0x0, S=1, 1 >> 0, 252000, 255600, 3600, 768, 0x89d774bc, F=0x0, S=1, 1 >> @@ -97,7 +97,7 @@ >> 0, 316800, 324000, 3600, 1501, 0xb3b8f001, F=0x0, S=1, 1 >> 0, 320400, 320400, 3600, 941, 0x92b0cb18, F=0x0, S=1, 1 >> 0, 324000, 327600, 3600, 823, 0x3d548355, F=0x0, S=1, 1 >> -0, 327600, 345600, 3600, 24042, 0x441e94fb, S=1, 1 >> +0, 327600, 345600, 3600, 24042, 0x441e94fb, F=0x0, S=1, 1 >> 0, 331200, 338400, 3600, 1582, 0x4f5d1049, F=0x0, S=1, 1 >> 0, 334800, 334800, 3600, 945, 0x4f3cc9e8, F=0x0, S=1, 1 >> 0, 338400, 342000, 3600, 815, 0x0ca790a4, F=0x0, S=1, 1 >> @@ -121,7 +121,7 @@ >> 0, 403200, 410400, 3600, 359, 0x11bdae52, F=0x0, S=1, 1 >> 0, 406800, 406800, 3600, 235, 0xbec26964, F=0x0, S=1, 1 >> 0, 410400, 414000, 3600, 221, 0x8380682c, F=0x0, S=1, 1 >> -0, 414000, 432000, 3600, 22588, 0xf0ecf072, S=1, 1 >> +0, 414000, 432000, 3600, 22588, 0xf0ecf072, F=0x0, S=1, 1 >> 0, 417600, 424800, 3600, 383, 0x4f3bb571, F=0x0, S=1, 1 >> 0, 421200, 421200, 3600, 257, 0x22e87802, F=0x0, S=1, 1 >> 0, 424800, 428400, 3600, 261, 0xdb988134, F=0x0, S=1, 1 >> diff --git a/tests/ref/lavf-fate/h264.mp4 b/tests/ref/lavf-fate/h264.mp4 >> index a9c3823c2c..54d8c407d2 100644 >> --- a/tests/ref/lavf-fate/h264.mp4 >> +++ b/tests/ref/lavf-fate/h264.mp4 >> @@ -1,3 +1,3 @@ >> -fe299ea5205b71a48281f917b1256a5d *tests/data/lavf-fate/lavf.h264.mp4 >> -547928 tests/data/lavf-fate/lavf.h264.mp4 >> +badb54efedaf0c7f725158b85339a8f4 *tests/data/lavf-fate/lavf.h264.mp4 >> +548177 tests/data/lavf-fate/lavf.h264.mp4 >> tests/data/lavf-fate/lavf.h264.mp4 CRC=0x9da2c999 >> > > _______________________________________________ > 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". >
James Almer: > On 7/17/2021 9:30 PM, Andreas Rheinhardt wrote: >> James Almer: >>> Tag only packets containing with IDR pictures as keyframes by >>> default, same as >>> the decoder. >>> Fixes MP4 and Matroska muxers marking incorrect samples as Sync >>> Samples in >>> scenarios where this AVParser is used. >>> >> >> 1. Could you please explain where you got the info that Matroska >> keyframes need to be ISOBMFF Sync Samples from? Because it's actually >> undefined what it exactly is; in case of AV1 (the only codec with a >> detailed codec mapping) said mapping allows delayed random access points >> to be marked as keyframes (without providing anything to actually signal >> that these are delayed random access points), so a key frame is simply a >> random access point. And that is how it is de-facto handled with all >> other codecs as well. IMO it is ISOBMFF which is the outlier here. > > It was an assumption considering the Matroska h264 encapsulation is > taken verbatim from ISOBMFF. But you're right, MKVToolnix does mark > those as key. > It's not taken verbatim -- Matroska doesn't have an analog to the sync samples table; actually, it is not even guaranteed that cues always reference keyframes (MKVToolNix even allows to reference all blocks!), although (lacking an alternative) demuxers operate under this assumption. >> >>> Signed-off-by: James Almer <jamrial@gmail.com> >>> --- >>> libavcodec/h264_parser.c | 16 ++++++++++------ >>> tests/fate-run.sh | 4 ++-- >>> tests/fate/ffmpeg.mak | 2 +- >>> tests/fate/lavf-container.mak | 12 ++++++------ >>> tests/fate/matroska.mak | 2 +- >>> tests/ref/fate/copy-trac2211-avi | 2 +- >>> tests/ref/fate/matroska-h264-remux | 4 ++-- >>> tests/ref/fate/segment-mp4-to-ts | 10 +++++----- >>> tests/ref/lavf-fate/h264.mp4 | 4 ++-- >>> 9 files changed, 30 insertions(+), 26 deletions(-) >>> >>> diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c >>> index d3c56cc188..532dc462b0 100644 >>> --- a/libavcodec/h264_parser.c >>> +++ b/libavcodec/h264_parser.c >>> @@ -344,9 +344,11 @@ static inline int >>> parse_nal_units(AVCodecParserContext *s, >>> get_ue_golomb_long(&nal.gb); // skip first_mb_in_slice >>> slice_type = get_ue_golomb_31(&nal.gb); >>> s->pict_type = ff_h264_golomb_to_pict_type[slice_type % >>> 5]; >>> - if (p->sei.recovery_point.recovery_frame_cnt >= 0) { >>> - /* key frame, since recovery_frame_cnt is set */ >>> - s->key_frame = 1; >>> + if (s->flags & PARSER_FLAG_USE_KEYFRAME_HEURISTICS) { >>> + if (p->sei.recovery_point.recovery_frame_cnt >= 0) { >>> + /* key frame, since recovery_frame_cnt is set */ >>> + s->key_frame = 1; >>> + } >> >> 2. This change means that files that don't use IDR pictures, but only >> recovery point SEIs won't have packets marked as keyframes at all any >> more; this affects every open-gop video. This is way worse than an >> incorrect sync sample list created by the mp4 muxer. > > I worked around this for the mpegts muxer as Kieran requested, but if > Matroska is the same then the situation changes. > > I can add a user settable demuxer option for the autoinserted parser > instead of hardcoding the behavior, and leave the current behavior as > the default. > So the mp4 muxer would create invalid files by default and in case this flag is set, it instead creates crippled files (where open gop random access points are not marked as such)? Does not sound good to me. An actual fix is to make the muxer aware of the difference between IDR and ordinary keyframes (there is already a MOV_PARTIAL_SYNC_SAMPLE for the latter; and there is already some parsing in case of MPEG-2). (As much as I like to reuse the parsers for this, I don't really know what exactly the parser should additionally return. Should this be solved, we might need to add another field to AVPacket (given that the new parsing API is supposed to be packet-based, said information should be stored there; alternatively, one could also use more of AV_PKT_FLAG_*). We do not even have to wait for the new parser API to get this information out of the parser: We can just add new fields to AVCodecParserContext; but we need to agree on whether this information should be part of AVPacket and if so, how.) - Andreas
On 7/17/2021 11:19 PM, Andreas Rheinhardt wrote: > James Almer: >> On 7/17/2021 9:30 PM, Andreas Rheinhardt wrote: >>> James Almer: >>>> Tag only packets containing with IDR pictures as keyframes by >>>> default, same as >>>> the decoder. >>>> Fixes MP4 and Matroska muxers marking incorrect samples as Sync >>>> Samples in >>>> scenarios where this AVParser is used. >>>> >>> >>> 1. Could you please explain where you got the info that Matroska >>> keyframes need to be ISOBMFF Sync Samples from? Because it's actually >>> undefined what it exactly is; in case of AV1 (the only codec with a >>> detailed codec mapping) said mapping allows delayed random access points >>> to be marked as keyframes (without providing anything to actually signal >>> that these are delayed random access points), so a key frame is simply a >>> random access point. And that is how it is de-facto handled with all >>> other codecs as well. IMO it is ISOBMFF which is the outlier here. >> >> It was an assumption considering the Matroska h264 encapsulation is >> taken verbatim from ISOBMFF. But you're right, MKVToolnix does mark >> those as key. >> > > It's not taken verbatim -- Matroska doesn't have an analog to the sync > samples table; actually, it is not even guaranteed that cues always > reference keyframes (MKVToolNix even allows to reference all blocks!), > although (lacking an alternative) demuxers operate under this assumption. > >>> >>>> Signed-off-by: James Almer <jamrial@gmail.com> >>>> --- >>>> libavcodec/h264_parser.c | 16 ++++++++++------ >>>> tests/fate-run.sh | 4 ++-- >>>> tests/fate/ffmpeg.mak | 2 +- >>>> tests/fate/lavf-container.mak | 12 ++++++------ >>>> tests/fate/matroska.mak | 2 +- >>>> tests/ref/fate/copy-trac2211-avi | 2 +- >>>> tests/ref/fate/matroska-h264-remux | 4 ++-- >>>> tests/ref/fate/segment-mp4-to-ts | 10 +++++----- >>>> tests/ref/lavf-fate/h264.mp4 | 4 ++-- >>>> 9 files changed, 30 insertions(+), 26 deletions(-) >>>> >>>> diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c >>>> index d3c56cc188..532dc462b0 100644 >>>> --- a/libavcodec/h264_parser.c >>>> +++ b/libavcodec/h264_parser.c >>>> @@ -344,9 +344,11 @@ static inline int >>>> parse_nal_units(AVCodecParserContext *s, >>>> get_ue_golomb_long(&nal.gb); // skip first_mb_in_slice >>>> slice_type = get_ue_golomb_31(&nal.gb); >>>> s->pict_type = ff_h264_golomb_to_pict_type[slice_type % >>>> 5]; >>>> - if (p->sei.recovery_point.recovery_frame_cnt >= 0) { >>>> - /* key frame, since recovery_frame_cnt is set */ >>>> - s->key_frame = 1; >>>> + if (s->flags & PARSER_FLAG_USE_KEYFRAME_HEURISTICS) { >>>> + if (p->sei.recovery_point.recovery_frame_cnt >= 0) { >>>> + /* key frame, since recovery_frame_cnt is set */ >>>> + s->key_frame = 1; >>>> + } >>> >>> 2. This change means that files that don't use IDR pictures, but only >>> recovery point SEIs won't have packets marked as keyframes at all any >>> more; this affects every open-gop video. This is way worse than an >>> incorrect sync sample list created by the mp4 muxer. >> >> I worked around this for the mpegts muxer as Kieran requested, but if >> Matroska is the same then the situation changes. >> >> I can add a user settable demuxer option for the autoinserted parser >> instead of hardcoding the behavior, and leave the current behavior as >> the default. >> > > So the mp4 muxer would create invalid files by default and in case this > flag is set, it instead creates crippled files (where open gop random > access points are not marked as such)? Does not sound good to me. > An actual fix is to make the muxer aware of the difference between IDR > and ordinary keyframes (there is already a MOV_PARTIAL_SYNC_SAMPLE for > the latter; and there is already some parsing in case of MPEG-2). The current code uses this flag to write stps atoms, and at least for h264 that's not apparently what should be done (We need roll sample groups instead). > > (As much as I like to reuse the parsers for this, I don't really know > what exactly the parser should additionally return. Should this be > solved, we might need to add another field to AVPacket (given that the > new parsing API is supposed to be packet-based, said information should > be stored there; alternatively, one could also use more of AV_PKT_FLAG_*). A packet flag would work to signal that the packet contains an ordinary key frame, but not to propagate values like recovery_frame_cnt (to write roll distance). That would probably require a side data struct. > We do not even have to wait for the new parser API to get this > information out of the parser: We can just add new fields to > AVCodecParserContext; but we need to agree on whether this information > should be part of AVPacket and if so, how.) It would need to be part of AVPacket if we do it as something AVCodecParsers and demuxers can output and propagate. Even a bsfs inserted by the muxer would require whatever we get out of it to be attached to the packet. But we could maybe avoid this with a new bsfs based parser API where for example reduced scope CBS-style codec-specific structs could be offered, and used by muxers in a similar way some of them are currently using bsfs. > > - Andreas > _______________________________________________ > 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 --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c index d3c56cc188..532dc462b0 100644 --- a/libavcodec/h264_parser.c +++ b/libavcodec/h264_parser.c @@ -344,9 +344,11 @@ static inline int parse_nal_units(AVCodecParserContext *s, get_ue_golomb_long(&nal.gb); // skip first_mb_in_slice slice_type = get_ue_golomb_31(&nal.gb); s->pict_type = ff_h264_golomb_to_pict_type[slice_type % 5]; - if (p->sei.recovery_point.recovery_frame_cnt >= 0) { - /* key frame, since recovery_frame_cnt is set */ - s->key_frame = 1; + if (s->flags & PARSER_FLAG_USE_KEYFRAME_HEURISTICS) { + if (p->sei.recovery_point.recovery_frame_cnt >= 0) { + /* key frame, since recovery_frame_cnt is set */ + s->key_frame = 1; + } } pps_id = get_ue_golomb(&nal.gb); if (pps_id >= MAX_PPS_COUNT) { @@ -370,9 +372,11 @@ static inline int parse_nal_units(AVCodecParserContext *s, p->ps.sps = p->ps.pps->sps; sps = p->ps.sps; - // heuristic to detect non marked keyframes - if (p->ps.sps->ref_frame_count <= 1 && p->ps.pps->ref_count[0] <= 1 && s->pict_type == AV_PICTURE_TYPE_I) - s->key_frame = 1; + if (s->flags & PARSER_FLAG_USE_KEYFRAME_HEURISTICS) { + // heuristic to detect non marked keyframes + if (p->ps.sps->ref_frame_count <= 1 && p->ps.pps->ref_count[0] <= 1 && s->pict_type == AV_PICTURE_TYPE_I) + s->key_frame = 1; + } p->poc.frame_num = get_bits(&nal.gb, sps->log2_max_frame_num); diff --git a/tests/fate-run.sh b/tests/fate-run.sh index ba437dfbb8..8680e35524 100755 --- a/tests/fate-run.sh +++ b/tests/fate-run.sh @@ -339,8 +339,8 @@ lavf_container_fate() outdir="tests/data/lavf-fate" file=${outdir}/lavf.$t input="${target_samples}/$1" - do_avconv $file -auto_conversion_filters $DEC_OPTS $2 -i "$input" "$ENC_OPTS -metadata title=lavftest" -vcodec copy -acodec copy - do_avconv_crc $file -auto_conversion_filters $DEC_OPTS -i $target_path/$file $3 + do_avconv $file -auto_conversion_filters $DEC_OPTS $2 -i "$input" "$ENC_OPTS -metadata title=lavftest" -vcodec copy -acodec copy $3 + do_avconv_crc $file -auto_conversion_filters $DEC_OPTS -i $target_path/$file $4 } lavf_image(){ diff --git a/tests/fate/ffmpeg.mak b/tests/fate/ffmpeg.mak index 4dfb77d250..57d16fba6f 100644 --- a/tests/fate/ffmpeg.mak +++ b/tests/fate/ffmpeg.mak @@ -110,7 +110,7 @@ fate-copy-trac4914-avi: CMD = transcode mpegts $(TARGET_SAMPLES)/mpeg2/xdcam8mp2 FATE_STREAMCOPY-$(call ALLYES, H264_DEMUXER AVI_MUXER) += fate-copy-trac2211-avi fate-copy-trac2211-avi: $(SAMPLES)/h264/bbc2.sample.h264 fate-copy-trac2211-avi: CMD = transcode "h264 -r 14" $(TARGET_SAMPLES)/h264/bbc2.sample.h264\ - avi "-c:a copy -c:v copy" + avi "-c:a copy -c:v copy -copyinkf" FATE_STREAMCOPY-$(call ENCDEC, APNG, APNG) += fate-copy-apng fate-copy-apng: fate-lavf-apng diff --git a/tests/fate/lavf-container.mak b/tests/fate/lavf-container.mak index 9e0eed4851..40250badc1 100644 --- a/tests/fate/lavf-container.mak +++ b/tests/fate/lavf-container.mak @@ -71,13 +71,13 @@ FATE_LAVF_CONTAINER_FATE = $(FATE_LAVF_CONTAINER_FATE-yes:%=fate-lavf-fate-%) $(FATE_LAVF_CONTAINER_FATE): REF = $(SRC_PATH)/tests/ref/lavf-fate/$(@:fate-lavf-fate-%=%) $(FATE_LAVF_CONTAINER_FATE): $(AREF) $(VREF) -fate-lavf-fate-av1.mp4: CMD = lavf_container_fate "av1-test-vectors/av1-1-b8-05-mv.ivf" "" "-c:v copy" -fate-lavf-fate-av1.mkv: CMD = lavf_container_fate "av1-test-vectors/av1-1-b8-05-mv.ivf" "" "-c:v copy" -fate-lavf-fate-h264.mp4: CMD = lavf_container_fate "h264/intra_refresh.h264" "" "-c:v copy" +fate-lavf-fate-av1.mp4: CMD = lavf_container_fate "av1-test-vectors/av1-1-b8-05-mv.ivf" "" "" "-c:v copy" +fate-lavf-fate-av1.mkv: CMD = lavf_container_fate "av1-test-vectors/av1-1-b8-05-mv.ivf" "" "" "-c:v copy" +fate-lavf-fate-h264.mp4: CMD = lavf_container_fate "h264/intra_refresh.h264" "" "-copyinkf" "-c:v copy -copyinkf" fate-lavf-fate-vp3.ogg: CMD = lavf_container_fate "vp3/coeff_level64.mkv" "-idct auto" -fate-lavf-fate-vp8.ogg: CMD = lavf_container_fate "vp8/RRSF49-short.webm" "" "-acodec copy" -fate-lavf-fate-latm: CMD = lavf_container_fate "aac/al04_44.mp4" "" "-acodec copy" -fate-lavf-fate-mp3: CMD = lavf_container_fate "mp3-conformance/he_32khz.bit" "" "-acodec copy" +fate-lavf-fate-vp8.ogg: CMD = lavf_container_fate "vp8/RRSF49-short.webm" "" "" "-acodec copy" +fate-lavf-fate-latm: CMD = lavf_container_fate "aac/al04_44.mp4" "" "" "-acodec copy" +fate-lavf-fate-mp3: CMD = lavf_container_fate "mp3-conformance/he_32khz.bit" "" "" "-acodec copy" fate-lavf-fate-qtrle_mace6.mov: CMD = lavf_container_fate "qtrle/Animation-16Greys.mov" "-idct auto" fate-lavf-fate-cram.avi: CMD = lavf_container_fate "cram/toon.avi" "-idct auto" diff --git a/tests/fate/matroska.mak b/tests/fate/matroska.mak index ca7193a055..545a0d1d50 100644 --- a/tests/fate/matroska.mak +++ b/tests/fate/matroska.mak @@ -105,7 +105,7 @@ FATE_MATROSKA_FFMPEG_FFPROBE-$(call ALLYES, FILE_PROTOCOL MPEGTS_DEMUXER \ MATROSKA_DEMUXER H264_DECODER \ FRAMECRC_MUXER PIPE_PROTOCOL) \ += fate-matroska-h264-remux -fate-matroska-h264-remux: CMD = transcode mpegts $(TARGET_SAMPLES)/h264/h264_intra_first-small.ts matroska "-map 0:0 -map 0 -c:v copy -sar:0 3:4 -bsf:v:1 h264_metadata=aud=remove:delete_filler=1 -disposition:v +hearing_impaired -af aresample -c:a:0 pcm_s32le -c:a:1 pcm_s32be -disposition:a:0 original -metadata:s:a:0 title=swedish_silence -metadata:s:a:1 title=norwegian_silence -disposition:a:1 dub" "-map 0:v" "" "-show_entries stream=index,codec_name:stream_tags=title,language" +fate-matroska-h264-remux: CMD = transcode mpegts $(TARGET_SAMPLES)/h264/h264_intra_first-small.ts matroska "-map 0:0 -map 0 -c:v copy -copyinkf -sar:0 3:4 -bsf:v:1 h264_metadata=aud=remove:delete_filler=1 -disposition:v +hearing_impaired -af aresample -c:a:0 pcm_s32le -c:a:1 pcm_s32be -disposition:a:0 original -metadata:s:a:0 title=swedish_silence -metadata:s:a:1 title=norwegian_silence -disposition:a:1 dub" "-map 0:v" "" "-show_entries stream=index,codec_name:stream_tags=title,language" # Tests writing BlockAdditional and BlockGroups with ReferenceBlock elements; # it also tests setting a track as suitable for hearing impaired. diff --git a/tests/ref/fate/copy-trac2211-avi b/tests/ref/fate/copy-trac2211-avi index 06d81e537d..1f71ae65f2 100644 --- a/tests/ref/fate/copy-trac2211-avi +++ b/tests/ref/fate/copy-trac2211-avi @@ -1,4 +1,4 @@ -0920978f3f8196413c43f0033b55a5b6 *tests/data/fate/copy-trac2211-avi.avi +ee1e66eac40569ae3cf9552286900900 *tests/data/fate/copy-trac2211-avi.avi 1777956 tests/data/fate/copy-trac2211-avi.avi #tb 0: 1/14 #media_type 0: video diff --git a/tests/ref/fate/matroska-h264-remux b/tests/ref/fate/matroska-h264-remux index 14e6758fa0..7b852f8266 100644 --- a/tests/ref/fate/matroska-h264-remux +++ b/tests/ref/fate/matroska-h264-remux @@ -1,5 +1,5 @@ -ded6da7e46ce7df1232b116afb0b2f0a *tests/data/fate/matroska-h264-remux.matroska -2036083 tests/data/fate/matroska-h264-remux.matroska +d5fc08094380fc8aba485c09b596ceee *tests/data/fate/matroska-h264-remux.matroska +2371935 tests/data/fate/matroska-h264-remux.matroska #tb 0: 1/25 #media_type 0: video #codec_id 0: rawvideo diff --git a/tests/ref/fate/segment-mp4-to-ts b/tests/ref/fate/segment-mp4-to-ts index 8b0746fa92..5c456cd0bc 100644 --- a/tests/ref/fate/segment-mp4-to-ts +++ b/tests/ref/fate/segment-mp4-to-ts @@ -25,7 +25,7 @@ 0, 57600, 64800, 3600, 1182, 0xbe1a4847, F=0x0, S=1, 1 0, 61200, 61200, 3600, 809, 0x8d948a4e, F=0x0, S=1, 1 0, 64800, 68400, 3600, 656, 0x4fa03c2b, F=0x0, S=1, 1 -0, 68400, 86400, 3600, 26555, 0x5629b584, S=1, 1 +0, 68400, 86400, 3600, 26555, 0x5629b584, F=0x0, S=1, 1 0, 72000, 79200, 3600, 1141, 0x761b31e8, F=0x0, S=1, 1 0, 75600, 75600, 3600, 717, 0x57746351, F=0x0, S=1, 1 0, 79200, 82800, 3600, 693, 0x78b24263, F=0x0, S=1, 1 @@ -49,7 +49,7 @@ 0, 144000, 151200, 3600, 1271, 0x46006870, F=0x0, S=1, 1 0, 147600, 147600, 3600, 849, 0x94dc99c7, F=0x0, S=1, 1 0, 151200, 154800, 3600, 753, 0xf4236cab, F=0x0, S=1, 1 -0, 154800, 172800, 3600, 25825, 0xd5464dee, S=1, 1 +0, 154800, 172800, 3600, 25825, 0xd5464dee, F=0x0, S=1, 1 0, 158400, 165600, 3600, 1206, 0x8ce84344, F=0x0, S=1, 1 0, 162000, 162000, 3600, 867, 0x312fa07d, F=0x0, S=1, 1 0, 165600, 169200, 3600, 719, 0x810666d1, F=0x0, S=1, 1 @@ -73,7 +73,7 @@ 0, 230400, 237600, 3600, 1545, 0x0099fc98, F=0x0, S=1, 1 0, 234000, 234000, 3600, 929, 0xfd72d049, F=0x0, S=1, 1 0, 237600, 241200, 3600, 829, 0xcfda9e96, F=0x0, S=1, 1 -0, 241200, 259200, 3600, 24220, 0x5ca21d71, S=1, 1 +0, 241200, 259200, 3600, 24220, 0x5ca21d71, F=0x0, S=1, 1 0, 244800, 252000, 3600, 1422, 0xcde6cc34, F=0x0, S=1, 1 0, 248400, 248400, 3600, 883, 0xedacbe25, F=0x0, S=1, 1 0, 252000, 255600, 3600, 768, 0x89d774bc, F=0x0, S=1, 1 @@ -97,7 +97,7 @@ 0, 316800, 324000, 3600, 1501, 0xb3b8f001, F=0x0, S=1, 1 0, 320400, 320400, 3600, 941, 0x92b0cb18, F=0x0, S=1, 1 0, 324000, 327600, 3600, 823, 0x3d548355, F=0x0, S=1, 1 -0, 327600, 345600, 3600, 24042, 0x441e94fb, S=1, 1 +0, 327600, 345600, 3600, 24042, 0x441e94fb, F=0x0, S=1, 1 0, 331200, 338400, 3600, 1582, 0x4f5d1049, F=0x0, S=1, 1 0, 334800, 334800, 3600, 945, 0x4f3cc9e8, F=0x0, S=1, 1 0, 338400, 342000, 3600, 815, 0x0ca790a4, F=0x0, S=1, 1 @@ -121,7 +121,7 @@ 0, 403200, 410400, 3600, 359, 0x11bdae52, F=0x0, S=1, 1 0, 406800, 406800, 3600, 235, 0xbec26964, F=0x0, S=1, 1 0, 410400, 414000, 3600, 221, 0x8380682c, F=0x0, S=1, 1 -0, 414000, 432000, 3600, 22588, 0xf0ecf072, S=1, 1 +0, 414000, 432000, 3600, 22588, 0xf0ecf072, F=0x0, S=1, 1 0, 417600, 424800, 3600, 383, 0x4f3bb571, F=0x0, S=1, 1 0, 421200, 421200, 3600, 257, 0x22e87802, F=0x0, S=1, 1 0, 424800, 428400, 3600, 261, 0xdb988134, F=0x0, S=1, 1 diff --git a/tests/ref/lavf-fate/h264.mp4 b/tests/ref/lavf-fate/h264.mp4 index a9c3823c2c..54d8c407d2 100644 --- a/tests/ref/lavf-fate/h264.mp4 +++ b/tests/ref/lavf-fate/h264.mp4 @@ -1,3 +1,3 @@ -fe299ea5205b71a48281f917b1256a5d *tests/data/lavf-fate/lavf.h264.mp4 -547928 tests/data/lavf-fate/lavf.h264.mp4 +badb54efedaf0c7f725158b85339a8f4 *tests/data/lavf-fate/lavf.h264.mp4 +548177 tests/data/lavf-fate/lavf.h264.mp4 tests/data/lavf-fate/lavf.h264.mp4 CRC=0x9da2c999
Tag only packets containing with IDR pictures as keyframes by default, same as the decoder. Fixes MP4 and Matroska muxers marking incorrect samples as Sync Samples in scenarios where this AVParser is used. Signed-off-by: James Almer <jamrial@gmail.com> --- libavcodec/h264_parser.c | 16 ++++++++++------ tests/fate-run.sh | 4 ++-- tests/fate/ffmpeg.mak | 2 +- tests/fate/lavf-container.mak | 12 ++++++------ tests/fate/matroska.mak | 2 +- tests/ref/fate/copy-trac2211-avi | 2 +- tests/ref/fate/matroska-h264-remux | 4 ++-- tests/ref/fate/segment-mp4-to-ts | 10 +++++----- tests/ref/lavf-fate/h264.mp4 | 4 ++-- 9 files changed, 30 insertions(+), 26 deletions(-)