[FFmpeg-devel] [PATCH] libavformat/rtspdec.c: flush pes buffer while rtsp seek

["tanwei (D)" <tanwei123@hisilicon.com>]

[-- Attachment #1: Type: text/plain, Size: 9357 bytes --]

>> On Dec 29, 2022, at 11:43, tanwei (D) <tanwei123@hisilicon.com> wrote:
>> 
>>>> 
>>>> +    if (rt->cur_transport_priv && rt->transport ==
>>>> + RTSP_TRANSPORT_RTP) {
>>>> 
>>>> +        ff_rtp_seek_flush(rt->cur_transport_priv);
>>>> 
>>>> +    } else if (CONFIG_RTPDEC && rt->ts) {
>>>> 
>>>> +        av_freep(&rt->recvbuf);
>> 
>>> Is it necessary to free rt->recvbuf? 
>> The recvbuf must be released. Otherwise, data before seek may be read in the RTSP+TS scenario.
>
>Read from recvbuf with recvbuf_len = 0 ? How is it possible?
I checked the code again, and you're right. The recvbuf does not need to be free.
The new modification is to set cur_transport_priv to NULL. I have fixed the previous review comments.
Please review the attached patch again. Thank you.
>>>> 
>>>> +        rt->recvbuf_pos = 0;
>>>> 
>>>> +        rt->recvbuf_len = 0;
>>>> 
>>>> +        ff_mpegts_seek_flush(rt->ts);
>>>> 
>>>> +    }> 
> 
> -----邮件原件-----
> 发件人: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] 代表 "zhilizhao(赵志立)"
> 发送时间: 2022年12月23日 10:34
> 收件人: FFmpeg development discussions and patches 
> <ffmpeg-devel@ffmpeg.org>
> 抄送: Wujian(Chin) <wujian2@huawei.com>; Lingzezhi 
> <steven.ling@hisilicon.com>
> 主题: Re: [FFmpeg-devel] [PATCH] libavformat/rtspdec.c: flush pes buffer 
> while rtsp seek
> 
> 
> 
>> On Dec 22, 2022, at 11:32, tanwei (D) <tanwei123@hisilicon.com> wrote:
>> 
>> Fixes ticket #9949.
>> 
>> 
>> Signed-off-by: t00660896 <tanwei123@hisilicon.com>
>> 
>> ---
>> 
>> libavformat/mpegts.c        | 20 ++++++++++++++++++++
>> 
>> libavformat/mpegts.h        |  1 +
>> 
>> libavformat/rtpdec.c        |  7 +++++++
>> 
>> libavformat/rtpdec.h        |  2 ++
>> 
>> libavformat/rtpdec_mpegts.c | 11 +++++++++++
>> 
>> libavformat/rtspdec.c       | 11 +++++++++++
>> 
>> 6 files changed, 52 insertions(+)
>> 
>> 
> 
> Please check the patch format (a lot of empty lines).
> 
>> diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
>> 
>> index d97702fcd7..c82971af87 100644
>> 
>> --- a/libavformat/mpegts.c
>> 
>> +++ b/libavformat/mpegts.c
>> 
>> @@ -3419,6 +3419,26 @@ int avpriv_mpegts_parse_packet(MpegTSContext
>> *ts, AVPacket *pkt,
>> 
>>    return len1 - len;
>> 
>> }
>> 
>> +void ff_mpegts_seek_flush(MpegTSContext *ts)
>> 
>> +{
>> 
>> +    int i;
>> 
>> +    /* flush pes buffer */
>> 
>> +    for (i = 0; i < NB_PID_MAX; i++) {
>> 
>> +        if (ts->pids[i]) {
>> 
>> +            if (ts->pids[i]->type == MPEGTS_PES) {
>> 
>> +                PESContext *pes = ts->pids[i]->u.pes_filter.opaque;
>> 
>> +                av_buffer_unref(&pes->buffer);
>> 
>> +                pes->data_index = 0;
>> 
>> +                pes->state = MPEGTS_SKIP; /* skip until pes header 
>> + */
>> 
>> +            } else if (ts->pids[i]->type == MPEGTS_SECTION) {
>> 
>> +                ts->pids[i]->u.section_filter.last_ver = -1;
>> 
>> +            }
>> 
>> +            ts->pids[i]->last_cc = -1;
>> 
>> +            ts->pids[i]->last_pcr = -1;
>> 
>> +        }
>> 
>> +    }
>> 
>> +}
>> 
>> +
> 
> Please don’t just duplicate the source code.
> 
>> 
>> void avpriv_mpegts_parse_close(MpegTSContext *ts)
>> 
>> {
>> 
>>    mpegts_free(ts);
>> 
>> diff --git a/libavformat/mpegts.h b/libavformat/mpegts.h
>> 
>> index a48f14e768..ea6b5106a4 100644
>> 
>> --- a/libavformat/mpegts.h
>> 
>> +++ b/libavformat/mpegts.h
>> 
>> @@ -170,6 +170,7 @@ MpegTSContext
>> *avpriv_mpegts_parse_open(AVFormatContext *s);
>> 
>> int avpriv_mpegts_parse_packet(MpegTSContext *ts, AVPacket *pkt,
>> 
>>                               const uint8_t *buf, int len);
>> 
>> void avpriv_mpegts_parse_close(MpegTSContext *ts);
>> 
>> +void ff_mpegts_seek_flush(MpegTSContext *ts);
>> 
>> typedef struct SLConfigDescr {
>> 
>>    int use_au_start;
>> 
>> diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c
>> 
>> index fa7544cc07..d688afd1c1 100644
>> 
>> --- a/libavformat/rtpdec.c
>> 
>> +++ b/libavformat/rtpdec.c
>> 
>> @@ -954,6 +954,13 @@ int ff_rtp_parse_packet(RTPDemuxContext *s, 
>> AVPacket *pkt,
>> 
>>    return rv ? rv : has_next_packet(s);
>> 
>> }
>> 
>> +void ff_rtp_seek_flush(RTPDemuxContext *s)
>> 
>> +{
>> 
>> +    ff_rtp_reset_packet_queue(s);
>> 
>> +    if (s->handler && s->handler->seek_flush)
>> 
>> +        s->handler->seek_flush(s->dynamic_protocol_context);
>> 
>> +}
>> 
>> +
>> 
>> void ff_rtp_parse_close(RTPDemuxContext *s)
>> 
>> {
>> 
>>    ff_rtp_reset_packet_queue(s);
>> 
>> diff --git a/libavformat/rtpdec.h b/libavformat/rtpdec.h
>> 
>> index 5a02e72dc2..8d6d857e28 100644
>> 
>> --- a/libavformat/rtpdec.h
>> 
>> +++ b/libavformat/rtpdec.h
>> 
>> @@ -52,6 +52,7 @@ int ff_rtp_parse_packet(RTPDemuxContext *s, 
>> AVPacket *pkt,
>> 
>> void ff_rtp_parse_close(RTPDemuxContext *s);
>> 
>> int64_t ff_rtp_queued_packet_time(RTPDemuxContext *s);
>> 
>> void ff_rtp_reset_packet_queue(RTPDemuxContext *s);
>> 
>> +void ff_rtp_seek_flush(RTPDemuxContext *s);
>> 
>> /**
>> 
>> * Send a dummy packet on both port pairs to set up the connection
>> 
>> @@ -135,6 +136,7 @@ struct RTPDynamicProtocolHandler {
>> 
>>    /** Parse handler for this dynamic packet */
>> 
>>    DynamicPayloadPacketHandlerProc parse_packet;
>> 
>>    int (*need_keyframe)(PayloadContext *context);
>> 
>> +    void (*seek_flush)(PayloadContext *protocol_data);
>> 
>> };
>> 
>> typedef struct RTPPacket {
>> 
>> diff --git a/libavformat/rtpdec_mpegts.c 
>> b/libavformat/rtpdec_mpegts.c
>> 
>> index 405271f744..46c1d36021 100644
>> 
>> --- a/libavformat/rtpdec_mpegts.c
>> 
>> +++ b/libavformat/rtpdec_mpegts.c
>> 
>> @@ -47,6 +47,16 @@ static av_cold int mpegts_init(AVFormatContext 
>> *ctx, int st_index,
>> 
>>    return 0;
>> 
>> }
>> 
>> +static void mpegts_seek_flush(PayloadContext *data)
>> 
>> +{
>> 
>> +    if (!data)
>> 
>> +        return;
> 
> Is it possible for data being NULL? It’s better to depends on a clear lifecycle management rather than NULL pointer check everywhere.
> 
>> 
>> +    memset(data->buf, 0, data->read_buf_size);
>> 
>> +    data->read_buf_size = 0;
> 
> What about data->read_buf_index ?
> 
>> 
>> +    if (data->ts)
>> 
>> +        ff_mpegts_seek_flush(data->ts);
>> 
>> +}
>> 
>> +
>> 
>> static int mpegts_handle_packet(AVFormatContext *ctx, PayloadContext 
>> *data,
>> 
>>                                AVStream *st, AVPacket *pkt, uint32_t 
>> *timestamp,
>> 
>>                                const uint8_t *buf, int len, uint16_t 
>> seq,
>> 
>> @@ -94,6 +104,7 @@ const RTPDynamicProtocolHandler 
>> ff_mpegts_dynamic_handler = {
>> 
>>    .priv_data_size    = sizeof(PayloadContext),
>> 
>>    .parse_packet      = mpegts_handle_packet,
>> 
>>    .init              = mpegts_init,
>> 
>> +    .seek_flush        = mpegts_seek_flush,
>> 
>>    .close             = mpegts_close_context,
>> 
>>    .static_payload_id = 33,
>> 
>> };
>> 
>> diff --git a/libavformat/rtspdec.c b/libavformat/rtspdec.c
>> 
>> index bbabec7db8..22b08a4c56 100644
>> 
>> --- a/libavformat/rtspdec.c
>> 
>> +++ b/libavformat/rtspdec.c
>> 
>> @@ -36,6 +36,7 @@
>> 
>> #include "rdt.h"
>> 
>> #include "tls.h"
>> 
>> #include "url.h"
>> 
>> +#include "mpegts.h"
>> 
>> #include "version.h"
>> 
>> static const struct RTSPStatusMessage {
>> 
>> @@ -982,6 +983,16 @@ static int rtsp_read_seek(AVFormatContext *s, 
>> int stream_index,
>> 
>>        rt->state = RTSP_STATE_IDLE;
>> 
>>        break;
>> 
>>    }
>> 
>> +
>> 
>> +    if (rt->cur_transport_priv && rt->transport ==
>> + RTSP_TRANSPORT_RTP) {
>> 
>> +        ff_rtp_seek_flush(rt->cur_transport_priv);
>> 
>> +    } else if (CONFIG_RTPDEC && rt->ts) {
>> 
>> +        av_freep(&rt->recvbuf);
> 
> Is it necessary to free rt->recvbuf? 
> 
>> 
>> +        rt->recvbuf_pos = 0;
>> 
>> +        rt->recvbuf_len = 0;
>> 
>> +        ff_mpegts_seek_flush(rt->ts);
>> 
>> +    }
>> 
>> +
>> 
>>    return 0;
>> 
>> }
>> 
>> --
>> 
>> 2.25.1
>> 
>> 
>> _______________________________________________
>> 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".
> _______________________________________________
> 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".

[-- Attachment #2: 0001-libavformat-rtspdec.c-flush-pes-buffer-while-rtsp-seek.patch --]
[-- Type: application/octet-stream, Size: 6347 bytes --]

[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".