[FFmpeg-devel] [PATCH 4/5] bsf: Add new bitstream filter to set pts_adjustment when reclocking
Devin Heitmueller
devin.heitmueller at ltnglobal.com
Mon Jun 19 16:15:38 EEST 2023
On Fri, Jun 16, 2023 at 5:58 PM Andreas Rheinhardt
<andreas.rheinhardt at outlook.com> wrote:
>
> Devin Heitmueller:
> > Because SCTE-35 messages are represented in TS streams as sections
> > rather than PES packets, we cannot rely on ffmpeg's standard
> > mechanisms to adjust PTS values if reclocking the stream.
> > This filter will leverage the SCTE-35 pts_adjust field to
> > compensate for any change in the PTS values in the stream.
> >
> > See SCTE-35 2019 Sec 9.6 for information about the use of
> > the pts_adjust field.
> >
> > This filter also tweaks the mpegtsenc mux to automatically
> > add it so the user doesn't have to include it manually.
> >
> > Signed-off-by: Devin Heitmueller <dheitmueller at ltnglobal.com>
> > ---
> > libavcodec/Makefile | 1 +
> > libavcodec/bitstream_filters.c | 1 +
> > libavcodec/scte35ptsadjust_bsf.c | 114 +++++++++++++++++++++++++++++++++++++++
> > libavformat/mpegtsenc.c | 2 +
> > 4 files changed, 118 insertions(+)
> > create mode 100644 libavcodec/scte35ptsadjust_bsf.c
> >
> > diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> > index 0ce8fe5..6944c82 100644
> > --- a/libavcodec/Makefile
> > +++ b/libavcodec/Makefile
> > @@ -1250,6 +1250,7 @@ OBJS-$(CONFIG_PCM_RECHUNK_BSF) += pcm_rechunk_bsf.o
> > OBJS-$(CONFIG_PGS_FRAME_MERGE_BSF) += pgs_frame_merge_bsf.o
> > OBJS-$(CONFIG_PRORES_METADATA_BSF) += prores_metadata_bsf.o
> > OBJS-$(CONFIG_REMOVE_EXTRADATA_BSF) += remove_extradata_bsf.o av1_parse.o
> > +OBJS-$(CONFIG_SCTE35PTSADJUST_BSF) += scte35ptsadjust_bsf.o
> > OBJS-$(CONFIG_SETTS_BSF) += setts_bsf.o
> > OBJS-$(CONFIG_TEXT2MOVSUB_BSF) += movsub_bsf.o
> > OBJS-$(CONFIG_TRACE_HEADERS_BSF) += trace_headers_bsf.o
> > diff --git a/libavcodec/bitstream_filters.c b/libavcodec/bitstream_filters.c
> > index 7512fcc..d30dfbd 100644
> > --- a/libavcodec/bitstream_filters.c
> > +++ b/libavcodec/bitstream_filters.c
> > @@ -57,6 +57,7 @@ extern const FFBitStreamFilter ff_pcm_rechunk_bsf;
> > extern const FFBitStreamFilter ff_pgs_frame_merge_bsf;
> > extern const FFBitStreamFilter ff_prores_metadata_bsf;
> > extern const FFBitStreamFilter ff_remove_extradata_bsf;
> > +extern const FFBitStreamFilter ff_scte35ptsadjust_bsf;
> > extern const FFBitStreamFilter ff_setts_bsf;
> > extern const FFBitStreamFilter ff_text2movsub_bsf;
> > extern const FFBitStreamFilter ff_trace_headers_bsf;
> > diff --git a/libavcodec/scte35ptsadjust_bsf.c b/libavcodec/scte35ptsadjust_bsf.c
> > new file mode 100644
> > index 0000000..e6e9ec9
> > --- /dev/null
> > +++ b/libavcodec/scte35ptsadjust_bsf.c
> > @@ -0,0 +1,114 @@
> > +/*
> > + * SCTE-35 PTS fixup bitstream filter
> > + * Copyright (c) 2023 LTN Global Communications, Inc.
> > + *
> > + * Author: Devin Heitmueller <dheitmueller at ltnglobal.com>
> > + *
> > + * Because SCTE-35 messages are represented in TS streams as sections
> > + * rather than PES packets, we cannot rely on ffmpeg's standard
> > + * mechanisms to adjust PTS values if reclocking the stream.
> > + * This filter will leverage the SCTE-35 pts_adjust field to
> > + * compensate for any change in the PTS values in the stream.
> > + *
> > + * See SCTE-35 2019 Sec 9.6 for information about the use of
> > + * the pts_adjust field.
> > + *
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * FFmpeg is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with FFmpeg; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> > + */
> > +
> > +#include "avcodec.h"
>
> What is that for? The BSF API is separate from the AVCodecContext stuff.
>
> > +#include "bsf.h"
> > +#include "bsf_internal.h"
> > +
> > +static int scte35ptsadjust_filter(AVBSFContext *ctx, AVPacket *out)
> > +{
> > + AVPacket *in;
> > + size_t size;
>
> av_packet_get_side_data() can work with NULL for the size pointer.
>
> > + const int64_t *orig_pts;
> > + int64_t cur_pts_adjust;
> > + int ret = 0;
> > +
> > + ret = ff_bsf_get_packet(ctx, &in);
> > + if (ret < 0)
> > + return ret;
> > +
> > + /* Retrieve the original PTS, which will be used to calculate the pts_adjust */
> > + orig_pts = (int64_t *) av_packet_get_side_data(in, AV_PKT_DATA_ORIG_PTS, &size);
> > + if (orig_pts == NULL) {
> > + /* No original PTS specified, so just pass the packet through */
> > + av_packet_move_ref(out, in);
> > + av_packet_free(&in);
> > + return 0;
> > + }
> > +
> > + av_log(ctx, AV_LOG_DEBUG, "%s pts=%" PRId64 " orig_pts=%" PRId64 "\n", __func__,
> > + in->pts, *orig_pts);
> > +
> > + /* The pts_adjust field is logically buf[4]-buf[8] of the payload */
> > + if (in->size < 8)
>
> You are reading data[8], so size should be at least 9.
>
> > + goto fail;
>
> This will return 0 instead of indicating an error.
> (But this is irrelevant, see below.)
>
> > +
> > + /* Extract the current pts_adjust value from the packet */
> > + cur_pts_adjust = ((int64_t) in->data[4] & (int64_t) 0x01 << 32) |
> > + ((int64_t) in->data[5] << 24) |
> > + ((int64_t) in->data[6] << 16) |
> > + ((int64_t) in->data[7] << 8) |
> > + ((int64_t) in->data[8] );
> > +
> > + av_log(ctx, AV_LOG_DEBUG, "%s pts_adjust=%" PRId64 "\n", __func__,
> > + cur_pts_adjust);
> > +
> > + /* Compute the new PTS adjust value */
> > + cur_pts_adjust -= *orig_pts;
> > + cur_pts_adjust += in->pts;
> > + cur_pts_adjust &= (int64_t) 0x1ffffffff;
> > +
> > + av_log(ctx, AV_LOG_DEBUG, "%s new pts_adjust=%" PRId64 "\n", __func__,
> > + cur_pts_adjust);
>
> We typically don't add __func__, because the actual information where
> the log comes from is contained in the logcontext. Furthermore, you
> could combine these two av_log().
>
> > +
> > + /* Clone the incoming packet since we need to modify it */
> > + ret = av_new_packet(out, in->size);
> > + if (ret < 0)
> > + goto fail;
> > + ret = av_packet_copy_props(out, in);
> > + if (ret < 0)
> > + goto fail;
> > + memcpy(out->data, in->data, in->size);
>
> Just use av_packet_make_writable() instead of this; use it in
> conjunction with ff_bsf_get_packet_ref(), avoiding the second packet. It
> also avoids allocations and copies in case the packet's data is already
> writable and avoids copying side-data etc.
>
> > +
> > + /* Insert the updated pts_adjust value */
> > + out->data[4] &= 0xfe; /* Preserve top 7 unrelated bits */
> > + out->data[4] |= cur_pts_adjust >> 32;
> > + out->data[5] = cur_pts_adjust >> 24;
> > + out->data[6] = cur_pts_adjust >> 16;
> > + out->data[7] = cur_pts_adjust >> 8;
> > + out->data[8] = cur_pts_adjust;
>
> The last four could be simplified to AV_WB32(out_data[5],
> (uint32_t)cur_pts_adjust);
>
> > +
> > +fail:
> > + av_packet_free(&in);
> > +
> > + return ret;
> > +}
> > +
> > +static const enum AVCodecID codec_ids[] = {
> > + AV_CODEC_ID_SCTE_35, AV_CODEC_ID_NONE,
> > +};
> > +
> > +const FFBitStreamFilter ff_scte35ptsadjust_bsf = {
> > + .p.name = "scte35ptsadjust",
> > + .p.codec_ids = codec_ids,
> > + .filter = scte35ptsadjust_filter,
> > +};
> > diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
> > index c6cd1fd..48d7833 100644
> > --- a/libavformat/mpegtsenc.c
> > +++ b/libavformat/mpegtsenc.c
> > @@ -2337,6 +2337,8 @@ static int mpegts_check_bitstream(AVFormatContext *s, AVStream *st,
> > (st->codecpar->extradata_size > 0 &&
> > st->codecpar->extradata[0] == 1)))
> > ret = ff_stream_add_bitstream_filter(st, "hevc_mp4toannexb", NULL);
> > + } else if (st->codecpar->codec_id == AV_CODEC_ID_SCTE_35) {
> > + ret = ff_stream_add_bitstream_filter(st, "scte35ptsadjust", NULL);
> > }
> >
> > return ret;
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
Thanks for reviewing. These all seem like reasonable comments and I
will incorporate them into the next patch series.
Regards,
Devin
--
Devin Heitmueller, Senior Software Engineer
LTN Global Communications
o: +1 (301) 363-1001
w: https://ltnglobal.com e: devin.heitmueller at ltnglobal.com
More information about the ffmpeg-devel
mailing list