[FFmpeg-devel] [PATCH 02/14] avformat/flvdec: add support for demuxing multi-track FLV

[Anton Khirnov]

Quoting Timo Rothenpieler (2024-12-14 22:48:17)
> On 14.12.2024 10:17, Anton Khirnov wrote:
> > Quoting Timo Rothenpieler (2024-12-12 20:55:27)
> >> From: Dennis Sädtler <dennis at obsproject.com>
> >>
> >> Based on enhanced-rtmp v2 spec published by Veovera:
> >> https://veovera.github.io/enhanced-rtmp/docs/enhanced/enhanced-rtmp-v2
> >>
> >> Signed-off-by: Dennis Sädtler <dennis at obsproject.com>
> >> ---
> >>   libavformat/flvdec.c | 117 +++++++++++++++++++++++++++++++++++--------
> >>   1 file changed, 96 insertions(+), 21 deletions(-)
> > 
> > This could use some adapting to our coding style.
> 
> Anything specific that seems off to you?
> Generally looks to be following our style.

missing spaces around = in some places
&& at the beginning of the line rather the end
broken vertical alignment at the third-to-last added line

Also, just noticed some unchecked av_mallocz() that should be checked
av_calloc().

> 
> The only thing I saw was one use of
> for (int i = ...)
> Which I think we even allow now?

Yes, that is fine.

> 
> >> @@ -1526,6 +1591,16 @@ retry_duration:
> >>               flv->new_extradata[stream_type]      = NULL;
> >>               flv->new_extradata_size[stream_type] = 0;
> >>           }
> >> +    } else if (multitrack
> >> +               && flv->mt_extradata_cnt > track_idx
> >> +               && flv->mt_extradata[track_idx]) {
> >> +        int ret = av_packet_add_side_data(pkt, AV_PKT_DATA_NEW_EXTRADATA,
> >> +                                          flv->mt_extradata[track_idx],
> >> +                                          flv->mt_extradata_sz[track_idx]);
> >> +        if (ret >= 0) {
> > 
> > This should fail when ret < 0
> 
> It follows the scheme of the pre-existing call, so I'd probably rather 
> change both in a separate commit.

Sure, fine with me.

-- 
Anton Khirnov