On Wed, 27 Jan 2021, Tomas Härdin wrote:
ons 2021-01-27 klockan 22:24 +0100 skrev Tomas Härdin:
ons 2021-01-27 klockan 21:38 +0100 skrev Marton Balint:
On Wed, 27 Jan 2021, Tomas Härdin wrote:
Hi
Ticket #9079 brought this about. This should prevent accidentally
adding local tags that are not registered in the primer. It also allows
us to omit tags that we know won't be used, in a manner that is more
elegant than the old code.
The actual meat of this patch is mxf_mark_tag_unused(),
mxf_write_primer_pack(), mxf_write_local_tag() and
ff_mxf_lookup_local_tag()
IMHO you should not move the local tags to mxf.c, because only encoding
uses them.
The only exception where sharing made sense is
ff_mxf_mastering_display_local_tags, but that is super ugly that you
now lookup them in mxfdec.c based on local tags we assign them for
encoding. Not to mention the linear search you use for each lookup...
We could sort them and use a binary search, but I wanted some feedback
on this idea before going further. There's not terribly many of them
I'd like to avoid having the full ULs twice in the code. The only way I
can see how to do that is with #defines
So I suggest you simply duplicate the 4 UL-s to the single local tags
array you make and keep them in mxfenc.c, that way you also don't have to
specify the array size manually...
That might conflict with Andreas' deduplication efforts. But yeah, the
thought did occur to me
Here's an updated patch. Feedback welcome.
Thanks, I like this version much more. One comment is that I'd put an
assert right into mxf_lookup_local_tag instead of returning NULL if a tag
is not found, this way you can remove NULL-check asserts from individual
places where mxf_lookup_local_tag is called. Otherwise seems all fine.
Regards,
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".
