[FFmpeg-devel] Fwd: Fate test for mxfdec.c: prefer metadata from Footer -
Am 2023-02-16 18:14, schrieb emco...@ffastrans.com: Am 2023-02-12 01:25, schrieb Tomas Härdin: fre 2023-02-03 klockan 14:54 +0100 skrev emco...@ffastrans.com: Am 2021-07-03 15:13, schrieb emco...@ffastrans.com: > Am 2021-06-28 21:58, schrieb emco...@ffastrans.com: > > Am 2021-06-28 03:00, schrieb Marton Balint: > > > On Sun, 27 Jun 2021, emco...@ffastrans.com wrote: > > > > > > > Am 2021-06-27 20:12, schrieb Marton Balint: > > > > > Why? I though it is enough if you store the partition > > > > > number in the > > > > > metadata set, that way you should be able to compare if the > > > > > existing > > > > > metadata set is better than the current one when adding a > > > > > new > > > > > metadata > > > > > set. Or am I missing something? > > > > > > > > OK, i just had a try on this but honestly i don't know how > > > > this > > > > could work without a very deep change of the whole mxfdec.c. > > > > The problem is that i cannot just add a field to the struct > > > > MXFMetadataSet as this points to raw data which has been read > > > > from > > > > the mxf file. I could add a field but if i initialize the > > > > value, i > > > > will automatically destroy the original raw data which was > > > > read from > > > > the mxf file. > > > > > > See the attached patch, that is what I had in mind. Or is it > > > overkill? > > > Can you test it with your dataset and see if it makes any > > > difference? > > > > > > Thanks, > > > Marton > > > ___ > > > ffmpeg-devel mailing list > > > ffmpeg-devel@ffmpeg.org > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > To unsubscribe, visit link above, or email > > > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". > > > > Tested your patch pleasure, thanks for the support! The "score > > approach" seems to work in practice exactly as my previous patch, > > the > > only thing i fear about is that it is a little harder now to > > foresee > > which metadata source is taken from a users perspective but i > > also > > think it is now more compliant than it was before. > > > > Using my test fileset which contains 4.476 mxf files (not all > > unique, > > maybe half is duplicates and most focus on xdcamhd and D10), we > > have > > 90 differences between ffprobe before your patch and after your > > patch. > > All of the differences are only in files that have openincomplete > > header. Most of the differences just changes the duration from a > > guessed one to the analyzed one: > > > > All STREAMS (NEW - OLD): > > "duration_ts": 3000, "duration_ts": 3099, > > "duration": "120.00", "duration": "123.96", > > FORMAT (NEW - OLD): > > "duration": "120.00", "duration": "123.969813", > > "bit_rate": "61178142", "bit_rate": "59219070", > > > > Exception one Op1b self contained file, where the "old" version > > did > > not spit out a "duration" value at all, so it was not even > > calculated > > from bitrate, it was just missing in the format section and set > > to 0 > > in the stream section. > > Exception two, there were 4 files (3 were samples from IRT and 1 > > a > > real world file from old omneon version) where the startc OLD was > > 0 > > and the new one was the MP starttimecode from MP, so perfect. > > So the conclusion is that of course your version had the same > > effect > > on my testfileset than my patch version, so thats nice. > > > > Also, the FATE samples i shared will still work and can be used > > for > > this patch. > > Attached a patch for adding only the fate samples. Note that > > these > > Fate tests of course fail when the patch for mxfdec.c is not > > applied. > > https://we.tl/t-MVmyG2mZHq > > > > Thanks a lot! > > -emcodem > > ___ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > To unsubscribe, visit link above, or email > > ffmpeg-
Re: [FFmpeg-devel] Fwd: Re: [PATCH] mxfdec.c: prefer metadata from Footer
Am 2021-07-04 20:31, schrieb emco...@ffastrans.com: Am 2021-07-04 20:28, schrieb emco...@ffastrans.com: Am 2021-07-04 17:28, schrieb Marton Balint: On Sat, 3 Jul 2021, Tomas Härdin wrote: lör 2021-07-03 klockan 15:13 +0200 skrev emco...@ffastrans.com: Unfortunately the wetransfer link for the fate samples expired, so i thought it might be a good idea to resend it as link to gdrive: https://drive.google.com/file/d/1yXTdS9RfOsduzg49vBLEshdmIzdaVQfd/view?usp=sharing Also attached the 2 patches: 1 from cus for mxfdec.c and one from myself for the corresponding fate samples. After applying the mxfdec.c patch, fate will pass with the currently existing tests but the files in the zip must be uploaded to the fate suite before applying my corresponding patch of course (otherwise the files don't exist). It would be cool if someone found the time and wants to apply this. The patch works (except for the samples not being in FATE) Actually I found one file where the packetization behaviour changes, because after the patch a fake index is generated based of the now recognized duration: samples/MXF/freemxf/freeMXF-mxf-dv-1.mxf but I guess the file is wrong because clip wrapped UL is used when the file seems frame wrapped instead? Nice finding! I can confirm this, it is actually clip wrapped looking at the mxf dump from bmx. These samples are pretty outdated anyway :D Unfortunately i don't have enough insight on the internals yet on the topic about adding/freeing/deleting metadata sets :-( FRAME Wrapped it is, not clip wrapped :rolleyes: ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". Anything left that needs investigation/correction on this one? ...it looks to be a very nice addition so far. Thanks -emcodem ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Fwd: Re: [PATCH] mxfdec.c: prefer metadata from Footer
Am 2021-07-04 20:28, schrieb emco...@ffastrans.com: Am 2021-07-04 17:28, schrieb Marton Balint: On Sat, 3 Jul 2021, Tomas Härdin wrote: lör 2021-07-03 klockan 15:13 +0200 skrev emco...@ffastrans.com: Unfortunately the wetransfer link for the fate samples expired, so i thought it might be a good idea to resend it as link to gdrive: https://drive.google.com/file/d/1yXTdS9RfOsduzg49vBLEshdmIzdaVQfd/view?usp=sharing Also attached the 2 patches: 1 from cus for mxfdec.c and one from myself for the corresponding fate samples. After applying the mxfdec.c patch, fate will pass with the currently existing tests but the files in the zip must be uploaded to the fate suite before applying my corresponding patch of course (otherwise the files don't exist). It would be cool if someone found the time and wants to apply this. The patch works (except for the samples not being in FATE) Actually I found one file where the packetization behaviour changes, because after the patch a fake index is generated based of the now recognized duration: samples/MXF/freemxf/freeMXF-mxf-dv-1.mxf but I guess the file is wrong because clip wrapped UL is used when the file seems frame wrapped instead? Nice finding! I can confirm this, it is actually clip wrapped looking at the mxf dump from bmx. These samples are pretty outdated anyway :D Unfortunately i don't have enough insight on the internals yet on the topic about adding/freeing/deleting metadata sets :-( FRAME Wrapped it is, not clip wrapped :rolleyes: ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Fwd: Re: [PATCH] mxfdec.c: prefer metadata from Footer
Am 2021-07-04 17:28, schrieb Marton Balint: On Sat, 3 Jul 2021, Tomas Härdin wrote: lör 2021-07-03 klockan 15:13 +0200 skrev emco...@ffastrans.com: Unfortunately the wetransfer link for the fate samples expired, so i thought it might be a good idea to resend it as link to gdrive: https://drive.google.com/file/d/1yXTdS9RfOsduzg49vBLEshdmIzdaVQfd/view?usp=sharing Also attached the 2 patches: 1 from cus for mxfdec.c and one from myself for the corresponding fate samples. After applying the mxfdec.c patch, fate will pass with the currently existing tests but the files in the zip must be uploaded to the fate suite before applying my corresponding patch of course (otherwise the files don't exist). It would be cool if someone found the time and wants to apply this. The patch works (except for the samples not being in FATE) Actually I found one file where the packetization behaviour changes, because after the patch a fake index is generated based of the now recognized duration: samples/MXF/freemxf/freeMXF-mxf-dv-1.mxf but I guess the file is wrong because clip wrapped UL is used when the file seems frame wrapped instead? Nice finding! I can confirm this, it is actually clip wrapped looking at the mxf dump from bmx. These samples are pretty outdated anyway :D Unfortunately i don't have enough insight on the internals yet on the topic about adding/freeing/deleting metadata sets :-( ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] Fwd: Re: [PATCH] mxfdec.c: prefer metadata from Footer
Am 2021-06-28 21:58, schrieb emco...@ffastrans.com: Am 2021-06-28 03:00, schrieb Marton Balint: On Sun, 27 Jun 2021, emco...@ffastrans.com wrote: Am 2021-06-27 20:12, schrieb Marton Balint: Why? I though it is enough if you store the partition number in the metadata set, that way you should be able to compare if the existing metadata set is better than the current one when adding a new metadata set. Or am I missing something? OK, i just had a try on this but honestly i don't know how this could work without a very deep change of the whole mxfdec.c. The problem is that i cannot just add a field to the struct MXFMetadataSet as this points to raw data which has been read from the mxf file. I could add a field but if i initialize the value, i will automatically destroy the original raw data which was read from the mxf file. See the attached patch, that is what I had in mind. Or is it overkill? Can you test it with your dataset and see if it makes any difference? Thanks, Marton ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". Tested your patch pleasure, thanks for the support! The "score approach" seems to work in practice exactly as my previous patch, the only thing i fear about is that it is a little harder now to foresee which metadata source is taken from a users perspective but i also think it is now more compliant than it was before. Using my test fileset which contains 4.476 mxf files (not all unique, maybe half is duplicates and most focus on xdcamhd and D10), we have 90 differences between ffprobe before your patch and after your patch. All of the differences are only in files that have openincomplete header. Most of the differences just changes the duration from a guessed one to the analyzed one: All STREAMS (NEW - OLD): "duration_ts": 3000, "duration_ts": 3099, "duration": "120.00", "duration": "123.96", FORMAT (NEW - OLD): "duration": "120.00", "duration": "123.969813", "bit_rate": "61178142", "bit_rate": "59219070", Exception one Op1b self contained file, where the "old" version did not spit out a "duration" value at all, so it was not even calculated from bitrate, it was just missing in the format section and set to 0 in the stream section. Exception two, there were 4 files (3 were samples from IRT and 1 a real world file from old omneon version) where the startc OLD was 0 and the new one was the MP starttimecode from MP, so perfect. So the conclusion is that of course your version had the same effect on my testfileset than my patch version, so thats nice. Also, the FATE samples i shared will still work and can be used for this patch. Attached a patch for adding only the fate samples. Note that these Fate tests of course fail when the patch for mxfdec.c is not applied. https://we.tl/t-MVmyG2mZHq Thanks a lot! -emcodem ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". Unfortunately the wetransfer link for the fate samples expired, so i thought it might be a good idea to resend it as link to gdrive: https://drive.google.com/file/d/1yXTdS9RfOsduzg49vBLEshdmIzdaVQfd/view?usp=sharing Also attached the 2 patches: 1 from cus for mxfdec.c and one from myself for the corresponding fate samples. After applying the mxfdec.c patch, fate will pass with the currently existing tests but the files in the zip must be uploaded to the fate suite before applying my corresponding patch of course (otherwise the files don't exist). It would be cool if someone found the time and wants to apply this. Thanks! -emcodemFrom 499a41f0db6cfda746fd8751d4e55e910f495687 Mon Sep 17 00:00:00 2001 From: emcodem Date: Mon, 28 Jun 2021 21:54:54 +0200 Subject: [PATCH 1/1] mxf Fate tests for openincomplete and truncated --- tests/fate/mxf.mak| 10 + tests/ref/fate/mxf-probe-xdcamhd-oit | 442 ++ tests/ref/fate/mxf-probe-xdcamhd-tcfooter | 442 ++ 3 files changed, 894 insertions(+) create mode 100644 tests/ref/fate/mxf-probe-xdcamhd-oit create mode 100644 tests/ref/fate/mxf-probe-xdcamhd-tcfooter diff --git a/tests/fate/mxf.mak b/tests/fate/mxf.mak index 3a1096176f..1b7be46c64 100644 --- a/tests/fate/mxf.mak +++ b/tests/fate/mxf.mak @@ -37,6 +37,16 @@ FATE_MXF_PROBE-$(call ENCDEC2, PRORES, PCM_S24LE, MXF) += fate-mxf-probe-applehd fate-mx
Re: [FFmpeg-devel] [PATCH] mxfdec.c: prefer metadata from Footer
Am 2021-06-28 03:00, schrieb Marton Balint: On Sun, 27 Jun 2021, emco...@ffastrans.com wrote: Am 2021-06-27 20:12, schrieb Marton Balint: Why? I though it is enough if you store the partition number in the metadata set, that way you should be able to compare if the existing metadata set is better than the current one when adding a new metadata set. Or am I missing something? OK, i just had a try on this but honestly i don't know how this could work without a very deep change of the whole mxfdec.c. The problem is that i cannot just add a field to the struct MXFMetadataSet as this points to raw data which has been read from the mxf file. I could add a field but if i initialize the value, i will automatically destroy the original raw data which was read from the mxf file. See the attached patch, that is what I had in mind. Or is it overkill? Can you test it with your dataset and see if it makes any difference? Thanks, Marton ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". Tested your patch pleasure, thanks for the support! The "score approach" seems to work in practice exactly as my previous patch, the only thing i fear about is that it is a little harder now to foresee which metadata source is taken from a users perspective but i also think it is now more compliant than it was before. Using my test fileset which contains 4.476 mxf files (not all unique, maybe half is duplicates and most focus on xdcamhd and D10), we have 90 differences between ffprobe before your patch and after your patch. All of the differences are only in files that have openincomplete header. Most of the differences just changes the duration from a guessed one to the analyzed one: All STREAMS (NEW - OLD): "duration_ts": 3000, "duration_ts": 3099, "duration": "120.00", "duration": "123.96", FORMAT (NEW - OLD): "duration": "120.00", "duration": "123.969813", "bit_rate": "61178142", "bit_rate": "59219070", Exception one Op1b self contained file, where the "old" version did not spit out a "duration" value at all, so it was not even calculated from bitrate, it was just missing in the format section and set to 0 in the stream section. Exception two, there were 4 files (3 were samples from IRT and 1 a real world file from old omneon version) where the startc OLD was 0 and the new one was the MP starttimecode from MP, so perfect. So the conclusion is that of course your version had the same effect on my testfileset than my patch version, so thats nice. Also, the FATE samples i shared will still work and can be used for this patch. Attached a patch for adding only the fate samples. Note that these Fate tests of course fail when the patch for mxfdec.c is not applied. https://we.tl/t-MVmyG2mZHq Thanks a lot! -emcodemFrom 499a41f0db6cfda746fd8751d4e55e910f495687 Mon Sep 17 00:00:00 2001 From: emcodem Date: Mon, 28 Jun 2021 21:54:54 +0200 Subject: [PATCH 1/1] mxf Fate tests for openincomplete and truncated --- tests/fate/mxf.mak| 10 + tests/ref/fate/mxf-probe-xdcamhd-oit | 442 ++ tests/ref/fate/mxf-probe-xdcamhd-tcfooter | 442 ++ 3 files changed, 894 insertions(+) create mode 100644 tests/ref/fate/mxf-probe-xdcamhd-oit create mode 100644 tests/ref/fate/mxf-probe-xdcamhd-tcfooter diff --git a/tests/fate/mxf.mak b/tests/fate/mxf.mak index 3a1096176f..1b7be46c64 100644 --- a/tests/fate/mxf.mak +++ b/tests/fate/mxf.mak @@ -37,6 +37,16 @@ FATE_MXF_PROBE-$(call ENCDEC2, PRORES, PCM_S24LE, MXF) += fate-mxf-probe-applehd fate-mxf-probe-applehdr10: SRC = $(TARGET_SAMPLES)/mxf/Meridian-Apple_ProResProxy-HDR10.mxf fate-mxf-probe-applehdr10: CMD = run $(PROBE_FORMAT_STREAMS_COMMAND) -i "$(SRC)" | sed -e "s/yuv422p10../yuv422p10/" +# openincomplete Header, truncated +FATE_MXF_PROBE-$(call ENCDEC2, MPEG2VIDEO, PCM_S16LE, MXF) += fate-mxf-probe-xdcamhd-oit +fate-mxf-probe-xdcamhd-oit: SRC = $(TARGET_SAMPLES)/mxf/omneon_6.4.1.0.1_xdcam_truncated.mxf +fate-mxf-probe-xdcamhd-oit: CMD = run $(PROBE_FORMAT_STREAMS_COMMAND) -i "$(SRC)" + +# openincomplete Header, starttc in header 0 but Footer MP 10:11:17:21, SP 10:11:17:17 +FATE_MXF_PROBE-$(call ENCDEC2, MPEG2VIDEO, PCM_S24LE, MXF) += fate-mxf-probe-xdcamhd-tcfooter +fate-mxf-probe-xdcamhd-tcfooter: SRC = $(TARGET_SAMPLES)/mxf/omneon_8.3.0.0_xdcam_startc_footer.mxf +fate-mxf-probe-xdcamhd-tcfooter: CMD = run $(PROBE_FORMAT_STREAMS_COMMAND) -i "$(SRC)" + FATE_MXF_REEL_NAME-$(call ENCDEC2, M
Re: [FFmpeg-devel] [PATCH] mxfdec.c: prefer metadata from Footer
Am 2021-06-27 20:12, schrieb Marton Balint: Why? I though it is enough if you store the partition number in the metadata set, that way you should be able to compare if the existing metadata set is better than the current one when adding a new metadata set. Or am I missing something? OK, i just had a try on this but honestly i don't know how this could work without a very deep change of the whole mxfdec.c. The problem is that i cannot just add a field to the struct MXFMetadataSet as this points to raw data which has been read from the mxf file. I could add a field but if i initialize the value, i will automatically destroy the original raw data which was read from the mxf file. In case i did not miss anything, i guess the 1-line change i sent is kind of best effort: it should not destroy any existing funtionality but in certain cases, it delivers more accurate metadata. Lemme know your thoughts! ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] mxfdec.c: prefer metadata from Footer
Am 2021-06-27 20:12, schrieb Marton Balint: On Sun, 27 Jun 2021, Tomas Härdin wrote: Order of parsing depends on the file, we usually do header, footer and then backwards the partitions. So after your patch metadata in the second partition will have priority over footer, which is also against spec I believe. I was going to say this is not right because mxf_read_partition_pack() will reverse the order in which partitions are added when parsing backwards, so everything ends up in the correct order. But this doesn't apply to mxf->metadata_sets, only mxf->partitions.. We've had a similar discussion on here roughly a month ago. There's a bit of finessing around the order in which to prefer any one metadata set. IIRC something like: FooterPartition, then any Complete non-footer Partition, then the latest OpenPartition /Tomas My original patch a month ago was more like this "decision" approach: https://patchwork.ffmpeg.org/project/ffmpeg/patch/20210524103027.30367-1-emco...@ffastrans.com/ I basically only wanted to correct a case where the starttc was 0 in header but the correct value in footer. Tomas in the chat then came up with the idea that it could work for all metadata, so i changed my stuff and tested the outcome on multiple thousand files containing about 150 different flavours. It turned out that in all cases where we had a new metadata value, it was always more accurate than before. "Comaring and Deciding" cannot be done in a very general way, there would be some special cases, like the Starttc: SMPTE 377 says whenever some value is unknown for the encoder, it shall put the "distinguished value" as a placeholder. But for the starttimecode (and many other values), it does not define such a distinguished value. Which makes sense because it feels kind of impossible that the encoder does not know the starttimecode at the start of recording. So one might argue that what i am doing is supporting the files of a broke encoder version. I just fear that this "Compare and decide" approach would end up with lots of special cases. SMPTE 377m 2009, 7.5: MXF decoders should always use Header Metadata from a Closed Partition. When processing files that contain updated Header Metadata repetitions and when a Closed Partition containing Header Metadata is not available, MXF decoders should use the repetition of a Header Metadata Set with the Generation UID value that equals the This Generation UID Property of the Identification Set at the highest index in the Identifications Property of the Preface Set. All other versions of the same Header Metadata Set should be considered outdated. Most important: i still think this version of the patch is good in general. Taking the Values from a possible Header Metadata Repetition in a Body IMHO does not directly "violate the rules" and in worst case it would spit out the same value that it does now (without this patch). The standard seems to say "should" instead of "must". So now the question for me is if Tomas maybe now prefers my initial patch, if that's so, i'd send a separate Patch for the corresponding Fate tests. Of course i'd still need someone to put my 2 testfiles to the fate-suite/mxf. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] mxfdec.c: prefer metadata from Footer
In case there is a Footer, regarding to SMPTE 377 all versions, the metadata in
Footer shall be correct (where in Header it can be incomplete)..
If there is no footer (stream, truncated...) it will still work as usual.
Tested with a huge set of files and compared old/new ffprobes, it will not
change lots of metadata, mainly duration and in some cases start timecode.
Without this change, especially Duration would be often inaccurate because it
is unknown in header and calculated from bitrate.
The new sample files should be added to \fate-suite\mxf, as i do not have an
account to upload the files, i shared them on wetransfer:
https://we.tl/t-MVmyG2mZHq
omneon_6.4.1.0.1_xdcam_truncated.mxf
An original Omneon File from an older Version, file is truncated. It shall
prove that Metadata is being parsed even when there is no Footer.
omneon_8.3.0.0_xdcam_startc_footer.mxf
An original Omneon File from a recent Version with "better" Metadata in Footer
than in Header. I needed to hexedit this file and set the MP and SP start
timecode in header to 0.
This test is for proving that metadata from Footer is preferred.
---
libavformat/mxfdec.c | 2 +-
tests/fate/mxf.mak| 10 +
tests/ref/fate/mxf-probe-xdcamhd-oit | 442 ++
tests/ref/fate/mxf-probe-xdcamhd-tcfooter | 442 ++
4 files changed, 895 insertions(+), 1 deletion(-)
create mode 100644 tests/ref/fate/mxf-probe-xdcamhd-oit
create mode 100644 tests/ref/fate/mxf-probe-xdcamhd-tcfooter
diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index 7b40076fb4..a7f552c753 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -1402,7 +1402,7 @@ static void *mxf_resolve_strong_ref(MXFContext *mxf, UID
*strong_ref, enum MXFMe
if (!strong_ref)
return NULL;
-for (i = 0; i < mxf->metadata_sets_count; i++) {
+for (i = mxf->metadata_sets_count-1; i >= 0; i--) {
if (!memcmp(*strong_ref, mxf->metadata_sets[i]->uid, 16) &&
(type == AnyType || mxf->metadata_sets[i]->type == type)) {
return mxf->metadata_sets[i];
diff --git a/tests/fate/mxf.mak b/tests/fate/mxf.mak
index 3a1096176f..1b7be46c64 100644
--- a/tests/fate/mxf.mak
+++ b/tests/fate/mxf.mak
@@ -37,6 +37,16 @@ FATE_MXF_PROBE-$(call ENCDEC2, PRORES, PCM_S24LE, MXF) +=
fate-mxf-probe-applehd
fate-mxf-probe-applehdr10: SRC =
$(TARGET_SAMPLES)/mxf/Meridian-Apple_ProResProxy-HDR10.mxf
fate-mxf-probe-applehdr10: CMD = run $(PROBE_FORMAT_STREAMS_COMMAND) -i
"$(SRC)" | sed -e "s/yuv422p10../yuv422p10/"
+# openincomplete Header, truncated
+FATE_MXF_PROBE-$(call ENCDEC2, MPEG2VIDEO, PCM_S16LE, MXF) +=
fate-mxf-probe-xdcamhd-oit
+fate-mxf-probe-xdcamhd-oit: SRC =
$(TARGET_SAMPLES)/mxf/omneon_6.4.1.0.1_xdcam_truncated.mxf
+fate-mxf-probe-xdcamhd-oit: CMD = run $(PROBE_FORMAT_STREAMS_COMMAND) -i
"$(SRC)"
+
+# openincomplete Header, starttc in header 0 but Footer MP 10:11:17:21, SP
10:11:17:17
+FATE_MXF_PROBE-$(call ENCDEC2, MPEG2VIDEO, PCM_S24LE, MXF) +=
fate-mxf-probe-xdcamhd-tcfooter
+fate-mxf-probe-xdcamhd-tcfooter: SRC =
$(TARGET_SAMPLES)/mxf/omneon_8.3.0.0_xdcam_startc_footer.mxf
+fate-mxf-probe-xdcamhd-tcfooter: CMD = run $(PROBE_FORMAT_STREAMS_COMMAND) -i
"$(SRC)"
+
FATE_MXF_REEL_NAME-$(call ENCDEC2, MPEG2VIDEO, PCM_S16LE, MXF) +=
fate-mxf-reel_name
fate-mxf-reel_name: $(SAMPLES)/mxf/Sony-1.mxf
fate-mxf-reel_name: CMD = md5 -y -i $(TARGET_SAMPLES)/mxf/Sony-1.mxf -c
copy -timecode 00:00:00:00 -metadata "reel_name=test_reel" -fflags +bitexact -f
mxf
diff --git a/tests/ref/fate/mxf-probe-xdcamhd-oit
b/tests/ref/fate/mxf-probe-xdcamhd-oit
new file mode 100644
index 00..040a4e0fba
--- /dev/null
+++ b/tests/ref/fate/mxf-probe-xdcamhd-oit
@@ -0,0 +1,442 @@
+[STREAM]
+index=0
+codec_name=mpeg2video
+profile=0
+codec_type=video
+codec_tag_string=[0][0][0][0]
+codec_tag=0x
+width=1920
+height=1080
+coded_width=0
+coded_height=0
+closed_captions=0
+has_b_frames=1
+sample_aspect_ratio=1:1
+display_aspect_ratio=16:9
+pix_fmt=yuv422p
+level=2
+color_range=tv
+color_space=unknown
+color_transfer=bt709
+color_primaries=unknown
+chroma_location=topleft
+field_order=tt
+refs=1
+id=N/A
+r_frame_rate=25/1
+avg_frame_rate=25/1
+time_base=1/25
+start_pts=0
+start_time=0.00
+duration_ts=6
+duration=0.24
+bit_rate=5000
+max_bit_rate=N/A
+bits_per_raw_sample=N/A
+nb_frames=N/A
+nb_read_frames=N/A
+nb_read_packets=N/A
+DISPOSITION:default=0
+DISPOSITION:dub=0
+DISPOSITION:original=0
+DISPOSITION:comment=0
+DISPOSITION:lyrics=0
+DISPOSITION:karaoke=0
+DISPOSITION:forced=0
+DISPOSITION:hearing_impaired=0
+DISPOSITION:visual_impaired=0
+DISPOSITION:clean_effects=0
+DISPOSITION:attached_pic=0
+DISPOSITION:timed_thumbnails=0
+DISPOSITION:captions=0
+DISPOSITION:descriptions=0
+DISPOSITION:metadata=0
+DISPOSITION:dependent=0
+DISPOSITION:still_image=0
+TAG:file_package_umid=0x060A2B340101010501010D2313003E792039C0579AD9E111BAFB00D028113
Re: [FFmpeg-devel] [PATCH] avisynth.c corrected interlace detection
Am 2021-05-27 22:38, Stephen Hutchinson wrote: The change seems okay, but the comment and commit message need to explain what's going on better, because the confusion that's erupted over this seems to derive from the rather poor way the AviSynth documentation describes the difference between field-based and frame-based clips, and how to access this information through the API. After having read through all of this a bit more, the situation appears to be as follows: AviSynth works on Frame-based video. Full stop. 'Frame-based', despite the name, is not a synonym for 'progressive', merely that it's being processed as a single frame rather than as a half-height field. This is the single point from which all the rest of this got confused. Using SeparateFields(), you can break the frame into its constituent half-height fields so that certain filters which don't like processing interlaced footage can filter the fields as a sort of 'fake progressive', before using Weave() to combine the fields again into a singular full height frame. That's its intention, rather than to signal to a client program that something is interlaced. The AssumeFieldBased() function and avs_is_field_based API check are there to allow other functions to understand this situation where the fields have been broken apart (or to override the field-based setting in cases that it didn't get set properly). The confusion seems to have arisen from mistaking these as ways that AviSynth indicates something is interlaced or not, and it isn't. For that purpose, you have to look at frame properties, something that was introduced in AviSynth+ as a flag that a source plugin can set to indicate whether the frame is interlaced (either tff or bff) or progressive. Adding frame property detection to the demuxer here would require larger changes that need to be guarded from 2.6, but it would accomplish the goal I *think* this is ultimately intended to address. The in-code comment should probably be something closer to: /* AviSynth works on frame-based video by default, which can /* be either progressive or interlaced. Some filters can break /* frames into half-height fields, at which point it considers /* the clip to be field-based (avs_is_field_based can be used /* to check for this situation). /* To properly detect the field order of a typical video clip, /* the frame needs to have been weaved back together already, /* so avs_is_field_based should actually report 'false' when /* checked. */ Yeah i think a huge part of all the confusion is that all existing descriptions are written from an "inside avisynth" perspective, while from feeling most readers see it from the "recieving applications perspective". From this perspective, it is kind of guaranteed that field_based clips have to be threated as progressive because is not really a chance that the delivered frames are interlaced - except a filter did not set the value correctly. Checking the Frame property as you suggest could make it for sure better but honestly i don't see a need for it because env->field- and frame based should be set correctly by the filters. It would be a shame if the demuxer would have to actually decode a frame to get this propety correctly - also it could potentially take many minutes for the first frame to be decoded when the script does lots of smart stuff and the resolution and bit depth is very high. Last but not least, you could potentially decode frame 0 while the script doesn't even need frame 0 because it starts at another one. However, leaving all this aside, i also hope my stuff makes the situation better than it was before and of course i am ok with your comment version, have nothing to add. You want me to change it and re-send the patch (i always struggle in how exactly to do that, you want me to work from my current fork where the commit i sent is included , alter it and commit again - or you want me to start from scratch and send a totally new patch?) Thanks a lot Steve -emcodem ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] mxfdec.c: Try TC from Footer if Header TC was < 1
Am 2021-05-24 17:31, schrieb Tomas Härdin:
mån 2021-05-24 klockan 12:30 +0200 skrev emcodem:
Added support for reading Start Timecode from Footer (if any).
Specifically targets Omneon 6.4.3.0 but also works on other Versions
and Vendors, e.g. when Header is OpenIncomplete.
Function mxf_resolve_strong_ref_reverse can potentially be re-used for
getting other values like Duration and
Origin from Footer.
This needs a sample and a test
Sure, Will add.
---
libavformat/mxfdec.c | 20
1 file changed, 20 insertions(+)
diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index 3bf480a3a6..557e01f8ed 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -1396,6 +1396,19 @@ static const MXFCodecUL *mxf_get_codec_ul(const
MXFCodecUL *uls, UID *uid)
return uls;
}
+static void *mxf_resolve_strong_ref_reverse(MXFContext *mxf, UID
*strong_ref, enum MXFMetadataSetType type)
This and mxf_resolve_strong_ref() could maybe be merged to one function
with an "int dir" option, and mxf_resolve_strong_ref() just calling it
with the value 1.
Will delete the revers function and alter the mxf_resolve_strong_ref as
suggested.
+{
+int i;
+if (!strong_ref)
+return NULL;
+for (i = mxf->metadata_sets_count-1; i >= 0 ; i--) {
+if (!memcmp(*strong_ref, mxf->metadata_sets[i]->uid, 16) &&
+(type == AnyType || mxf->metadata_sets[i]->type == type))
{
+return mxf->metadata_sets[i];
+}
+}
+return NULL;
+}
Missing newline, but I can add that locally
static void *mxf_resolve_strong_ref(MXFContext *mxf, UID *strong_ref,
enum MXFMetadataSetType type)
{
int i;
@@ -2328,8 +2341,15 @@ static int
mxf_parse_structural_metadata(MXFContext *mxf)
continue;
mxf_tc = (MXFTimecodeComponent*)component;
+if (mxf_tc->start_frame <= 0) {
I feel like this should trigger on OpenIncomplete instead. I wouldn't
be surprised if start_frame < 0 is perfectly valid.
OK thats an interesting discussion, from my perspective there is a
little gray area here and what i did should deliver the best effort. My
thoughts are:
*) openincomplete may or may not carry the correct value, it is not
guaranteed to carry the wrong value
*) there may not be a footer (file truncated, file growing)
*) if a footer is there, it may or may not repeat the header metadata
*) all 377m versions say "if values are unknonwn (open/incomplete), one
must use the "distinguished value", but none of the 377m up to 2011
define such a value for the start timecode - BUT my guess was that this
distinguished value shall be a valid TC, instead of e.g. "-1" (...)
because the value -1 would potentially fail in the TC parsing code
(thinking globally here, not only for libav*)
*) if the Start TC is actually 0 AND there is a Footer Metadata
repetition, my code still works because it will take the value 0 from
the footer
So after all i believe checking for "openincomplete" would be kind of
superfluous for this usecase. Checking if there is another value than 0
in the footer does not really hurt and as the footer is guaranteed to
carry the correct timecode, it should never return a really wrong value.
The really correct solution i believe would be to parse ALL metadata
from footer (if any) first but i don't want to change the mxfdec.c
fundamentally just for this sidecase.
+if (mxf_tc->start_frame <= 0) {
+av_log(mxf->fc, AV_LOG_TRACE, "Header Start
Timecode was %d, trying reversed parsing\n",mxf_tc->start_frame);
+component = mxf_resolve_strong_ref_reverse(mxf,
&material_track->sequence->structural_components_refs[j],
TimecodeComponent);
+mxf_tc = (MXFTimecodeComponent*)component;
Wrong indent. I was also going to comment that mxf_tc might end up NULL
here but it can't since it's non-NULL when resolving in the forward
direction.
Yeah, in worst case the reverse parsing should return exactly the same
value as the forward parsing did, so i guess we are safe here. My
intention was to be backward compatible as far as possible.
/Tomas
Hi Tomas, thanks for the quick reply.
OK i'll try to get hold of a short example file, upload to the fate
suite and add a test. If i cannot get a very short sample (XDCAM, ~1-2
GOP's), i'll make some up by altering values using hex editor if thats
ok for you.
Other comments inline, please let me know your thougts - AND as i am
pretty new to sending patches here, please also let me know if you like
me to send ONE patch with the new code for mxfdec.c AND the test or you
want me to send it separately?
Thanks,
-emcodem
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] avisynth.c corrected interlace detection
Sorry for the delay on this, should have corrected it much earlier.
There was some confusion in the interlaced analysis. From 3rdparty decoders
perspective, a clip
can only be interlaced when it is NOT field_based. This is because in a
field_based clip, the fields
are separate images, so it is actually progressive. In that case, avisynth
still shows is_tff or bff because those
values are needed in case the script decides to weave the separated fields
later on.
---
libavformat/avisynth.c | 11 ---
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/libavformat/avisynth.c b/libavformat/avisynth.c
index 350ac6d11d..ff6e6e1bfa 100644
--- a/libavformat/avisynth.c
+++ b/libavformat/avisynth.c
@@ -250,15 +250,12 @@ static int avisynth_create_stream_video(AVFormatContext
*s, AVStream *st)
st->nb_frames = avs->vi->num_frames;
avpriv_set_pts_info(st, 32, avs->vi->fps_denominator,
avs->vi->fps_numerator);
-av_log(s, AV_LOG_TRACE, "avs_is_field_based: %d\n",
avs_is_field_based(avs->vi));
-av_log(s, AV_LOG_TRACE, "avs_is_parity_known: %d\n",
avs_is_parity_known(avs->vi));
-/* The following typically only works when assumetff (-bff) and
- * assumefieldbased is used in-script. Additional
- * logic using GetParity() could deliver more accurate results
- * but also decodes a frame which we want to avoid. */
st->codecpar->field_order = AV_FIELD_UNKNOWN;
-if (avs_is_field_based(avs->vi)) {
+/* separatefields makes field_based==true, weave makes field_based==false
+ * only non field_based clips can therefore be interlaced */
+av_log(s, AV_LOG_TRACE, "avs_is_field_based: %d\n",
avs_is_field_based(avs->vi));
+if (avs_is_field_based(avs->vi) == 0) {
if (avs_is_tff(avs->vi)) {
st->codecpar->field_order = AV_FIELD_TT;
}
--
2.25.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] mxfdec.c: Try TC from Footer if Header TC was < 1
Added support for reading Start Timecode from Footer (if any).
Specifically targets Omneon 6.4.3.0 but also works on other Versions and
Vendors, e.g. when Header is OpenIncomplete.
Function mxf_resolve_strong_ref_reverse can potentially be re-used for getting
other values like Duration and
Origin from Footer.
---
libavformat/mxfdec.c | 20
1 file changed, 20 insertions(+)
diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index 3bf480a3a6..557e01f8ed 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -1396,6 +1396,19 @@ static const MXFCodecUL *mxf_get_codec_ul(const
MXFCodecUL *uls, UID *uid)
return uls;
}
+static void *mxf_resolve_strong_ref_reverse(MXFContext *mxf, UID *strong_ref,
enum MXFMetadataSetType type)
+{
+int i;
+if (!strong_ref)
+return NULL;
+for (i = mxf->metadata_sets_count-1; i >= 0 ; i--) {
+if (!memcmp(*strong_ref, mxf->metadata_sets[i]->uid, 16) &&
+(type == AnyType || mxf->metadata_sets[i]->type == type)) {
+return mxf->metadata_sets[i];
+}
+}
+return NULL;
+}
static void *mxf_resolve_strong_ref(MXFContext *mxf, UID *strong_ref, enum
MXFMetadataSetType type)
{
int i;
@@ -2328,8 +2341,15 @@ static int mxf_parse_structural_metadata(MXFContext *mxf)
continue;
mxf_tc = (MXFTimecodeComponent*)component;
+if (mxf_tc->start_frame <= 0) {
+av_log(mxf->fc, AV_LOG_TRACE, "Header Start Timecode was
%d, trying reversed parsing\n",mxf_tc->start_frame);
+component = mxf_resolve_strong_ref_reverse(mxf,
&material_track->sequence->structural_components_refs[j], TimecodeComponent);
+mxf_tc = (MXFTimecodeComponent*)component;
+}
+
flags = mxf_tc->drop_frame == 1 ? AV_TIMECODE_FLAG_DROPFRAME : 0;
if (av_timecode_init(&tc, mxf_tc->rate, flags,
mxf_tc->start_frame, mxf->fc) == 0) {
+av_log(mxf->fc, AV_LOG_TRACE, "Setting Start Timecode: %d
\n",mxf_tc->start_frame);
mxf_add_timecode_metadata(&mxf->fc->metadata, "timecode", &tc);
break;
}
--
2.25.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] mov.c log qt ref extenal essence metadata
Am 2021-03-06 10:48, schrieb Jan Ekström:
On Sat, Mar 6, 2021 at 10:38 AM emcodem wrote:
---
libavformat/mov.c | 8
1 file changed, 8 insertions(+)
In quicktime reference files, exposing the parsed info for external
essences location can be very handy for users
Unfortunately, as per the discussion we had yesterday on #ffmpeg, Data
References are not as simple as mov.c might make it feel like.
Thanks again for the chat yesterday! I thought i better open the topic
here so i can do the work async :D
So, if we think that a single MOV/MP4 Track is:
1. A set of decode'able packets of a certain media type (as far as I
can tell that has been the limitation, while other things can change
during a Track the media type is one that doesn't).
2. Which is then presented according to a virtual time line (edit
lists, which we will in this case ignore since they get applied on top
of the decoded result on the presentation layer, and data references
are on the packet set level).
Thus if we go through the layers:
1. We have samples (packets in FFmpeg parliance more or less, stsz
defines the sizes of them and so forth).
2. Samples get put into chunks, which are basically tuple of
(sample_description_index, data offset) - see stsc, stco, co64.
3. Sample Description can thought of as a tuple of (AVCodec, the
extradata (if any) required, data reference index), there is a list of
them in the Track's stsd box.
4. Then finally we get to the data reference list in the dref box of
the track.
Currently as far as I can tell from reading mov_read_stsd /
ff_mov_read_stsd_entries, it does generate extradata buffer for each
sample description, but effectively only keeps a single data reference
around in the MOVStreamContext, skipping the whole chunk matching etc
part of things :) (if I am reading the code correctly, which I might
not be).
Hmmm not sure why you refer to extradata, how is this connected to the
dref besides both are stored on streamcontext level? (but yes, i also
understand that it would be overwritten for the current pseudotrack in
case it is called multiple times, not sure if that can happen tough)
So yea, there's two questions:
1. Should this be exposed?
Well it is vital information. mov.c unfortunately misses the
functionality that the original quicktime engine has: try to resolve the
referenced path on multiple different locations (e.g. try every
connected root device/driveletter), so it occcasionally fails to process
qt ref files.
Now i am not that experienced that i can add this missing part cross-OS
in C but exposing it is cheap and simple. When it's exposed, API users
or scripters have a much easier locating the media and set cwd
accordingly or even work with the referenced media directly.
2. If it should be exposed, how? A set of metadata this should not be,
as this at the very least would end up being a weird set/list of byte
offsets/sizes and references :)
So yea, sorry for things not actually being as simple as they look by
the code in mov.c.
How could it end up as a weird set of byte offests/references? I mean i
totally see your point that dref is more like on the same level as media
type, so kind of top-level but i miss any example how to present an
array of objects on that level.
What my code definitely misses is to add the dref_id, so i imagine the
"key", e.g. dref_path should be better presented as
sprintf("dref_path_%d",sc->dref_id).
What you think?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] mov.c log qt ref extenal essence metadata
---
libavformat/mov.c | 8
1 file changed, 8 insertions(+)
In quicktime reference files, exposing the parsed info for external essences
location can be very handy for users
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 1c07cff6b5..e9625c0cf9 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -4275,6 +4275,14 @@ static int mov_read_trak(MOVContext *c, AVIOContext *pb,
MOVAtom atom)
if (sc->dref_id-1 < sc->drefs_count && sc->drefs[sc->dref_id-1].path) {
MOVDref *dref = &sc->drefs[sc->dref_id - 1];
+
+av_dict_set(&st->metadata, "dref_path", dref->path, 0);
+av_dict_set(&st->metadata, "dref_dir", dref->dir, 0);
+av_dict_set(&st->metadata, "dref_filename", dref->filename, 0);
+av_dict_set(&st->metadata, "dref_volume", dref->volume, 0);
+av_dict_set_int(&st->metadata, "dref_nlvl_from", dref->nlvl_from, 0);
+av_dict_set_int(&st->metadata, "nlvl_to", dref->nlvl_to, 0);
+
if (c->enable_drefs) {
if (mov_open_dref(c, &sc->pb, c->fc->url, dref) < 0)
av_log(c->fc, AV_LOG_ERROR,
--
2.25.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] Populate field order returned by avs script, Trac Ticket 8757
Am 2021-02-01 09:44, schrieb emco...@ffastrans.com:
Am 2021-01-21 19:08, schrieb emco...@ffastrans.com:
On 2021-01-21 14:10, Stephen Hutchinson wrote:
Yeah, never mind about that. I didn't notice that those are declared
AVSC_INLINE, not AVSC_API, so they don't get used through the dynamic
API loader.
The comment formatting seems to have been messed up in the second
version, though.
/* The following typically only works when assumetff (-bff) and
* assumefieldbased is used in-script. Additional
* logic using GetParity() could deliver more accurate results
* but also decodes a frame which we want to avoid. */
OK, i have to admit formatting comments is in the top 10 of my
greatest weaknesses :D
Thanks for your patience and also thanks for telling me directly how
the formatting is done correctly.
New patch with formatted comment attached
Is it OK to ping this?
Sorry for Pinging again :-(
I know it's not much that the patch does but the resulting functionality
from a user perspective can be really useful, especially when using
programatically calculated avs scripts.From 243bb2cbae9fabc78fe7f48cb90ae1c620c51a51 Mon Sep 17 00:00:00 2001
From: emcodem
Date: Thu, 21 Jan 2021 18:59:45 +0100
Subject: [PATCH] Populate field order returned by avs script, Trac Ticket 8757
---
libavformat/avisynth.c | 17 +
1 file changed, 17 insertions(+)
diff --git a/libavformat/avisynth.c b/libavformat/avisynth.c
index 2c08ace8db..22ae8c0dd3 100644
--- a/libavformat/avisynth.c
+++ b/libavformat/avisynth.c
@@ -241,6 +241,23 @@ static int avisynth_create_stream_video(AVFormatContext *s, AVStream *st)
st->nb_frames = avs->vi->num_frames;
avpriv_set_pts_info(st, 32, avs->vi->fps_denominator, avs->vi->fps_numerator);
+av_log(s, AV_LOG_TRACE, "avs_is_field_based: %d\n", avs_is_field_based(avs->vi));
+av_log(s, AV_LOG_TRACE, "avs_is_parity_known: %d\n", avs_is_parity_known(avs->vi));
+
+/* The following typically only works when assumetff (-bff) and
+ * assumefieldbased is used in-script. Additional
+ * logic using GetParity() could deliver more accurate results
+ * but also decodes a frame which we want to avoid. */
+st->codecpar->field_order = AV_FIELD_UNKNOWN;
+if (avs_is_field_based(avs->vi)) {
+if (avs_is_tff(avs->vi)) {
+st->codecpar->field_order = AV_FIELD_TT;
+}
+else if (avs_is_bff(avs->vi)) {
+st->codecpar->field_order = AV_FIELD_BB;
+}
+}
+
switch (avs->vi->pixel_type) {
/* 10~16-bit YUV pix_fmts (AviSynth+) */
case AVS_CS_YUV444P10:
--
2.25.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] Populate field order returned by avs script, Trac Ticket 8757
Am 2021-01-21 19:08, schrieb emco...@ffastrans.com: On 2021-01-21 14:10, Stephen Hutchinson wrote: Yeah, never mind about that. I didn't notice that those are declared AVSC_INLINE, not AVSC_API, so they don't get used through the dynamic API loader. The comment formatting seems to have been messed up in the second version, though. /* The following typically only works when assumetff (-bff) and * assumefieldbased is used in-script. Additional * logic using GetParity() could deliver more accurate results * but also decodes a frame which we want to avoid. */ OK, i have to admit formatting comments is in the top 10 of my greatest weaknesses :D Thanks for your patience and also thanks for telling me directly how the formatting is done correctly. New patch with formatted comment attached Is it OK to ping this? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v4 3/3] avformat/mxfenc: prefer to use the configured metadta
Am 2021-01-28 02:21, schrieb lance.lmw...@gmail.com: I haven't found the s337m freely, so I'm not sure about the Platforma metadata. I think we haven't support the field yet, I guess it's 0x3C08 tag, but I haven't the document in hand so it's not OK to add it by me. Yeah the SMPTE are unfortunately not free :-( Maybe this helps: a citate from 377-1-2009c which i believe is the only versoin of SMPTE 377 we should refer to unless we change the "Minor Version" property of the partition pack to another value than "3" (which it currently is): Item Name: Platform Type: UTF-16 string Len: var Local Tag: 3C.08 Item UL: 06.0E.2B.3401.01.01.0205.20.07.0106.01.00.00 Req?: Opt Meaning: Human readable name of the toolkit and operating system used. Best practice is to use the form “SDK name (OS name)” [RP 210 Specifies the platform on which the application was run] ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v4 3/3] avformat/mxfenc: prefer to use the configured metadta
Am 2021-01-26 00:34, schrieb Marton Balint: Uhm guys, it is very bad practice: if you just insert a different manufacturer name then you just cheat. SMPTE 377 has a clear statement about what goes to the identification, you cannot just write the infos from a different program there. Libavformat is a library/SDK, not an application. For most identification fields SMPTE 377 talks about the application which created the file, which can be anything using libavformat libraries. Feel free to quote if you have a more exact specification. What you can do instead is to push both identifications, the old one and the one from the current program into the identification array, this way the processing chain can be reconstructed. Unforutnately i have never seen anyone doing this besides Opencube. Also, note that broadcasters currently are using the identification string, looking for "ffmpeg" in order to sort out non compatible XDCAMHD mxf: ffmpeg does not write some mandatory metadata fields as mentioned here: https://trac.ffmpeg.org/ticket/5097 this leads to sony devices not accepting the ffmpeg mxf container - which again leads to ffmpeg mxf wrapper for XDCAMHD not being accepted by our public broadcaster. Maybe they should post patches instead of having workarounds? And I explained, libavformat MXF muxer will still be identifiable, but the proper field will be used for identifying the SDK used, not Company/Product but Toolkit/Platform. Sorry, there seems to be some confusion in this thread, the first submission seemed to target the "automatic copy" of the existing metadata but the updated version removes this automatism. Please ignore my comment as it is totally ok to provide input options for company_name, product_name, product_version but not automatically reflect the ones from the input. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v4 3/3] avformat/mxfenc: prefer to use the configured metadta
Am 2021-01-20 16:41, schrieb Tomas Härdin: ons 2021-01-20 klockan 00:27 +0100 skrev Marton Balint: On Tue, 19 Jan 2021, Tobias Rapp wrote: > On 18.01.2021 23:53, Tomas Härdin wrote: > > lör 2021-01-16 klockan 08:43 +0800 skrev lance.lmw...@gmail.com: > > > On Fri, Jan 15, 2021 at 09:43:58PM +0100, Marton Balint wrote: > > > > On Fri, 15 Jan 2021, Tomas Härdin wrote: > > > > > Again, why? If you have a company that maintains a fork of FFmpeg then > > > > > compile that info in here instead. Compare with FFmbc which always puts > > > > > "FFmbc" as CompanyName. > > > > > > > > And how can a product based on libavformat set the company name, product > > > > name and product version? It seems a valid use case for me that these are > > > > overridable. Also note that this product version is only the "user > friendly" > > > > version string, for the numeric version still LIBAVFORMAT_VERSION values > are > > > > used. > > > > > > Yes, my use case is the product is using libavformat as library, so it's > > > prefer to have way to override these information as requirements. > > > > What I'm worried about here is that we're going to get files which > > claim to have been written by something other than libavformat. I've > > had situations like this before, and it is a source of headache. For > > example, if mxfenc writes some field incorrectly then this might cause > > us to hack mxfdec to accept that field instead of fixing mxfenc. > > I agree that especially for the MXF format with its flexible structure > it is more relevant to know the muxing library rather than the hosting > application. Have seen MXF output files of other commercial products > that also contain library identifiers like "libMXF" or "MXFtk" here. > > Other formats in FFmpeg use the "encoder" metadata key for embedding > library information in the output file. A quick test with AVI output > shows that this metadata is generated internally and cannot be > overridden on the command-line. If your concern is being able to identify our mxf muxer, then why not use the Platform metadata item for this? "Human readable name of the toolkit and operating system used. Best practice is to use the form “SDK name (OS name)”" Uhm guys, it is very bad practice: if you just insert a different manufacturer name then you just cheat. SMPTE 377 has a clear statement about what goes to the identification, you cannot just write the infos from a different program there. What you can do instead is to push both identifications, the old one and the one from the current program into the identification array, this way the processing chain can be reconstructed. Unforutnately i have never seen anyone doing this besides Opencube. Also, note that broadcasters currently are using the identification string, looking for "ffmpeg" in order to sort out non compatible XDCAMHD mxf: ffmpeg does not write some mandatory metadata fields as mentioned here: https://trac.ffmpeg.org/ticket/5097 this leads to sony devices not accepting the ffmpeg mxf container - which again leads to ffmpeg mxf wrapper for XDCAMHD not being accepted by our public broadcaster. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] Populate field order returned by avs script, Trac Ticket 8757
On 2021-01-21 14:10, Stephen Hutchinson wrote:
Yeah, never mind about that. I didn't notice that those are declared
AVSC_INLINE, not AVSC_API, so they don't get used through the dynamic
API loader.
The comment formatting seems to have been messed up in the second
version, though.
/* The following typically only works when assumetff (-bff) and
* assumefieldbased is used in-script. Additional
* logic using GetParity() could deliver more accurate results
* but also decodes a frame which we want to avoid. */
OK, i have to admit formatting comments is in the top 10 of my greatest
weaknesses :D
Thanks for your patience and also thanks for telling me directly how the
formatting is done correctly.
New patch with formatted comment attached
From 243bb2cbae9fabc78fe7f48cb90ae1c620c51a51 Mon Sep 17 00:00:00 2001
From: emcodem
Date: Thu, 21 Jan 2021 18:59:45 +0100
Subject: [PATCH] Populate field order returned by avs script, Trac Ticket 8757
---
libavformat/avisynth.c | 17 +
1 file changed, 17 insertions(+)
diff --git a/libavformat/avisynth.c b/libavformat/avisynth.c
index 2c08ace8db..22ae8c0dd3 100644
--- a/libavformat/avisynth.c
+++ b/libavformat/avisynth.c
@@ -241,6 +241,23 @@ static int avisynth_create_stream_video(AVFormatContext *s, AVStream *st)
st->nb_frames = avs->vi->num_frames;
avpriv_set_pts_info(st, 32, avs->vi->fps_denominator, avs->vi->fps_numerator);
+av_log(s, AV_LOG_TRACE, "avs_is_field_based: %d\n", avs_is_field_based(avs->vi));
+av_log(s, AV_LOG_TRACE, "avs_is_parity_known: %d\n", avs_is_parity_known(avs->vi));
+
+/* The following typically only works when assumetff (-bff) and
+ * assumefieldbased is used in-script. Additional
+ * logic using GetParity() could deliver more accurate results
+ * but also decodes a frame which we want to avoid. */
+st->codecpar->field_order = AV_FIELD_UNKNOWN;
+if (avs_is_field_based(avs->vi)) {
+if (avs_is_tff(avs->vi)) {
+st->codecpar->field_order = AV_FIELD_TT;
+}
+else if (avs_is_bff(avs->vi)) {
+st->codecpar->field_order = AV_FIELD_BB;
+}
+}
+
switch (avs->vi->pixel_type) {
/* 10~16-bit YUV pix_fmts (AviSynth+) */
case AVS_CS_YUV444P10:
--
2.25.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] Populate field order returned by avs script, Trac Ticket 8757
On 2021-01-17 09:02, wrote Stephen Hutchinson:
Going into detail about GetParity wouldn't be necessary if it's not
used (and there aren't any other invoke-parsed functions aside from
checking with Import() whether the script actually returns a clip),
so the comment could be shortened. Also, since this is the avisynth
demuxer, 'in the avs' would be better rendered as 'in the script',
and simply refer to 'source plugins' rather than 'avisynth source
plugins'.
Comment bikeshedding aside, LGTM, but the avs_is* API usage added here
needs to be reflected in the AVSC_DECLARE_FUNC and LOAD_AVS_FUNC
blocks.
If those parts of the API are present in 2.5, the LOAD_AVS_FUNC can be
a
0. If they were added in 2.6 (or Plus, but I know these would have to
be
from classic AviSynth), then it should be 1.
Thanks for the very quick reply, i shortened the comment but as the
whole comment is basically just there to explain why i decided to not
involve getparity, i believe it is worth to mention in order to save
some time for possible future committers.
What i am not able to do is to add the used convenience functions
avs_is_tff and bff to AVSC_DECLARE_FUNC and LOAD_AVS_FUNC, it refuses to
compile when i do so. IMHO this is because it is just convenience
functions that's function body is defined in the linked avisynth_c.h
file instead of being exported by the avisynth api lib.
The existing code in avisynth.c also uses such convenience functions
without adding them to the declaration, examples:
avs_has_video, line 524
avs_is_clip, line 571
Also, i found it safe to use the convenience functions avs_is_tff and
bff because the minimum required version is 2.6 and Plus or above, so i
hoped the functions will be always available.From 0725b04b8855723309662a9b663b346d98117ff1 Mon Sep 17 00:00:00 2001
From: emcodem
Date: Sun, 17 Jan 2021 18:59:41 +0100
Subject: [PATCH] Populate field order returned by avs script, Trac Ticket 8757
---
libavformat/avisynth.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/libavformat/avisynth.c b/libavformat/avisynth.c
index 2c08ace8db..75c7b18c22 100644
--- a/libavformat/avisynth.c
+++ b/libavformat/avisynth.c
@@ -241,6 +241,21 @@ static int avisynth_create_stream_video(AVFormatContext *s, AVStream *st)
st->nb_frames = avs->vi->num_frames;
avpriv_set_pts_info(st, 32, avs->vi->fps_denominator, avs->vi->fps_numerator);
+av_log(s, AV_LOG_TRACE, "avs_is_field_based: %d\n", avs_is_field_based(avs->vi));
+av_log(s, AV_LOG_TRACE, "avs_is_parity_known: %d\n", avs_is_parity_known(avs->vi));
+
+/* The following typically only works when assumetff (-bff) and assumefieldbased is used in the source avs script.
+Additional logic using GetParity() could deliver more accurate results but also decodes a frame which we want to avoid. */
+st->codecpar->field_order = AV_FIELD_UNKNOWN;
+if (avs_is_field_based(avs->vi)) {
+if (avs_is_tff(avs->vi)) {
+st->codecpar->field_order = AV_FIELD_TT;
+}
+else if (avs_is_bff(avs->vi)) {
+st->codecpar->field_order = AV_FIELD_BB;
+}
+}
+
switch (avs->vi->pixel_type) {
/* 10~16-bit YUV pix_fmts (AviSynth+) */
case AVS_CS_YUV444P10:
--
2.25.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] Populate field order returned by avs script, Trac Ticket 8757
The purpose of this is to tell ffmpeg which field order the returned
clip of the avisynth .avs script decided to finally deliver, which is
handy in an automated environment.
Interlacing is generally a very hard topic in avisynth, the huge comment
is neccessary to explain my reasons.
Additionally to the comment in the patch, i can tell that i set unknown
instead of progressive for backward compatibility. (i was struggeling
with this, maybe i should have even left it unset in case it is not
clearly tff or bff interlaced.
/* Set interlacing type.
http://avisynth.nl/index.php/Interlaced_fieldbased
* The following typically only works when assumetff (-bff) and
assumefieldbased is used in the avs.
* This is because most avisynth source plugins do not set the
parity info in the clip properties.
* We could use GetParity() to be more accurate but it decodes a
frame which is
* expensive and can potentially lead to unforeseen behaviour when
seeking later.
* In case Parity is not known, we still cannot guarantee that
*/
Tests with different avisynth source plugins, e.g. directshowsource,
ffms2, mpeg2source delivered the expected result.
Make FATE passed.From 31a37518afa5f9b635e6c52a8f29513bc756c715 Mon Sep 17 00:00:00 2001
From: emcodem
Date: Sun, 17 Jan 2021 01:02:08 +0100
Subject: [PATCH] Populate field order returned by avs script, Trac Ticket 8757
---
libavformat/avisynth.c | 20
1 file changed, 20 insertions(+)
diff --git a/libavformat/avisynth.c b/libavformat/avisynth.c
index 2c08ace8db..67348a61d7 100644
--- a/libavformat/avisynth.c
+++ b/libavformat/avisynth.c
@@ -241,6 +241,26 @@ static int avisynth_create_stream_video(AVFormatContext *s, AVStream *st)
st->nb_frames = avs->vi->num_frames;
avpriv_set_pts_info(st, 32, avs->vi->fps_denominator, avs->vi->fps_numerator);
+av_log(s, AV_LOG_TRACE, "avs_is_field_based: %d\n", avs_is_field_based(avs->vi));
+av_log(s, AV_LOG_TRACE, "avs_is_parity_known: %d\n", avs_is_parity_known(avs->vi));
+
+st->codecpar->field_order = AV_FIELD_UNKNOWN;
+if (avs_is_field_based(avs->vi)) {
+/* Set interlacing type. http://avisynth.nl/index.php/Interlaced_fieldbased
+* The following typically only works when assumetff (-bff) and assumefieldbased is used in the avs.
+* This is because most avisynth source plugins do not set the parity info in the clip properties.
+* We could use GetParity() to be more accurate but it decodes a frame which is
+* expensive and can potentially lead to unforeseen behaviour when seeking later.
+* In case Parity is not known, we still cannot guarantee that
+*/
+if (avs_is_tff(avs->vi)) {
+st->codecpar->field_order = AV_FIELD_TT;
+}
+else if (avs_is_bff(avs->vi)) {
+st->codecpar->field_order = AV_FIELD_BB;
+}
+}
+
switch (avs->vi->pixel_type) {
/* 10~16-bit YUV pix_fmts (AviSynth+) */
case AVS_CS_YUV444P10:
--
2.25.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
