| Message ID | 20240216211804.4546-2-cus@passwd.hu |
|---|---|
| State | Accepted |
| Commit | 68f2b32ef2b29aa95488531b007adde92ca82165 |
| Headers | show |
| Series | [FFmpeg-devel,1/2] avformat/mxfdec: move resolving Descriptors to the multi descriptor resolve function | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
fre 2024-02-16 klockan 22:18 +0100 skrev Marton Balint: > Using AnyType should not be a problem for proper MXF files because > UIDs are > supposed to be unique themselves. Unfortunately that is not the case > for some > broken files, so let's check the type more strictly. Here's what S377m-2004 says: > StrongRef: “One-to-one” relationship between sets and implemented in > MXF with UUIDs. Strong references > are typed which means that the definition identifies the kind of set > which is the target of the reference. So non-unique UUIDs are fine so long as their type sets them apart. Therefore we should not use AnyType unless there is a good reason to do so. UMID lookup would be such a reason. Resolving SourceClips and TimeCodeComponents are not. And since we don't do any UMID lookup we could just as well ditch AnyType entirely. > Fixes ticket #10865. > > Signed-off-by: Marton Balint <cus@passwd.hu> > --- > libavformat/mxfdec.c | 19 ++++++++----------- > 1 file changed, 8 insertions(+), 11 deletions(-) > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c > index 4e4e3e7a84..446bcf3276 100644 > --- a/libavformat/mxfdec.c > +++ b/libavformat/mxfdec.c > @@ -2258,16 +2258,14 @@ static MXFPackage* > mxf_resolve_source_package(MXFContext *mxf, UID package_ul, U > > static MXFDescriptor* mxf_resolve_descriptor(MXFContext *mxf, UID > *strong_ref, int track_id) > { > - MXFDescriptor *descriptor, *file_descriptor = NULL; > - int i; Mixing functional and style changes make the patch annoying to read. Anyway it looks fine /Tomas
On Mon, 19 Feb 2024, Tomas Härdin wrote: > fre 2024-02-16 klockan 22:18 +0100 skrev Marton Balint: >> Using AnyType should not be a problem for proper MXF files because >> UIDs are >> supposed to be unique themselves. Unfortunately that is not the case >> for some >> broken files, so let's check the type more strictly. > > Here's what S377m-2004 says: > >> StrongRef: “One-to-one” relationship between sets and implemented in >> MXF with UUIDs. Strong references >> are typed which means that the definition identifies the kind of set >> which is the target of the reference. > > > So non-unique UUIDs are fine so long as their type sets them apart. > Therefore we should not use AnyType unless there is a good reason to do > so. UMID lookup would be such a reason. Resolving SourceClips and > TimeCodeComponents are not. And since we don't do any UMID lookup we > could just as well ditch AnyType entirely. Thanks, I was not aware of this, with MXF, you can never be sure :) I have already prepared patches to get rid of AnyType, will send it soon. > >> Fixes ticket #10865. >> >> Signed-off-by: Marton Balint <cus@passwd.hu> >> --- >> libavformat/mxfdec.c | 19 ++++++++----------- >> 1 file changed, 8 insertions(+), 11 deletions(-) >> >> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c >> index 4e4e3e7a84..446bcf3276 100644 >> --- a/libavformat/mxfdec.c >> +++ b/libavformat/mxfdec.c >> @@ -2258,16 +2258,14 @@ static MXFPackage* >> mxf_resolve_source_package(MXFContext *mxf, UID package_ul, U >> >> static MXFDescriptor* mxf_resolve_descriptor(MXFContext *mxf, UID >> *strong_ref, int track_id) >> { >> - MXFDescriptor *descriptor, *file_descriptor = NULL; >> - int i; > > Mixing functional and style changes make the patch annoying to read. > Anyway it looks fine Sorry, it was such a tiny change, I did not think it would matter in practice. Thanks for your feedback, I will push this set and send a follow up. Regards, Marton
diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c index 4e4e3e7a84..446bcf3276 100644 --- a/libavformat/mxfdec.c +++ b/libavformat/mxfdec.c @@ -2258,16 +2258,14 @@ static MXFPackage* mxf_resolve_source_package(MXFContext *mxf, UID package_ul, U static MXFDescriptor* mxf_resolve_descriptor(MXFContext *mxf, UID *strong_ref, int track_id) { - MXFDescriptor *descriptor, *file_descriptor = NULL; - int i; - - descriptor = mxf_resolve_strong_ref(mxf, strong_ref, AnyType); - if (!descriptor) - return NULL; + MXFDescriptor *descriptor = mxf_resolve_strong_ref(mxf, strong_ref, Descriptor); + if (descriptor) + return descriptor; - if (descriptor->meta.type == MultipleDescriptor) { - for (i = 0; i < descriptor->file_descriptors_count; i++) { - file_descriptor = mxf_resolve_strong_ref(mxf, &descriptor->file_descriptors_refs[i], Descriptor); + descriptor = mxf_resolve_strong_ref(mxf, strong_ref, MultipleDescriptor); + if (descriptor) { + for (int i = 0; i < descriptor->file_descriptors_count; i++) { + MXFDescriptor *file_descriptor = mxf_resolve_strong_ref(mxf, &descriptor->file_descriptors_refs[i], Descriptor); if (!file_descriptor) { av_log(mxf->fc, AV_LOG_ERROR, "could not resolve file descriptor strong ref\n"); @@ -2277,8 +2275,7 @@ static MXFDescriptor* mxf_resolve_descriptor(MXFContext *mxf, UID *strong_ref, i return file_descriptor; } } - } else if (descriptor->meta.type == Descriptor) - return descriptor; + } return NULL; }
Using AnyType should not be a problem for proper MXF files because UIDs are supposed to be unique themselves. Unfortunately that is not the case for some broken files, so let's check the type more strictly. Fixes ticket #10865. Signed-off-by: Marton Balint <cus@passwd.hu> --- libavformat/mxfdec.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-)