[FFmpeg-devel] [PATCH 0/5] Add passthrough support for SCTE-35

[FFmpeg-devel] [PATCH 0/5] Add passthrough support for SCTE-35

[Devin Heitmueller]

Properly set up the MPEG-TS mux and recalculate the pts_adjust field
in SCTE_35 packets, such that a user can transparently pass through
SCTE-35 streams when both the input and output are MPEG-TS.

Devin Heitmueller (5):
  avcodec: Add new side data type to contain original PTS value
  mpegts: Stash original PTS for SCTE-35 sections for processing later
  mpegtsenc: Add support for output of SCTE-35 streams over TS
  bsf: Add new bitstream filter to set pts_adjustment when reclocking
  mpegtsenc: Don't periodically announce PCR on SCTE-35 streams

 libavcodec/Makefile              |   1 +
 libavcodec/bitstream_filters.c   |   1 +
 libavcodec/packet.h              |  10 ++++
 libavcodec/scte35ptsadjust_bsf.c | 114 +++++++++++++++++++++++++++++++++++++++
 libavformat/mpegts.c             |   9 +++-
 libavformat/mpegts.h             |   1 +
 libavformat/mpegtsenc.c          |  78 +++++++++++++++++++++++++--
 libavformat/mux.c                |   6 ++-
 8 files changed, 212 insertions(+), 8 deletions(-)
 create mode 100644 libavcodec/scte35ptsadjust_bsf.c

-- 
1.8.3.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] [PATCH 1/5] avcodec: Add new side data type to contain original PTS value

[Devin Heitmueller]

In order to properly process SCTE-35 packets, we need the original
PTS value from the demux (i.e. not mangled by the application or
reclocked for the output).  This allows us to set the pts_adjustment
field in an BSF on the output side.

Introduce a new side data type to store the original PTS.

Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com>
---
 libavcodec/packet.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/libavcodec/packet.h b/libavcodec/packet.h
index f28e7e7..a86a550 100644
--- a/libavcodec/packet.h
+++ b/libavcodec/packet.h
@@ -300,6 +300,16 @@ enum AVPacketSideDataType {
     AV_PKT_DATA_DYNAMIC_HDR10_PLUS,
 
     /**
+     * Provides the original PTS when passed through the demux.  This can
+     * be used to offset any subsequent changes made by the caller to
+     * adjust PTS values (such as pts_offset).  We need this for SCTE-35,
+     * since by the time the packets reach the output the PTS values have
+     * already been re-written, and we cannot calculate pre-roll values
+     * using the PTS values embedded in the packet content
+     */
+    AV_PKT_DATA_ORIG_PTS,
+
+    /**
      * The number of side data types.
      * This is not part of the public API/ABI in the sense that it may
      * change when new side data types are added.
-- 
1.8.3.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".

Re: [FFmpeg-devel] [PATCH 1/5] avcodec: Add new side data type to contain original PTS value

[Andreas Rheinhardt]

Devin Heitmueller:
> In order to properly process SCTE-35 packets, we need the original
> PTS value from the demux (i.e. not mangled by the application or
> reclocked for the output).  This allows us to set the pts_adjustment
> field in an BSF on the output side.
> 
> Introduce a new side data type to store the original PTS.
> 
> Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com>
> ---
>  libavcodec/packet.h | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/libavcodec/packet.h b/libavcodec/packet.h
> index f28e7e7..a86a550 100644
> --- a/libavcodec/packet.h
> +++ b/libavcodec/packet.h
> @@ -300,6 +300,16 @@ enum AVPacketSideDataType {
>      AV_PKT_DATA_DYNAMIC_HDR10_PLUS,
>  
>      /**
> +     * Provides the original PTS when passed through the demux.  This can
> +     * be used to offset any subsequent changes made by the caller to
> +     * adjust PTS values (such as pts_offset).  We need this for SCTE-35,
> +     * since by the time the packets reach the output the PTS values have
> +     * already been re-written, and we cannot calculate pre-roll values
> +     * using the PTS values embedded in the packet content
> +     */
> +    AV_PKT_DATA_ORIG_PTS,
> +
> +    /**
>       * The number of side data types.
>       * This is not part of the public API/ABI in the sense that it may
>       * change when new side data types are added.

A timestamp without a timebase? Doesn't sound good to me. And it also
seems quite hacky.
Apart from that: It needs to specify that the data is a int64_t.

- Andreas

_______________________________________________
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 1/5] avcodec: Add new side data type to contain original PTS value

[Devin Heitmueller]

Hi Andreas,

Thanks for the feedback.  I put out an RFC back in March but got no comments.

On Fri, Jun 16, 2023 at 6:01 PM Andreas Rheinhardt
<andreas.rheinhardt@outlook.com> wrote:
> A timestamp without a timebase? Doesn't sound good to me. And it also
> seems quite hacky.
> Apart from that: It needs to specify that the data is a int64_t.

So you're suggesting a struct that contains both the timestamp and a
timebase?  I don't have any real objection to this.

I agree it seems hacky, but don't have a better solution.  I welcome
constructive suggestions.  I had considered using an AVPacket metadata
field rather than a new side data type (as that won't necessarily lock
us into a new side data type that we would have to support), and the
functionality is really specific to one use case.  However I figured
side data might be better since it avoids the conversion of the PTS to
a string and back.

Devin

-- 
Devin Heitmueller, Senior Software Engineer
LTN Global Communications
o: +1 (301) 363-1001
w: https://ltnglobal.com  e: devin.heitmueller@ltnglobal.com
_______________________________________________
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 1/5] avcodec: Add new side data type to contain original PTS value

[Anton Khirnov]

Quoting Devin Heitmueller (2023-06-19 15:14:38)
> Hi Andreas,
> 
> Thanks for the feedback.  I put out an RFC back in March but got no comments.
> 
> On Fri, Jun 16, 2023 at 6:01 PM Andreas Rheinhardt
> <andreas.rheinhardt@outlook.com> wrote:
> > A timestamp without a timebase? Doesn't sound good to me. And it also
> > seems quite hacky.
> > Apart from that: It needs to specify that the data is a int64_t.
> 
> So you're suggesting a struct that contains both the timestamp and a
> timebase?  I don't have any real objection to this.
> 
> I agree it seems hacky, but don't have a better solution.  I welcome
> constructive suggestions.  I had considered using an AVPacket metadata
> field rather than a new side data type (as that won't necessarily lock
> us into a new side data type that we would have to support),

This reasoning is backwards IMO. You'd be creating an implicit API while
pretending not to. If anyone uses it, they would expect stability,
otherwise what's the point.

> functionality is really specific to one use case.  However I figured
> side data might be better since it avoids the conversion of the PTS to
> a string and back.

One one hand it does feel hacky, on the other I can see valid uses for
it. Though 'orig' is rather vague, I'd call it something like 'transport
timestamp' instead. A timebase should definitely be bundled with it.

-- 
Anton Khirnov
_______________________________________________
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 1/5] avcodec: Add new side data type to contain original PTS value

[Devin Heitmueller]

On Mon, Jun 19, 2023 at 10:32 AM Anton Khirnov <anton@khirnov.net> wrote:
> This reasoning is backwards IMO. You'd be creating an implicit API while
> pretending not to. If anyone uses it, they would expect stability,
> otherwise what's the point.

My concern was that given right now it only has a single use case that
we might not have thought of other ways it may be used in the future,
so using a metadata field might allow us to keep it "semi-private"
until others wanted the functionality.  My fear was that we wouldn't
be able to reach a consensus on a new public API definition that we
would be willing to support indefinitely.

> > functionality is really specific to one use case.  However I figured
> > side data might be better since it avoids the conversion of the PTS to
> > a string and back.
>
> One one hand it does feel hacky, on the other I can see valid uses for
> it. Though 'orig' is rather vague, I'd call it something like 'transport
> timestamp' instead. A timebase should definitely be bundled with it.

I don't feel strongly about the naming.  My goal was to convey that
this was the original input timestamp on arrival, and not something
that has been transformed, and I worry that "transport" might be a bit
vague as it isn't clear whether this was on input, output, or
somewhere else in the workflow.  That said, pick a name that will be
considered acceptable to be merged upstream, and I'll adjust the patch
accordingly.

Devin

-- 
Devin Heitmueller, Senior Software Engineer
LTN Global Communications
o: +1 (301) 363-1001
w: https://ltnglobal.com  e: devin.heitmueller@ltnglobal.com
_______________________________________________
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 2/5] mpegts: Stash original PTS for SCTE-35 sections for processing later

[Devin Heitmueller]

We need the original PTS value in order to do subsequent processing,
so set it as packet side data.

Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com>
---
 libavformat/mpegts.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
index 385d78b..b8f1d7d 100644
--- a/libavformat/mpegts.c
+++ b/libavformat/mpegts.c
@@ -1782,8 +1782,15 @@ static void scte_data_cb(MpegTSFilter *filter, const uint8_t *section,
     prg = av_find_program_from_stream(ts->stream, NULL, idx);
     if (prg && prg->pcr_pid != -1 && prg->discard != AVDISCARD_ALL) {
         MpegTSFilter *f = ts->pids[prg->pcr_pid];
-        if (f && f->last_pcr != -1)
+        if (f && f->last_pcr != -1) {
+            int64_t *orig_pts;
             ts->pkt->pts = ts->pkt->dts = f->last_pcr/300;
+            orig_pts = (int64_t *) av_packet_new_side_data(ts->pkt,
+                                                           AV_PKT_DATA_ORIG_PTS,
+                                                           sizeof(int64_t));
+            if (orig_pts)
+                *orig_pts = ts->pkt->pts;
+        }
     }
     ts->stop_parse = 1;
 
-- 
1.8.3.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] [PATCH 3/5] mpegtsenc: Add support for output of SCTE-35 streams over TS

[Devin Heitmueller]

Introduce the ability to pass through SCTE-35 packets when creating
MPEG transport streams.  Note that this patch makes no effort to
change the PTS values in the SCTE-35 packets, and thus only works
when not reclocking the stream (i.e. using -copyts).  A subsequent
patch includes a BSF to recompute the PTS values.

Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com>
---
 libavformat/mpegts.h    |  1 +
 libavformat/mpegtsenc.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++---
 libavformat/mux.c       |  6 ++--
 3 files changed, 75 insertions(+), 6 deletions(-)

diff --git a/libavformat/mpegts.h b/libavformat/mpegts.h
index a48f14e..a7aaaba 100644
--- a/libavformat/mpegts.h
+++ b/libavformat/mpegts.h
@@ -137,6 +137,7 @@
 #define STREAM_TYPE_AUDIO_AC3       0x81
 #define STREAM_TYPE_AUDIO_DTS       0x82
 #define STREAM_TYPE_AUDIO_TRUEHD    0x83
+#define STREAM_TYPE_SCTE_35         0x86
 #define STREAM_TYPE_AUDIO_EAC3      0x87
 
 /* ISO/IEC 13818-1 Table 2-22 */
diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
index 700fc54..c6cd1fd 100644
--- a/libavformat/mpegtsenc.c
+++ b/libavformat/mpegtsenc.c
@@ -425,6 +425,9 @@ static int get_dvb_stream_type(AVFormatContext *s, AVStream *st)
     case AV_CODEC_ID_SMPTE_2038:
         stream_type = STREAM_TYPE_PRIVATE_DATA;
         break;
+    case AV_CODEC_ID_SCTE_35:
+        stream_type = STREAM_TYPE_SCTE_35;
+        break;
     case AV_CODEC_ID_DVB_SUBTITLE:
     case AV_CODEC_ID_DVB_TELETEXT:
     case AV_CODEC_ID_ARIB_CAPTION:
@@ -522,6 +525,16 @@ static int mpegts_write_pmt(AVFormatContext *s, MpegTSService *service)
         *q++ = 0xfc;        // private_data_byte
     }
 
+    /* If there is an SCTE-35 stream, we need a registration descriptor
+       at the program level (SCTE 35 2016 Sec 8.1) */
+    for (i = 0; i < s->nb_streams; i++) {
+        AVStream *st = s->streams[i];
+        if (st->codecpar->codec_id == AV_CODEC_ID_SCTE_35) {
+            put_registration_descriptor(&q, MKTAG('C', 'U', 'E', 'I'));
+            break;
+        }
+    }
+
     val = 0xf000 | (q - program_info_length_ptr - 2);
     program_info_length_ptr[0] = val >> 8;
     program_info_length_ptr[1] = val;
@@ -533,6 +546,14 @@ static int mpegts_write_pmt(AVFormatContext *s, MpegTSService *service)
         const char default_language[] = "und";
         const char *language = lang && strlen(lang->value) >= 3 ? lang->value : default_language;
         enum AVCodecID codec_id = st->codecpar->codec_id;
+        uint16_t pid;
+
+        if (st->codecpar->codec_id == AV_CODEC_ID_SCTE_35) {
+            MpegTSSection *sect = st->priv_data;
+            pid = sect->pid;
+        } else {
+            pid = ts_st->pid;
+        }
 
         if (s->nb_programs) {
             int k, found = 0;
@@ -556,7 +577,7 @@ static int mpegts_write_pmt(AVFormatContext *s, MpegTSService *service)
         stream_type = ts->m2ts_mode ? get_m2ts_stream_type(s, st) : get_dvb_stream_type(s, st);
 
         *q++ = stream_type;
-        put16(&q, 0xe000 | ts_st->pid);
+        put16(&q, 0xe000 | pid);
         desc_length_ptr = q;
         q += 2; /* patched after */
 
@@ -819,6 +840,10 @@ static int mpegts_write_pmt(AVFormatContext *s, MpegTSService *service)
                 putbuf(&q, tag, strlen(tag));
                 *q++ = 0;            /* metadata service ID */
                 *q++ = 0xF;          /* metadata_locator_record_flag|MPEG_carriage_flags|reserved */
+            } else if (st->codecpar->codec_id == AV_CODEC_ID_SCTE_35) {
+                *q++ = 0x8a; /* Cue Identifier Descriptor */
+                *q++ = 0x01; /* length */
+                *q++ = 0x01; /* Cue Stream Type (see Sec 8.2) */
             }
             break;
         }
@@ -1159,6 +1184,33 @@ static int mpegts_init(AVFormatContext *s)
         AVStream *st = s->streams[i];
         MpegTSWriteStream *ts_st;
 
+        if (st->codecpar->codec_id == AV_CODEC_ID_SCTE_35) {
+            struct MpegTSSection *sect;
+            sect = av_mallocz(sizeof(MpegTSSection));
+            if (!sect) {
+                ret = AVERROR(ENOMEM);
+                continue;
+            }
+
+            if (st->id < 16) {
+                sect->pid = ts->start_pid + i;
+            } else if (st->id < 0x1FFF) {
+                sect->pid = st->id;
+            } else {
+                av_log(s, AV_LOG_ERROR,
+                       "Invalid stream id %d, must be less than 8191\n", st->id);
+                ret = AVERROR(EINVAL);
+                continue;
+            }
+
+            sect->write_packet = section_write_packet;
+            sect->opaque       = s;
+            sect->cc           = 15;
+            sect->discontinuity= ts->flags & MPEGTS_FLAG_DISCONT;
+            st->priv_data = sect;
+            continue;
+        }
+
         ts_st = av_mallocz(sizeof(MpegTSWriteStream));
         if (!ts_st) {
             return AVERROR(ENOMEM);
@@ -1877,6 +1929,19 @@ static int mpegts_write_packet_internal(AVFormatContext *s, AVPacket *pkt)
             dts += delay;
     }
 
+    if (st->codecpar->codec_id == AV_CODEC_ID_SCTE_35) {
+        MpegTSSection *s = st->priv_data;
+        uint8_t data[SECTION_LENGTH];
+
+        if (size > SECTION_LENGTH) {
+            av_log(s, AV_LOG_ERROR, "SCTE-35 section too long\n");
+            return AVERROR_INVALIDDATA;
+        }
+        memcpy(data, buf, size);
+        mpegts_write_section(s, data, size);
+        return 0;
+    }
+
     if (!ts_st->first_timestamp_checked && (pts == AV_NOPTS_VALUE || dts == AV_NOPTS_VALUE)) {
         av_log(s, AV_LOG_ERROR, "first pts and dts value must be set\n");
         return AVERROR_INVALIDDATA;
@@ -2149,7 +2214,8 @@ static int mpegts_write_packet_internal(AVFormatContext *s, AVPacket *pkt)
         return 0;
     }
 
-    if (ts_st->payload_size && (ts_st->payload_size + size > ts->pes_payload_size ||
+    if (st->codecpar->codec_id != AV_CODEC_ID_SCTE_35 &&
+        ts_st->payload_size && (ts_st->payload_size + size > ts->pes_payload_size ||
         (dts != AV_NOPTS_VALUE && ts_st->payload_dts != AV_NOPTS_VALUE &&
          dts - ts_st->payload_dts >= max_audio_delay) ||
         ts_st->opus_queued_samples + opus_samples >= 5760 /* 120ms */)) {
@@ -2194,7 +2260,7 @@ static void mpegts_write_flush(AVFormatContext *s)
     for (i = 0; i < s->nb_streams; i++) {
         AVStream *st = s->streams[i];
         MpegTSWriteStream *ts_st = st->priv_data;
-        if (ts_st->payload_size > 0) {
+        if (st->codecpar->codec_id != AV_CODEC_ID_SCTE_35 && ts_st->payload_size > 0) {
             mpegts_write_pes(s, st, ts_st->payload, ts_st->payload_size,
                              ts_st->payload_pts, ts_st->payload_dts,
                              ts_st->payload_flags & AV_PKT_FLAG_KEY, -1);
@@ -2237,7 +2303,7 @@ static void mpegts_deinit(AVFormatContext *s)
     for (i = 0; i < s->nb_streams; i++) {
         AVStream *st = s->streams[i];
         MpegTSWriteStream *ts_st = st->priv_data;
-        if (ts_st) {
+        if (ts_st && st->codecpar->codec_id != AV_CODEC_ID_SCTE_35) {
             av_freep(&ts_st->dvb_ac3_desc);
             av_freep(&ts_st->payload);
             if (ts_st->amux) {
diff --git a/libavformat/mux.c b/libavformat/mux.c
index 415bd39..55bec25 100644
--- a/libavformat/mux.c
+++ b/libavformat/mux.c
@@ -307,7 +307,8 @@ FF_ENABLE_DEPRECATION_WARNINGS
         }
 
         if (par->codec_type != AVMEDIA_TYPE_ATTACHMENT &&
-            par->codec_id != AV_CODEC_ID_SMPTE_2038)
+            par->codec_id != AV_CODEC_ID_SMPTE_2038 &&
+            par->codec_id != AV_CODEC_ID_SCTE_35)
             si->nb_interleaved_streams++;
     }
     si->interleave_packet = of->interleave_packet;
@@ -946,7 +947,8 @@ int ff_interleave_packet_per_dts(AVFormatContext *s, AVPacket *pkt,
         } else if (par->codec_type != AVMEDIA_TYPE_ATTACHMENT &&
                    par->codec_id != AV_CODEC_ID_VP8 &&
                    par->codec_id != AV_CODEC_ID_VP9 &&
-                   par->codec_id != AV_CODEC_ID_SMPTE_2038) {
+                   par->codec_id != AV_CODEC_ID_SMPTE_2038 &&
+                   par->codec_id != AV_CODEC_ID_SCTE_35) {
             ++noninterleaved_count;
         }
     }
-- 
1.8.3.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] [PATCH 4/5] bsf: Add new bitstream filter to set pts_adjustment when reclocking

[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@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@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"
+#include "bsf.h"
+#include "bsf_internal.h"
+
+static int scte35ptsadjust_filter(AVBSFContext *ctx, AVPacket *out)
+{
+    AVPacket *in;
+    size_t size;
+    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)
+        goto fail;
+
+    /* 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);
+
+    /* 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);
+
+    /* 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;
+
+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;
-- 
1.8.3.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".

Re: [FFmpeg-devel] [PATCH 4/5] bsf: Add new bitstream filter to set pts_adjustment when reclocking

[Andreas Rheinhardt]

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@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@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@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 4/5] bsf: Add new bitstream filter to set pts_adjustment when reclocking

[Devin Heitmueller]

On Fri, Jun 16, 2023 at 5:58 PM Andreas Rheinhardt
<andreas.rheinhardt@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@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@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@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@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@ltnglobal.com
_______________________________________________
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 5/5] mpegtsenc: Don't periodically announce PCR on SCTE-35 streams

[Devin Heitmueller]

Changes were made between in the last two years to periodically
send PCR-only packets on all PIDs, but for cases where the stream
may send packets very infrequently (like SCTE-35), this results in
extra TR101290 errors because it fails the PCR interval test.

I am not quite sure what the "right" fix should be for this, but
for now just disable all periodic sending of PCR-only packets on
SCTE-35 streams.

Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com>
---
 libavformat/mpegtsenc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
index 48d7833..728057e 100644
--- a/libavformat/mpegtsenc.c
+++ b/libavformat/mpegtsenc.c
@@ -1579,7 +1579,7 @@ static void mpegts_write_pes(AVFormatContext *s, AVStream *st,
                     int st2_index = i < st->index ? i : (i + 1 == s->nb_streams ? st->index : i + 1);
                     AVStream *st2 = s->streams[st2_index];
                     MpegTSWriteStream *ts_st2 = st2->priv_data;
-                    if (ts_st2->pcr_period) {
+                    if (ts_st2->pcr_period && st2->codecpar->codec_id != AV_CODEC_ID_SCTE_35) {
                         if (pcr - ts_st2->last_pcr >= ts_st2->pcr_period) {
                             ts_st2->last_pcr = FFMAX(pcr - ts_st2->pcr_period, ts_st2->last_pcr + ts_st2->pcr_period);
                             if (st2 != st) {
-- 
1.8.3.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".