Do experts have any good ideas on this fuction? -----邮件原件----- 发件人: Wujian(Chin) 发送时间: 2022年9月26日 14:48 收件人: ffmpeg-devel@ffmpeg.org 抄送: yangzhiyong (F) <yangzhiyo...@huawei.com> 主题: Re: [FFmpeg-devel] [PATCH] RTP: get the buffer information of the RTP extension header.
Andreas: >Wujian(Chin): >> Signed-off-by: wujian_nanjing <wuji...@huawei.com> >> --- >> libavcodec/avpacket.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ >> libavcodec/packet.h | 17 +++++++++++++++++ >> libavformat/demux.c | 7 +++++++ >> libavformat/rtpdec.c | 37 ++++++++++++++++++++++++++++--------- >> 4 files changed, 96 insertions(+), 9 deletions(-) >> >> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c index >> bcbc416..45c131a 100644 >> --- a/libavcodec/avpacket.c >> +++ b/libavcodec/avpacket.c >> @@ -46,6 +46,8 @@ void av_init_packet(AVPacket *pkt) >> pkt->opaque = NULL; >> pkt->opaque_ref = NULL; >> pkt->time_base = av_make_q(0, 1); >> + pkt->ext = NULL; >> + pkt->extlen = 0; >> } >> #endif >> >> @@ -109,6 +111,46 @@ int av_new_packet(AVPacket *pkt, int size) >> return 0; >> } >> >> +static void av_packet_free_ext(AVPacket *pkt) { >> + if (!pkt) >> + return; >> + >> + if (pkt->ext) { > >+ av_free(pkt->ext); > >+ pkt->ext = NULL; > >+ } >> + >> + pkt->extlen = 0; >> +} > >+ > >+int av_set_packet_ext(AVPacket *pkt, uint8_t *buf, int len) { > >+ if (!buf || (len <= 0)) > >+ return 0; > >+ >> + if (pkt->ext == buf) { >> + pkt->extlen = len; >> + return 0; >> + } >> + >> + pkt->ext = av_malloc(len); >> + pkt->extlen = len; >> + (void)memcpy(pkt->ext, buf, len); >> + >> + return 0; >> +} >> + >> +static int av_copy_packet_ext(AVPacket *pkt, const AVPacket *src) { >> + pkt->ext = NULL; >> + pkt->extlen = 0; >> + >> + av_set_packet_ext(pkt, src->ext, src->extlen); >> + >> + return 0; >> +} >> + >> void av_shrink_packet(AVPacket *pkt, int size) { >> if (pkt->size <= size) >> @@ -422,6 +464,7 @@ int av_packet_copy_props(AVPacket *dst, const >> AVPacket *src) void av_packet_unref(AVPacket *pkt) { >> av_packet_free_side_data(pkt); >> + av_packet_free_ext(pkt); >> av_buffer_unref(&pkt->opaque_ref); >> av_buffer_unref(&pkt->buf); >> get_packet_defaults(pkt); >> @@ -456,6 +499,7 @@ int av_packet_ref(AVPacket *dst, const AVPacket *src) >> } >> >> dst->size = src->size; >> + av_copy_packet_ext(dst, src); >> >> return 0; >> fail: >> diff --git a/libavcodec/packet.h b/libavcodec/packet.h index >> 404d520..137d1fa 100644 >> --- a/libavcodec/packet.h >> +++ b/libavcodec/packet.h >> @@ -374,6 +374,13 @@ typedef struct AVPacket { >> uint8_t *data; >> int size; >> int stream_index; >> + >> + /** >> + * Extension header data and size >> + */ >> + uint8_t *ext; >> + uint32_t extlen; >> + >> /** >> * A combination of AV_PKT_FLAG values >> */ >> @@ -523,6 +530,16 @@ void av_init_packet(AVPacket *pkt); int >> av_new_packet(AVPacket *pkt, int size); >> >> /** >> + * Allocate the ext data from buf with len lenght. >> + * Make sure pkt->ext is null, otherwise cause memory leak. >> + * >> + * @param pkt packet >> + * @param buf buffer with ext data >> + * @param len buffer size >> + */ >> +int av_set_packet_ext(AVPacket *pkt, uint8_t *buf, int len); >> + >> +/** >> * Reduce packet size, correctly zeroing padding >> * >> * @param pkt packet >> diff --git a/libavformat/demux.c b/libavformat/demux.c index >> 1620716..2a8b201 100644 >> --- a/libavformat/demux.c >> +++ b/libavformat/demux.c >> @@ -1175,6 +1175,13 @@ static int parse_packet(AVFormatContext *s, AVPacket >> *pkt, >> pkt->side_data_elems = 0; >> } >> >> + if (pkt->extlen > 0) { >> + out_pkt->extlen = pkt->extlen; >> + out_pkt->ext = pkt->ext; >> + pkt->extlen = 0; >> + pkt->ext = NULL; >> + } >> + >> /* set the duration */ >> out_pkt->duration = (sti->parser->flags & >> PARSER_FLAG_COMPLETE_FRAMES) ? pkt->duration : 0; >> if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) { diff >> --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c index >> fa7544c..2a539e0 100644 >> --- a/libavformat/rtpdec.c >> +++ b/libavformat/rtpdec.c >> @@ -699,13 +699,14 @@ static int >> rtp_parse_packet_internal(RTPDemuxContext *s, AVPacket *pkt, { >> unsigned int ssrc; >> int payload_type, seq, flags = 0; >> - int ext, csrc; >> + int has_ext, csrc, extlen; >> + uint8_t *extbuf; >> AVStream *st; >> uint32_t timestamp; >> int rv = 0; >> >> csrc = buf[0] & 0x0f; >> - ext = buf[0] & 0x10; >> + has_ext = buf[0] & 0x10; >> payload_type = buf[1] & 0x7f; >> if (buf[1] & 0x80) >> flags |= RTP_FLAG_MARKER; >> @@ -744,18 +745,25 @@ static int rtp_parse_packet_internal(RTPDemuxContext >> *s, AVPacket *pkt, >> return AVERROR_INVALIDDATA; >> >> /* RFC 3550 Section 5.3.1 RTP Header Extension handling */ >> - if (ext) { >> + if (has_ext) { >> if (len < 4) >> return -1; >> /* calculate the header extension length (stored as number >> * of 32-bit words) */ >> - ext = (AV_RB16(buf + 2) + 1) << 2; >> + extlen = (AV_RB16(buf + 2) + 1) << 2; >> >> - if (len < ext) >> + if (len < extlen) >> return -1; >> - // skip past RTP header extension >> - len -= ext; >> - buf += ext; >> + >> + // Save RTP header extension data >> + if (extlen >> 0) { >> + extbuf = av_malloc(extlen); >> + if (extbuf) >> + (void)memcpy(extbuf, buf, extlen); >> + } >> + >> + len -= extlen; >> + buf += extlen; >> } >> >> if (s->>handler && s->>handler->>parse_packet) { @@ -763,14 >> +771,25 @@ static int rtp_parse_packet_internal(RTPDemuxContext *s, AVPacket >> *pkt, >> s->>st, pkt, ×tamp, buf, len, >> seq, >> flags); >> } else if (st) { >> - if ((rv = av_new_packet(pkt, len)) < 0) >> + if ((rv = av_new_packet(pkt, len)) < 0) { >> + if ((extlen >> 0) && extbuf) >> + av_free(extbuf); >> return rv; >> + } >> memcpy(pkt->>data, buf, len); >> pkt->>stream_index = st->>index; >> } else { >> + if ((extlen >> 0) && extbuf) >> + av_free(extbuf); >> return AVERROR(EINVAL); >> } >> >> + // attach ext data >> + if ((extlen >> 0) && extbuf) { >> + av_set_packet_ext(pkt, extbuf, extlen); >> + av_free(extbuf); >> + } >> + >> // now perform timestamp things.... >> finalize_packet(s, pkt, timestamp); >> >1. Adding something in the middle of a public struct is an ABI break. >2. Currently sizeof(AVPacket) is still part of the public ABI, so one can't >even add something add the end of AVPacket. >3. But it doesn't matter, because such stuff belongs into packet side data. >4. The allocation in av_set_packet_ext() is unchecked. >5. Don't cast the return value of memcpy() to void. >6. There is a problem with parse_packet(): In case the parser adds delay, the >side data and (if this patch were merged as-is) also your extension header >data will end up with the wrong packet (if your parser adds a one-packet >delay, then packet n will end up with the side data for packet n + 1). The >reason for this is that the parsing API is ancient and directly works with >buffers and not with AVPackets. >- Andreas 1. Adding something in the middle of a public struct is an ABI break. --Do you have any good advice? 6. There is a problem with parse_packet(): In case the parser adds delay, the side data and (if this patch were merged as-is) also your extension header data will end up with the wrong packet (if your parser adds a one-packet delay, then packet n will end up with the side data for packet n + 1). The reason for this is that the parsing API is ancient and directly works with buffers and not with AVPackets. --Only parses and obtains the buffer. The caller determines whether the buffer is correct. _______________________________________________ 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". _______________________________________________ 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".