[FFmpeg-devel] [PATCH 2/3] fate/mxf: Fix d10-user-comments test
Andreas Rheinhardt
andreas.rheinhardt at gmail.com
Sun Dec 6 01:02:50 EET 2020
Andriy Gelman:
> On Fri, 04. Dec 23:57, Andreas Rheinhardt wrote:
>> The mxf_d10 muxer is very picky regarding the input it accepts:
>> The only video accepted is MPEG-2 with absolutely constant bitrate,
>> i.e. all packets need to have exactly the same size; and only a few
>> bitrates are accepted.
>>
>> The sample file used did not abide by this: Writing the first packet
>> (a video packet) errors out and afterwards an audio packet from the
>> muxing queue has been written. That's all besides metadata (which this
>> test is about). The FFmpeg cli returned an error, but said error has
>> been ignored by the md5 test.
>>
>> This commit changes the test to actually send a compliant stream to the
>> muxer, so that it does not error out; furthermore, the test is changed
>> to explicitly check the metadata instead of it only being implicitly
>> included in the md5 checksum. The compliant stream is created by our
>> encoder at runtime.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
>> ---
>> This test needs to be fixed in preparation for the next patch.
>>
>> tests/fate/mxf.mak | 8 ++++----
>> tests/ref/fate/mxf-d10-user-comments | 26 +++++++++++++++++++++++++-
>> 2 files changed, 29 insertions(+), 5 deletions(-)
>>
>> diff --git a/tests/fate/mxf.mak b/tests/fate/mxf.mak
>> index ca119fa677..467357ed2d 100644
>> --- a/tests/fate/mxf.mak
>> +++ b/tests/fate/mxf.mak
>> @@ -45,9 +45,8 @@ FATE_MXF_USER_COMMENTS-$(call ENCDEC2, MPEG2VIDEO, PCM_S16LE, MXF) += fate-mxf-u
>> fate-mxf-user-comments: $(SAMPLES)/mxf/Sony-00001.mxf
>> fate-mxf-user-comments: CMD = md5 -y -i $(TARGET_SAMPLES)/mxf/Sony-00001.mxf -c copy -metadata "comment_test=value" -fflags +bitexact -f mxf
>>
>> -FATE_MXF_D10_USER_COMMENTS-$(call ENCDEC2, MPEG2VIDEO, PCM_S16LE, MXF) += fate-mxf-d10-user-comments
>> -fate-mxf-d10-user-comments: $(SAMPLES)/mxf/Sony-00001.mxf
>> -fate-mxf-d10-user-comments: CMD = md5 -y -i $(TARGET_SAMPLES)/mxf/Sony-00001.mxf -c copy -metadata "comment_test=value" -store_user_comments 1 -fflags +bitexact -f mxf_d10
>> +FATE_MXF_D10_USER_COMMENTS-$(call ALLYES, FILE_PROTOCOL MXF_DEMUXER DVVIDEO_DECODER SCALE_FILTER MPEG2VIDEO_ENCODER MXF_D10_MUXER EXTRACT_EXTRADATA_BSF MPEGVIDEO_PARSER PIPE_PROTOCOL FRAMECRC_MUXER) += fate-mxf-d10-user-comments
>> +fate-mxf-d10-user-comments: CMD = transcode mxf $(TARGET_SAMPLES)/mxf/Avid-00005.mxf mxf_d10 "-c:v mpeg2video -b:v 30000k -minrate:v 30000k -maxrate:v 30000k -bufsize:v 30000k -rc_init_occupancy 30000k -vf scale=w=1280:h=720 -an -metadata comment_test=value -store_user_comments 1" "-c copy -frames:v 5" "" "-show_entries format_tags"
>>
>> FATE_MXF_OPATOM_USER_COMMENTS-$(call ENCDEC2, MPEG2VIDEO, PCM_S16LE, MXF) += fate-mxf-opatom-user-comments
>> fate-mxf-opatom-user-comments: $(SAMPLES)/mxf/Sony-00001.mxf
>> @@ -56,7 +55,8 @@ fate-mxf-opatom-user-comments: CMD = md5 -y -i $(TARGET_SAMPLES)/mxf/Sony-00001.
>> FATE_MXF-$(CONFIG_MXF_DEMUXER) += $(FATE_MXF)
>>
>> FATE_SAMPLES_AVCONV += $(FATE_MXF-yes) $(FATE_MXF_REEL_NAME-yes)
>> -FATE_SAMPLES_AVCONV += $(FATE_MXF_USER_COMMENTS-yes) $(FATE_MXF_D10_USER_COMMENTS-yes) $(FATE_MXF_OPATOM_USER_COMMENTS-yes)
>> +FATE_SAMPLES_AVCONV += $(FATE_MXF_USER_COMMENTS-yes) $(FATE_MXF_OPATOM_USER_COMMENTS-yes)
>> +FATE_SAMPLES_FFMPEG_FFPROBE += $(FATE_MXF_D10_USER_COMMENTS-yes)
>> FATE_SAMPLES_FFPROBE += $(FATE_MXF_PROBE-yes)
>>
>> fate-mxf: $(FATE_MXF-yes) $(FATE_MXF_PROBE-yes) $(FATE_MXF_REEL_NAME-yes) $(FATE_MXF_USER_COMMENTS-yes) $(FATE_MXF_D10_USER_COMMENTS-yes) $(FATE_MXF_OPATOM_USER_COMMENTS-yes)
>> diff --git a/tests/ref/fate/mxf-d10-user-comments b/tests/ref/fate/mxf-d10-user-comments
>> index de4f26c6f8..a2fae7ec01 100644
>> --- a/tests/ref/fate/mxf-d10-user-comments
>> +++ b/tests/ref/fate/mxf-d10-user-comments
>> @@ -1 +1,25 @@
>> -68f0fa62b6a676894afbbe4c34ebf70b
>> +eea0dd8376b059d41a3063064d3ac811 *tests/data/fate/mxf-d10-user-comments.mxf_d10
>> +3782189 tests/data/fate/mxf-d10-user-comments.mxf_d10
>> +#extradata 0: 34, 0x716b05c4
>> +#tb 0: 1/25
>> +#media_type 0: video
>> +#codec_id 0: mpeg2video
>> +#dimensions 0: 1280x720
>> +#sar 0: 3/4
>> +0, -1, 0, 1, 150000, 0x0547870d, S=1, 24, 0x5aa90ad0
>> +0, 0, 1, 1, 150000, 0xe80a1612, F=0x0
>> +0, 1, 2, 1, 150000, 0xc5c50e2f, F=0x0
>> +0, 2, 3, 1, 150000, 0x51e28a04, F=0x0
>> +0, 3, 4, 1, 150000, 0x9bbe6feb, F=0x0
>> +[FORMAT]
>> +TAG:operational_pattern_ul=060e2b34.04010101.0d010201.01010900
>> +TAG:uid=adab4424-2f25-4dc7-92ff-29bd000c0000
>> +TAG:generation_uid=adab4424-2f25-4dc7-92ff-29bd000c0001
>> +TAG:company_name=FFmpeg
>> +TAG:product_name=OP1a Muxer
>> +TAG:product_version=0.0.0
>> +TAG:product_uid=adab4424-2f25-4dc7-92ff-29bd000c0002
>> +TAG:material_package_umid=0x060A2B340101010501010D001300000000000000000000000000000000000000
>> +TAG:comment_test=value
>> +TAG:timecode=01:00:00:00
>> +[/FORMAT]
>
> The fate test fails on PPC64.
> I think the problem is that packet side data AV_PKT_DATA_CPB_PROPERTIES is
> just bitwise struct AVCPBProperties which is going to be different on BE/LE.
>
> fate-ts-demux has the same issue and also fails on BE.
>
> TEST mxf-d10-user-comments
> --- ./tests/ref/fate/mxf-d10-user-comments 2020-12-05 15:28:17.894070313 +0000
> +++ tests/data/fate/mxf-d10-user-comments 2020-12-05 16:50:14.160783464 +0000
> @@ -6,7 +6,7 @@
> #codec_id 0: mpeg2video
> #dimensions 0: 1280x720
> #sar 0: 3/4
> -0, -1, 0, 1, 150000, 0x0547870d, S=1, 24, 0x5aa90ad0
> +0, -1, 0, 1, 150000, 0x0547870d, S=1, 24, 0x59ff0ad0
> 0, 0, 1, 1, 150000, 0xe80a1612, F=0x0
> 0, 1, 2, 1, 150000, 0xc5c50e2f, F=0x0
> 0, 2, 3, 1, 150000, 0x51e28a04, F=0x0
> Test mxf-d10-user-comments failed. Look at tests/data/fate/mxf-d10-user-comments.err for details.
> make: *** [tests/Makefile:256: fate-mxf-d10-user-comments] Error 1
>
Thanks for noticing this. It seems that the workaround for
AV_PKT_DATA_PALETTE that is already implemented can be reused for
AV_PKT_DATA_REPLAYGAIN, AV_PKT_DATA_DISPLAYMATRIX, AV_PKT_DATA_STEREO3D,
AV_PKT_DATA_AUDIOSERVICETYPE, AV_PKT_DATA_FALLBACK_TRACK,
AV_PKT_DATA_MASTERING_DISPLAY_METADATA, AV_PKT_DATA_SPHERICAL,
AV_PKT_DATA_CONTENT_LIGHT_LEVEL and AV_PKT_DATA_S12M_TIMECODE.
It does not work for AV_PKT_DATA_PRFT (AVProducerReferenceTime has an
int64_t) and AV_PKT_DATA_CPB_PROPERTIES (which already has an uint64_t
in it and will have several more int64_t after the next mayor version
bump). I'll send a patch for all of them; it will make use of
sizeof(AVCPBProperties), but it has no note that it's size is not part
of the public ABI (and anyway, I am not blindly allocating such a thing
on the stack).
(There are other possible fixes for this very test: Test the decoded
frames and not the copied packets (no side data); or don't use a
transcode test (with its implicit framecrc) at all. But if it affects
more tests, then it should be fixed at the root.)
- Andreas
PS: All of the above statements of course presume 32bit integer/unsigned
as well as enums; furthermore it presumes that the compiler does not add
unnecessary padding beyond the requirements of alignment.
More information about the ffmpeg-devel
mailing list