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" (FFFF...)
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".
