Re: [FFmpeg-devel] [PATCH 4/5] avformat/iamf: Don't mix ownership and non-ownership pointers

[James Almer <jamrial@gmail.com>]

On 2/19/2024 7:06 PM, Andreas Rheinhardt wrote:
> James Almer:
>> On 2/19/2024 6:52 PM, Andreas Rheinhardt wrote:
>>> IAMFAudioElement and IAMFMixPresentation currently contain
>>> pointers to independently allocated objects that are sometimes
>>> owned by said structures and sometimes not.
>>>
>>> More precisely, upon success the demuxer transfers ownership
>>> of these other objects newly created AVStreamGroups, but it
>>> keeps its pointers. iamf_read_close() therefore always resets
>>> these pointers (because the cleanup code always treats them
>>> as ownership pointers). This leads to memory leaks in case
>>> iamf_read_header() without having attached all of these
>>> objects to stream groups.
>>>
>>> The muxer has a similar issue: It also clears these pointers
>>> (pointing to objects owned by stream groups created by the user)
>>> in its deinit function.
>>>
>>> This commit fixes this memleak by explicitly adding non-ownership
>>> pointers; this also allows to remove the code to reset the
>>> ownership pointers.
>>>
>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>>> ---
>>>    libavformat/iamf.h        | 10 ++++++++++
>>>    libavformat/iamf_parse.c  |  2 ++
>>>    libavformat/iamf_writer.c | 16 ++++++++--------
>>>    libavformat/iamfdec.c     | 22 ++++++++--------------
>>>    libavformat/iamfenc.c     | 12 +-----------
>>>    5 files changed, 29 insertions(+), 33 deletions(-)
>>>
>>> diff --git a/libavformat/iamf.h b/libavformat/iamf.h
>>> index d88a24c435..0cb0902e86 100644
>>> --- a/libavformat/iamf.h
>>> +++ b/libavformat/iamf.h
>>> @@ -86,6 +86,11 @@ typedef struct IAMFSubStream {
>>>    } IAMFSubStream;
>>>      typedef struct IAMFAudioElement {
>>> +    const AVIAMFAudioElement *celement;
>>> +    /**
>>> +     * element backs celement iff the AVIAMFAudioElement
>>
>> Typo. Also in IAMFMixPresentation.
>>
>> Should be ok otherwise.
> 
> What typo? ("iff" is shorthand for "if and only if".)

First time i see it. But no strong feelings about it.

> 
> - Andreas
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".