[FFmpeg-devel] [PATCH v3] avformat/rtpdec_rfc4175: support non-zero based line numbers
Paul B Mahol
onemda at gmail.com
Sun Jan 19 14:37:40 EET 2020
Ping those people directly instead.
On 11/7/19, Michael Niedermayer <michael at niedermayer.cc> wrote:
> On Thu, Nov 07, 2019 at 10:27:31PM +0800, Kah Goh wrote:
>> On Mon, Sep 30, 2019 at 09:27:20PM +0800, 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 says it should start at 0.
>> >
>> > This change adds support for non-zero based line numbers and addresses
>> > the following issues when it starts 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 <kobe at live.com.au>
>> > Co-Authored-By: Kah Goh <villastar at yahoo.com.au>
>> > Signed-off-by: Kah Goh <villastar at yahoo.com.au>
>> > ---
>> > libavformat/rtpdec_rfc4175.c | 49 +++++++++++++++++++++++++++++++++---
>> > 1 file changed, 45 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/libavformat/rtpdec_rfc4175.c
>> > b/libavformat/rtpdec_rfc4175.c
>> > index e9c62c1389..47d5d23dd6 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 <stdbool.h>
>> >
>> > 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,
>> > @@ -188,7 +202,7 @@ static int rfc4175_handle_packet(AVFormatContext
>> > *ctx, PayloadContext *data,
>> >
>> > /* and now iterate over every scan lines */
>> > do {
>> > - int copy_offset;
>> > + int copy_offset, copy_to_line;
>> >
>> > if (payload_len < data->pgroup)
>> > return AVERROR_INVALIDDATA;
>> > @@ -199,17 +213,34 @@ 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;
>> >
>> > if (length > payload_len)
>> > 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)
>> > + copy_to_line = line - data->first_line_number;
>> > + if (copy_to_line < 0)
>> > + /* This means the first line number we have calculated is
>> > too large, which indicates that we
>> > + may have received some bad data. */
>> > return AVERROR_INVALIDDATA;
>> >
>> > + /* prevent ill-formed packets to write after buffer's end */
>> > + copy_offset = (copy_to_line * 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 +249,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 +272,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,
>> > };
>> > --
>> > 2.23.0
>> >
>>
>> Ping
>
> I see there are multiple People working on this and others who worked
> previously on this. Can one of the people working on this code
> please review this ?
> Iam happy to apply it if its reviewed (assuming noone spots anything
> bad of course)
>
> Thanks
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> He who knows, does not speak. He who speaks, does not know. -- Lao Tsu
>
More information about the ffmpeg-devel
mailing list