[FFmpeg-devel] [PATCH 3/4] mpegts: add fix_teletext_pts mpegts demuxer option

[Marton Balint]

On Tue, 30 Jul 2013, Michael Niedermayer wrote:

> On Sat, Jul 27, 2013 at 09:14:46PM +0200, Marton Balint wrote:
>> The option (if set) overrides teletext packet pts with the PCR of a program
>> which the teletext stream is part of that is not discarded. Using the same
>> teletext PID for multiple programs is possible, therefore we need some kind of
>> heuristics to know which program PCR we should synchronize to. Using the first
>> non-discarded PCR pid among the programs of the teletext stream seemed like a
>> good choice.
>>
>> Based on a patch by Reimar Döffinger.
>> http://lists.ffmpeg.org/pipermail/ffmpeg-devel/2012-September/131610.html
>>
>> Signed-off-by: Marton Balint <cus at passwd.hu>
>> ---
>>  libavformat/mpegts.c  | 46 ++++++++++++++++++++++++++++++++++++++++++++--
>>  libavformat/version.h |  2 +-
>>  2 files changed, 45 insertions(+), 3 deletions(-)
>>
>> diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
>> index c7c957f..5e2af02 100644
>> --- a/libavformat/mpegts.c
>> +++ b/libavformat/mpegts.c
>> @@ -106,6 +106,9 @@ struct MpegTSContext {
>>      /** compute exact PCR for each transport stream packet   */
>>      int mpeg2ts_compute_pcr;
>>
>> +    /** fix dvb teletext pts                                 */
>> +    int fix_teletext_pts;
>> +
>>      int64_t cur_pcr;    /**< used to estimate the exact PCR  */
>>      int pcr_incr;       /**< used to estimate the exact PCR  */
>>
>> @@ -131,7 +134,7 @@ struct MpegTSContext {
>>      int current_pid;
>>  };
>>
>> -static const AVOption options[] = {
>> +static const AVOption mpegtsraw_options[] = {
>>      {"compute_pcr", "Compute exact PCR for each transport stream packet.", offsetof(MpegTSContext, mpeg2ts_compute_pcr), AV_OPT_TYPE_INT,
>>       {.i64 = 0}, 0, 1, AV_OPT_FLAG_DECODING_PARAM },
>>      { NULL },
>
>> @@ -140,7 +143,20 @@ static const AVOption options[] = {
>>  static const AVClass mpegtsraw_class = {
>>      .class_name = "mpegtsraw demuxer",
>>      .item_name  = av_default_item_name,
>> -    .option     = options,
>> +    .option     = mpegtsraw_options,
>> +    .version    = LIBAVUTIL_VERSION_INT,
>> +};
>> +
>> +static const AVOption mpegts_options[] = {
>> +    {"fix_teletext_pts", "Try to fix pts values of dvb teletext streams.", offsetof(MpegTSContext, fix_teletext_pts), AV_OPT_TYPE_INT,
>> +     {.i64 = 0}, 0, 1, AV_OPT_FLAG_DECODING_PARAM },
>> +    { NULL },
>> +};
>
> i dont know how commonly this is needed but the defaults should be so
> that the demuxer works in as many cases as possibe
> requireing the user to know and override defaults is a big problem
> as normal users wont know where to look if something doesnt work

I'd say the fix is needed quite often, here are two possible scenarios 
which usually cause out of sync pts-es:
  - using the same teletext PID for multiple programs
  - adding teletext service to a HD channel by using the teletext PID of an 
SD channel - this is common because as far as I know there is no widely 
used standardised method of transferring teletext in a HD-SDI signal, so 
the teletext is typically generated by the SD encoder and not the HD...

Since most decoders have no problem with invalid teletext packet pts 
values, broadcasters don't care about it, and use it shamelessly...

On the other hand, guessing which program we should synchornize the 
teletext to will always be a guess, so this is a hack in the end of the 
day. That is why I disabled it by default, I kind of disliked the idea 
that the mpegts demuxer will return some guessed PTS values by default, 
instead of the actual ones in the transport stream...

Anyway, I can be convinced to change the default to 1, and spare the 
majority of the users the google search of finding the magic option to 
fix their teletext subtitles.

Or another option can be considered: keep it disabled by default in the 
API, but set it by default in the ff* tools.

Regards,
Marton