Bugfix/memory leak

[ohjiwoo123]

First, Solved Memory Leak in traf (MP4TrackFragmentAtom)
Second, Solved Memory Leak in trex (MP4TrackExtendsAtom)

If user makes Fragment Mp4, There are some memory leak.
There is no memory free for MP4TrackFragmentAtom.
There is no memory free for MP4TrackExtendsAtom.

After add Code and completing the test, There is no memory leak.

[podborski]

Thanks, there seem to be a problem with the compilation on linux and some basic clang-format issues.

[ohjiwoo123]

Complete the Change Formatting with clang-format.
Could you try again?
Thank you.

[ohjiwoo123]

Do I must use formatting clang-format vesion 13 ? ( as written in isobmff/.github/workflows/clang-format.yml )
I am formatting file with clang-format version 18.0.0

[ohjiwoo123]

I think that you need to change the clang-format.yml file (version up)
If I use jidicula/[email protected] version, The error raised like below.

Installing clang-format-13
E: Unable to locate package clang-format-13
Not a directory in the workspace, fallback to all files.
/entrypoint.sh: line 23: /usr/bin/clang-format-13: No such file or directory

https://github.com/MPEGGroup/isobmff/blob/master/.github/workflows/clang-format.yml

jidicula/[email protected] -> jidicula/[email protected]
or
jidicula/[email protected]
Thank you

[podborski]

Yep, I remember we fixed some of those issues in the sgpd_versions branch. It now has a lot of useful changes in it. I suggest we merge this one to that branch as well.

[podborski]

Not so sure about this one. Below moof->destroy is called which should already do this.

[ohjiwoo123]

I understand this library's design and understand what you said.
But, If there is no below code

for(i = 0; i < moof->atomList->entryCount; i++)
    {
      err = MP4GetListEntry(moof->atomList, i, (char **)&traf);
      if(err) goto bail;
      traf->destroy((MP4AtomPtr)traf);
    }

traf is still in memory.

If this code is not matching this library design,
you need to check moof destory's flow.

[podborski]

In this part child boxes are destroyed which should also include the traf box that you destroy manually in MP4MovieFile.c

[podborski]

This is already a part of the macro above.

[ohjiwoo123]

In side of Macro code

#define DESTROY_ATOM_LIST                                      \
  if(self->atomList)                                           \
  {                                                            \
    u32 atomListSize;                                          \
    err = MP4GetListEntryCount(self->atomList, &atomListSize); \
    if(err) goto bail;                                         \
    for(i = 0; i < atomListSize; i++)                          \
    {                                                          \
      MP4AtomPtr a;                                            \
      err = MP4GetListEntry(self->atomList, i, (char **)&a);   \
      if(err) goto bail;                                       \
      if(a) a->destroy(a);                                     \
    }                                                          \
    err = MP4DeleteLinkedList(self->atomList);                 \
    if(err) goto bail;                                         \
  }

if(a) a->destroy(a);
err = MP4DeleteLinkedList(self->atomList);
I think that those free the memory same thing
Is it Right or not ?

My Thought is we use only err = MP4DeleteLinkedList(self->atomList);
for free the self->atomList memory.

I do not understand LinkedList have several ListEntry and several entry count.

I add this code due to 'Segmentation Fault Error Raised'
when I free the MP4MovieFragmentAtomPtr

[podborski]

Is the above not already handled by this call?

[ohjiwoo123]

If It makes movie atom for fragment mode,
It need to free the memory traf atom and trex atom.

if(moov->moovAtomPtr) moov->moovAtomPtr->destroy(moov->moovAtomPtr);

This part do not free the memory of atoms derived from fragments