Re: [FFmpeg-devel] [PATCH] avformat/rtpdec_rfc4175: fix interlace format

Re: [FFmpeg-devel] [PATCH] avformat/rtpdec_rfc4175: fix interlace format

[lance.lmwang]

On Wed, Feb 02, 2022 at 03:45:10PM -0500, Patrick Keroulas wrote:
> In previous state, a new frame was allocated on each timestamp step,
> i.e. each frame/field transition. However, for interlace, a new frame
> should be allocated on 1st field, completed with the 2nd and finally
> freed.
> 
> This commit fixes the frame allocation and the detection of missing RTP
> markers.
> 
> Signed-off-by: Patrick Keroulas <patrick.keroulas@radio-canada.ca>
> ---
>  libavformat/rtpdec_rfc4175.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/libavformat/rtpdec_rfc4175.c b/libavformat/rtpdec_rfc4175.c
> index 8e73c07838..83abe499f8 100644
> --- a/libavformat/rtpdec_rfc4175.c
> +++ b/libavformat/rtpdec_rfc4175.c
> @@ -234,20 +234,21 @@ static int rfc4175_handle_packet(AVFormatContext *ctx, PayloadContext *data,
>      uint8_t *dest;
>  
>      if (*timestamp != data->timestamp) {
> -        if (data->frame) {
> +        if (data->frame && (!data->interlaced || data->field)) {
>              /*
> -             * if we're here, it means that two RTP packets didn't have the
> -             * same timestamp, which is a sign that they were packets from two
> -             * different frames, but we didn't get the flag RTP_FLAG_MARKER on
> -             * the first one of these frames (last packet of a frame).
> -             * Finalize the previous frame anyway by filling the AVPacket.
> +             * if we're here, it means that we missed the cue to return
> +             * the previous AVPacket, that cue being the RTP_FLAG_MARKER
> +             * in the last packet of either the previous frame (progressive)
> +             * or the previous second field (interlace). Let's finalize the
> +             * previous frame (or pair of fields) anyway by filling the AVPacket.
>               */
>              av_log(ctx, AV_LOG_ERROR, "Missed previous RTP Marker\n");
>              missed_last_packet = 1;
>              rfc4175_finalize_packet(data, pkt, st->index);
>          }
>  
> -        data->frame = av_malloc(data->frame_size);
> +        if (!data->frame)
> +            data->frame = av_malloc(data->frame_size);
>  
>          data->timestamp = *timestamp;

LGTM 

>  
> -- 
> 2.25.1
> 

-- 
Thanks,
Limin Wang
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] avformat/rtpdec_rfc4175: fix interlace format

[lance.lmwang]

On Wed, Feb 02, 2022 at 03:45:10PM -0500, Patrick Keroulas wrote:
> In previous state, a new frame was allocated on each timestamp step,
> i.e. each frame/field transition. However, for interlace, a new frame
> should be allocated on 1st field, completed with the 2nd and finally
> freed.
> 
> This commit fixes the frame allocation and the detection of missing RTP
> markers.
> 
> Signed-off-by: Patrick Keroulas <patrick.keroulas@radio-canada.ca>
> ---
>  libavformat/rtpdec_rfc4175.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/libavformat/rtpdec_rfc4175.c b/libavformat/rtpdec_rfc4175.c
> index 8e73c07838..83abe499f8 100644
> --- a/libavformat/rtpdec_rfc4175.c
> +++ b/libavformat/rtpdec_rfc4175.c
> @@ -234,20 +234,21 @@ static int rfc4175_handle_packet(AVFormatContext *ctx, PayloadContext *data,
>      uint8_t *dest;
>  
>      if (*timestamp != data->timestamp) {
> -        if (data->frame) {
> +        if (data->frame && (!data->interlaced || data->field)) {
>              /*
> -             * if we're here, it means that two RTP packets didn't have the
> -             * same timestamp, which is a sign that they were packets from two
> -             * different frames, but we didn't get the flag RTP_FLAG_MARKER on
> -             * the first one of these frames (last packet of a frame).
> -             * Finalize the previous frame anyway by filling the AVPacket.
> +             * if we're here, it means that we missed the cue to return
> +             * the previous AVPacket, that cue being the RTP_FLAG_MARKER
> +             * in the last packet of either the previous frame (progressive)
> +             * or the previous second field (interlace). Let's finalize the
> +             * previous frame (or pair of fields) anyway by filling the AVPacket.
>               */
>              av_log(ctx, AV_LOG_ERROR, "Missed previous RTP Marker\n");
>              missed_last_packet = 1;
>              rfc4175_finalize_packet(data, pkt, st->index);
>          }
>  
> -        data->frame = av_malloc(data->frame_size);
> +        if (!data->frame)
> +            data->frame = av_malloc(data->frame_size);
>  
>          data->timestamp = *timestamp;
>  
> -- 
> 2.25.1
> 

applied, thanks.


-- 
Thanks,
Limin Wang
_______________________________________________
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] [PATCH] avformat/rtpdec_rfc4175: fix interlace format

[Patrick Keroulas]

In previous state, a new frame was allocated on each timestamp step,
i.e. each frame/field transition. However, for interlace, a new frame
should be allocated on 1st field, completed with the 2nd and finally
freed.

This commit fixes the frame allocation and the detection of missing RTP
markers.

Signed-off-by: Patrick Keroulas <patrick.keroulas@radio-canada.ca>
---
 libavformat/rtpdec_rfc4175.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/libavformat/rtpdec_rfc4175.c b/libavformat/rtpdec_rfc4175.c
index 8e73c07838..83abe499f8 100644
--- a/libavformat/rtpdec_rfc4175.c
+++ b/libavformat/rtpdec_rfc4175.c
@@ -234,20 +234,21 @@ static int rfc4175_handle_packet(AVFormatContext *ctx, PayloadContext *data,
     uint8_t *dest;
 
     if (*timestamp != data->timestamp) {
-        if (data->frame) {
+        if (data->frame && (!data->interlaced || data->field)) {
             /*
-             * if we're here, it means that two RTP packets didn't have the
-             * same timestamp, which is a sign that they were packets from two
-             * different frames, but we didn't get the flag RTP_FLAG_MARKER on
-             * the first one of these frames (last packet of a frame).
-             * Finalize the previous frame anyway by filling the AVPacket.
+             * if we're here, it means that we missed the cue to return
+             * the previous AVPacket, that cue being the RTP_FLAG_MARKER
+             * in the last packet of either the previous frame (progressive)
+             * or the previous second field (interlace). Let's finalize the
+             * previous frame (or pair of fields) anyway by filling the AVPacket.
              */
             av_log(ctx, AV_LOG_ERROR, "Missed previous RTP Marker\n");
             missed_last_packet = 1;
             rfc4175_finalize_packet(data, pkt, st->index);
         }
 
-        data->frame = av_malloc(data->frame_size);
+        if (!data->frame)
+            data->frame = av_malloc(data->frame_size);
 
         data->timestamp = *timestamp;
 
-- 
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".