[FFmpeg-devel] [PATCH 1/3] mpegtsenc: Allow user to set SDT retransmission period

[Michael Niedermayer]

On Sun, Apr 05, 2015 at 02:37:06PM -0400, Derek Buitenhuis wrote:
> Signed-off-by: Derek Buitenhuis <derek.buitenhuis at gmail.com>
> ---
>  libavformat/mpegtsenc.c | 10 +++++++---
>  libavformat/version.h   |  2 +-
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
> index ae0593d..861da95 100644
> --- a/libavformat/mpegtsenc.c
> +++ b/libavformat/mpegtsenc.c
> @@ -96,6 +96,7 @@ typedef struct MpegTSWrite {
>      int reemit_pat_pmt; // backward compatibility
>  
>      int pcr_period;
> +    int sdt_period;
>  #define MPEGTS_FLAG_REEMIT_PAT_PMT  0x01
>  #define MPEGTS_FLAG_AAC_LATM        0x02
>      int flags;
> @@ -205,7 +206,7 @@ static int mpegts_write_section1(MpegTSSection *s, int tid, int id,
>  #define DEFAULT_SERVICE_NAME    "Service01"
>  
>  /* we retransmit the SI info at this rate */
> -#define SDT_RETRANS_TIME 500
> +#define SDT_RETRANS_TIME 200
>  #define PAT_RETRANS_TIME 100
>  #define PCR_RETRANS_TIME 20

the change for the one of 2 cases should be documented in the commit
message


>  
> @@ -753,7 +754,7 @@ static int mpegts_write_header(AVFormatContext *s)
>      if (ts->mux_rate > 1) {
>          service->pcr_packet_period = (ts->mux_rate * ts->pcr_period) /
>                                       (TS_PACKET_SIZE * 8 * 1000);
> -        ts->sdt_packet_period      = (ts->mux_rate * SDT_RETRANS_TIME) /
> +        ts->sdt_packet_period      = (ts->mux_rate * ts->sdt_period) /
>                                       (TS_PACKET_SIZE * 8 * 1000);
>          ts->pat_packet_period      = (ts->mux_rate * PAT_RETRANS_TIME) /
>                                       (TS_PACKET_SIZE * 8 * 1000);
> @@ -762,7 +763,7 @@ static int mpegts_write_header(AVFormatContext *s)
>              ts->first_pcr = av_rescale(s->max_delay, PCR_TIME_BASE, AV_TIME_BASE);
>      } else {
>          /* Arbitrary values, PAT/PMT will also be written on video key frames */
> -        ts->sdt_packet_period = 200;
> +        ts->sdt_packet_period = ts->sdt_period;
>          ts->pat_packet_period = 40;
>          if (pcr_st->codec->codec_type == AVMEDIA_TYPE_AUDIO) {
>              if (!pcr_st->codec->frame_size) {
> @@ -1520,6 +1521,9 @@ static const AVOption options[] = {
>      { "pcr_period", "PCR retransmission time",
>        offsetof(MpegTSWrite, pcr_period), AV_OPT_TYPE_INT,
>        { .i64 = PCR_RETRANS_TIME }, 0, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM },
> +    { "sdt_period", "SDT retransmission time",
> +      offsetof(MpegTSWrite, sdt_period), AV_OPT_TYPE_INT,
> +      { .i64 = SDT_RETRANS_TIME }, 0, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM },

this is missing documentation about the units in which sdt_period is.
as is this depends on "mux_rate > 1"
if its intended to stay that way, then this should be documented

similar comments apply to the other patch


>      { NULL },
>  };
>  
> diff --git a/libavformat/version.h b/libavformat/version.h
> index 1c78df2..55287ae 100644
> --- a/libavformat/version.h
> +++ b/libavformat/version.h
> @@ -30,7 +30,7 @@
>  #include "libavutil/version.h"
>  
>  #define LIBAVFORMAT_VERSION_MAJOR 56
> -#define LIBAVFORMAT_VERSION_MINOR  29
> +#define LIBAVFORMAT_VERSION_MINOR  30

1 or 2 bumps LGTM, whaever is preferred

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150406/993d1c7d/attachment.asc>