[FFmpeg-devel] [PATCH] Add optional NIT table generation

[Ubaldo Porcheddu]

Hi Marton,

Il 2021-05-12 19:18 Marton Balint ha scritto:
> On Wed, 12 May 2021, Ubaldo Porcheddu wrote:
> 
>> Hi Marton,
>> 
>>>> +    }
>>>> +
>>>> +    //private data
>>>> +    desc_len += 6 + 2;
>>>> +    *q++ = 0x5F;
>>>> +    *q++ = 4;
>>>> +    *q++ = 0x00;
>>>> +    *q++ = 0x00;
>>>> +    put16(&q, 40);
>>> 
>>> What are these?
>> 
>> I didn't find any official document about it but this seems to be the 
>> way many national (FR,IT,UK) broadcasters are writing the virtual 
>> channel private data header and the one recognized by many popular tv 
>> on the market.
> 
> But this looks like a separate descriptor from the virtual channels
> (logical_channel_descriptor).

I think it is better to remove it until it is more clear what it does, 
or maybe we add an extra mpegts flag like "nit_lcn_extra" ?

> logical_channel_descriptor() is documented for example here:
> https://forums.mediaspy.org/uploads/short-url/2wA2rGhOkh2yjlbcWMtcQizBv8L.pdf
> 
> So you should use the terminology that is used in the document above.

Ok, rewritten accordly.

>>>> +    ts->nit_period = av_rescale(ts->nit_period_us, PCR_TIME_BASE,
>>> AV_TIME_BASE);
>>>> 
>>>>     if (ts->mux_rate == 1)
>>>>         av_log(s, AV_LOG_VERBOSE, "muxrate VBR, ");
>>>> @@ -1154,12 +1237,14 @@ static int mpegts_init(AVFormatContext *s)
>>>>            "sdt every %"PRId64" ms, pat/pmt every %"PRId64" ms\n",
>>>>            av_rescale(ts->sdt_period, 1000, PCR_TIME_BASE),
>>>>            av_rescale(ts->pat_period, 1000, PCR_TIME_BASE));
>>>> +    if (ts->nit_enable > 0)
>>>> +        av_log(s, AV_LOG_VERBOSE, "nit every %"PRId64" ms\n",
>>> av_rescale(ts->nit_period, 1000, PCR_TIME_BASE));
>>> 
>>> Maybe you should continue the last log line which already describes
>>> the
>>> interval of various tables, and not start a new line.
>> 
>> The idea is to have the NIT emission optional so I don't know if is a 
>> good idea to stat its periodical transmission on the same line, when 
>> disabled?
> 
> I meant something like:
> 
> av_log(s, AV_LOG_VERBOSE,
>        "sdt every %"PRId64" ms, pat/pmt every %"PRId64" ms",
>        av_rescale(ts->sdt_period, 1000, PCR_TIME_BASE),
>        av_rescale(ts->pat_period, 1000, PCR_TIME_BASE));
> if (nit_enabled)
>      av_log(s, AV_LOG_VERBOSE, ", nit every %"PRId64" ms")
> av_log(s, AV_LOG_VERBOSE, "\n")

Ok, clear now :)

>>> Also docs/muxers.texi update is missing for the new feature.
>> 
>> is there any guide on what an how to add it? :)
> 
> No guide, but should be straightforward based on the existing
> documentation of the mpegts muxer.

You mean something like this (I am changing "nit_enable" option with 
"nit_emit" flag):

--- ffmpeg.old/doc/muxers.texi  2021-05-09 18:20:04.000000000 +0200
+++ ffmpeg.new/doc/muxers.texi  2021-05-12 16:52:38.368198223 +0200
@@ -1876,6 +1876,8 @@
  Conform to System B (DVB) instead of System A (ATSC).
  @item initial_discontinuity
  Mark the initial packet of each stream as discontinuity.
+ at item nit_emit
+Emit NIT table.
  @end table