From patchwork Wed Aug 28 14:55:25 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Kah Goh X-Patchwork-Id: 14751 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id 5EDE944945D for ; Wed, 28 Aug 2019 17:55:37 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 420F568AE52; Wed, 28 Aug 2019 17:55:37 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from sonic314-19.consmr.mail.gq1.yahoo.com (sonic314-19.consmr.mail.gq1.yahoo.com [98.137.69.82]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 71FBE68AD42 for ; Wed, 28 Aug 2019 17:55:30 +0300 (EEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.com.au; s=s2048; t=1567004128; bh=KRXRMrUZWdTR+4X+PvvtmY8yKtp8koQmnur915oUvxI=; h=Date:From:To:In-Reply-To:References:Subject:From:Subject; b=GnfxpO0is5+YcaMnpZjsQyWa/HHodzMZdeJUU0e2zgAcZPNSpRkjCeO2Dmrk+QvxJqHK3i4QfgQ95pD4uokmeVUlWpJlQMO/Ty6pG+WRrzixaQoZOm0yB+UuTTqMS+/K9AfOKls4BrNOfXkVhbYzZEGTaeZVlT58ui9BXyy0hBg0R5+CjjtKjjERlDx0Kc9UzIzAKsKI4xBSeAXcIzDtSvxg+3CpqlzNmwMpzDlQFIDDaLNFSd+MXuHKdqt+X2iAclYma9KbcDAaCtqcGI+/DbyynKeguOilko6wDlQaZXbaYEzMJc7yQloC6wMgq2MPMV2lFDmoZDXBw6LtVOZ6ig== X-YMail-OSG: 0clPlQ8VM1kvbYpcrYaIDlt4nQlZ9ikyjuZ1QSz7DGq2wO69B3lwTMQQpGNBPaE zRVoPOMpM3tCgWHW2j7rbaiIO6ickjgdHEmfiB._jFwjqwnqRLhmQF8fY_Mscn3cLrGUQRJc2kwQ Gq6cqGAzVArkjdPpObLT0sPz3zqNxVCRMl1yb.c4dqqEXnym4yppxwbNSpGYq3G6CdcVB4jLs3kE Pj7RjogjssQKpUYfnjTM9eM__SluYp8nrtiNTTKLzQFKUcHxzKuuXZtF1EyT4NjGP.jiGQj6fZLU s.aPGdjgqgpcwfnX8lYhq65NgHnQnOkUoFkudWyB1Heyet3wOgXNnChhb0.EwgvoWyaxq9Lgieea dKvVTYjG8w5jhfEqHabPJUbqy7VLux5Uyplc465rXw9zMWA1ah9_gj_PbmDfvCclKvlwJw.MgEA2 SvWurf8ZoggeMHfjkrwwSSGp6gDD0AKBPljgsex18yvDyshj5hxQY9wqA7cl8Xm4.wNljiLR9PgV tdEM1tg6o500x5egMUqKWX7cutSTmRSiRFWgK6ASXIsEs7.sWhiryr5Bog20AutGr0AQnjGAuYm2 SRMbpeZLXpf8HEcdLhS4LNgAfVnRcpSRA5mZOKLa5f9FCIBVng2uTfMpOfhRM5BCAvrTO8hrc.kG NCMMtXZ5GBsUVRveWivjGJifzLey9WByc2J83IFIYCDAREdpGIQwoflo0hp68YVz24oSMxHYbTpO N8Er.tbvUh_VYpOVOu36za6TMp5NMXQ.N00ugWi4uv4b2bsH2cvmWbTIoDJNdZJa7B462yms4PCC Qh5__FfUqDIpPxxScIGIC0_G6vEw5m9pUSZM3YQGkUXnMd0EtlA5BlprOr1rW.eL2pPwI5SUQoAI kn_N6_UuCVf5mNAXSUV3SiQEq1bHlMWAJ2cDm.ae7Xx0iXz0N7o_34jZZJ_EHCDoeXZXEtBAQPOx AxKqWvgVbWCriDKau9nFj91.grUkAjp1RLWaj7FIiVFhgXKOZhydiivNGZ9_gdLGy0NhwSYT.PFq sTUI_ebCZhVWhtQOolE4eCpkC2Q.niMilnQzWXeASTOc7D_YveGBl2OSnSeV3knWyLJKe1NsTbbc hSMtG4O.4IzF3AUfRUDD3oudICWryFuNJyZB3gEBGL_piCuE8rWHjIKcnz7zdElGPOYey6at_3JA aX39H5bQp.irDqhtezefQp0OCihznEBxGTIyaZhqOTRdzfmr_sAbvVMWrYLBVWtI3SeE99_C1mQU jDg4oEZlE Received: from sonic.gate.mail.ne1.yahoo.com by sonic314.consmr.mail.gq1.yahoo.com with HTTP; Wed, 28 Aug 2019 14:55:28 +0000 Date: Wed, 28 Aug 2019 14:55:25 +0000 (UTC) From: "villastar@yahoo.com.au" To: FFmpeg development discussions and patches Message-ID: <996971237.1181926.1567004125198@mail.yahoo.com> In-Reply-To: <20190828143310.30558-1-villastar@yahoo.com.au> References: <20190828143310.30558-1-villastar@yahoo.com.au> MIME-Version: 1.0 X-Mailer: WebService/1.1.14253 YMailNorrin Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Firefox/68.0 X-Content-Filtered-By: Mailman/MimeDel 2.1.20 Subject: Re: [FFmpeg-devel] [PATCH] avformat/rtpdec_rfc4175: support non-zero based line numbers X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" On Wednesday, 28 August 2019, 10:33:35 pm AWST, Kah Goh wrote: > There are differing standards that define different starting line > numbers. For example, VSF TR-03 says the line numbers starts at 1, > whereas SMPTE 2110-20. I have just realised the last sentence is incomplete. > > This change fixes the following issues when the line numbering start > at 1: > - The first scan line was being incorrectly interpreted as the second >   scan line. This means the first line in the frame was never being >   populated. > - The last packet for the video frame would be treated as invalid >   because it would have been copied outside of the frame. Consequently, >   the packet would never be "finalized" and the next packet triggers a >   missed RTP marker ("Missed previous RTP marker" would keep being >   logged). > > VSF TR-03: http://www.videoservicesforum.org/download/technical_recommendations/VSF_TR-03_2015-11-12.pdf > > Co-Authored-By: Jacob Siddall > Co-Authored-By: Kah Goh And I have forgotten to sign off on the patch. I'll fix up the patch and resubmit. On Wednesday, 28 August 2019, 10:33:35 pm AWST, Kah Goh wrote: There are differing standards that define different starting line numbers. For example, VSF TR-03 says the line numbers starts at 1, whereas SMPTE 2110-20. This change fixes the following issues when the line numbering start at 1: - The first scan line was being incorrectly interpreted as the second   scan line. This means the first line in the frame was never being   populated. - The last packet for the video frame would be treated as invalid   because it would have been copied outside of the frame. Consequently,   the packet would never be "finalized" and the next packet triggers a   missed RTP marker ("Missed previous RTP marker" would keep being   logged). VSF TR-03: http://www.videoservicesforum.org/download/technical_recommendations/VSF_TR-03_2015-11-12.pdf Co-Authored-By: Jacob Siddall Co-Authored-By: Kah Goh --- libavformat/rtpdec_rfc4175.c | 41 +++++++++++++++++++++++++++++++++--- 1 file changed, 38 insertions(+), 3 deletions(-) diff --git a/libavformat/rtpdec_rfc4175.c b/libavformat/rtpdec_rfc4175.c index e9c62c1389..427d4b31e2 100644 --- a/libavformat/rtpdec_rfc4175.c +++ b/libavformat/rtpdec_rfc4175.c @@ -25,6 +25,7 @@ #include "rtpdec_formats.h" #include "libavutil/avstring.h" #include "libavutil/pixdesc.h" +#include struct PayloadContext {     char *sampling; @@ -37,6 +38,12 @@ struct PayloadContext {     unsigned int pgroup; /* size of the pixel group in bytes */     unsigned int xinc; +    /* The line number of the first line in the frame (usually either 0 or 1). */ +    int first_line_number; + +    /* This is set to true once the first line number is confirmed. */ +    bool first_line_number_known; +     uint32_t timestamp; }; @@ -136,6 +143,13 @@ static int rfc4175_finalize_packet(PayloadContext *data, AVPacket *pkt,     return ret; } +static int rfc4175_initialize(AVFormatContext *s, int st_index, PayloadContext *data) +{ +    data->first_line_number = 0; +    data->first_line_number_known = false; +    return 0; +} + static int rfc4175_handle_packet(AVFormatContext *ctx, PayloadContext *data,                                   AVStream *st, AVPacket *pkt, uint32_t *timestamp,                                   const uint8_t * buf, int len, @@ -199,6 +213,11 @@ static int rfc4175_handle_packet(AVFormatContext *ctx, PayloadContext *data,         cont = headers[4] & 0x80;         headers += 6; +        if (line == 0) { +            data->first_line_number = 0; +            data->first_line_number_known = true; +        } +         if (length % data->pgroup)             return AVERROR_INVALIDDATA; @@ -206,9 +225,15 @@ static int rfc4175_handle_packet(AVFormatContext *ctx, PayloadContext *data,             length = payload_len;         /* prevent ill-formed packets to write after buffer's end */ -        copy_offset = (line * data->width + offset) * data->pgroup / data->xinc; -        if (copy_offset + length > data->frame_size) -            return AVERROR_INVALIDDATA; +        copy_offset = ((line - data->first_line_number) * data->width + offset) * data->pgroup / data->xinc; +        if (copy_offset + length > data->frame_size) { +            if (data->first_line_number_known) +                return AVERROR_INVALIDDATA; + +            // This would happen if the line numbering is 1 based. We still need to check for the RTP flag +            // marker (as per after the while loop). +            break; +        }         dest = data->frame + copy_offset;         memcpy(dest, payload, length); @@ -218,6 +243,15 @@ static int rfc4175_handle_packet(AVFormatContext *ctx, PayloadContext *data,     } while (cont);     if ((flags & RTP_FLAG_MARKER)) { +        if (!data->first_line_number_known) { +            data->first_line_number = line - data->height + 1; +            if (data->first_line_number < 0) { +                // This could happen if the frame does not fill up the entire height. +                data->first_line_number = 0; +                av_log(ctx, AV_LOG_WARNING, "Video frame does not fill entire height"); +            } +            data->first_line_number_known = true; +        }         return rfc4175_finalize_packet(data, pkt, st->index);     } else if (missed_last_packet) {         return 0; @@ -232,5 +266,6 @@ const RTPDynamicProtocolHandler ff_rfc4175_rtp_handler = {     .codec_id          = AV_CODEC_ID_BITPACKED,     .priv_data_size    = sizeof(PayloadContext),     .parse_sdp_a_line  = rfc4175_parse_sdp_line, +    .init              = rfc4175_initialize,     .parse_packet      = rfc4175_handle_packet, };